[PATCH 0/3] mdadm patches for some trivial fixes

[PATCH 0/3] mdadm patches for some trivial fixes

[Zhilong Liu]

Hi, Jes;
  mdadm is not willing to set component size as '0' actually,
does the first patch make sense for this scenario?

Thanks,
Zhilong

Zhilong Liu (3):
  mdadm/grow: Component size must be larger than chunk size
  mdadm/manpage: disable bitmap_resize for external file bitmap
  mdadm: fixes some trivial typos in comments

 Grow.c     | 6 ++++++
 ReadMe.c   | 2 +-
 mdadm.8.in | 5 +++++
 super1.c   | 8 ++++----
 4 files changed, 16 insertions(+), 5 deletions(-)

-- 
2.6.6

[PATCH 1/3] mdadm/grow: Component size must be larger than chunk size

[Zhilong Liu]

Grow: Changing component size must be larger than current
chunk size against stripe raids, otherwise Grow_reshape()
would set s->size to '0'.

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 Grow.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Grow.c b/Grow.c
index 534ba80..f0a21ff 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1816,6 +1816,12 @@ int Grow_reshape(char *devname, int fd,
 		return 1;
 	}
 
+	if (array.level > 1 &&
+	   (array.chunk_size / 1024) > (int)s->size) {
+		pr_err("component size must be larger than chunk size.\n");
+		return 1;
+	}
+
 	st = super_by_fd(fd, &subarray);
 	if (!st) {
 		pr_err("Unable to determine metadata format for %s\n", devname);
-- 
2.6.6

Re: [PATCH 1/3] mdadm/grow: Component size must be larger than chunk size

[Zhilong Liu]

> 在 2017年10月5日，19:44，Tomasz Majchrzak <tomasz.majchrzak@intel.com> 写道：
> 
>> On Tue, Sep 05, 2017 at 05:41:36PM +0800, Zhilong Liu wrote:
>> Grow: Changing component size must be larger than current
>> chunk size against stripe raids, otherwise Grow_reshape()
>> would set s->size to '0'.
>> 
>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>> ---
>> Grow.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>> 
>> diff --git a/Grow.c b/Grow.c
>> index 534ba80..f0a21ff 100644
>> --- a/Grow.c
>> +++ b/Grow.c
>> @@ -1816,6 +1816,12 @@ int Grow_reshape(char *devname, int fd,
>>        return 1;
>>    }
>> 
>> +    if (array.level > 1 &&
>> +       (array.chunk_size / 1024) > (int)s->size) {
>> +        pr_err("component size must be larger than chunk size.\n");
>> +        return 1;
>> +    }
>> +
>>    st = super_by_fd(fd, &subarray);
>>    if (!st) {
>>        pr_err("Unable to determine metadata format for %s\n", devname);
>> -- 
>> 2.6.6
> 
> 
> Hi Zhilong Liu,
> 
> Above patch has broken RAID level conversion:
> 
> Before the patch:
> 
> mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
> /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
> MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
> mdadm: level of /dev/md/raid changed to raid0
> 
> After the patch:
> 
> mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
> /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
> MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
> mdadm: component size must be larger than chunk size.
> 
> Could you take a look at it, please?
> 

Hi, Tomek;
   Thanks for your information. I would verify it when I'm back from vacation.
  Yes, this condition should be proper only in create mode.

Thanks very much,
Zhilong

> Thanks,
> 
> Tomek
> 

Re: [PATCH 1/3] mdadm/grow: Component size must be larger than chunk size

[Zhilong Liu]

> 在 2017年10月7日，20:17，Zhilong Liu <zlliu@suse.com> 写道：
> 
> 
> 
>>> 在 2017年10月5日，19:44，Tomasz Majchrzak <tomasz.majchrzak@intel.com> 写道：
>>> 
>>> On Tue, Sep 05, 2017 at 05:41:36PM +0800, Zhilong Liu wrote:
>>> Grow: Changing component size must be larger than current
>>> chunk size against stripe raids, otherwise Grow_reshape()
>>> would set s->size to '0'.
>>> 
>>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>>> ---
>>> Grow.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>> 
>>> diff --git a/Grow.c b/Grow.c
>>> index 534ba80..f0a21ff 100644
>>> --- a/Grow.c
>>> +++ b/Grow.c
>>> @@ -1816,6 +1816,12 @@ int Grow_reshape(char *devname, int fd,
>>>       return 1;
>>>   }
>>> 
>>> +    if (array.level > 1 &&
>>> +       (array.chunk_size / 1024) > (int)s->size) {
>>> +        pr_err("component size must be larger than chunk size.\n");
>>> +        return 1;
>>> +    }
>>> +
>>>   st = super_by_fd(fd, &subarray);
>>>   if (!st) {
>>>       pr_err("Unable to determine metadata format for %s\n", devname);
>>> -- 
>>> 2.6.6
>> 
>> 
>> Hi Zhilong Liu,
>> 
>> Above patch has broken RAID level conversion:
>> 
>> Before the patch:
>> 
>> mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
>> /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
>> MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
>> mdadm: level of /dev/md/raid changed to raid0
>> 
>> After the patch:
>> 
>> mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
>> /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
>> MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
>> mdadm: component size must be larger than chunk size.
>> 
>> Could you take a look at it, please?
>> 
> 
> Hi, Tomek;
>   Thanks for your information. I would verify it when I'm back from vacation.
>  Yes, this condition should be proper only in create mode.
> 

Ignore the last mail, sorry. I would look at it.

Thanks again,
Zhilong 

> Thanks very much,
> Zhilong
> 
>> Thanks,
>> 
>> Tomek
>> 

Re: [PATCH 1/3] mdadm/grow: Component size must be larger than chunk size

[Zhilong Liu]

On 10/05/2017 07:44 PM, Tomasz Majchrzak wrote:
> On Tue, Sep 05, 2017 at 05:41:36PM +0800, Zhilong Liu wrote:
>> Grow: Changing component size must be larger than current
>> chunk size against stripe raids, otherwise Grow_reshape()
>> would set s->size to '0'.
>>
>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>> ---
>>   Grow.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Grow.c b/Grow.c
>> index 534ba80..f0a21ff 100644
>> --- a/Grow.c
>> +++ b/Grow.c
>> @@ -1816,6 +1816,12 @@ int Grow_reshape(char *devname, int fd,
>>   		return 1;
>>   	}
>>   
>> +	if (array.level > 1 &&
>> +	   (array.chunk_size / 1024) > (int)s->size) {
>> +		pr_err("component size must be larger than chunk size.\n");
>> +		return 1;
>> +	}
>> +
>>   	st = super_by_fd(fd, &subarray);
>>   	if (!st) {
>>   		pr_err("Unable to determine metadata format for %s\n", devname);
>> -- 
>> 2.6.6
>
> Hi Zhilong Liu,
>
> Above patch has broken RAID level conversion:
>
> Before the patch:
>
> mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
> /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
> MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
> mdadm: level of /dev/md/raid changed to raid0
>
> After the patch:
>
> mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
> /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
> MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
> mdadm: component size must be larger than chunk size.
>
> Could you take a look at it, please?

Hi, Tomek;
   I just back from vacation, and this is very important info for me, 
thanks very much.
Against this issue, the codes should be proper like:

diff --git a/Grow.c b/Grow.c
index 1149753..180fd78 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1814,7 +1814,8 @@ int Grow_reshape(char *devname, int fd,
         }

         if (array.level > 1 &&
-          (array.chunk_size / 1024) > (int)s->size) {
+          (array.chunk_size / 1024) > (int)s->size &&
+           s->level == UnSet) {
                 pr_err("component size must be larger than chunk size.\n");
                 return 1;
         }

Is this changing good for you? Any ideas is very welcome.

Thanks,
-Zhilong

> Thanks,
>
> Tomek
>

Re: [PATCH 1/3] mdadm/grow: Component size must be larger than chunk size

[Tomasz Majchrzak]

On Mon, Oct 09, 2017 at 11:32:09AM +0800, Zhilong Liu wrote:
> 
> 
> On 10/05/2017 07:44 PM, Tomasz Majchrzak wrote:
> >On Tue, Sep 05, 2017 at 05:41:36PM +0800, Zhilong Liu wrote:
> >>Grow: Changing component size must be larger than current
> >>chunk size against stripe raids, otherwise Grow_reshape()
> >>would set s->size to '0'.
> >>
> >>Signed-off-by: Zhilong Liu <zlliu@suse.com>
> >>---
> >>  Grow.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >>diff --git a/Grow.c b/Grow.c
> >>index 534ba80..f0a21ff 100644
> >>--- a/Grow.c
> >>+++ b/Grow.c
> >>@@ -1816,6 +1816,12 @@ int Grow_reshape(char *devname, int fd,
> >>  		return 1;
> >>  	}
> >>+	if (array.level > 1 &&
> >>+	   (array.chunk_size / 1024) > (int)s->size) {
> >>+		pr_err("component size must be larger than chunk size.\n");
> >>+		return 1;
> >>+	}
> >>+
> >>  	st = super_by_fd(fd, &subarray);
> >>  	if (!st) {
> >>  		pr_err("Unable to determine metadata format for %s\n", devname);
> >>-- 
> >>2.6.6
> >
> >Hi Zhilong Liu,
> >
> >Above patch has broken RAID level conversion:
> >
> >Before the patch:
> >
> >mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
> >/dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
> >MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
> >mdadm: level of /dev/md/raid changed to raid0
> >
> >After the patch:
> >
> >mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
> >/dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
> >MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
> >mdadm: component size must be larger than chunk size.
> >
> >Could you take a look at it, please?
> 
> Hi, Tomek;
>   I just back from vacation, and this is very important info for me,
> thanks very much.
> Against this issue, the codes should be proper like:
> 
> diff --git a/Grow.c b/Grow.c
> index 1149753..180fd78 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1814,7 +1814,8 @@ int Grow_reshape(char *devname, int fd,
>         }
> 
>         if (array.level > 1 &&
> -          (array.chunk_size / 1024) > (int)s->size) {
> +          (array.chunk_size / 1024) > (int)s->size &&
> +           s->level == UnSet) {
>                 pr_err("component size must be larger than chunk size.\n");
>                 return 1;
>         }
> 
> Is this changing good for you? Any ideas is very welcome.
> 

Yes, the above scenario works fine. Please send it as official patch.

Thanks,

Tomek

Re: [PATCH 1/3] mdadm/grow: Component size must be larger than chunk size

[NeilBrown]

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

On Mon, Oct 09 2017, Zhilong Liu wrote:

> On 10/05/2017 07:44 PM, Tomasz Majchrzak wrote:
>> On Tue, Sep 05, 2017 at 05:41:36PM +0800, Zhilong Liu wrote:
>>> Grow: Changing component size must be larger than current
>>> chunk size against stripe raids, otherwise Grow_reshape()
>>> would set s->size to '0'.
>>>
>>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>>> ---
>>>   Grow.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Grow.c b/Grow.c
>>> index 534ba80..f0a21ff 100644
>>> --- a/Grow.c
>>> +++ b/Grow.c
>>> @@ -1816,6 +1816,12 @@ int Grow_reshape(char *devname, int fd,
>>>   		return 1;
>>>   	}
>>>   
>>> +	if (array.level > 1 &&
>>> +	   (array.chunk_size / 1024) > (int)s->size) {
>>> +		pr_err("component size must be larger than chunk size.\n");
>>> +		return 1;
>>> +	}
>>> +
>>>   	st = super_by_fd(fd, &subarray);
>>>   	if (!st) {
>>>   		pr_err("Unable to determine metadata format for %s\n", devname);
>>> -- 
>>> 2.6.6
>>
>> Hi Zhilong Liu,
>>
>> Above patch has broken RAID level conversion:
>>
>> Before the patch:
>>
>> mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
>> /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
>> MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
>> mdadm: level of /dev/md/raid changed to raid0
>>
>> After the patch:
>>
>> mdadm --create /dev/md/raid --level=10 --raid-devices=4 /dev/nvme0n1
>> /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 --run --assume-clean
>> MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/raid --level=0
>> mdadm: component size must be larger than chunk size.
>>
>> Could you take a look at it, please?
>
> Hi, Tomek;
>    I just back from vacation, and this is very important info for me, 
> thanks very much.
> Against this issue, the codes should be proper like:
>
> diff --git a/Grow.c b/Grow.c
> index 1149753..180fd78 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1814,7 +1814,8 @@ int Grow_reshape(char *devname, int fd,
>          }
>
>          if (array.level > 1 &&
> -          (array.chunk_size / 1024) > (int)s->size) {
> +          (array.chunk_size / 1024) > (int)s->size &&
> +           s->level == UnSet) {
>                  pr_err("component size must be larger than chunk size.\n");
>                  return 1;
>          }

I think the correct fix for this bug is to add a test for "s->size > 0".
if "s->size == 0", no size change has been requested.

NeilBrown

>
> Is this changing good for you? Any ideas is very welcome.
>
> Thanks,
> -Zhilong
>
>> Thanks,
>>
>> Tomek
>>
>
> --
> 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: 832 bytes --]

[PATCH 2/3] mdadm/manpage: disable bitmap_resize for external file bitmap

[Zhilong Liu]

Update the manpage in "SIZE CHANGES" against the md commit.
Commit: e8a27f836f165c26f867ece7f31eb5c811692319
(md/bitmap: disable bitmap_resize for file-backed bitmaps.)

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 mdadm.8.in | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mdadm.8.in b/mdadm.8.in
index e0747fb..f0fd1fc 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -2758,6 +2758,11 @@ Also the size of an array cannot be changed while it has an active
 bitmap.  If an array has a bitmap, it must be removed before the size
 can be changed. Once the change is complete a new bitmap can be created.
 
+.PP
+Note:
+.B "--grow --size"
+is not yet supported for external file bitmap.
+
 .SS RAID\-DEVICES CHANGES
 
 A RAID1 array can work with any number of devices from 1 upwards
-- 
2.6.6

[PATCH 3/3] mdadm: fixes some trivial typos in comments

[Zhilong Liu]

fix some trivial typos in comments.

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 ReadMe.c | 2 +-
 super1.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/ReadMe.c b/ReadMe.c
index eb8fb4b..4d871e9 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -342,7 +342,7 @@ char OptionHelp[] =
 */
 
 char Help_create[] =
-"Usage:  mdadm --create device -chunk=X --level=Y --raid-devices=Z devices\n"
+"Usage:  mdadm --create device --chunk=X --level=Y --raid-devices=Z devices\n"
 "\n"
 " This usage will initialise a new md array, associate some\n"
 " devices with it, and activate the array.   In order to create an\n"
diff --git a/super1.c b/super1.c
index f6a1045..3cfbbc0 100644
--- a/super1.c
+++ b/super1.c
@@ -89,12 +89,12 @@ struct mdp_superblock_1 {
 	/* bad block log.  If there are any bad blocks the feature flag is set.
 	 * if offset and size are non-zero, that space is reserved and available.
 	 */
-	__u8	bblog_shift;	/* shift from sectors to block size for badblocklist */
-	__u16	bblog_size;	/* number of sectors reserved for badblocklist */
+	__u8	bblog_shift;	/* shift from sectors to block size for badblock list */
+	__u16	bblog_size;	/* number of sectors reserved for badblock list */
 	__u32	bblog_offset;	/* sector offset from superblock to bblog, signed */
 
 	/* array state information - 64 bytes */
-	__u64	utime;		/* 40 bits second, 24 btes microseconds */
+	__u64	utime;		/* 40 bits second, 24 bits microseconds */
 	__u64	events;		/* incremented when superblock updated */
 	__u64	resync_offset;	/* data before this offset (from data_offset) known to be in sync */
 	__u32	sb_csum;	/* checksum upto dev_roles[max_dev] */
@@ -2396,7 +2396,7 @@ add_internal_bitmap1(struct supertype *st,
 	/*
 	 * If not may_change, then this is a 'Grow' without sysfs support for
 	 * bitmaps, and the bitmap must fit after the superblock at 1K offset.
-	 * If may_change, then this is create or a Grow with sysfs syupport,
+	 * If may_change, then this is create or a Grow with sysfs support,
 	 * and we can put the bitmap wherever we like.
 	 *
 	 * size is in sectors,  chunk is in bytes !!!
-- 
2.6.6

Re: [PATCH 0/3] mdadm patches for some trivial fixes

[Jes Sorensen]

On 09/05/2017 05:41 AM, Zhilong Liu wrote:
> Hi, Jes;
>    mdadm is not willing to set component size as '0' actually,
> does the first patch make sense for this scenario?
> 
> Thanks,
> Zhilong
> 
> Zhilong Liu (3):
>    mdadm/grow: Component size must be larger than chunk size
>    mdadm/manpage: disable bitmap_resize for external file bitmap
>    mdadm: fixes some trivial typos in comments
> 
>   Grow.c     | 6 ++++++
>   ReadMe.c   | 2 +-
>   mdadm.8.in | 5 +++++
>   super1.c   | 8 ++++----
>   4 files changed, 16 insertions(+), 5 deletions(-)
> 

Applied!

Thanks,
Jes