[PATCH 00/13] OLCE for external meta Raid5/single array

This patches enables raid5/single array Online Capacity Expansion using container operation.
I've decided to post smaller series to allow us for closer/adaptive work.

Main changes in code:
- metadata is reloaded between reshape_super() and reshape_array(), reshape_super() doesn't modify local anchor
- disk slot is set by md (index == -1), this propably has to be fixed in md
- suspend_lo has to be set to suspend_hi value after reshape to unblock array
- fixes


The main problems I can see:
- cannot add spare disks to container (add spare to raid0 works),
I've payed no attantion for it so far as I'm using workaround for tests: create container with bigger disks number before any array is created.
- adding disks to md with set index/slot should be fixed/investigated in md (for details look patches description)
- array cannot be opened excl. in reshape_array()
As I've put information in patch, it is possible caused by mdmon, I cannot see double array open operation in mdadm.
On mdadm begin open_dev_excl() is possible.

BR
Adam

---

Adam Kwolek (13):
FIX: Fill sys_name for disk add
FIX: parity disks use for redundant levels only
FIX/WORKAROUND: Arrays cannot be opened exclusively
FIX: Cannot unmount array after reshape
Finalize reshape after adding disks to array
FIX/WORKAROUND: added disks are not used by reshape process /md/
FIX: wrong flags are passed to function
imsm: FIX: update disks status in container_contents()
msm: FIX: when mdadm reloads anchor not need for local modyfication
FIX: load anchor to get all information
imsm: FIX: when monitor is active post update
imsm: FIX: allow for container reshape any unset size value
Manage reshape process in manage_reshape vector.


Grow.c | 29 +++++++++++++++++++++--------
monitor.c | 11 +++++++++++
super-intel.c | 30 +++++++++++++-----------------
3 files changed, 45 insertions(+), 25 deletions(-)

--
Signature
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 01/13] Manage reshape process in manage_reshape vector.

The manage_reshape vector is intended to be be used as
reshape process supervisor for external metadata types.
Sometimes there is no need for separate control,
and main code for native meta can be used instead.

Signed-off-by: Krzysztof Wojcik
Signed-off-by: Adam Kwolek
---

