* [PATCH] Grow: fix can't change bitmap type from none to clustered.
@ 2023-02-23 14:39 Heming Zhao
2023-02-23 15:56 ` Coly Li
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Heming Zhao @ 2023-02-23 14:39 UTC (permalink / raw)
To: linux-raid, jes, ncroxon; +Cc: Heming Zhao, colyli
Commit a042210648ed ("disallow create or grow clustered bitmap with
writemostly set") introduced this bug. We should use 'true' logic not
'== 0' to deny setting up clustered array under WRITEMOSTLY condition.
How to trigger
```
~/mdadm # ./mdadm -Ss && ./mdadm --zero-superblock /dev/sd{a,b}
~/mdadm # ./mdadm -C /dev/md0 -l mirror -b clustered -e 1.2 -n 2 \
/dev/sda /dev/sdb --assume-clean
mdadm: array /dev/md0 started.
~/mdadm # ./mdadm --grow /dev/md0 --bitmap=none
~/mdadm # ./mdadm --grow /dev/md0 --bitmap=clustered
mdadm: /dev/md0 disks marked write-mostly are not supported with clustered bitmap
```
Signed-off-by: Heming Zhao <heming.zhao@suse.com>
---
Grow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Grow.c b/Grow.c
index 8f5cf07d10d9..bb5fe45c851c 100644
--- a/Grow.c
+++ b/Grow.c
@@ -429,7 +429,7 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
dv = map_dev(disk.major, disk.minor, 1);
if (!dv)
continue;
- if (((disk.state & (1 << MD_DISK_WRITEMOSTLY)) == 0) &&
+ if ((disk.state & (1 << MD_DISK_WRITEMOSTLY)) &&
(strcmp(s->bitmap_file, "clustered") == 0)) {
pr_err("%s disks marked write-mostly are not supported with clustered bitmap\n",devname);
free(mdi);
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] Grow: fix can't change bitmap type from none to clustered.
2023-02-23 14:39 [PATCH] Grow: fix can't change bitmap type from none to clustered Heming Zhao
@ 2023-02-23 15:56 ` Coly Li
2023-02-23 16:24 ` Paul Menzel
2023-02-23 18:23 ` Jes Sorensen
2 siblings, 0 replies; 6+ messages in thread
From: Coly Li @ 2023-02-23 15:56 UTC (permalink / raw)
To: Heming Zhao; +Cc: linux-raid, jes, ncroxon
On Thu, Feb 23, 2023 at 10:39:39PM +0800, Heming Zhao wrote:
> Commit a042210648ed ("disallow create or grow clustered bitmap with
> writemostly set") introduced this bug. We should use 'true' logic not
> '== 0' to deny setting up clustered array under WRITEMOSTLY condition.
>
> How to trigger
>
> ```
> ~/mdadm # ./mdadm -Ss && ./mdadm --zero-superblock /dev/sd{a,b}
> ~/mdadm # ./mdadm -C /dev/md0 -l mirror -b clustered -e 1.2 -n 2 \
> /dev/sda /dev/sdb --assume-clean
> mdadm: array /dev/md0 started.
> ~/mdadm # ./mdadm --grow /dev/md0 --bitmap=none
> ~/mdadm # ./mdadm --grow /dev/md0 --bitmap=clustered
> mdadm: /dev/md0 disks marked write-mostly are not supported with clustered bitmap
> ```
>
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
Acked-by: Coly Li <colyli@suse.de>
> ---
> Grow.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Grow.c b/Grow.c
> index 8f5cf07d10d9..bb5fe45c851c 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -429,7 +429,7 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
> dv = map_dev(disk.major, disk.minor, 1);
> if (!dv)
> continue;
> - if (((disk.state & (1 << MD_DISK_WRITEMOSTLY)) == 0) &&
> + if ((disk.state & (1 << MD_DISK_WRITEMOSTLY)) &&
> (strcmp(s->bitmap_file, "clustered") == 0)) {
> pr_err("%s disks marked write-mostly are not supported with clustered bitmap\n",devname);
> free(mdi);
--
Coly Li
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Grow: fix can't change bitmap type from none to clustered.
2023-02-23 14:39 [PATCH] Grow: fix can't change bitmap type from none to clustered Heming Zhao
2023-02-23 15:56 ` Coly Li
@ 2023-02-23 16:24 ` Paul Menzel
2023-02-23 18:23 ` Jes Sorensen
2 siblings, 0 replies; 6+ messages in thread
From: Paul Menzel @ 2023-02-23 16:24 UTC (permalink / raw)
To: Heming Zhao; +Cc: linux-raid, jes, ncroxon, colyli
Dear Heming,
Thank you for your patch.
It’d be great, if you removed the dot/period from the end of the commit
message summary.
Kind regards,
Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Grow: fix can't change bitmap type from none to clustered.
2023-02-23 14:39 [PATCH] Grow: fix can't change bitmap type from none to clustered Heming Zhao
2023-02-23 15:56 ` Coly Li
2023-02-23 16:24 ` Paul Menzel
@ 2023-02-23 18:23 ` Jes Sorensen
2023-02-23 23:50 ` Heming Zhao
2 siblings, 1 reply; 6+ messages in thread
From: Jes Sorensen @ 2023-02-23 18:23 UTC (permalink / raw)
To: Heming Zhao, linux-raid, ncroxon; +Cc: colyli
On 2/23/23 09:39, Heming Zhao wrote:
> Commit a042210648ed ("disallow create or grow clustered bitmap with
> writemostly set") introduced this bug. We should use 'true' logic not
> '== 0' to deny setting up clustered array under WRITEMOSTLY condition.
>
> How to trigger
>
> ```
> ~/mdadm # ./mdadm -Ss && ./mdadm --zero-superblock /dev/sd{a,b}
> ~/mdadm # ./mdadm -C /dev/md0 -l mirror -b clustered -e 1.2 -n 2 \
> /dev/sda /dev/sdb --assume-clean
> mdadm: array /dev/md0 started.
> ~/mdadm # ./mdadm --grow /dev/md0 --bitmap=none
> ~/mdadm # ./mdadm --grow /dev/md0 --bitmap=clustered
> mdadm: /dev/md0 disks marked write-mostly are not supported with clustered bitmap
> ```
>
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
Applied!
Thanks,
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Grow: fix can't change bitmap type from none to clustered.
2023-02-23 18:23 ` Jes Sorensen
@ 2023-02-23 23:50 ` Heming Zhao
2023-03-01 2:10 ` Jes Sorensen
0 siblings, 1 reply; 6+ messages in thread
From: Heming Zhao @ 2023-02-23 23:50 UTC (permalink / raw)
To: Jes Sorensen, linux-raid, ncroxon; +Cc: colyli, pmenzel
Hello Jes,
On 2/24/23 2:23 AM, Jes Sorensen wrote:
> On 2/23/23 09:39, Heming Zhao wrote:
>> Commit a042210648ed ("disallow create or grow clustered bitmap with
>> writemostly set") introduced this bug. We should use 'true' logic not
>> '== 0' to deny setting up clustered array under WRITEMOSTLY condition.
>>
>> How to trigger
>>
>> ```
>> ~/mdadm # ./mdadm -Ss && ./mdadm --zero-superblock /dev/sd{a,b}
>> ~/mdadm # ./mdadm -C /dev/md0 -l mirror -b clustered -e 1.2 -n 2 \
>> /dev/sda /dev/sdb --assume-clean
>> mdadm: array /dev/md0 started.
>> ~/mdadm # ./mdadm --grow /dev/md0 --bitmap=none
>> ~/mdadm # ./mdadm --grow /dev/md0 --bitmap=clustered
>> mdadm: /dev/md0 disks marked write-mostly are not supported with clustered bitmap
>> ```
>>
>> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
>
> Applied!
>
> Thanks,
> Jes
>
With Paul Menzel comment, I will remove the dot/period in patch subject then
send a v2.
------------------
@Nigel Croxon or other people
Yesterday my brain only focused on fixing bug, no thinking the a042210648ed
use case. Why or what reason makes the rule for clustered array denies to use
write-mostly disk?
I could image a scenario to use write-mostly disk. In cloud env, VMs have two
legs, one is local shared disk for speed up IOs, another is remote shared disk
(e.g: JBD) for back up. If anyone think this scenario makes sense, the best
solution is to revert a042210648ed.
Thanks,
Heming
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Grow: fix can't change bitmap type from none to clustered.
2023-02-23 23:50 ` Heming Zhao
@ 2023-03-01 2:10 ` Jes Sorensen
0 siblings, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2023-03-01 2:10 UTC (permalink / raw)
To: Heming Zhao, linux-raid, ncroxon; +Cc: colyli, pmenzel
On 2/23/23 18:50, Heming Zhao wrote:
> Hello Jes,
>
> On 2/24/23 2:23 AM, Jes Sorensen wrote:
>> On 2/23/23 09:39, Heming Zhao wrote:
>>> Commit a042210648ed ("disallow create or grow clustered bitmap with
>>> writemostly set") introduced this bug. We should use 'true' logic not
>>> '== 0' to deny setting up clustered array under WRITEMOSTLY condition.
>>>
>>> How to trigger
>>>
>>> ```
>>> ~/mdadm # ./mdadm -Ss && ./mdadm --zero-superblock /dev/sd{a,b}
>>> ~/mdadm # ./mdadm -C /dev/md0 -l mirror -b clustered -e 1.2 -n 2 \
>>> /dev/sda /dev/sdb --assume-clean
>>> mdadm: array /dev/md0 started.
>>> ~/mdadm # ./mdadm --grow /dev/md0 --bitmap=none
>>> ~/mdadm # ./mdadm --grow /dev/md0 --bitmap=clustered
>>> mdadm: /dev/md0 disks marked write-mostly are not supported with
>>> clustered bitmap
>>> ```
>>>
>>> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
>>
>> Applied!
>>
>> Thanks,
>> Jes
>>
>
> With Paul Menzel comment, I will remove the dot/period in patch subject
> then
> send a v2.
I already applied it. I don't think the dot is worth respinning the
patch for.
Thanks,
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-01 2:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-23 14:39 [PATCH] Grow: fix can't change bitmap type from none to clustered Heming Zhao
2023-02-23 15:56 ` Coly Li
2023-02-23 16:24 ` Paul Menzel
2023-02-23 18:23 ` Jes Sorensen
2023-02-23 23:50 ` Heming Zhao
2023-03-01 2:10 ` Jes Sorensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.