diff mbox series

[v3,4/4] Manage: forbid re-add to the array without metadata

Message ID 20241120064637.3657385-5-yukuai1@huaweicloud.com (mailing list archive)
State New
Headers show
Series mdadm: remove bitmap file support | expand

Checks

Context Check Description
mdraidci/vmtest-md-6_13-PR fail merge-conflict

Commit Message

Yu Kuai Nov. 20, 2024, 6:46 a.m. UTC 
From: Yu Kuai <yukuai3@huawei.com>

re-add will not trigger full disk recovery, user should add a new disk
to the array instead. Also update test/05r1-re-add-nosuper to reflect
this.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 Manage.c                  | 12 ++++++++++++
 tests/05r1-re-add-nosuper | 27 +++++----------------------
 2 files changed, 17 insertions(+), 22 deletions(-)

Comments

Mariusz Tkaczyk Nov. 20, 2024, 10:36 a.m. UTC | #1 
On Wed, 20 Nov 2024 14:46:37 +0800
Yu Kuai <yukuai1@huaweicloud.com> wrote:

> From: Yu Kuai <yukuai3@huawei.com>
> 
> re-add will not trigger full disk recovery, user should add a new disk
> to the array instead. Also update test/05r1-re-add-nosuper to reflect
> this.
> 

Description miss information about blocking --re-add for external :)

Please add something. I know it is not supported, I just need some note like:
"I was never supported for external metadata and behavior is undefined? (please
check at least with IMSM), block it"

If it fallbacks to standard --add I'm also fine with the change because we are
making clear user interface and this will be help to avoid abuses and bug
reports. Probably no one is using that with external- I think we can take a
risk here.

For the code, LGTM.

Thanks!
Mariusz
diff mbox series

Patch

diff --git a/Manage.c b/Manage.c
index b9e55c43..b14a9ab9 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1448,6 +1448,18 @@  int Manage_subdevs(char *devname, int fd,
 		int rv, err = 0;
 		int mj, mn;
 
+		if (tst->ss->external && dv->disposition == 'A') {
+			pr_err("Cannot re-add member device %s to %s, it is not supported for external metadata, aborting.\n",
+			       dv->devname, fd2devnm(fd));
+			goto abort;
+		}
+
+		if (array.not_persistent == 1 && dv->disposition == 'A') {
+			pr_err("Cannot re-add member device %s to %s, array is not persistent, aborting.\n",
+			       dv->devname, fd2devnm(fd));
+			goto abort;
+		}
+
 		raid_slot = -1;
 		if (dv->disposition == 'c') {
 			rv = parse_cluster_confirm_arg(dv->devname, &dv->devname, &raid_slot);
diff --git a/tests/05r1-re-add-nosuper b/tests/05r1-re-add-nosuper
index a3fa9503..750d7c14 100644
--- a/tests/05r1-re-add-nosuper
+++ b/tests/05r1-re-add-nosuper
@@ -1,7 +1,6 @@ 
 #
-# create a raid1, remove a drive, and readd it.
-# resync should be instant.
-# Then do some IO first.  Resync should still be very fast
+# create a raid1 without superblock, remove a drive, and readd it.
+# readd should fail.
 #
 mdadm -B $md0 -l1 -n2 -d1 $dev1 $dev2
 check resync
@@ -12,24 +11,8 @@  sleep 4
 mdadm $md0 -f $dev2
 sleep 1
 mdadm $md0 -r $dev2
-mdadm $md0 --re-add $dev2
-check nosync
+if mdadm $md0 --re-add $dev2; then
+	err "re-add should fail"
+fi
 
-mdadm $md0 -f $dev2
-sleep 1
-mdadm $md0 -r $dev2
-testdev $md0 1 $size 1
-mdadm $md0 --re-add $dev2
-check wait
-cmp --bytes=$[$mdsize0*1024] $dev1 $dev2
-
-mdadm $md0 -f $dev2; sleep 1
-mdadm $md0 -r $dev2
-if dd if=/dev/zero of=$md0 ; then : ; fi
-blockdev --flushbufs $md0 # make sure writes have been sent
-mdadm $md0 --re-add $dev2
-check recovery
-check wait
-# should BLKFLSBUF and then read $dev1/$dev2...
-cmp --bytes=$[$mdsize0*1024] $file1 $file2
 mdadm -S $md0