Re: [PATCH] mdadm: fix --grow with --add for linear

[Mariusz Tkaczyk <mtkaczyk@xxxxxxxxxx>]

On Fri, 27 Dec 2024 14:07:02 +0800
Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:

> From: Yu Kuai <yukuai3@xxxxxxxxxx>
> 
> 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