[PATCH 0/4] OLCE continue reshape for all arrays in container (rebased)

[PATCH 0/4] OLCE continue reshape for all arrays in container (rebased)

[Adam Kwolek]

Hi,
Thank you for new code base, I've re-based OLCE to this version and changed reshape continuation method as you mention.
Finish of reshape of first array triggers changes in metadata for next array in set_array_state().
This simplifies Grow.c changes. The only one bug is fixed in series for multi-array case when first reshaped array is raid0
(second is monitored array).

I'm resending patch for adding spares problem also (as you not commented it previously), and adding workaraound 
for check-pointing as reminder only.

BR
Adam


---

Adam Kwolek (4):
      WORKAROUND: mdadm hangs during reshape
      FIX: monitor doesn't handshake with md
      imsm: FIX: spare cannot be added
      imsm: Update metadata for second array


 Grow.c        |    7 +++++--
 super-intel.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 60 insertions(+), 6 deletions(-)

-- 
Signature

[PATCH 1/4] imsm: Update metadata for second array

[Adam Kwolek]

When second array reshape is about to start external metadata should be updated
by mdmon in imsm_set_array_state().
for this purposes imsm_progress_container_reshape() is reused.

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

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

diff --git a/super-intel.c b/super-intel.c
index d86e8d8..7f907ef 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -4863,7 +4863,9 @@ static void handle_missing(struct intel_super *super, struct imsm_dev *dev)
 
 static void imsm_set_disk(struct active_array *a, int n, int state);
 
-static void imsm_progress_container_reshape(struct intel_super *super)
+static void imsm_progress_container_reshape(struct intel_super *super,
+					    struct imsm_dev *dev_in,
+					    int disk_count)
 {
 	/* if no device has a migr_state, but some device has a
 	 * different number of members than the previous device, start
@@ -4871,7 +4873,7 @@ static void imsm_progress_container_reshape(struct intel_super *super)
 	 * previous.
 	 */
 	struct imsm_super *mpb = super->anchor;
-	int prev_disks = -1;
+	int prev_disks = disk_count;
 	int i;
 
 	for (i = 0; i < mpb->num_raid_devs; i++) {
@@ -4884,6 +4886,10 @@ static void imsm_progress_container_reshape(struct intel_super *super)
 		if (dev->vol.migr_state)
 			return;
 
+		if (((dev_in != NULL) && disk_count > 0) &&
+		     (dev_in != dev))
+			continue;
+
 		if (prev_disks == -1)
 			prev_disks = map->num_members;
 		if (prev_disks == map->num_members)
@@ -4999,7 +5005,9 @@ static int imsm_set_array_state(struct active_array *a, int consistent)
 						       * array size
 						       */
 				super->updates_pending++;
-				imsm_progress_container_reshape(super);
+				imsm_progress_container_reshape(super,
+								NULL,
+								-1);
 			}				
 		}
 	}
@@ -5065,13 +5073,47 @@ static int imsm_set_array_state(struct active_array *a, int consistent)
 		super->updates_pending++;
 	}
 
-	/* finalize online capacity expansion/reshape */
 	if ((a->curr_action != reshape) &&
 	    (a->prev_action == reshape)) {
 		struct mdinfo *mdi;
+		struct imsm_dev *dev_next;
+		int next_inst;
 
+
+		/* finalize online capacity expansion/reshape */
 		for (mdi = a->info.devs; mdi; mdi = mdi->next)
 			imsm_set_disk(a, mdi->disk.raid_disk, mdi->curr_state);
+
+		/* check next volume reshape
+		 * 2 volumes support only
+		 * for more volumes supported next_inst should be found
+		 * in another way
+		 */
+		next_inst = inst ? 0 : 1;
+		dev_next = get_imsm_dev(super, next_inst);
+		if (dev_next && dev_next->vol.migr_state == 0) {
+			/* check here if this is container action
+			* and next metadata should be prepared
+			*/
+			struct imsm_map *map_next = get_imsm_map(dev_next, 0);
+			struct dl *dl = NULL;
+			int disks_count = 0;
+
+			/* check if this array should be reshaped
+			*/
+			for (dl = super->disks; dl; dl = dl->next)
+				if (dl->index >= 0)
+					disks_count++;
+
+			if (disks_count > map_next->num_members) {
+				/* manage changes in volume
+				*/
+				imsm_progress_container_reshape(super,
+							dev_next,
+							disks_count);
+				super->updates_pending++;
+			}
+		}
 	}
 
 	return consistent;

