Re: [PATCH 6/8] FIX: Do not set array size after reshape in mdadm

[NeilBrown <neilb@suse.de>]

On Tue, 01 Feb 2011 14:49:44 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Do not set array size after reshape in mdadm,
> as it is up to mdmon metadata handler (set_array_state()) now.

I'm not sure about this...

I agree that it is probably more appropriate for mdmon to impose the new
array_size rather than for mdadm to do it.
In general, if the kernel does something for native metadata, then mdmon
should probably do it for external metadata (though there might be exceptions
to this).

However it is not the metadata handler which should do this (and it
currently doesn't, which is good). The metadata handler should set
custom_array_array, and the core code of mdmon should use this number
to set array_size.
And I don't see that mdmon does this at the moment.  It sets the array size
when the reshape starts (which is possibly wrong) but it does not set
it when the reshape finishes (which is the right time).

Could you please review all of this and either point out to me where
I am wrong, or fix up the code to do the right thing, thanks.

I won't apply this patch now, but am happy to apply it once I'm sure
mdmon is performing this task.

NeilBrown


> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |   35 -----------------------------------
>  1 files changed, 0 insertions(+), 35 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 8229b4d..a74da89 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2045,41 +2045,6 @@ started:
>  		}
>  	}
>  
> -	/* set new array size if required customer_array_size is used
> -	 * by this metadata.
> -	 */
> -	if (reshape.before.data_disks !=
> -	    reshape.after.data_disks &&
> -	    info->custom_array_size) {
> -		struct mdinfo *info2;
> -		char *subarray = strchr(info->text_version+1, '/')+1;
> -
> -		info2 = st->ss->container_content(st, subarray);
> -		if (info2) {
> -			unsigned long long current_size = 0;
> -			unsigned long long new_size =
> -				info2->custom_array_size/2;
> -
> -			if (sysfs_get_ll(sra,
> -					 NULL,
> -					 "array_size",
> -					 &current_size) == 0 &&
> -			    new_size > current_size) {
> -				if (sysfs_set_num(sra, NULL,
> -						  "array_size", new_size)
> -				    < 0)
> -					dprintf("Error: Cannot"
> -						" set array size");
> -				else
> -					dprintf("Array size "
> -						"changed");
> -				dprintf(" from %llu to %llu.\n",
> -					current_size, new_size);
> -			}
> -			sysfs_free(info2);
> -		}
> -	}
> -
>  	if (info->new_level != reshape.level) {
>  
>  		c = map_num(pers, info->new_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