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

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

[Adam Kwolek]

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

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

[Adam Kwolek]

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 <krzysztof.wojcik@intel.com>
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

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

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

[NeilBrown]

On Mon, 10 Jan 2011 12:27:33 +0100 Adam Kwolek <adam.kwolek@intel.com> 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 <krzysztof.wojcik@intel.com>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---


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

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

[Adam Kwolek]

Any request for no size change has to be accepted.

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

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

[NeilBrown]

On Mon, 10 Jan 2011 12:27:42 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Any request for no size change has to be accepted.
> 
> 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 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

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

[Kwolek, Adam]

> -----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 <adam.kwolek@intel.com>
> wrote:
> 
> > Any request for no size change has to be accepted.
> >
> > 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 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 <adam.kwolek@intel.com>
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 <adam.kwolek@intel.com>
---
 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

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

[NeilBrown]

On Wed, 12 Jan 2011 08:32:10 +0000 "Kwolek, Adam" <adam.kwolek@intel.com>
wrote:


> This issue can be addressed in the following way:
> 
> >From ce0a720c162c425ae46684ac7866f7076e65d5e5 Mon Sep 17 00:00:00 2001
> From: Adam Kwolek <adam.kwolek@intel.com>
> 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 <adam.kwolek@intel.com>
> ---
>  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))

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

[Adam Kwolek]

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

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

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

[NeilBrown]

On Mon, 10 Jan 2011 12:27:50 +0100 Adam Kwolek <adam.kwolek@intel.com> 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 <adam.kwolek@intel.com>
> ---
> 
>  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

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

[Adam Kwolek]

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

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

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

[NeilBrown]

On Mon, 10 Jan 2011 12:27:58 +0100 Adam Kwolek <adam.kwolek@intel.com> 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 <adam.kwolek@intel.com>

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

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

[Adam Kwolek]

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

 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

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

[Adam Kwolek]

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

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

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

[NeilBrown]

On Mon, 10 Jan 2011 12:28:15 +0100 Adam Kwolek <adam.kwolek@intel.com> 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 <adam.kwolek@intel.com>
> ---
> 
>  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 <adam.kwolek@intel.com>
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 <adam.kwolek@intel.com>
    Signed-off-by: NeilBrown <neilb@suse.de>

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

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

[Adam Kwolek]

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

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

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

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

[Adam Kwolek]

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

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

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

[NeilBrown]

On Mon, 10 Jan 2011 12:28:32 +0100 Adam Kwolek <adam.kwolek@intel.com> 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 <adam.kwolek@intel.com>
> ---
> 
>  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

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

[Adam Kwolek]

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

 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

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

[NeilBrown]

On Mon, 10 Jan 2011 12:28:40 +0100 Adam Kwolek <adam.kwolek@intel.com> 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 <adam.kwolek@intel.com>

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

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

[Adam Kwolek]

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

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

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

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

[NeilBrown]

On Mon, 10 Jan 2011 12:28:48 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

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

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

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

[Kwolek, Adam]

> -----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 <adam.kwolek@intel.com>
> wrote:
> 
> > After reshape array remains blocked (i.e. cannot be umounted) due to
> suspend_lo
> > sysfs entry setting.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> 
> 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);
> >  			}
> >  		}

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

[Adam Kwolek]

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

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

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

[Adam Kwolek]

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

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 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,

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

[NeilBrown]

On Mon, 10 Jan 2011 12:29:04 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> For raid0 reshape spares should not be increased by parity disk.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  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 <neilb@suse.de>
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 <adam.kwolek@intel.com>
    Signed-off-by: NeilBrown <neilb@suse.de>

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"

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

[Adam Kwolek]

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

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

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

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

[NeilBrown]

On Mon, 10 Jan 2011 12:29:12 +0100 Adam Kwolek <adam.kwolek@intel.com> 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 <adam.kwolek@intel.com>

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