Re: [PATCH 1/4] imsm: Update metadata for second array

[Neil Brown]

On Thu, 20 Jan 2011 11:57:22 +0100
Adam Kwolek <adam.kwolek@intel.com> wrote:

> When second array reshape is about to start external metadata should
> be updated by mdmon in imsm_set_array_state().
> for this purposes imsm_progress_container_reshape() is reused.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |   50
> ++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed,
> 46 insertions(+), 4 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index d86e8d8..7f907ef 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -4863,7 +4863,9 @@ static void handle_missing(struct intel_super
> *super, struct imsm_dev *dev) 
>  static void imsm_set_disk(struct active_array *a, int n, int state);
>  
> -static void imsm_progress_container_reshape(struct intel_super
> *super) +static void imsm_progress_container_reshape(struct
> intel_super *super,
> +					    struct imsm_dev *dev_in,
> +					    int disk_count)
>  {


imsm_progress_container_reshape does not need to be passed a dev and a
count.

It finds the first devices that has a different number of disks than
the previous device, and changes it to have the same number.  
If there is some reason that this is not sufficient, please be more 
explicit.

Thanks,
NeilBrown



>  	/* if no device has a migr_state, but some
device has a
>  	 * different number of members than the previous device,
> start @@ -4871,7 +4873,7 @@ static void
> imsm_progress_container_reshape(struct intel_super *super)
>  	 * previous.
>  	 */
>  	struct imsm_super *mpb = super->anchor;
> -	int prev_disks = -1;
> +	int prev_disks = disk_count;
>  	int i;
>  
>  	for (i = 0; i < mpb->num_raid_devs; i++) {
> @@ -4884,6 +4886,10 @@ static void
> imsm_progress_container_reshape(struct intel_super *super) if
> (dev->vol.migr_state) return;
>  
> +		if (((dev_in != NULL) && disk_count > 0) &&
> +		     (dev_in != dev))
> +			continue;
> +
>  		if (prev_disks == -1)
>  			prev_disks = map->num_members;
>  		if (prev_disks == map->num_members)
> @@ -4999,7 +5005,9 @@ static int imsm_set_array_state(struct
> active_array *a, int consistent)
>  						       * array size
>  						       */
>  				super->updates_pending++;
> -
> imsm_progress_container_reshape(super);
> +
> imsm_progress_container_reshape(super,
> +								NULL,
> +								-1);
>  			}				
>  		}
>  	}
> @@ -5065,13 +5073,47 @@ static int imsm_set_array_state(struct
> active_array *a, int consistent) super->updates_pending++;
>  	}
>  
> -	/* finalize online capacity expansion/reshape */
>  	if ((a->curr_action != reshape) &&
>  	    (a->prev_action == reshape)) {
>  		struct mdinfo *mdi;
> +		struct imsm_dev *dev_next;
> +		int next_inst;
>  
> +
> +		/* finalize online capacity expansion/reshape */
>  		for (mdi = a->info.devs; mdi; mdi = mdi->next)
>  			imsm_set_disk(a, mdi->disk.raid_disk,
> mdi->curr_state); +
> +		/* check next volume reshape
> +		 * 2 volumes support only
> +		 * for more volumes supported next_inst should be
> found
> +		 * in another way
> +		 */
> +		next_inst = inst ? 0 : 1;
> +		dev_next = get_imsm_dev(super, next_inst);
> +		if (dev_next && dev_next->vol.migr_state == 0) {
> +			/* check here if this is container action
> +			* and next metadata should be prepared
> +			*/
> +			struct imsm_map *map_next =
> get_imsm_map(dev_next, 0);
> +			struct dl *dl = NULL;
> +			int disks_count = 0;
> +
> +			/* check if this array should be reshaped
> +			*/
> +			for (dl = super->disks; dl; dl = dl->next)
> +				if (dl->index >= 0)
> +					disks_count++;
> +
> +			if (disks_count > map_next->num_members) {
> +				/* manage changes in volume
> +				*/
> +
> imsm_progress_container_reshape(super,
> +							dev_next,
> +							disks_count);
> +				super->updates_pending++;
> +			}
> +		}
>  	}
>  
>  	return consistent;