Grow.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/Grow.c b/Grow.c
index 1222263..4cbe427 100644
--- a/Grow.c
+++ b/Grow.c
@@ -574,8 +574,7 @@ static int reshape_super(struct supertype *st, long long size, int level,
/* nothing extra to check in the native case */
if (!st->ss->external)
return 0;
- if (!st->ss->reshape_super ||
- !st->ss->manage_reshape) {
+ if (!st->ss->reshape_super) {
fprintf(stderr, Name ": %s metadata does not support reshape\n",
st->ss->name);
return 1;
@@ -1940,7 +1939,7 @@ static int reshape_array(char *container, int fd, char *devname,
}

start_reshape(sra);
- if (st->ss->external) {
+ if ((st->ss->external) && (st->ss->manage_reshape)) {
/* metadata handler takes it from here */
ping_manager(container);
st->ss->manage_reshape(st, backup_file);

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 02/13] imsm: FIX: allow for container reshape any unset size

Any request for no size change has to be accepted.

Signed-off-by: Adam Kwolek
---

super-intel.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index d70a7df..58a776e 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6322,7 +6322,7 @@ static int imsm_reshape_is_allowed_on_container(struct supertype *st,
"st->devnum = (%i)\n",
st->devnum);

- if (geo->size != -1 ||
+ if (geo->size >= 0 ||
geo->level != UnSet ||
geo->layout != UnSet ||
geo->chunksize != 0 ||

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 03/13] imsm: FIX: when monitor is active post update

Updates has to be posted when monitor is active for this array/container.
Only when monitor is not active mdadm has update metadata itself.

Signed-off-by: Adam Kwolek
---

super-intel.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 58a776e..8644c39 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6570,9 +6570,15 @@ static int imsm_reshape_super(struct supertype *st, long long size, int level,
goto exit_imsm_reshape_super;
}

- /* As well as creating update, we apply update.
- */
+ if (mdmon_running(st->container_dev)) {
+ ret_val = 0;
+ append_metadata_update(st, u, len);
+ goto exit_imsm_reshape_super;
+ }
+

+ /* no mdmon - apply update
+ */
dprintf("imsm:prepare space list for update_reshape\n");
for (dl = super->devlist; dl;
dl = dl->next) {

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 04/13] FIX: load anchor to get all information

When reshape_super() puts updated metadata to array, mdadm should reload it,
to get all information that are changed during update operation.
To avoid (follow curent implementation) reload operation mdadm should update metadata 2 times.
One time for mdmon, second for internal mdadm purposes. Both updates has to update whole
array information (not disks only) to know everything about reshape process.
Local/mdadm update ie. for imsm should contain:
- reshape_active field update
- second map update
- migration state update
.....
- and most important during disk add metadata anchor should be reallocated
to make space for new/greater disks number,
and then metadata contents has to be copied to new area ...

This makes also problem with keeping in sync both those updates, so I'm resending
patches for reloading metadata after update (this one)
and removing anchor modifications in mdadm (next patch)

Signed-off-by: Adam Kwolek
---

Grow.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Grow.c b/Grow.c
index 4cbe427..1d0ef12 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2159,6 +2159,7 @@ int reshape_container(char *container, int cfd, char *devname,

sysfs_free(cc);

+ st->ss->load_container(st, cfd, NULL);
cc = st->ss->container_content(st, NULL);

for (content = cc; content ; content = content->next) {

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 05/13] msm: FIX: when mdadm reloads anchor not need for local

imsm part to patch: "FIX: load anchor to get all information"
mdadm should not update anchor but reload metadata.
For details look previous patch.

Signed-off-by: Adam Kwolek
---

super-intel.c | 13 -------------
1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 8644c39..9687993 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6483,19 +6483,6 @@ static int imsm_create_metadata_update_for_reshape(
dl = get_disk_super(super, dev->disk.major, dev->disk.minor);
dl->index = mpb->num_disks++;
}
- /* Now update the metadata so that container_content will find
- * the new devices
- */
- for (i = 0; i < mpb->num_raid_devs; i++) {
- int d;
- struct imsm_dev *dev = get_imsm_dev(super, i);
- struct imsm_map *map = get_imsm_map(dev, 0);
- map->num_members = geo->raid_disks;
- for (d = 0; d < delta_disks; d++) {
- set_imsm_ord_tbl_ent(map, old_raid_disks + d,
- mpb->num_disks - delta_disks + d);
- }
- }

abort:
/* free spares

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 06/13] imsm: FIX: update disks status in container_contents()

Based on status information disks are added to array during grow (in reshape_array()).
This information currently is not present and all disks (old and new) were added to md.
To avoid adding already present disks, disk.state has to be set.

Signed-off-by: Adam Kwolek
---

super-intel.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 9687993..2700401 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -4541,6 +4541,7 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
for (i = 0; i < mpb->num_raid_devs; i++) {
struct imsm_dev *dev;
struct imsm_map *map;
+ struct imsm_map *map2;
struct mdinfo *this;
int slot;
char *ep;
@@ -4551,6 +4552,7 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra

dev = get_imsm_dev(super, i);
map = get_imsm_map(dev, 0);
+ map2 = get_imsm_map(dev, 1);

/* do not publish arrays that are in the middle of an
* unsupported migration
@@ -4632,7 +4634,8 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
info_d->disk.minor = d->minor;
info_d->disk.raid_disk = slot;
info_d->recovery_start = recovery_start;
-
+ if (map2 && (map2->num_members > slot))
+ info_d->disk.state = (1 << MD_DISK_ACTIVE);
if (info_d->recovery_start == MaxSector)
this->array.working_disks++;


--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 07/13] FIX: wrong flags are passed to function

flags == 1 are always passed to sysfs_read() due to '||' usage.

Signed-off-by: Adam Kwolek
---

Grow.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index 1d0ef12..0efa106 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1788,7 +1788,7 @@ static int reshape_array(char *container, int fd, char *devname,
}

sra = sysfs_read(fd, 0,
- GET_COMPONENT|GET_DEVS|GET_OFFSET|GET_STATE||GET_CHUNK|
+ GET_COMPONENT|GET_DEVS|GET_OFFSET|GET_STATE|GET_CHUNK|
GET_CACHE);

if (!sra) {

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 08/13] FIX/WORKAROUND: added disks are not used by reshape

md runs reshape but added disk is not touched
/proc/mdstat reports that disk is used.

To fix this let md to set slot for reshape.
It is possible that slot verification mechanism has to be added.

Second possibility is to investigate problem in md.
md/raid5 treats disks as spare with disk index == -1 during raid5 reshape_start.
Probably rqaid5 should treat as spare disks when disk index > used disks

Signed-off-by: Adam Kwolek
---

Grow.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Grow.c b/Grow.c
index 0efa106..6b2889e 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1697,6 +1697,7 @@ static int reshape_array(char *container, int fd, char *devname,
/* This is a spare that wants to
* be part of the array.
*/
+ d->disk.raid_disk = -1;
add_disk(fd, st, info2, d);
}
}

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 09/13] Finalize reshape after adding disks to array

When reshape is finished monitor, has to finalize reshape in metadata.
To do this set_array_state() should be called.
This finishes migration and stores metadata on disks.

This finishes reshape flow in mdmon.

Signed-off-by: Adam Kwolek
---

monitor.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 1107d47..a0068d3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -322,6 +322,17 @@ static int read_and_act(struct active_array *a)
*/
check_reshape = 1;

+ /* finalize reshape detection
+ */
+ if ((a->curr_action != reshape) &&
+ (a->prev_action == reshape)) {
+ /* A reshape has finished.
+ * Some disks may be in sync now.
+ */
+ a->container->ss->set_array_state(a, a->curr_state <= clean);
+ check_degraded = 1;
+ }
+
/* Check for failures and if found:
* 1/ Record the failure in the metadata and unblock the device.
* FIXME update the kernel to stop notifying on failed drives when

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 10/13] FIX: Cannot unmount array after reshape

After reshape array remains blocked (i.e. cannot be umounted) due to suspend_lo
sysfs entry setting.

Signed-off-by: Adam Kwolek
---

Grow.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/Grow.c b/Grow.c
index 6b2889e..f083ba5 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2026,6 +2026,7 @@ static int reshape_array(char *container, int fd, char *devname,
unsigned long long current_size = 0;
unsigned long long new_size =
info2->custom_array_size/2;
+ unsigned long long suspend_value;

if (sysfs_get_ll(sra,
NULL,
@@ -2043,6 +2044,13 @@ static int reshape_array(char *container, int fd, char *devname,
dprintf(" from %llu to %llu.\n",
current_size, new_size);
}
+ /* manage suspend_* entries
+ * set suspend_lo to suspend_hi value
+ * to unblock array
+ */
+ sysfs_get_ll(sra, NULL, "suspend_hi", &suspend_value);
+ sysfs_set_num(sra, NULL, "suspend_lo", suspend_value);
+
sysfs_free(info2);
}
}

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 11/13] FIX/WORKAROUND: Arrays cannot be opened exclusively

