[PATCH 0/3] Reshape restart after file system pivot (missing part)

[PATCH 0/3] Reshape restart after file system pivot (missing part)

[Adam Kwolek]

This patch series (3) adds missing part for reshape restart after file system
pivot during OS boot.
Patches makes changes to Grow_continue() execute it on already run arrays:

start_reshape() function is modified (in simpler way) to set correct
restart position and allow reshape process to be reported by mdstat:

Note: 
  This patch series requires: Remove freeze() call from Grow_continue()
  patch to be applied (sent yesterday).

BR
Adam

---

Adam Kwolek (3):
      Set correct reshape restart position
      Monitor reshaped array
      Always run Grow_continue() for started array.


 Assemble.c |   23 +++++++++++++++++++----
 Grow.c     |   60 ++++++++++++++++++++++++++++++------------------------------
 2 files changed, 49 insertions(+), 34 deletions(-)

-- 
Signature

[PATCH 1/3] Always run Grow_continue() for started array.

[Adam Kwolek]

So far there were 2 reshape continuation cases:
 1. array is started /e.g. reshape was already invoked during initrd
                      start-up stage using "--freeze-reshape" option/
 2. array is not started yet /"normal" assembling array under reshape case/

This patch narrows continuation cases in to single one. To do this
array should be started /set readonly in to array_state/ before calling
Grow_continue() function.

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

 Assemble.c |   17 +++++++++++++----
 Grow.c     |   43 +++++++++++++++++++------------------------
 2 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/Assemble.c b/Assemble.c
index 4511f4d..0d3730b 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1343,10 +1343,14 @@ int Assemble(struct supertype *st, char *mddev,
 			int rv;
 #ifndef MDASSEMBLE
 			if (content->reshape_active &&
-			    content->delta_disks <= 0)
-				rv = Grow_continue(mdfd, st, content,
-						   backup_file, freeze_reshape);
-			else
+			    content->delta_disks <= 0) {
+				rv = sysfs_set_str(content, NULL,
+						   "array_state", "readonly");
+				if (rv == 0)
+					rv = Grow_continue(mdfd, st, content,
+							   backup_file,
+							   freeze_reshape);
+			} else
 #endif
 				rv = ioctl(mdfd, RUN_ARRAY, NULL);
 			if (rv == 0) {
@@ -1561,6 +1565,11 @@ int assemble_container_content(struct supertype *st, int mdfd,
 					   spare, backup_file, verbose) == 1)
 				return 1;
 
+			err = sysfs_set_str(content, NULL,
+					    "array_state", "readonly");
+			if (err)
+				return 1;
+
 			err = Grow_continue(mdfd, st, content, backup_file,
 					    freeze_reshape);
 		} else switch(content->array.level) {
diff --git a/Grow.c b/Grow.c
index 0b58d5e..076375a 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3797,33 +3797,28 @@ Grow_continue_command_exit:
 int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
 		  char *backup_file, int freeze_reshape)
 {
-	char buf[40];
-	char *container = NULL;
-	int err;
+	int ret_val = 2;
+
+	if (!info->reshape_active)
+		return ret_val;
 
-	err = sysfs_set_str(info, NULL, "array_state", "readonly");
-	if (err)
-		return err;
 	if (st->ss->external) {
-		fmt_devname(buf, st->container_dev);
-		container = buf;
+		char container[40];
+		int cfd = open_dev(st->container_dev);
 
-		if (!mdmon_running(st->container_dev))
-			start_mdmon(st->container_dev);
-		ping_monitor_by_id(st->container_dev);
+		if (cfd < 0)
+			return 1;
 
+		fmt_devname(container, st->container_dev);
+		st->ss->load_container(st, cfd, container);
+		close(cfd);
+		ret_val = reshape_container(container, NULL,
+					    st, info, 0, backup_file,
+					    0, 1, freeze_reshape);
+	} else
+		ret_val = reshape_array(NULL, mdfd, "array", st, info, 1,
+					NULL, backup_file, 0, 0, 1,
+					freeze_reshape);
 
-		if (info->reshape_active == 2) {
-			int cfd = open_dev(st->container_dev);
-			if (cfd < 0)
-				return 1;
-			st->ss->load_container(st, cfd, container);
-			close(cfd);
-			return reshape_container(container, NULL,
-						 st, info, 0, backup_file,
-						 0, 1, freeze_reshape);
-		}
-	}
-	return reshape_array(container, mdfd, "array", st, info, 1,
-			     NULL, backup_file, 0, 0, 1, freeze_reshape);
+	return ret_val;
 }