[PATCH 2/4] imsm: FIX: spare cannot be added

[Adam Kwolek]

When Manage.c adds spare, it calls write_init_super() and this function
is responsible for closing disk handle (Manage.c:812).
For imsm case this handle is reused for managing this disk and handle
(due to this it is not closed).
As handle was opened with flag O_EXCL, adding disk to md fails on writing
to new_dev (md cannot set lock on device).
To resolve situation close current handle and open new one without O_EXCL flag.

Signed-off-by: Anna Czarnowska <anna.czarnowska@intel.com>
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

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

diff --git a/super-intel.c b/super-intel.c
index 7f907ef..0d6d2ee 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -3714,6 +3714,7 @@ static int mgmt_disk(struct supertype *st)
 	struct intel_super *super = st->sb;
 	size_t len;
 	struct imsm_update_add_remove_disk *u;
+	struct dl *d;
 
 	if (!super->disk_mgmt_list)
 		return 0;
@@ -3729,6 +3730,14 @@ static int mgmt_disk(struct supertype *st)
 	u->type = update_add_remove_disk;
 	append_metadata_update(st, u, len);
 
+	for (d = super->disk_mgmt_list; d ; d = d->next) {
+		char buf[PATH_MAX];
+
+		close(d->fd);
+		sprintf(buf, "%d:%d", d->major, d->minor);
+		d->fd = dev_open(buf, O_RDWR | O_DIRECT);
+	}
+
 	return 0;
 }
 

Re: [PATCH 2/4] imsm: FIX: spare cannot be added

[Neil Brown]

On Thu, 20 Jan 2011 11:57:30 +0100
Adam Kwolek <adam.kwolek@intel.com> wrote:

> When Manage.c adds spare, it calls write_init_super() and this
> function is responsible for closing disk handle (Manage.c:812).
> For imsm case this handle is reused for managing this disk and handle
> (due to this it is not closed).
> As handle was opened with flag O_EXCL, adding disk to md fails on
> writing to new_dev (md cannot set lock on device).
> To resolve situation close current handle and open new one without
> O_EXCL flag.

Thanks.
I have created a different solution. write_init_super doesn't close
anything any more. 
Hopefully the new approach works better.

Thanks,
NeilBrown

> 
> Signed-off-by: Anna Czarnowska <anna.czarnowska@intel.com>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index 7f907ef..0d6d2ee 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -3714,6 +3714,7 @@ static int mgmt_disk(struct supertype *st)
>  	struct intel_super *super = st->sb;
>  	size_t len;
>  	struct imsm_update_add_remove_disk *u;
> +	struct dl *d;
>  
>  	if (!super->disk_mgmt_list)
>  		return 0;
> @@ -3729,6 +3730,14 @@ static int mgmt_disk(struct supertype *st)
>  	u->type = update_add_remove_disk;
>  	append_metadata_update(st, u, len);
>  
> +	for (d = super->disk_mgmt_list; d ; d = d->next) {
> +		char buf[PATH_MAX];
> +
> +		close(d->fd);
> +		sprintf(buf, "%d:%d", d->major, d->minor);
> +		d->fd = dev_open(buf, O_RDWR | O_DIRECT);
> +	}
> +
>  	return 0;
>  }
>  

RE: [PATCH 2/4] imsm: FIX: spare cannot be added

[Kwolek, Adam]

> -----Original Message-----
> From: Neil Brown [mailto:neilb@suse.de]
> Sent: Thursday, January 27, 2011 4:03 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 2/4] imsm: FIX: spare cannot be added
> 
> On Thu, 20 Jan 2011 11:57:30 +0100
> Adam Kwolek <adam.kwolek@intel.com> wrote:
> 
> > When Manage.c adds spare, it calls write_init_super() and this
> > function is responsible for closing disk handle (Manage.c:812).
> > For imsm case this handle is reused for managing this disk and handle
> > (due to this it is not closed).
> > As handle was opened with flag O_EXCL, adding disk to md fails on
> > writing to new_dev (md cannot set lock on device).
> > To resolve situation close current handle and open new one without
> > O_EXCL flag.
> 
> Thanks.
> I have created a different solution. write_init_super doesn't close
> anything any more.
> Hopefully the new approach works better.
> 
> Thanks,
> NeilBrown

