* [PATCH] mdadm/bitmap: fix wrong bitmap nodes num when adding new disk
@ 2020-10-24 15:07 Zhao Heming
2020-10-28 5:51 ` Xiao Ni
0 siblings, 1 reply; 4+ messages in thread
From: Zhao Heming @ 2020-10-24 15:07 UTC (permalink / raw)
To: linux-raid, jes, guoqing.jiang; +Cc: Zhao Heming
The bug introduced from commit 81306e021ebdcc4baef866da82d25c3f0a415d2d
In this patch, it modified two place to support NodeNumUpdate:
Grow_addbitmap, write_init_super1
The path write_init_super1 is wrong.
reproducible steps:
```
node1 # mdadm -S --scan
node1 # for i in {a,b,c};do dd if=/dev/zero of=/dev/sd$i bs=1M count=10;
done
... ...
node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
/dev/sdb
mdadm: array /dev/md0 started.
node1 # mdadm --manage --add /dev/md0 /dev/sdc
mdadm: added /dev/sdc
node1 # mdadm --grow --raid-devices=3 /dev/md0
raid_disks for /dev/md0 set to 3
node1 # mdadm -X /dev/sdc
Filename : /dev/sdc
Magic : 6d746962
Version : 5
UUID : 9b0ff8c8:2a274a64:088ade6e:a88286a3
Chunksize : 64 MB
Daemon : 5s flush period
Write Mode : Normal
Sync Size : 306176 (299.00 MiB 313.52 MB)
Cluster nodes : 4
Cluster name : tb-ha-tst
Node Slot : 0
Events : 44
Events Cleared : 0
State : OK
Bitmap : 5 bits (chunks), 0 dirty (0.0%)
mdadm: invalid bitmap magic 0x0, the bitmap file appears to be corrupted
Node Slot : 1
Events : 0
Events Cleared : 0
State : OK
Bitmap : 0 bits (chunks), 0 dirty (0.0%)
```
There are three paths to call write_bitmap:
Assemble, Grow, write_init_super.
1. Assemble & Grow should concern NodeNumUpdate
Grow: Grow_addbitmap => write_bitmap
trigger steps:
```
node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
/dev/sdb
node1 # mdadm -G /dev/md0 -b none
node1 # mdadm -G /dev/md0 -b clustered
```
Assemble: Assemble => load_devices => write_bitmap1
trigger steps:
```
node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
/dev/sdb
node2 # mdadm -A /dev/md0 /dev/sda /dev/sdb --update=nodes --nodes=5
```
2. write_init_super should use NoUpdate.
write_init_super is called by Create & Manage path.
Create: Create => write_init_super => write_bitmap
This is initialization, it doesn't need to care node changing.
trigger step:
```
node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
/dev/sdb
```
Manage: Manage_subdevs => Manage_add => write_init_super => write_bitmap
This is disk add, not node add. So it doesn't need to care node
changing.
trigger steps:
```
mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda /dev/sdb
mdadm --manage --add /dev/md0 /dev/sdc
```
Signed-off-by: Zhao Heming <heming.zhao@suse.com>
---
super1.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/super1.c b/super1.c
index 8b0d6ff..06fa3ad 100644
--- a/super1.c
+++ b/super1.c
@@ -2093,7 +2093,7 @@ static int write_init_super1(struct supertype *st)
if (rv == 0 &&
(__le32_to_cpu(sb->feature_map) &
MD_FEATURE_BITMAP_OFFSET)) {
- rv = st->ss->write_bitmap(st, di->fd, NodeNumUpdate);
+ rv = st->ss->write_bitmap(st, di->fd, NoUpdate);
} else if (rv == 0 &&
md_feature_any_ppl_on(sb->feature_map)) {
struct mdinfo info;
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mdadm/bitmap: fix wrong bitmap nodes num when adding new disk
2020-10-24 15:07 [PATCH] mdadm/bitmap: fix wrong bitmap nodes num when adding new disk Zhao Heming
@ 2020-10-28 5:51 ` Xiao Ni
2020-10-28 12:37 ` heming.zhao
0 siblings, 1 reply; 4+ messages in thread
From: Xiao Ni @ 2020-10-28 5:51 UTC (permalink / raw)
To: Zhao Heming, linux-raid, jes, guoqing.jiang
Hi Heming
In fact it's not a wrong cluster nodes. This patch only avoids to do the
job that doesn't
need to do, right? The cluster nodes don't change when adding a new
disk. Even NodeNumUpdate
is specified, it'll not change the cluster nodes. If so, the subject is
a little confusing.
Regards
Xiao
On 10/24/2020 11:07 PM, Zhao Heming wrote:
> The bug introduced from commit 81306e021ebdcc4baef866da82d25c3f0a415d2d
> In this patch, it modified two place to support NodeNumUpdate:
> Grow_addbitmap, write_init_super1
>
> The path write_init_super1 is wrong.
>
> reproducible steps:
> ```
> node1 # mdadm -S --scan
> node1 # for i in {a,b,c};do dd if=/dev/zero of=/dev/sd$i bs=1M count=10;
> done
> ... ...
> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
> /dev/sdb
> mdadm: array /dev/md0 started.
> node1 # mdadm --manage --add /dev/md0 /dev/sdc
> mdadm: added /dev/sdc
> node1 # mdadm --grow --raid-devices=3 /dev/md0
> raid_disks for /dev/md0 set to 3
> node1 # mdadm -X /dev/sdc
> Filename : /dev/sdc
> Magic : 6d746962
> Version : 5
> UUID : 9b0ff8c8:2a274a64:088ade6e:a88286a3
> Chunksize : 64 MB
> Daemon : 5s flush period
> Write Mode : Normal
> Sync Size : 306176 (299.00 MiB 313.52 MB)
> Cluster nodes : 4
> Cluster name : tb-ha-tst
> Node Slot : 0
> Events : 44
> Events Cleared : 0
> State : OK
> Bitmap : 5 bits (chunks), 0 dirty (0.0%)
> mdadm: invalid bitmap magic 0x0, the bitmap file appears to be corrupted
> Node Slot : 1
> Events : 0
> Events Cleared : 0
> State : OK
> Bitmap : 0 bits (chunks), 0 dirty (0.0%)
> ```
>
> There are three paths to call write_bitmap:
> Assemble, Grow, write_init_super.
>
> 1. Assemble & Grow should concern NodeNumUpdate
>
> Grow: Grow_addbitmap => write_bitmap
> trigger steps:
> ```
> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
> /dev/sdb
> node1 # mdadm -G /dev/md0 -b none
> node1 # mdadm -G /dev/md0 -b clustered
> ```
>
> Assemble: Assemble => load_devices => write_bitmap1
> trigger steps:
> ```
> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
> /dev/sdb
> node2 # mdadm -A /dev/md0 /dev/sda /dev/sdb --update=nodes --nodes=5
> ```
>
> 2. write_init_super should use NoUpdate.
>
> write_init_super is called by Create & Manage path.
>
> Create: Create => write_init_super => write_bitmap
> This is initialization, it doesn't need to care node changing.
> trigger step:
> ```
> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
> /dev/sdb
> ```
>
> Manage: Manage_subdevs => Manage_add => write_init_super => write_bitmap
> This is disk add, not node add. So it doesn't need to care node
> changing.
> trigger steps:
> ```
> mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda /dev/sdb
> mdadm --manage --add /dev/md0 /dev/sdc
> ```
>
> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
> ---
> super1.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/super1.c b/super1.c
> index 8b0d6ff..06fa3ad 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -2093,7 +2093,7 @@ static int write_init_super1(struct supertype *st)
> if (rv == 0 &&
> (__le32_to_cpu(sb->feature_map) &
> MD_FEATURE_BITMAP_OFFSET)) {
> - rv = st->ss->write_bitmap(st, di->fd, NodeNumUpdate);
> + rv = st->ss->write_bitmap(st, di->fd, NoUpdate);
> } else if (rv == 0 &&
> md_feature_any_ppl_on(sb->feature_map)) {
> struct mdinfo info;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mdadm/bitmap: fix wrong bitmap nodes num when adding new disk
2020-10-28 5:51 ` Xiao Ni
@ 2020-10-28 12:37 ` heming.zhao
2020-10-29 17:03 ` Xiao Ni
0 siblings, 1 reply; 4+ messages in thread
From: heming.zhao @ 2020-10-28 12:37 UTC (permalink / raw)
To: Xiao Ni, linux-raid, jes, guoqing.jiang
Hello Xiao,
Thank you for your comment.
The patch code is right, but the patch subject need modification.
The bug only generate one bitmap area in clustered env.
So the more suitable subject: fixing wrong bitmap area number when adding new disk in clustered env
Do you feel better for it?
On 10/28/20 1:51 PM, Xiao Ni wrote:
> Hi Heming
>
> In fact it's not a wrong cluster nodes. This patch only avoids to do the job that doesn't
> need to do, right? The cluster nodes don't change when adding a new disk. Even NodeNumUpdate
> is specified, it'll not change the cluster nodes. If so, the subject is a little confusing.
>
> Regards
> Xiao
>
> On 10/24/2020 11:07 PM, Zhao Heming wrote:
>> The bug introduced from commit 81306e021ebdcc4baef866da82d25c3f0a415d2d
>> In this patch, it modified two place to support NodeNumUpdate:
>> Grow_addbitmap, write_init_super1
>>
>> The path write_init_super1 is wrong.
>>
>> reproducible steps:
>> ```
>> node1 # mdadm -S --scan
>> node1 # for i in {a,b,c};do dd if=/dev/zero of=/dev/sd$i bs=1M count=10;
>> done
>> ... ...
>> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
>> /dev/sdb
>> mdadm: array /dev/md0 started.
>> node1 # mdadm --manage --add /dev/md0 /dev/sdc
>> mdadm: added /dev/sdc
>> node1 # mdadm --grow --raid-devices=3 /dev/md0
>> raid_disks for /dev/md0 set to 3
>> node1 # mdadm -X /dev/sdc
>> Filename : /dev/sdc
>> Magic : 6d746962
>> Version : 5
>> UUID : 9b0ff8c8:2a274a64:088ade6e:a88286a3
>> Chunksize : 64 MB
>> Daemon : 5s flush period
>> Write Mode : Normal
>> Sync Size : 306176 (299.00 MiB 313.52 MB)
>> Cluster nodes : 4
>> Cluster name : tb-ha-tst
>> Node Slot : 0
>> Events : 44
>> Events Cleared : 0
>> State : OK
>> Bitmap : 5 bits (chunks), 0 dirty (0.0%)
>> mdadm: invalid bitmap magic 0x0, the bitmap file appears to be corrupted
>> Node Slot : 1
>> Events : 0
>> Events Cleared : 0
>> State : OK
>> Bitmap : 0 bits (chunks), 0 dirty (0.0%)
>> ```
>>
>> There are three paths to call write_bitmap:
>> Assemble, Grow, write_init_super.
>>
>> 1. Assemble & Grow should concern NodeNumUpdate
>>
>> Grow: Grow_addbitmap => write_bitmap
>> trigger steps:
>> ```
>> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
>> /dev/sdb
>> node1 # mdadm -G /dev/md0 -b none
>> node1 # mdadm -G /dev/md0 -b clustered
>> ```
>>
>> Assemble: Assemble => load_devices => write_bitmap1
>> trigger steps:
>> ```
>> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
>> /dev/sdb
>> node2 # mdadm -A /dev/md0 /dev/sda /dev/sdb --update=nodes --nodes=5
>> ```
>>
>> 2. write_init_super should use NoUpdate.
>>
>> write_init_super is called by Create & Manage path.
>>
>> Create: Create => write_init_super => write_bitmap
>> This is initialization, it doesn't need to care node changing.
>> trigger step:
>> ```
>> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
>> /dev/sdb
>> ```
>>
>> Manage: Manage_subdevs => Manage_add => write_init_super => write_bitmap
>> This is disk add, not node add. So it doesn't need to care node
>> changing.
>> trigger steps:
>> ```
>> mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda /dev/sdb
>> mdadm --manage --add /dev/md0 /dev/sdc
>> ```
>>
>> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
>> ---
>> super1.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/super1.c b/super1.c
>> index 8b0d6ff..06fa3ad 100644
>> --- a/super1.c
>> +++ b/super1.c
>> @@ -2093,7 +2093,7 @@ static int write_init_super1(struct supertype *st)
>> if (rv == 0 &&
>> (__le32_to_cpu(sb->feature_map) &
>> MD_FEATURE_BITMAP_OFFSET)) {
>> - rv = st->ss->write_bitmap(st, di->fd, NodeNumUpdate);
>> + rv = st->ss->write_bitmap(st, di->fd, NoUpdate);
>> } else if (rv == 0 &&
>> md_feature_any_ppl_on(sb->feature_map)) {
>> struct mdinfo info;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mdadm/bitmap: fix wrong bitmap nodes num when adding new disk
2020-10-28 12:37 ` heming.zhao
@ 2020-10-29 17:03 ` Xiao Ni
0 siblings, 0 replies; 4+ messages in thread
From: Xiao Ni @ 2020-10-29 17:03 UTC (permalink / raw)
To: heming.zhao, linux-raid, jes, guoqing.jiang
Hi Heming
I can reproduce this bug myself. It indeed writes the wrong cluster
nodes to the new disk.
So the original subject is good for me. Thanks for the explanation.
Regards
Xiao
On 10/28/2020 08:37 PM, heming.zhao@suse.com wrote:
> Hello Xiao,
>
> Thank you for your comment.
> The patch code is right, but the patch subject need modification.
>
> The bug only generate one bitmap area in clustered env.
> So the more suitable subject: fixing wrong bitmap area number when adding new disk in clustered env
>
> Do you feel better for it?
>
> On 10/28/20 1:51 PM, Xiao Ni wrote:
>> Hi Heming
>>
>> In fact it's not a wrong cluster nodes. This patch only avoids to do the job that doesn't
>> need to do, right? The cluster nodes don't change when adding a new disk. Even NodeNumUpdate
>> is specified, it'll not change the cluster nodes. If so, the subject is a little confusing.
>>
>> Regards
>> Xiao
>>
>> On 10/24/2020 11:07 PM, Zhao Heming wrote:
>>> The bug introduced from commit 81306e021ebdcc4baef866da82d25c3f0a415d2d
>>> In this patch, it modified two place to support NodeNumUpdate:
>>> Grow_addbitmap, write_init_super1
>>>
>>> The path write_init_super1 is wrong.
>>>
>>> reproducible steps:
>>> ```
>>> node1 # mdadm -S --scan
>>> node1 # for i in {a,b,c};do dd if=/dev/zero of=/dev/sd$i bs=1M count=10;
>>> done
>>> ... ...
>>> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
>>> /dev/sdb
>>> mdadm: array /dev/md0 started.
>>> node1 # mdadm --manage --add /dev/md0 /dev/sdc
>>> mdadm: added /dev/sdc
>>> node1 # mdadm --grow --raid-devices=3 /dev/md0
>>> raid_disks for /dev/md0 set to 3
>>> node1 # mdadm -X /dev/sdc
>>> Filename : /dev/sdc
>>> Magic : 6d746962
>>> Version : 5
>>> UUID : 9b0ff8c8:2a274a64:088ade6e:a88286a3
>>> Chunksize : 64 MB
>>> Daemon : 5s flush period
>>> Write Mode : Normal
>>> Sync Size : 306176 (299.00 MiB 313.52 MB)
>>> Cluster nodes : 4
>>> Cluster name : tb-ha-tst
>>> Node Slot : 0
>>> Events : 44
>>> Events Cleared : 0
>>> State : OK
>>> Bitmap : 5 bits (chunks), 0 dirty (0.0%)
>>> mdadm: invalid bitmap magic 0x0, the bitmap file appears to be corrupted
>>> Node Slot : 1
>>> Events : 0
>>> Events Cleared : 0
>>> State : OK
>>> Bitmap : 0 bits (chunks), 0 dirty (0.0%)
>>> ```
>>>
>>> There are three paths to call write_bitmap:
>>> Assemble, Grow, write_init_super.
>>>
>>> 1. Assemble & Grow should concern NodeNumUpdate
>>>
>>> Grow: Grow_addbitmap => write_bitmap
>>> trigger steps:
>>> ```
>>> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
>>> /dev/sdb
>>> node1 # mdadm -G /dev/md0 -b none
>>> node1 # mdadm -G /dev/md0 -b clustered
>>> ```
>>>
>>> Assemble: Assemble => load_devices => write_bitmap1
>>> trigger steps:
>>> ```
>>> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
>>> /dev/sdb
>>> node2 # mdadm -A /dev/md0 /dev/sda /dev/sdb --update=nodes --nodes=5
>>> ```
>>>
>>> 2. write_init_super should use NoUpdate.
>>>
>>> write_init_super is called by Create & Manage path.
>>>
>>> Create: Create => write_init_super => write_bitmap
>>> This is initialization, it doesn't need to care node changing.
>>> trigger step:
>>> ```
>>> node1 # mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda
>>> /dev/sdb
>>> ```
>>>
>>> Manage: Manage_subdevs => Manage_add => write_init_super => write_bitmap
>>> This is disk add, not node add. So it doesn't need to care node
>>> changing.
>>> trigger steps:
>>> ```
>>> mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sda /dev/sdb
>>> mdadm --manage --add /dev/md0 /dev/sdc
>>> ```
>>>
>>> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
>>> ---
>>> super1.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/super1.c b/super1.c
>>> index 8b0d6ff..06fa3ad 100644
>>> --- a/super1.c
>>> +++ b/super1.c
>>> @@ -2093,7 +2093,7 @@ static int write_init_super1(struct supertype *st)
>>> if (rv == 0 &&
>>> (__le32_to_cpu(sb->feature_map) &
>>> MD_FEATURE_BITMAP_OFFSET)) {
>>> - rv = st->ss->write_bitmap(st, di->fd, NodeNumUpdate);
>>> + rv = st->ss->write_bitmap(st, di->fd, NoUpdate);
>>> } else if (rv == 0 &&
>>> md_feature_any_ppl_on(sb->feature_map)) {
>>> struct mdinfo info;
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-29 17:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-24 15:07 [PATCH] mdadm/bitmap: fix wrong bitmap nodes num when adding new disk Zhao Heming
2020-10-28 5:51 ` Xiao Ni
2020-10-28 12:37 ` heming.zhao
2020-10-29 17:03 ` Xiao Ni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).