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

[NeilBrown <neilb@suse.de>]

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