I found the same issue in ne code yesterday, so I've resent this patch (reminder for myself ;))
I'll have a closer look on this problem next week.

BR
Adam

> >
> > Signed-off-by: Anna Czarnowska <anna.czarnowska@intel.com>
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> >  super-intel.c |    9 +++++++++
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> >
> > diff --git a/super-intel.c b/super-intel.c
> > index 7f907ef..0d6d2ee 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -3714,6 +3714,7 @@ static int mgmt_disk(struct supertype *st)
> >  	struct intel_super *super = st->sb;
> >  	size_t len;
> >  	struct imsm_update_add_remove_disk *u;
> > +	struct dl *d;
> >
> >  	if (!super->disk_mgmt_list)
> >  		return 0;
> > @@ -3729,6 +3730,14 @@ static int mgmt_disk(struct supertype *st)
> >  	u->type = update_add_remove_disk;
> >  	append_metadata_update(st, u, len);
> >
> > +	for (d = super->disk_mgmt_list; d ; d = d->next) {
> > +		char buf[PATH_MAX];
> > +
> > +		close(d->fd);
> > +		sprintf(buf, "%d:%d", d->major, d->minor);
> > +		d->fd = dev_open(buf, O_RDWR | O_DIRECT);
> > +	}
> > +
> >  	return 0;
> >  }
> >

[PATCH 3/4] FIX: monitor doesn't handshake with md

[Adam Kwolek]

when in container are present raid0 and raid5 arrays, and reshape order is:
1. raid0 array
2. raid5 array

