[PATCH] imsm: fix, check all volumes state in Grow_continue function

[PATCH] imsm: fix, check all volumes state in Grow_continue function

[Labun, Marcin]

The Grow_continue grows all volumes in the container that are in container
reshape state, therefore we  need to check blocking bits
of all volumes in container, not just the one that is pointed by the command
line argument.

Signed-off-by: Marcin Labun <marcin.labun@intel.com>
---
 Grow.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Grow.c b/Grow.c
index e7fd7c4..65aa851 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3769,18 +3769,19 @@ int Grow_continue_command(char *devname, int fd,
 			goto Grow_continue_command_exit;
 		}
 
-		cc = st->ss->container_content(st, subarray);
+		cc = st->ss->container_content(st, NULL);
 		for (content = cc; content ; content = content->next) {
 			char *array;
 			int allow_reshape = 1;
 
 			if (content->reshape_active == 0)
 				continue;
-			/* The decision about array or container wide
-			 * reshape is taken in Grow_continue based
-			 * content->reshape_active state, therefore we
-			 * need to check_reshape based on
-			 * reshape_active and subarray name
+			/* The Grow_continue grows all volumes
+			 * in the container that are in container
+			 * reshape state, 
+			 * therefore we  need to check blocking bits
+			 * of all volumes in container, not just the 
+			 * one that is pointed by command line argument.
 			 */
 			if (content->array.state & (1<<MD_SB_BLOCK_VOLUME))
 				allow_reshape = 0;

Re: [PATCH] imsm: fix, check all volumes state in Grow_continue function

[NeilBrown]

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

On Wed, 23 Nov 2011 15:26:01 +0000 "Labun, Marcin" <Marcin.Labun@intel.com>
wrote:

> The Grow_continue grows all volumes in the container that are in container
> reshape state, therefore we  need to check blocking bits
> of all volumes in container, not just the one that is pointed by the command
> line argument.
> 
> Signed-off-by: Marcin Labun <marcin.labun@intel.com>
> ---
>  Grow.c |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index e7fd7c4..65aa851 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -3769,18 +3769,19 @@ int Grow_continue_command(char *devname, int fd,
>  			goto Grow_continue_command_exit;
>  		}
>  
> -		cc = st->ss->container_content(st, subarray);
> +		cc = st->ss->container_content(st, NULL);
>  		for (content = cc; content ; content = content->next) {
>  			char *array;
>  			int allow_reshape = 1;
>  
>  			if (content->reshape_active == 0)
>  				continue;
> -			/* The decision about array or container wide
> -			 * reshape is taken in Grow_continue based
> -			 * content->reshape_active state, therefore we
> -			 * need to check_reshape based on
> -			 * reshape_active and subarray name
> +			/* The Grow_continue grows all volumes
> +			 * in the container that are in container
> +			 * reshape state, 
> +			 * therefore we  need to check blocking bits
> +			 * of all volumes in container, not just the 
> +			 * one that is pointed by command line argument.
>  			 */
>  			if (content->array.state & (1<<MD_SB_BLOCK_VOLUME))
>  				allow_reshape = 0;


I don't think this is right.

There are two sorts of reshapes - those that affect the whole container and
those that affect a single array.

If I run
  mdadm --grow --continue /dev/md/CONTAINER

then it should examine every array and restart any reshape that is pending,
whichever type it is.  You patch doesn't affect this case and I assume it
works.

If I run
  mdadm --grow --contine /dev/md/MEMBER_ARRAY

then it should refuse to restart a container-wide reshape, whether it is
blocked for some reason or not.  This usage should only be allowed to start a
just-this-array reshape and should succeed or fail without any reference to
any other arrays in the container.

Does that make sense?  Can you make that happen?

Thanks,
NeilBrown


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