[PATCH 0/4] Report new_disks number when migration is started

External metadata handler reports starting disks number during expansion
until migration is finished. It is opposite to native metadata behavior
where new disks number is reported immediately when expansion is started.
This causes problem during expansion restart and causes exception
due to wrong disks number information.

This patch series unifies reported raid disks number. After reshape
is started external metadata handler reports new disks number now.

BR
Adam

---

Adam Kwolek (4):
FIX: Count correctly added devices
FIX: Set proper raid disks during migration
FIX: Fiddle raid_disks number for external metadta
FIX: Always report new raid_disks during migration


Assemble.c | 5 +++--
Grow.c | 2 ++
super-intel.c | 2 +-
sysfs.c | 7 +++++--
4 files changed, 11 insertions(+), 5 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 1/4] FIX: Always report new raid_disks during migration

To behalf in the similar way as native metadata during migration,
new raid disks number has to be reported by metadata handler.

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 dc5e34e..83135a6 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1755,7 +1755,7 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
if (dl->raiddisk == info->disk.raid_disk)
break;
info->container_member = super->current_vol;
- info->array.raid_disks = map_to_analyse->num_members;
+ info->array.raid_disks = map->num_members;
info->array.level = get_imsm_raid_level(map_to_analyse);
info->array.layout = imsm_level_to_layout(info->array.level);
info->array.md_minor = -1;

--
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 2/4] FIX: Fiddle raid_disks number for external metadta

For external metadata migration is started already.
For native metadata migration starts when md is pushed to reshape state.
Change raid disks to currently present in md to calculate required
configuration change in analyse_change() and add required disks to md.

Signed-off-by: Adam Kwolek
---

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