mdadm cannot set new raid_disks for raid0 array. For this action md has to have
handshake with mdmon. We have the following conditions:
1. Raid0 is not monitored
2. raid0 has been just takeovered to raid4/5 (it has to be monitored
3. monitor has to start monitor new raid4/5 array
4. monitor is not started (it is started to second raid5 array)
In such situation pig_monitor is required to let know to m monitor about new array
(not in the starting monitor case only)

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 23ef88e..717d087 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1662,8 +1662,8 @@ static int reshape_array(char *container, int fd, char *devname,
 	if (reshape.level > 0 && st->ss->external &&
 	    !mdmon_running(st->container_dev)) {
 		start_mdmon(st->container_dev);
-		ping_monitor(container);
 	}
+	ping_monitor(container);
 
 	/* ->reshape_super might have chosen some spares from the
 	 * container that it wants to be part of the new array.

Re: [PATCH 3/4] FIX: monitor doesn't handshake with md

[Neil Brown]

On Thu, 20 Jan 2011 11:57:38 +0100
Adam Kwolek <adam.kwolek@intel.com> wrote:

> when in container are present raid0 and raid5 arrays, and reshape
> order is: 1. raid0 array
> 2. raid5 array
> 
> mdadm cannot set new raid_disks for raid0 array. For this action md
> has to have handshake with mdmon. We have the following conditions:
> 1. Raid0 is not monitored
> 2. raid0 has been just takeovered to raid4/5 (it has to be monitored
> 3. monitor has to start monitor new raid4/5 array
> 4. monitor is not started (it is started to second raid5 array)
> In such situation pig_monitor is required to let know to m monitor
> about new array (not in the starting monitor case only)
> 
> 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 23ef88e..717d087 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1662,8 +1662,8 @@ static int reshape_array(char *container, int
> fd, char *devname, if (reshape.level > 0 && st->ss->external &&
>  	    !mdmon_running(st->container_dev)) {
>  		start_mdmon(st->container_dev);
> -		ping_monitor(container);
>  	}
> +	ping_monitor(container);
>

I see the need, but calling ping_monitor out side the test for
->external is wrong.
I've re-written the patch a bit so it should be safer and more general.

Thanks,
NeilBrown

RE: [PATCH 3/4] FIX: monitor doesn't handshake with md

[Kwolek, Adam]

> -----Original Message-----
> From: Neil Brown [mailto:neilb@suse.de]
> Sent: Thursday, January 27, 2011 4:04 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 3/4] FIX: monitor doesn't handshake with md
> 
> On Thu, 20 Jan 2011 11:57:38 +0100
> Adam Kwolek <adam.kwolek@intel.com> wrote:
> 
> > when in container are present raid0 and raid5 arrays, and reshape
> > order is: 1. raid0 array
> > 2. raid5 array
> >
> > mdadm cannot set new raid_disks for raid0 array. For this action md
> > has to have handshake with mdmon. We have the following conditions:
> > 1. Raid0 is not monitored
> > 2. raid0 has been just takeovered to raid4/5 (it has to be monitored
> > 3. monitor has to start monitor new raid4/5 array
> > 4. monitor is not started (it is started to second raid5 array)
> > In such situation pig_monitor is required to let know to m monitor
> > about new array (not in the starting monitor case only)
> >
> > 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 23ef88e..717d087 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -1662,8 +1662,8 @@ static int reshape_array(char *container, int
> > fd, char *devname, if (reshape.level > 0 && st->ss->external &&
> >  	    !mdmon_running(st->container_dev)) {
> >  		start_mdmon(st->container_dev);
> > -		ping_monitor(container);
> >  	}
> > +	ping_monitor(container);
> >
> 
> I see the need, but calling ping_monitor out side the test for
> ->external is wrong.
> I've re-written the patch a bit so it should be safer and more general.
>
> Thanks,
> NeilBrown

I've corrected this in last patch series also (yesterday).
BR
Adam

[PATCH 4/4] WORKAROUND: mdadm hangs during reshape

[Adam Kwolek]

During reshape when reshape is finished in md, progress_reshape() hangs
on select(). Timeout is introduced to allow for reshape conditions check.

You point in the email that you do not like this solution.
It is put in to series as reminder.

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

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

diff --git a/Grow.c b/Grow.c
index 717d087..c9f4b9c 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2394,6 +2394,7 @@ int progress_reshape(struct mdinfo *info, struct reshape *reshape,
 		 * waiting forever on a dead array
 		 */
 		char action[20];
+		struct timeval timeout;
 		fd_set rfds;
 		if (sysfs_get_str(info, NULL, "sync_action",
 				  action, 20) <= 0 ||
@@ -2401,7 +2402,9 @@ int progress_reshape(struct mdinfo *info, struct reshape *reshape,
 			break;
 		FD_ZERO(&rfds);
 		FD_SET(fd, &rfds);
-		select(fd+1, NULL, NULL, &rfds, NULL);
+		timeout.tv_sec = 1;
+		timeout.tv_usec = 0;
+		select(fd+1, NULL, NULL, &rfds, &timeout);
 		if (sysfs_fd_get_ll(fd, &completed) < 0) {
 			close(fd);
 			goto check_progress;

Re: [PATCH 4/4] WORKAROUND: mdadm hangs during reshape

[Neil Brown]

On Thu, 20 Jan 2011 11:57:46 +0100
Adam Kwolek <adam.kwolek@intel.com> wrote:

> During reshape when reshape is finished in md, progress_reshape()
> hangs on select(). Timeout is introduced to allow for reshape
> conditions check.
> 
> You point in the email that you do not like this solution.
> It is put in to series as reminder.

Thanks for the reminder.  
I figured out what is really happening and wrote a better work-around.
You can find it in the git history - fully commented.

Thanks,
NeilBrown

> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 717d087..c9f4b9c 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2394,6 +2394,7 @@ int progress_reshape(struct mdinfo *info,
> struct reshape *reshape,
>  		 * waiting forever on a dead array
>  		 */
>  		char action[20];
> +		struct timeval timeout;
>  		fd_set rfds;
>  		if (sysfs_get_str(info, NULL, "sync_action",
>  				  action, 20) <= 0 ||
> @@ -2401,7 +2402,9 @@ int progress_reshape(struct mdinfo *info,
> struct reshape *reshape, break;
>  		FD_ZERO(&rfds);
>  		FD_SET(fd, &rfds);
> -		select(fd+1, NULL, NULL, &rfds, NULL);
> +		timeout.tv_sec = 1;
> +		timeout.tv_usec = 0;
> +		select(fd+1, NULL, NULL, &rfds, &timeout);
>  		if (sysfs_fd_get_ll(fd, &completed) < 0) {
>  			close(fd);
>  			goto check_progress;