[PATCH 2/3] Monitor reshaped array

[Adam Kwolek]

Reshape can be run for monitored arrays only /external metadata case/.
Before reshape can be executed, make sure that just starter array/container
is monitored. If not, run mdmon for it.

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

 Assemble.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Assemble.c b/Assemble.c
index 0d3730b..2000dd0 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1570,6 +1570,12 @@ int assemble_container_content(struct supertype *st, int mdfd,
 			if (err)
 				return 1;
 
+			if (st->ss->external) {
+				if (!mdmon_running(st->container_dev))
+					start_mdmon(st->container_dev);
+				ping_monitor_by_id(st->container_dev);
+			}
+
 			err = Grow_continue(mdfd, st, content, backup_file,
 					    freeze_reshape);
 		} else switch(content->array.level) {

[PATCH 3/3] Set correct reshape restart position

[Adam Kwolek]

This patch version is simplified compared to previous one.
There is no use of freeze_reshape flag in start_reshape(). It is assumed
that for reshape starting condition reshape_progress field contains
0 value /correct start position/. For reshape restart case, it contains
correct restart position. This approach doesn't make start_reshape()
difficult to read/manage and /imho/ kernel changes to change mdstat
reporting behavior are not necessary.

Setting correct position allows user to see it in the mdstat during
reshape restart and reshape process is not reported as resync.

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

 Grow.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/Grow.c b/Grow.c
index 076375a..44505b3 100644
--- a/Grow.c
+++ b/Grow.c
@@ -696,15 +696,19 @@ static int subarray_set_num(char *container, struct mdinfo *sra, char *name, int
 	return rc;
 }
 
-int start_reshape(struct mdinfo *sra, int already_running)
+int start_reshape(struct mdinfo *sra, int already_running, int data_disks)
 {
 	int err;
+	unsigned long long sync_max_to_set;
+
 	sysfs_set_num(sra, NULL, "suspend_lo", 0x7FFFFFFFFFFFFFFFULL);
-	err = sysfs_set_num(sra, NULL, "suspend_hi", 0);
-	err = err ?: sysfs_set_num(sra, NULL, "suspend_lo", 0);
+	err = sysfs_set_num(sra, NULL, "suspend_hi", sra->reshape_progress);
+	err = err ?: sysfs_set_num(sra, NULL, "suspend_lo",
+				   sra->reshape_progress);
+	sync_max_to_set = sra->reshape_progress / data_disks;
 	if (!already_running)
-		sysfs_set_num(sra, NULL, "sync_min", 0);
-	err = err ?: sysfs_set_num(sra, NULL, "sync_max", 0);
+		sysfs_set_num(sra, NULL, "sync_min", sync_max_to_set);
+	err = err ?: sysfs_set_num(sra, NULL, "sync_max", sync_max_to_set);
 	if (!already_running)
 		err = err ?: sysfs_set_str(sra, NULL, "sync_action", "reshape");
 
@@ -2241,7 +2245,8 @@ started:
 		}
 	}
 
