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@xxxxxxxx>
>> ---
>> 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;
>
