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

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

[Adam Kwolek]

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

[PATCH 1/4] FIX: Always report new raid_disks during migration

[Adam Kwolek]

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 <adam.kwolek@intel.com>
---

 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;

[PATCH 2/4] FIX: Fiddle raid_disks number for external metadta

[Adam Kwolek]

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 <adam.kwolek@intel.com>
---

 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)

[PATCH 3/4] FIX: Set proper raid disks during migration

[Adam Kwolek]

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 <adam.kwolek@intel.com>
---

 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.
 		 */

[PATCH 4/4] FIX: Count correctly added devices

[Adam Kwolek]

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 <adam.kwolek@intel.com>
---

 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

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

[NeilBrown]

On Fri, 15 Apr 2011 14:30:22 +0200 Adam Kwolek <adam.kwolek@intel.com> 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 <adam.kwolek@intel.com>
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 <adam.kwolek@intel.com>
    Signed-off-by: NeilBrown <neilb@suse.de>

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

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

[Kwolek, Adam]

> -----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 <adam.kwolek@intel.com>
> 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 <adam.kwolek@intel.com>
> 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 <adam.kwolek@intel.com>
>     Signed-off-by: NeilBrown <neilb@suse.de>
> 
> 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);

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

[NeilBrown]

On Mon, 18 Apr 2011 11:52:12 +0100 "Kwolek, Adam" <adam.kwolek@intel.com>
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 <adam.kwolek@intel.com>
> > 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 <adam.kwolek@intel.com>
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 <adam.kwolek@intel.com>
Signed-off-by: NeilBrown <neilb@suse.de>
---
 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

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

[Kwolek, Adam]

> -----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"
> <adam.kwolek@intel.com>
> 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
> <adam.kwolek@intel.com>
> > > 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 <adam.kwolek@intel.com>
> 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 <adam.kwolek@intel.com>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  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
> 
>