-	err = start_reshape(sra, restart);
+	err = start_reshape(sra, restart,
+			    info->array.raid_disks - reshape.parity);
 	if (err) {
 		fprintf(stderr, 
 			Name ": Cannot %s reshape for %s\n",

Re: [PATCH 1/3] Always run Grow_continue() for started array.

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 4310 bytes --]

On Tue, 04 Oct 2011 17:53:49 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> So far there were 2 reshape continuation cases:
>  1. array is started /e.g. reshape was already invoked during initrd
>                       start-up stage using "--freeze-reshape" option/
>  2. array is not started yet /"normal" assembling array under reshape case/
> 
> This patch narrows continuation cases in to single one. To do this
> array should be started /set readonly in to array_state/ before calling
> Grow_continue() function.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>

I mostly like this patch.
However it seems to have lost something in Grow_continue.
The distinction between ->reshape_active==1 and ->reshape_active==2
is lost.

==1 means just this array is in the middle of a reshape.
==2 means that the whole container is being reshaped and after this array
is done we need to move on to the next one.

Was there are reason you removed that?  If not, please put it back.

Thanks,
NeilBrown


> ---
> 
>  Assemble.c |   17 +++++++++++++----
>  Grow.c     |   43 +++++++++++++++++++------------------------
>  2 files changed, 32 insertions(+), 28 deletions(-)
> 
> diff --git a/Assemble.c b/Assemble.c
> index 4511f4d..0d3730b 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1343,10 +1343,14 @@ int Assemble(struct supertype *st, char *mddev,
>  			int rv;
>  #ifndef MDASSEMBLE
>  			if (content->reshape_active &&
> -			    content->delta_disks <= 0)
> -				rv = Grow_continue(mdfd, st, content,
> -						   backup_file, freeze_reshape);
> -			else
> +			    content->delta_disks <= 0) {
> +				rv = sysfs_set_str(content, NULL,
> +						   "array_state", "readonly");
> +				if (rv == 0)
> +					rv = Grow_continue(mdfd, st, content,
> +							   backup_file,
> +							   freeze_reshape);
> +			} else
>  #endif
>  				rv = ioctl(mdfd, RUN_ARRAY, NULL);
>  			if (rv == 0) {
> @@ -1561,6 +1565,11 @@ int assemble_container_content(struct supertype *st, int mdfd,
>  					   spare, backup_file, verbose) == 1)
>  				return 1;
>  
> +			err = sysfs_set_str(content, NULL,
> +					    "array_state", "readonly");
> +			if (err)
> +				return 1;
> +
>  			err = Grow_continue(mdfd, st, content, backup_file,
>  					    freeze_reshape);
>  		} else switch(content->array.level) {
> diff --git a/Grow.c b/Grow.c
> index 0b58d5e..076375a 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -3797,33 +3797,28 @@ Grow_continue_command_exit:
>  int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
>  		  char *backup_file, int freeze_reshape)
>  {
> -	char buf[40];
> -	char *container = NULL;
> -	int err;
> +	int ret_val = 2;
> +
> +	if (!info->reshape_active)
> +		return ret_val;
>  
> -	err = sysfs_set_str(info, NULL, "array_state", "readonly");
> -	if (err)
> -		return err;
>  	if (st->ss->external) {
> -		fmt_devname(buf, st->container_dev);
> -		container = buf;
> +		char container[40];
> +		int cfd = open_dev(st->container_dev);
>  
> -		if (!mdmon_running(st->container_dev))
> -			start_mdmon(st->container_dev);
> -		ping_monitor_by_id(st->container_dev);
> +		if (cfd < 0)
> +			return 1;
>  
> +		fmt_devname(container, st->container_dev);
> +		st->ss->load_container(st, cfd, container);
> +		close(cfd);
> +		ret_val = reshape_container(container, NULL,
> +					    st, info, 0, backup_file,
> +					    0, 1, freeze_reshape);
> +	} else
> +		ret_val = reshape_array(NULL, mdfd, "array", st, info, 1,
> +					NULL, backup_file, 0, 0, 1,
> +					freeze_reshape);
>  
> -		if (info->reshape_active == 2) {
> -			int cfd = open_dev(st->container_dev);
> -			if (cfd < 0)
> -				return 1;
> -			st->ss->load_container(st, cfd, container);
> -			close(cfd);
> -			return reshape_container(container, NULL,
> -						 st, info, 0, backup_file,
> -						 0, 1, freeze_reshape);
> -		}
> -	}
> -	return reshape_array(container, mdfd, "array", st, info, 1,
> -			     NULL, backup_file, 0, 0, 1, freeze_reshape);
> +	return ret_val;
>  }
> 
> --
> 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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH 2/3] Monitor reshaped array

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

