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:

quoted

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 <redacted>
---
  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;