diff --git a/Grow.c b/Grow.c
index 017a79d..4c221ad 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1676,6 +1676,8 @@ static int reshape_array(char *container, int fd, char *devname,
if (info->reshape_active) {
int new_level = info->new_level;
info->new_level = UnSet;
+ if (container)
+ info->array.raid_disks -= info->delta_disks;
msg = analyse_change(info, &reshape);
info->new_level = new_level;
if (!restart)

--
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 3/4] FIX: Set proper raid disks during migration

During migration raid_disks field contains new disks number now.
It should be set old disks number first and then new disks number
to allow md to calculate e.g. delta_disks parameter.

Signed-off-by: Adam Kwolek
---

sysfs.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sysfs.c b/sysfs.c
index b806e14..f1c6669 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -531,6 +531,7 @@ int sysfs_set_array(struct mdinfo *info, int vers)
{
int rv = 0;
char ver[100];
+ int raid_disks = info->array.raid_disks;

ver[0] = 0;
if (info->array.major_version == -1 &&
@@ -549,7 +550,9 @@ int sysfs_set_array(struct mdinfo *info, int vers)
return 0; /* FIXME */
rv |= sysfs_set_str(info, NULL, "level",
map_num(pers, info->array.level));
- rv |= sysfs_set_num(info, NULL, "raid_disks", info->array.raid_disks);
+ if (info->reshape_active && info->delta_disks != UnSet)
+ raid_disks -= info->delta_disks;
+ rv |= sysfs_set_num(info, NULL, "raid_disks", raid_disks);
rv |= sysfs_set_num(info, NULL, "chunk_size", info->array.chunk_size);
rv |= sysfs_set_num(info, NULL, "layout", info->array.layout);
rv |= sysfs_set_num(info, NULL, "component_size", info->component_size/2);
@@ -576,7 +579,7 @@ int sysfs_set_array(struct mdinfo *info, int vers)
rv |= sysfs_set_num(info, NULL, "chunk_size", info->new_chunk);
rv |= sysfs_set_num(info, NULL, "layout", info->new_layout);
rv |= sysfs_set_num(info, NULL, "raid_disks",
- info->array.raid_disks + info->delta_disks);
+ info->array.raid_disks);
/* We don't set 'new_level' here. That can only happen
* once the reshape completes.
*/

--
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 4/4] FIX: Count correctly added devices

When array is in reshape state raid_disks field contains final disks number.
To know how many disks were added, disk.raid_disk index has to be compared
against old disk number computed using delta_disks.

Signed-off-by: Adam Kwolek
---

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

diff --git a/Assemble.c b/Assemble.c
index 268e248..8b05829 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1516,6 +1516,7 @@ int assemble_container_content(struct supertype *st, int mdfd,
int working = 0, preexist = 0;
int expansion = 0;
struct map_ent *map = NULL;
+ int old_raid_disks;

sysfs_init(content, mdfd, 0);

@@ -1529,10 +1530,10 @@ int assemble_container_content(struct supertype *st, int mdfd,

if (sra)
sysfs_free(sra);
-
+ old_raid_disks = content->array.raid_disks - content->delta_disks;
for (dev = content->devs; dev; dev = dev->next)
if (sysfs_add_disk(content, dev, 1) == 0) {
- if (dev->disk.raid_disk >= content->array.raid_disks &&
+ if (dev->disk.raid_disk >= old_raid_disks &&
content->reshape_active)
expansion++;
else

--
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 0/4] Report new_disks number when migration is started

On Fri, 15 Apr 2011 14:30:22 +0200 Adam Kwolek wrote:

> External metadata handler reports starting disks number during expansion
> until migration is finished. It is opposite to native metadata behavior
> where new disks number is reported immediately when expansion is started.
> This causes problem during expansion restart and causes exception
> due to wrong disks number information.
>
> This patch series unifies reported raid disks number. After reshape
> is started external metadata handler reports new disks number now.
>
> BR
> Adam
>
> ---
>
> Adam Kwolek (4):
> FIX: Count correctly added devices
> FIX: Set proper raid disks during migration
> FIX: Fiddle raid_disks number for external metadta
> FIX: Always report new raid_disks during migration
>
>

Thanks. I've applied all this.

I change the second one a bit - please check that it still works for you.

Thanks.
NeilBrown


commit 178b8f353c4b4ffdf3bd4cd8c9dde37f64097da8
Author: Adam Kwolek
Date: Mon Apr 18 10:31:06 2011 +1000

FIX: Fiddle raid_disks number when restarting reshape

When restarting a reshape, the value of 'raid_disks' is the *new*
value. The old value is found by subtracting delta_disks.
So before calling analyse_change we must set raid_disks to be the
old value, and then reset it afterwards.

All other fields are cleanly separated with the main field being
the 'old' value and a new_* field available.

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

diff --git a/Grow.c b/Grow.c
index 017a79d..9c1f096 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1676,8 +1676,10 @@ static int reshape_array(char *container, int fd, char *devname,
if (info->reshape_active) {
int new_level = info->new_level;
info->new_level = UnSet;
+ info->array.raid_disks -= info->delta_disks;
msg = analyse_change(info, &reshape);
info->new_level = new_level;
+ info->array.raid_disks += info->delta_disks;
if (!restart)
/* Make sure the array isn't read-only */
ioctl(fd, RESTART_ARRAY_RW, 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 0/4] Report new_disks number when migration is started

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Monday, April 18, 2011 2:34 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 0/4] Report new_disks number when migration is
> started
>
> On Fri, 15 Apr 2011 14:30:22 +0200 Adam Kwolek
> wrote:
>
> > External metadata handler reports starting disks number during
> expansion
> > until migration is finished. It is opposite to native metadata
> behavior
> > where new disks number is reported immediately when expansion is
> started.
> > This causes problem during expansion restart and causes exception
> > due to wrong disks number information.
> >
> > This patch series unifies reported raid disks number. After reshape
> > is started external metadata handler reports new disks number now.
> >
> > BR
> > Adam
> >
> > ---
> >
> > Adam Kwolek (4):
> > FIX: Count correctly added devices
> > FIX: Set proper raid disks during migration
> > FIX: Fiddle raid_disks number for external metadta
> > FIX: Always report new raid_disks during migration
> >
> >
>
> Thanks. I've applied all this.
>
> I change the second one a bit - please check that it still works for
> you.

Unfortunately it doesn't work.
During restart 3-disks raid5 to 5-disks expansion it stops in reshape_array() on first condition after change (Grow.c:1695)
reshape.before.raid_disks +reshape.parity != info->array.raid_disks
where:
reshape.before.raid_disks = 2
reshape.parity = 1
info->array.raid_disks = 5

It is used during restart only (checked for restart flag).

Do not you think that in this condition instead reshape.before.raid_disks, reshape.after.raid_disks should be used?
For both (native and external) metadata formats during reshape restart new raid_disks will be reported so 'after' filed should be used.
Patch for this will follow this post.

BR
Adam













>
> Thanks.
> NeilBrown
>
>
> commit 178b8f353c4b4ffdf3bd4cd8c9dde37f64097da8
> Author: Adam Kwolek
> Date: Mon Apr 18 10:31:06 2011 +1000
>
> FIX: Fiddle raid_disks number when restarting reshape
>
> When restarting a reshape, the value of 'raid_disks' is the *new*
> value. The old value is found by subtracting delta_disks.
> So before calling analyse_change we must set raid_disks to be the
> old value, and then reset it afterwards.
>
> All other fields are cleanly separated with the main field being
> the 'old' value and a new_* field available.
>
> Signed-off-by: Adam Kwolek
> Signed-off-by: NeilBrown
>
> diff --git a/Grow.c b/Grow.c
> index 017a79d..9c1f096 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1676,8 +1676,10 @@ static int reshape_array(char *container, int fd,
> char *devname,
> if (info->reshape_active) {
> int new_level = info->new_level;
> info->new_level = UnSet;
> + info->array.raid_disks -= info->delta_disks;
> msg = analyse_change(info, &reshape);
> info->new_level = new_level;
> + info->array.raid_disks += info->delta_disks;
> if (!restart)
> /* Make sure the array isn't read-only */
> ioctl(fd, RESTART_ARRAY_RW, 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 0/4] Report new_disks number when migration is started

On Mon, 18 Apr 2011 11:52:12 +0100 "Kwolek, Adam"
wrote:

>
>
> > -----Original Message-----
> > From: NeilBrown [mailto:neilb@suse.de]
> > Sent: Monday, April 18, 2011 2:34 AM
> > To: Kwolek, Adam
> > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> > Neubauer, Wojciech
> > Subject: Re: [PATCH 0/4] Report new_disks number when migration is
> > started
> >
> > On Fri, 15 Apr 2011 14:30:22 +0200 Adam Kwolek
> > wrote:
> >
> > > External metadata handler reports starting disks number during
> > expansion
> > > until migration is finished. It is opposite to native metadata
> > behavior
> > > where new disks number is reported immediately when expansion is
> > started.
> > > This causes problem during expansion restart and causes exception
> > > due to wrong disks number information.
> > >
> > > This patch series unifies reported raid disks number. After reshape
> > > is started external metadata handler reports new disks number now.
> > >
> > > BR
> > > Adam
> > >
> > > ---
> > >
> > > Adam Kwolek (4):
> > > FIX: Count correctly added devices
> > > FIX: Set proper raid disks during migration
> > > FIX: Fiddle raid_disks number for external metadta
> > > FIX: Always report new raid_disks during migration
> > >
> > >
> >
> > Thanks. I've applied all this.
> >
> > I change the second one a bit - please check that it still works for
> > you.
>
> Unfortunately it doesn't work.
> During restart 3-disks raid5 to 5-disks expansion it stops in reshape_array() on first condition after change (Grow.c:1695)
> reshape.before.raid_disks +reshape.parity != info->array.raid_disks
> where:
> reshape.before.raid_disks = 2
> reshape.parity = 1
> info->array.raid_disks = 5
>
> It is used during restart only (checked for restart flag).
>
> Do not you think that in this condition instead reshape.before.raid_disks, reshape.after.raid_disks should be used?
> For both (native and external) metadata formats during reshape restart new raid_disks will be reported so 'after' filed should be used.
> Patch for this will follow this post.
>
> BR
> Adam
>

Thanks for testing and for the patch.

However I think I do want to still test before.raid_disk... though it is all
a bit messy.

Anyway I've commited the patch like this...

Thanks.
NeilBrown

From 384e9be1330c29b40559f85dd0e6124bd0dfa535 Mon Sep 17 00:00:00 2001
From: Adam Kwolek
Date: Tue, 19 Apr 2011 17:25:43 +1000
Subject: [PATCH] FIX: Check correctly raid disks during reshape restart

During reshape restart info->array.raid_disks contains new raid_disks number
It cannot be compared against old disks number. Such check will always fail.

Check raid disks array field against final disks number for restart.

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

diff --git a/Grow.c b/Grow.c
index 9c1f096..9c63036 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1692,7 +1692,8 @@ static int reshape_array(char *container, int fd, char *devname,
if (restart &&
(reshape.level != info->array.level ||
reshape.before.layout != info->array.layout ||
- reshape.before.data_disks + reshape.parity != info->array.raid_disks)) {
+ reshape.before.data_disks + reshape.parity
+ != info->array.raid_disks - info->delta_disks)) {
fprintf(stderr, Name ": reshape info is not in native format -"
" cannot continue.\n");
goto release;
--
1.7.3.4



--
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 0/4] Report new_disks number when migration is started

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Tuesday, April 19, 2011 9:28 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 0/4] Report new_disks number when migration is
> started
>
> On Mon, 18 Apr 2011 11:52:12 +0100 "Kwolek, Adam"
>
> wrote:
>
> >
> >
> > > -----Original Message-----
> > > From: NeilBrown [mailto:neilb@suse.de]
> > > Sent: Monday, April 18, 2011 2:34 AM
> > > To: Kwolek, Adam
> > > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> > > Neubauer, Wojciech
> > > Subject: Re: [PATCH 0/4] Report new_disks number when migration is
> > > started
> > >
> > > On Fri, 15 Apr 2011 14:30:22 +0200 Adam Kwolek
>
> > > wrote:
> > >
> > > > External metadata handler reports starting disks number during
> > > expansion
> > > > until migration is finished. It is opposite to native metadata
> > > behavior
> > > > where new disks number is reported immediately when expansion is
> > > started.
> > > > This causes problem during expansion restart and causes exception
> > > > due to wrong disks number information.
> > > >
> > > > This patch series unifies reported raid disks number. After
> reshape
> > > > is started external metadata handler reports new disks number now.
> > > >
> > > > BR
> > > > Adam
> > > >
> > > > ---
> > > >
> > > > Adam Kwolek (4):
> > > > FIX: Count correctly added devices
> > > > FIX: Set proper raid disks during migration
> > > > FIX: Fiddle raid_disks number for external metadta
> > > > FIX: Always report new raid_disks during migration
> > > >
> > > >
> > >
> > > Thanks. I've applied all this.
> > >
> > > I change the second one a bit - please check that it still works for
> > > you.
> >
> > Unfortunately it doesn't work.
> > During restart 3-disks raid5 to 5-disks expansion it stops in
> reshape_array() on first condition after change (Grow.c:1695)
> > reshape.before.raid_disks +reshape.parity != info-
> >array.raid_disks
> > where:
> > reshape.before.raid_disks = 2
> > reshape.parity = 1
> > info->array.raid_disks = 5
> >
> > It is used during restart only (checked for restart flag).
> >
> > Do not you think that in this condition instead
> reshape.before.raid_disks, reshape.after.raid_disks should be used?
> > For both (native and external) metadata formats during reshape restart
> new raid_disks will be reported so 'after' filed should be used.
> > Patch for this will follow this post.
> >
> > BR
> > Adam
> >
>
> Thanks for testing and for the patch.
>
> However I think I do want to still test before.raid_disk... though it is
> all
> a bit messy.
>
> Anyway I've commited the patch like this...


Thanks, it's ok :)

BR
Adam


>
> Thanks.
> NeilBrown
>
> From 384e9be1330c29b40559f85dd0e6124bd0dfa535 Mon Sep 17 00:00:00 2001
> From: Adam Kwolek
> Date: Tue, 19 Apr 2011 17:25:43 +1000
> Subject: [PATCH] FIX: Check correctly raid disks during reshape restart
>
> During reshape restart info->array.raid_disks contains new raid_disks
> number
> It cannot be compared against old disks number. Such check will always
> fail.
>
> Check raid disks array field against final disks number for restart.
>
> Signed-off-by: Adam Kwolek
> Signed-off-by: NeilBrown
> ---
> Grow.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 9c1f096..9c63036 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1692,7 +1692,8 @@ static int reshape_array(char *container, int fd,
> char *devname,
> if (restart &&
> (reshape.level != info->array.level ||
> reshape.before.layout != info->array.layout ||
> - reshape.before.data_disks + reshape.parity != info-
> >array.raid_disks)) {
> + reshape.before.data_disks + reshape.parity
> + != info->array.raid_disks - info->delta_disks)) {
> fprintf(stderr, Name ": reshape info is not in native format
> -"
> " cannot continue.\n");
> goto release;
> --
> 1.7.3.4
>
>

--
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