On Tue, 04 Oct 2011 17:53:56 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Reshape can be run for monitored arrays only /external metadata case/.
> Before reshape can be executed, make sure that just starter array/container
> is monitored. If not, run mdmon for it.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Assemble.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/Assemble.c b/Assemble.c
> index 0d3730b..2000dd0 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1570,6 +1570,12 @@ int assemble_container_content(struct supertype *st, int mdfd,
>  			if (err)
>  				return 1;
>  
> +			if (st->ss->external) {
> +				if (!mdmon_running(st->container_dev))
> +					start_mdmon(st->container_dev);
> +				ping_monitor_by_id(st->container_dev);
> +			}
> +
>  			err = Grow_continue(mdfd, st, content, backup_file,
>  					    freeze_reshape);
>  		} else switch(content->array.level) {
> 
> --
> 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

Applied, thanks.

NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH 3/3] Set correct reshape restart position

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2377 bytes --]

On Tue, 04 Oct 2011 17:54:04 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> This patch version is simplified compared to previous one.
> There is no use of freeze_reshape flag in start_reshape(). It is assumed
> that for reshape starting condition reshape_progress field contains
> 0 value /correct start position/. For reshape restart case, it contains
> correct restart position. This approach doesn't make start_reshape()
> difficult to read/manage and /imho/ kernel changes to change mdstat
> reporting behavior are not necessary.
> 
> Setting correct position allows user to see it in the mdstat during
> reshape restart and reshape process is not reported as resync.

Applied, thanks.

NeilBrown


> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 076375a..44505b3 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -696,15 +696,19 @@ static int subarray_set_num(char *container, struct mdinfo *sra, char *name, int
>  	return rc;
>  }
>  
> -int start_reshape(struct mdinfo *sra, int already_running)
> +int start_reshape(struct mdinfo *sra, int already_running, int data_disks)
>  {
>  	int err;
> +	unsigned long long sync_max_to_set;
> +
>  	sysfs_set_num(sra, NULL, "suspend_lo", 0x7FFFFFFFFFFFFFFFULL);
> -	err = sysfs_set_num(sra, NULL, "suspend_hi", 0);
> -	err = err ?: sysfs_set_num(sra, NULL, "suspend_lo", 0);
> +	err = sysfs_set_num(sra, NULL, "suspend_hi", sra->reshape_progress);
> +	err = err ?: sysfs_set_num(sra, NULL, "suspend_lo",
> +				   sra->reshape_progress);
> +	sync_max_to_set = sra->reshape_progress / data_disks;
>  	if (!already_running)
> -		sysfs_set_num(sra, NULL, "sync_min", 0);
> -	err = err ?: sysfs_set_num(sra, NULL, "sync_max", 0);
> +		sysfs_set_num(sra, NULL, "sync_min", sync_max_to_set);
> +	err = err ?: sysfs_set_num(sra, NULL, "sync_max", sync_max_to_set);
>  	if (!already_running)
>  		err = err ?: sysfs_set_str(sra, NULL, "sync_action", "reshape");
>  
> @@ -2241,7 +2245,8 @@ started:
>  		}
>  	}
>  
> -	err = start_reshape(sra, restart);
> +	err = start_reshape(sra, restart,
> +			    info->array.raid_disks - reshape.parity);
>  	if (err) {
>  		fprintf(stderr, 
>  			Name ": Cannot %s reshape for %s\n",


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

RE: [PATCH 1/3] Always run Grow_continue() for started array.

[Kwolek, Adam]

> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> owner@vger.kernel.org] On Behalf Of NeilBrown
> Sent: Wednesday, October 05, 2011 4:59 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin; Williams,
> Dan J
> Subject: Re: [PATCH 1/3] Always run Grow_continue() for started array.
> 
> On Tue, 04 Oct 2011 17:53:49 +0200 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > So far there were 2 reshape continuation cases:
> >  1. array is started /e.g. reshape was already invoked during initrd
> >                       start-up stage using "--freeze-reshape" option/
> > 2. array is not started yet /"normal" assembling array under reshape
> > case/
> >
> > This patch narrows continuation cases in to single one. To do this
> > array should be started /set readonly in to array_state/ before
> > calling
> > Grow_continue() function.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> 
> I mostly like this patch.
> However it seems to have lost something in Grow_continue.
> The distinction between ->reshape_active==1 and ->reshape_active==2 is
> lost.
> 
> ==1 means just this array is in the middle of a reshape.
> ==2 means that the whole container is being reshaped and after this array is
> done we need to move on to the next one.
> 
> Was there are reason you removed that?  If not, please put it back.

In both cases metadata is updated already and container _reshape() goes (somehow) directly to reshape array.
In container reshape case mdmon moves changes to next array for reshape and container_reshape() will continue with next one.

In case that single array is reshaped only mdmon finalizes reshape and container_reshape() finds no more arrays under reshape,
So this would means end of work also.

In my opinion during reshape continuation we do not need to know if this is container or single array reshape,
Both cases can be handled by container_reshape() /external metadata/ and we do not need to track what kind of reshape occurs.

Let me know your opinion.

BR
Adam

> 
> Thanks,
> NeilBrown
> 
> 
> > ---
> >
> >  Assemble.c |   17 +++++++++++++----
> >  Grow.c     |   43 +++++++++++++++++++------------------------
> >  2 files changed, 32 insertions(+), 28 deletions(-)
> >
> > diff --git a/Assemble.c b/Assemble.c
> > index 4511f4d..0d3730b 100644
> > --- a/Assemble.c
> > +++ b/Assemble.c
> > @@ -1343,10 +1343,14 @@ int Assemble(struct supertype *st, char
> *mddev,
> >  			int rv;
> >  #ifndef MDASSEMBLE
> >  			if (content->reshape_active &&
> > -			    content->delta_disks <= 0)
> > -				rv = Grow_continue(mdfd, st, content,
> > -						   backup_file,
> freeze_reshape);
> > -			else
> > +			    content->delta_disks <= 0) {
> > +				rv = sysfs_set_str(content, NULL,
> > +						   "array_state", "readonly");
> > +				if (rv == 0)
> > +					rv = Grow_continue(mdfd, st,
> content,
> > +							   backup_file,
> > +							   freeze_reshape);
> > +			} else
> >  #endif
> >  				rv = ioctl(mdfd, RUN_ARRAY, NULL);
> >  			if (rv == 0) {
> > @@ -1561,6 +1565,11 @@ int assemble_container_content(struct
> supertype *st, int mdfd,
> >  					   spare, backup_file, verbose) == 1)
> >  				return 1;
> >
> > +			err = sysfs_set_str(content, NULL,
> > +					    "array_state", "readonly");
> > +			if (err)
> > +				return 1;
> > +
> >  			err = Grow_continue(mdfd, st, content, backup_file,
> >  					    freeze_reshape);
> >  		} else switch(content->array.level) { diff --git a/Grow.c
> b/Grow.c
> > index 0b58d5e..076375a 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -3797,33 +3797,28 @@ Grow_continue_command_exit:
> >  int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
> >  		  char *backup_file, int freeze_reshape)  {
> > -	char buf[40];
> > -	char *container = NULL;
> > -	int err;
> > +	int ret_val = 2;
> > +
> > +	if (!info->reshape_active)
> > +		return ret_val;
> >
> > -	err = sysfs_set_str(info, NULL, "array_state", "readonly");
> > -	if (err)
> > -		return err;
> >  	if (st->ss->external) {
> > -		fmt_devname(buf, st->container_dev);
> > -		container = buf;
> > +		char container[40];
> > +		int cfd = open_dev(st->container_dev);
> >
> > -		if (!mdmon_running(st->container_dev))
> > -			start_mdmon(st->container_dev);
> > -		ping_monitor_by_id(st->container_dev);
> > +		if (cfd < 0)
> > +			return 1;
> >
> > +		fmt_devname(container, st->container_dev);
> > +		st->ss->load_container(st, cfd, container);
> > +		close(cfd);
> > +		ret_val = reshape_container(container, NULL,
> > +					    st, info, 0, backup_file,
> > +					    0, 1, freeze_reshape);
> > +	} else
> > +		ret_val = reshape_array(NULL, mdfd, "array", st, info, 1,
> > +					NULL, backup_file, 0, 0, 1,
> > +					freeze_reshape);
> >
> > -		if (info->reshape_active == 2) {
> > -			int cfd = open_dev(st->container_dev);
> > -			if (cfd < 0)
> > -				return 1;
> > -			st->ss->load_container(st, cfd, container);
> > -			close(cfd);
> > -			return reshape_container(container, NULL,
> > -						 st, info, 0, backup_file,
> > -						 0, 1, freeze_reshape);
> > -		}
> > -	}
> > -	return reshape_array(container, mdfd, "array", st, info, 1,
> > -			     NULL, backup_file, 0, 0, 1, freeze_reshape);
> > +	return ret_val;
> >  }
> >
> > --
> > 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 1/3] Always run Grow_continue() for started array.

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]

