在 2024/12/31 16:49, Mariusz Tkaczyk 写道:
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.
Hi,
Sorry for the late reply, I forgot about this patch somehow :(
Changes from commit 581ba1341017:
@@ -1634,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.bitmap_file) {
+ s.layout_str || s.btype != BitmapNone) {
pr_err("--add cannot be used with other
geometry changes in --grow mode\n");
rv = 1;
break;
Hence before the commit, bitmap=none is not valid in this case as well,
because s.bitmap_file will set to "none" in this case.
Thanks,
Kuai
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
.