During container operation arrays cannot be opened exclusively.
Use open_dev() instead.

On mdadm processing begin this is possible but when array_reshape() tries to do it,
it is not possible. I've found that there is no other opens in mdadm so it is caused
/possible/ by mdmon interaction.

Signed-off-by: Adam Kwolek
---

Grow.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index f083ba5..74ec429 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2186,7 +2186,7 @@ int reshape_container(char *container, int cfd, char *devname,
if (!content)
break;

- fd = open_dev_excl(mdstat->devnum);
+ fd = open_dev(mdstat->devnum);
if (fd < 0)
break;
adev = map_dev(dev2major(mdstat->devnum),

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 12/13] FIX: parity disks use for redundant levels only

For raid0 reshape spares should not be increased by parity disk.

Signed-off-by: Adam Kwolek
---

Grow.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index 74ec429..9d31087 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1637,7 +1637,9 @@ static int reshape_array(char *container, int fd, char *devname,
}
spares_needed = max(reshape.before.data_disks,
reshape.after.data_disks)
- + reshape.parity - array.raid_disks;
+ - array.raid_disks;
+ if (info->array.level != 0)
+ spares_needed += reshape.parity;

if (!force && spares_needed < info->array.spare_disks) {
fprintf(stderr,

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 13/13] FIX: Fill sys_name for disk add

Add disks fails due to empty sys name field.
sysfs_init fills out required fields for add disk operation.

Signed-off-by: Adam Kwolek
---

Grow.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Grow.c b/Grow.c
index 9d31087..2ca9835 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1692,7 +1692,8 @@ static int reshape_array(char *container, int fd, char *devname,
st->ss->container_content(st, subarray);
struct mdinfo *d;

- if (info2)
+ if (info2) {
+ sysfs_init(info2, fd, st->devnum);
for (d = info2->devs; d; d = d->next) {
if (d->disk.state == 0 &&
d->disk.raid_disk >= 0) {
@@ -1703,7 +1704,8 @@ static int reshape_array(char *container, int fd, char *devname,
add_disk(fd, st, info2, d);
}
}
- sysfs_free(info2);
+ sysfs_free(info2);
+ }
}

if (reshape.blocks == 0) {

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/13] Manage reshape process in manage_reshape vector.

On Mon, 10 Jan 2011 12:27:33 +0100 Adam Kwolek wrote:

> The manage_reshape vector is intended to be be used as
> reshape process supervisor for external metadata types.
> Sometimes there is no need for separate control,
> and main code for native meta can be used instead.
>
> Signed-off-by: Krzysztof Wojcik
> Signed-off-by: Adam Kwolek
> ---


Thanks.
I have address this issue a different way.
->manage_reshape is still required, but it can trivially be implemented by
calling child_monitor to use the native backup mechanism.

NeilBrown


>
> Grow.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 1222263..4cbe427 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -574,8 +574,7 @@ static int reshape_super(struct supertype *st, long long size, int level,
> /* nothing extra to check in the native case */
> if (!st->ss->external)
> return 0;
> - if (!st->ss->reshape_super ||
> - !st->ss->manage_reshape) {
> + if (!st->ss->reshape_super) {
> fprintf(stderr, Name ": %s metadata does not support reshape\n",
> st->ss->name);
> return 1;
> @@ -1940,7 +1939,7 @@ static int reshape_array(char *container, int fd, char *devname,
> }
>
> start_reshape(sra);
> - if (st->ss->external) {
> + if ((st->ss->external) && (st->ss->manage_reshape)) {
> /* metadata handler takes it from here */
> ping_manager(container);
> st->ss->manage_reshape(st, backup_file);

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/13] imsm: FIX: allow for container reshape any unsetsize value

On Mon, 10 Jan 2011 12:27:42 +0100 Adam Kwolek wrote:

> Any request for no size change has to be accepted.
>
> Signed-off-by: Adam Kwolek
> ---
>
> super-intel.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index d70a7df..58a776e 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -6322,7 +6322,7 @@ static int imsm_reshape_is_allowed_on_container(struct supertype *st,
> "st->devnum = (%i)\n",
> st->devnum);
>
> - if (geo->size != -1 ||
> + if (geo->size >= 0 ||
> geo->level != UnSet ||
> geo->layout != UnSet ||
> geo->chunksize != 0 ||


As far as I can tell, this is a not op.
'size' will only ever be -1, 0, or positive.
Given that, "size != -1" and "size >= 0" produce identical results.

If there is some way that 'size' can be some other negative number, we should
fix that.

NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/13] imsm: FIX: when monitor is active post update

On Mon, 10 Jan 2011 12:27:50 +0100 Adam Kwolek wrote:

> Updates has to be posted when monitor is active for this array/container.
> Only when monitor is not active mdadm has update metadata itself.
>
> Signed-off-by: Adam Kwolek
> ---
>
> super-intel.c | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index 58a776e..8644c39 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -6570,9 +6570,15 @@ static int imsm_reshape_super(struct supertype *st, long long size, int level,
> goto exit_imsm_reshape_super;
> }
>
> - /* As well as creating update, we apply update.
> - */
> + if (mdmon_running(st->container_dev)) {
> + ret_val = 0;
> + append_metadata_update(st, u, len);
> + goto exit_imsm_reshape_super;
> + }
> +
>
> + /* no mdmon - apply update
> + */
> dprintf("imsm:prepare space list for update_reshape\n");
> for (dl = super->devlist; dl;
> dl = dl->next) {


I really don't like have code in super-* calling "mdmon_running". It feels
like a layering violation.


imsm_reshape_super should:
- *always* change the metadata as required
- if st->update_tail is non-null, then a metadata update should also
be added with append_metadata_update

The common code will then call sync_metadata() to either flush the updates
through mdmon or write them directly.

NeilBrown

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/13] FIX: load anchor to get all information

On Mon, 10 Jan 2011 12:27:58 +0100 Adam Kwolek wrote:

> When reshape_super() puts updated metadata to array, mdadm should reload it,
> to get all information that are changed during update operation.
> To avoid (follow curent implementation) reload operation mdadm should update metadata 2 times.
> One time for mdmon, second for internal mdadm purposes. Both updates has to update whole
> array information (not disks only) to know everything about reshape process.
> Local/mdadm update ie. for imsm should contain:
> - reshape_active field update
> - second map update
> - migration state update
> ....
> - and most important during disk add metadata anchor should be reallocated
> to make space for new/greater disks number,
> and then metadata contents has to be copied to new area ...
>
> This makes also problem with keeping in sync both those updates, so I'm resending
> patches for reloading metadata after update (this one)
> and removing anchor modifications in mdadm (next patch)
>
> Signed-off-by: Adam Kwolek

I don't agree with this.
Is there some specific problem that you are trying to solve? If so, please
explain that symptoms.

Thanks,
NeilBrown


> ---
>
> Grow.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 4cbe427..1d0ef12 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2159,6 +2159,7 @@ int reshape_container(char *container, int cfd, char *devname,
>
> sysfs_free(cc);
>
> + st->ss->load_container(st, cfd, NULL);
> cc = st->ss->container_content(st, NULL);
>
> for (content = cc; content ; content = content->next) {

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/13] imsm: FIX: update disks status incontainer_contents()

On Mon, 10 Jan 2011 12:28:15 +0100 Adam Kwolek wrote:

> Based on status information disks are added to array during grow (in reshape_array()).
> This information currently is not present and all disks (old and new) were added to md.
> To avoid adding already present disks, disk.state has to be set.
>
> Signed-off-by: Adam Kwolek
> ---
>
> super-intel.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index 9687993..2700401 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -4541,6 +4541,7 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
> for (i = 0; i < mpb->num_raid_devs; i++) {
> struct imsm_dev *dev;
> struct imsm_map *map;
> + struct imsm_map *map2;
> struct mdinfo *this;
> int slot;
> char *ep;
> @@ -4551,6 +4552,7 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
>
> dev = get_imsm_dev(super, i);
> map = get_imsm_map(dev, 0);
> + map2 = get_imsm_map(dev, 1);
>
> /* do not publish arrays that are in the middle of an
> * unsupported migration
> @@ -4632,7 +4634,8 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
> info_d->disk.minor = d->minor;
> info_d->disk.raid_disk = slot;
> info_d->recovery_start = recovery_start;
> -
> + if (map2 && (map2->num_members > slot))
> + info_d->disk.state = (1 << MD_DISK_ACTIVE);
> if (info_d->recovery_start == MaxSector)
> this->array.working_disks++;
>


This patch looks a little odd. It should still be setting disk.state even if
map2 == NULL.

The real problem here is simply that container_content isn't setting
disk.state properly.

I have change the patch to the following which should be closed to correct.

NeilBrown


commit 86e3692b06f255ead5c290288f0fe9318f75440d
Author: Adam Kwolek
Date: Wed Jan 12 15:12:44 2011 +1100

imsm: FIX: update disks status in container_contents()

Based on status information disks are added to array during grow (in reshape_array()).
This information currently is not present and all disks (old and new) were added to md.
To avoid adding already present disks, disk.state has to be set.

Signed-off-by: Adam Kwolek
Signed-off-by: NeilBrown

diff --git a/super-intel.c b/super-intel.c
index d62e6b7..f158163 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -4541,6 +4541,7 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
for (i = 0; i < mpb->num_raid_devs; i++) {
struct imsm_dev *dev;
struct imsm_map *map;
+ struct imsm_map *map2;
struct mdinfo *this;
int slot;
char *ep;
@@ -4551,6 +4552,7 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra

dev = get_imsm_dev(super, i);
map = get_imsm_map(dev, 0);
+ map2 = get_imsm_map(dev, 1);

/* do not publish arrays that are in the middle of an
* unsupported migration
@@ -4632,7 +4634,13 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
info_d->disk.minor = d->minor;
info_d->disk.raid_disk = slot;
info_d->recovery_start = recovery_start;
-
+ if (map2) {
+ if (slot < map2->num_members)
+ info_d->disk.state = (1 << MD_DISK_ACTIVE);
+ } else {
+ if (slot < map->num_members)
+ info_d->disk.state = (1 << MD_DISK_ACTIVE);
+ }
if (info_d->recovery_start == MaxSector)
this->array.working_disks++;


--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/13] FIX/WORKAROUND: added disks are not used byreshape process /md/

On Mon, 10 Jan 2011 12:28:32 +0100 Adam Kwolek wrote:

> md runs reshape but added disk is not touched
> /proc/mdstat reports that disk is used.
>
> To fix this let md to set slot for reshape.
> It is possible that slot verification mechanism has to be added.
>
> Second possibility is to investigate problem in md.
> md/raid5 treats disks as spare with disk index == -1 during raid5 reshape_start.
> Probably rqaid5 should treat as spare disks when disk index > used disks
>
> Signed-off-by: Adam Kwolek
> ---
>
> Grow.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 0efa106..6b2889e 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1697,6 +1697,7 @@ static int reshape_array(char *container, int fd, char *devname,
> /* This is a spare that wants to
> * be part of the array.
> */
> + d->disk.raid_disk = -1;
> add_disk(fd, st, info2, d);
> }
> }

We really should fix md.
I think the following might help. I'll try testing at some stage, but if you
could let me know if it helps, that would be great.

NeilBrown

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index d223a6c..5044bab 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5527,8 +5527,8 @@ static int raid5_start_reshape(mddev_t *mddev)
return -ENOSPC;

list_for_each_entry(rdev, &mddev->disks, same_set)
- if (rdev->raid_disk < 0 &&
- !test_bit(Faulty, &rdev->flags))
+ if ((rdev->raid_disk < 0 || rdev->raid_disk >= conf->raid_disks)
+ && !test_bit(Faulty, &rdev->flags))
spares++;

if (spares - mddev->degraded < mddev->delta_disks - conf->max_degraded)
@@ -5588,6 +5588,11 @@ static int raid5_start_reshape(mddev_t *mddev)
/* Failure here is OK */;
} else
break;
+ } else if (rdev->raid_disk >= conf->previous_raid_disks
+ && !test_bit(Faulty, &rdev->flags)) {
+ /* This is a spare that was manually added */
+ set_bit(In_sync, &rdev->flags);
+ added_devices++;
}

/* When a reshape changes the number of devices, ->degraded
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/13] Finalize reshape after adding disks to array

On Mon, 10 Jan 2011 12:28:40 +0100 Adam Kwolek wrote:

> When reshape is finished monitor, has to finalize reshape in metadata.
> To do this set_array_state() should be called.
> This finishes migration and stores metadata on disks.
>
> This finishes reshape flow in mdmon.
>
> Signed-off-by: Adam Kwolek

I think I had rejected this patch before asking for more explanation, and you
haven't given any....

There is already code to handle the case:

(a->curr_action == idle && a->prev_action == reshape)

Why do you think you need more code to handle this case?

NeilBrown


> ---
>
> monitor.c | 11 +++++++++++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 1107d47..a0068d3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -322,6 +322,17 @@ static int read_and_act(struct active_array *a)
> */
> check_reshape = 1;
>
> + /* finalize reshape detection
> + */
> + if ((a->curr_action != reshape) &&
> + (a->prev_action == reshape)) {
> + /* A reshape has finished.
> + * Some disks may be in sync now.
> + */
> + a->container->ss->set_array_state(a, a->curr_state <= clean);
> + check_degraded = 1;
> + }
> +
> /* Check for failures and if found:
> * 1/ Record the failure in the metadata and unblock the device.
> * FIXME update the kernel to stop notifying on failed drives when

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/13] FIX: Cannot unmount array after reshape

On Mon, 10 Jan 2011 12:28:48 +0100 Adam Kwolek wrote:

> After reshape array remains blocked (i.e. cannot be umounted) due to suspend_lo
> sysfs entry setting.
>
> Signed-off-by: Adam Kwolek

I think I have address this issues elsewhere in the code.

Thanks,
NeilBrown


> ---
>
> Grow.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 6b2889e..f083ba5 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2026,6 +2026,7 @@ static int reshape_array(char *container, int fd, char *devname,
> unsigned long long current_size = 0;
> unsigned long long new_size =
> info2->custom_array_size/2;
> + unsigned long long suspend_value;
>
> if (sysfs_get_ll(sra,
> NULL,
> @@ -2043,6 +2044,13 @@ static int reshape_array(char *container, int fd, char *devname,
> dprintf(" from %llu to %llu.\n",
> current_size, new_size);
> }
> + /* manage suspend_* entries
> + * set suspend_lo to suspend_hi value
> + * to unblock array
> + */
> + sysfs_get_ll(sra, NULL, "suspend_hi", &suspend_value);
> + sysfs_set_num(sra, NULL, "suspend_lo", suspend_value);
> +
> sysfs_free(info2);
> }
> }

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/13] FIX: parity disks use for redundant levels only

On Mon, 10 Jan 2011 12:29:04 +0100 Adam Kwolek wrote:

> For raid0 reshape spares should not be increased by parity disk.
>
> Signed-off-by: Adam Kwolek
> ---
>
> Grow.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 74ec429..9d31087 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1637,7 +1637,9 @@ static int reshape_array(char *container, int fd, char *devname,
> }
> spares_needed = max(reshape.before.data_disks,
> reshape.after.data_disks)
> - + reshape.parity - array.raid_disks;
> + - array.raid_disks;
> + if (info->array.level != 0)
> + spares_needed += reshape.parity;
>
> if (!force && spares_needed < info->array.spare_disks) {
> fprintf(stderr,

Thanks.
I have fixed this issue differently, as follows.

NeilBrown

commit 88c1a083919b53b2e6b4ca71d17e86a26d6e4b8b
Author: NeilBrown
Date: Wed Jan 12 15:59:24 2011 +1100

Don't complain about missing spares when reshaping a raid0.

To reshape a RAID0 we convert to RAID4 first. This makes it look
like it could be degraded and so we are tempted to ensure there are
enough spares. However this is not appropriate for RAID0, so
explicitly exclude new_level == RAID0 in this check

Reported-by: Adam Kwolek
Signed-off-by: NeilBrown

diff --git a/Grow.c b/Grow.c
index 9edd4c7..109b0c1 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1613,7 +1613,9 @@ static int reshape_array(char *container, int fd, char *devname,
reshape.after.data_disks)
+ reshape.parity - array.raid_disks;

- if (!force && spares_needed > info->array.spare_disks) {
+ if (!force &&
+ info->new_level > 0 &&
+ spares_needed > info->array.spare_disks) {
fprintf(stderr,
Name ": Need %d spare%s to avoid degraded array,"
" and only have %d.\n"
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/13] FIX: Fill sys_name for disk add

On Mon, 10 Jan 2011 12:29:12 +0100 Adam Kwolek wrote:

> Add disks fails due to empty sys name field.
> sysfs_init fills out required fields for add disk operation.
>
> Signed-off-by: Adam Kwolek

Applied, thanks.

NeilBrown


> ---
>
> Grow.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 9d31087..2ca9835 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1692,7 +1692,8 @@ static int reshape_array(char *container, int fd, char *devname,
> st->ss->container_content(st, subarray);
> struct mdinfo *d;
>
> - if (info2)
> + if (info2) {
> + sysfs_init(info2, fd, st->devnum);
> for (d = info2->devs; d; d = d->next) {
> if (d->disk.state == 0 &&
> d->disk.raid_disk >= 0) {
> @@ -1703,7 +1704,8 @@ static int reshape_array(char *container, int fd, char *devname,
> add_disk(fd, st, info2, d);
> }
> }
> - sysfs_free(info2);
> + sysfs_free(info2);
> + }
> }
>
> if (reshape.blocks == 0) {

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

RE: [PATCH 02/13] imsm: FIX: allow for container reshape any unsetsize value

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Wednesday, January 12, 2011 4:53 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 02/13] imsm: FIX: allow for container reshape any
> unset size value
>
> On Mon, 10 Jan 2011 12:27:42 +0100 Adam Kwolek
> wrote:
>
> > Any request for no size change has to be accepted.
> >
> > Signed-off-by: Adam Kwolek
> > ---
> >
> > super-intel.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/super-intel.c b/super-intel.c
> > index d70a7df..58a776e 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -6322,7 +6322,7 @@ static int
> imsm_reshape_is_allowed_on_container(struct supertype *st,
> > "st->devnum = (%i)\n",
> > st->devnum);
> >
> > - if (geo->size != -1 ||
> > + if (geo->size >= 0 ||
> > geo->level != UnSet ||
> > geo->layout != UnSet ||
> > geo->chunksize != 0 ||
>
>
> As far as I can tell, this is a not op.
> 'size' will only ever be -1, 0, or positive.
> Given that, "size != -1" and "size >= 0" produce identical results.
>
> If there is some way that 'size' can be some other negative number, we
> should
> fix that.
>
> NeilBrown

This issue can be addressed in the following way:

From ce0a720c162c425ae46684ac7866f7076e65d5e5 Mon Sep 17 00:00:00 2001
From: Adam Kwolek
Date: Wed, 12 Jan 2011 09:23:53 +0100
Subject: [PATCH] FIX: size is passed incorrectly

reshape_super() called from reshape_container() with size set to info->component_size
gives size in reshape_super == -2 due to unsigned signed conversion
(info->component_size is not initializes).

As size is not changed during container reshape '-1' value is passed to indicate this.

Signed-off-by: Adam Kwolek
---
Grow.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index 4cbe427..466d546 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2120,7 +2120,7 @@ int reshape_container(char *container, int cfd, char *devname,
{
struct mdinfo *cc = NULL;

- if (reshape_super(st, info->component_size, info->new_level,
+ if (reshape_super(st, -1, info->new_level,
info->new_layout, info->new_chunk,
info->array.raid_disks + info->delta_disks,
backup_file, devname, quiet))
--
1.6.0.2

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/13] imsm: FIX: allow for container reshape any unsetsize value

On Wed, 12 Jan 2011 08:32:10 +0000 "Kwolek, Adam"
wrote:


> This issue can be addressed in the following way:
>
> >From ce0a720c162c425ae46684ac7866f7076e65d5e5 Mon Sep 17 00:00:00 2001
> From: Adam Kwolek
> Date: Wed, 12 Jan 2011 09:23:53 +0100
> Subject: [PATCH] FIX: size is passed incorrectly
>
> reshape_super() called from reshape_container() with size set to info->component_size
> gives size in reshape_super == -2 due to unsigned signed conversion
> (info->component_size is not initializes).
>
> As size is not changed during container reshape '-1' value is passed to indicate this.

Excellent problem description that allows me to immediately see the that the
solution is obviously correct.
Thanks. Applied and pushed out.

NeilBrown


>
> Signed-off-by: Adam Kwolek
> ---
> Grow.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 4cbe427..466d546 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2120,7 +2120,7 @@ int reshape_container(char *container, int cfd, char *devname,
> {
> struct mdinfo *cc = NULL;
>
> - if (reshape_super(st, info->component_size, info->new_level,
> + if (reshape_super(st, -1, info->new_level,
> info->new_layout, info->new_chunk,
> info->array.raid_disks + info->delta_disks,
> backup_file, devname, quiet))

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

RE: [PATCH 10/13] FIX: Cannot unmount array after reshape

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Wednesday, January 12, 2011 5:53 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 10/13] FIX: Cannot unmount array after reshape
>
> On Mon, 10 Jan 2011 12:28:48 +0100 Adam Kwolek
> wrote:
>
> > After reshape array remains blocked (i.e. cannot be umounted) due to
> suspend_lo
> > sysfs entry setting.
> >
> > Signed-off-by: Adam Kwolek
>
> I think I have address this issues elsewhere in the code.
>
> Thanks,
> NeilBrown

I agree, it is addressed now.

Thanks
Adam

>
> > ---
> >
> > Grow.c | 8 ++++++++
> > 1 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/Grow.c b/Grow.c
> > index 6b2889e..f083ba5 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -2026,6 +2026,7 @@ static int reshape_array(char *container, int
> fd, char *devname,
> > unsigned long long current_size = 0;
> > unsigned long long new_size =
> > info2->custom_array_size/2;
> > + unsigned long long suspend_value;
> >
> > if (sysfs_get_ll(sra,
> > NULL,
> > @@ -2043,6 +2044,13 @@ static int reshape_array(char *container, int
> fd, char *devname,
> > dprintf(" from %llu to %llu.\n",
> > current_size, new_size);
> > }
> > + /* manage suspend_* entries
> > + * set suspend_lo to suspend_hi value
> > + * to unblock array
> > + */
> > + sysfs_get_ll(sra, NULL, "suspend_hi",
> &suspend_value);
> > + sysfs_set_num(sra, NULL, "suspend_lo",
> suspend_value);
> > +
> > sysfs_free(info2);
> > }
> > }

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html