mdadm: fix --grow with --add for linear

Commit Message

Yu Kuai Dec. 27, 2024, 6:07 a.m. UTC

From: Yu Kuai <[e-mail]>

For the case mdadm --grow with --add, the s.btype should not be
initialized yet, hence BitmapUnknown should be checked instead of
BitmapNone.

Noted that this behaviour should only support by md-linear, which is
removed from kernel, howerver, it turns out md-linear is used widely
in home NAS and we're planning to reintroduce it soon.

Fixes: 581ba1341017 ("mdadm: remove bitmap file support")
Signed-off-by: Yu Kuai <[e-mail]>
---
 mdadm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mariusz Tkaczyk Dec. 31, 2024, 8:49 a.m. UTC | #1

On Fri, 27 Dec 2024 14:07:02 +0800
Yu Kuai <[e-mail]> wrote:

> From: Yu Kuai <[e-mail]>
> 
> For the case mdadm --grow with --add, the s.btype should not be
> initialized yet, hence BitmapUnknown should be checked instead of
> BitmapNone.

Hi Kuai,

For commit extra clarity it would be nice to include command you are
executing.

What if someone will do (not tested):
#mdadm --grow /dev/md0 --add /dev/sdx --bitmap=none

I think that it is perfectly valid, now it may work but I expect your
change to broke it.

I would say we need:

bool is_bitmap_set(struct shape *s) {
	if (s.layout)
		return true;
	if (s.btype == BitmapNone || s.btype != BitmapUnknown)
		return false;

	return true;
}

And respect both cases. Setting property to default should never be a
mistake.

Has it some sense? If no, I miss some explanation in commit message (or
better comment).

> 
> Noted that this behaviour should only support by md-linear, which is
> removed from kernel, howerver, it turns out md-linear is used widely
> in home NAS and we're planning to reintroduce it soon.

Wow. We get a lesson.

For the code, LGTM.

Thanks,
Mariusz

Patch

diff --git a/mdadm.c b/mdadm.c
index 7d3b656b..1fd4dcba 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -1625,7 +1625,7 @@  int main(int argc, char *argv[])
 		if (devs_found > 1 && s.raiddisks == 0 && s.level == UnSet) {
 			/* must be '-a'. */
 			if (s.size > 0 || s.chunk ||
-			    s.layout_str || s.btype != BitmapNone) {
+			    s.layout_str || s.btype != BitmapUnknown) {
 				pr_err("--add cannot be used with other geometry changes in --grow mode\n");
 				rv = 1;
 				break;