Re: [PATCH V3 06/11] Convert a bitmap=none device to clustered

[NeilBrown <neilb@suse.de>]

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

On Wed, 20 May 2015 11:20:38 +0800 Guoqing Jiang <gqjiang@suse.com> wrote:

> This adds the ability to convert a regular md without bitmap
> (--bitmap=none) to a clustered device (--bitmap=clustered).
> 
> To convert a device with --bitmap=internal or --bitmap=external,
> you have to convert to --bitmap=none and then re-execute the
> command with --bitmap=clustered.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
>  Grow.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 9a573fd..1122cec 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -330,9 +330,16 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
>  			}
>  			return 0;
>  		}
> -		pr_err("Internal bitmap already present on %s\n",
> -			devname);
> -		return 1;
> +		if ((strcmp(s->bitmap_file, "clustered")==0) && (array.state & (1<<MD_SB_CLUSTERED))) {
> +			pr_err("Clustered bitmap already present on %s\n",
> +					devname);
> +			return 1;
> +		}
> +		if ((strcmp(s->bitmap_file, "internal")==0) && (!(array.state & (1<<MD_SB_CLUSTERED)))) {
> +			pr_err("Internal bitmap already present on %s\n",
> +					devname);
> +			return 1;
> +		}

You shouldn't be checking the value of s->bitmap_file here.  No matter what
value it has, there is some sort of bitmap present and you should report what
sort and return.


>  	}
>  
>  	if (strcmp(s->bitmap_file, "none") == 0) {
> @@ -375,7 +382,8 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
>  		free(st);
>  		return 1;
>  	}
> -	if (strcmp(s->bitmap_file, "internal") == 0) {
> +	if ((strcmp(s->bitmap_file, "internal") == 0) ||
> +		(strcmp(s->bitmap_file, "clustered") == 0)) {

Indentation is wrong.  It should be:

   if ((strcmp(......) == 0) ||
       (strcmp(......) == 0)) {

except that the extra parentheses are not needed. so

   if (strcmp(......) == 0 ||
       strcmp(......) == 0) {


Thanks,
NeilBrown

>  		int rv;
>  		int d;
>  		int offset_setable = 0;
> @@ -384,6 +392,8 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
>  			pr_err("Internal bitmaps not supported with %s metadata\n", st->ss->name);
>  			return 1;
>  		}
> +		st->nodes = c->nodes;
> +		st->cluster_name = c->homecluster;
>  		mdi = sysfs_read(fd, NULL, GET_BITMAP_LOCATION);
>  		if (mdi)
>  			offset_setable = 1;
> @@ -426,6 +436,8 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
>  			rv = sysfs_set_num_signed(mdi, NULL, "bitmap/location",
>  						  mdi->bitmap_offset);
>  		} else {
> +			if (strcmp(s->bitmap_file, "clustered") == 0)
> +				array.state |= (1<<MD_SB_CLUSTERED);
>  			array.state |= (1<<MD_SB_BITMAP_PRESENT);
>  			rv = ioctl(fd, SET_ARRAY_INFO, &array);
>  		}


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]