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

[NeilBrown <neilb@suse.de>]

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