On Wed, 5 Oct 2011 07:04:01 +0000 "Kwolek, Adam" <adam.kwolek@intel.com>
wrote:

> 
> 
> > -----Original Message-----
> > From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> > owner@vger.kernel.org] On Behalf Of NeilBrown
> > Sent: Wednesday, October 05, 2011 4:59 AM
> > To: Kwolek, Adam
> > Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin; Williams,
> > Dan J
> > Subject: Re: [PATCH 1/3] Always run Grow_continue() for started array.
> > 
> > On Tue, 04 Oct 2011 17:53:49 +0200 Adam Kwolek <adam.kwolek@intel.com>
> > wrote:
> > 
> > > So far there were 2 reshape continuation cases:
> > >  1. array is started /e.g. reshape was already invoked during initrd
> > >                       start-up stage using "--freeze-reshape" option/
> > > 2. array is not started yet /"normal" assembling array under reshape
> > > case/
> > >
> > > This patch narrows continuation cases in to single one. To do this
> > > array should be started /set readonly in to array_state/ before
> > > calling
> > > Grow_continue() function.
> > >
> > > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > 
> > I mostly like this patch.
> > However it seems to have lost something in Grow_continue.
> > The distinction between ->reshape_active==1 and ->reshape_active==2 is
> > lost.
> > 
> > ==1 means just this array is in the middle of a reshape.
> > ==2 means that the whole container is being reshaped and after this array is
> > done we need to move on to the next one.
> > 
> > Was there are reason you removed that?  If not, please put it back.
> 
> In both cases metadata is updated already and container _reshape() goes (somehow) directly to reshape array.
> In container reshape case mdmon moves changes to next array for reshape and container_reshape() will continue with next one.
> 
> In case that single array is reshaped only mdmon finalizes reshape and container_reshape() finds no more arrays under reshape,
> So this would means end of work also.
> 
> In my opinion during reshape continuation we do not need to know if this is container or single array reshape,
> Both cases can be handled by container_reshape() /external metadata/ and we do not need to track what kind of reshape occurs.
> 
> Let me know your opinion.

Yes, that makes sense.

I don't seem to have the original patch any more.  If you could send it again
I'll apply it.

NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]