* [PATCH v2] mdadm: Clear extra flags when initializing metadata
[not found] <b894d081-eda9-6b28-5fef-75753838a916@huawei.com>
@ 2025-03-10 11:09 ` Wu Guanghao
2025-03-10 14:41 ` Blazej Kucman
0 siblings, 1 reply; 6+ messages in thread
From: Wu Guanghao @ 2025-03-10 11:09 UTC (permalink / raw)
To: Mariusz Tkaczyk, Yu Kuai, xni; +Cc: ncroxon, linux-raid, liubo254
When adding a disk to a RAID1 array, the metadata is read from the existing
member disks for sync. However, only the bad_blocks flag are copied,
the bad_blocks records are not copied, so the bad_blocks records are all zeros.
The kernel function super_1_load() detects bad_blocks flag and reads the
bad_blocks record, then sets the bad block using badblocks_set().
After the kernel commit 1726c7746("badblocks: improve badblocks_set() for
multiple ranges handling"), if the length of a bad_blocks record is 0, it will
return a failure. Therefore the device addition will fail.
So when adding a new disk, some flags cannot be sync and need to be cleared.
Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
---
Changelog:
v2:
Add a testcase.
Clear extra replace flag.
---
super1.c | 4 ++++
tests/05r1-add-badblocks | 25 +++++++++++++++++++++++++
2 files changed, 29 insertions(+)
create mode 100644 tests/05r1-add-badblocks
diff --git a/super1.c b/super1.c
index fe3c4c64..f4a29f4f 100644
--- a/super1.c
+++ b/super1.c
@@ -1971,6 +1971,10 @@ static int write_init_super1(struct supertype *st)
long bm_offset;
bool raid0_need_layout = false;
+ /* Clear extra flags */
+ sb->feature_map &= ~__cpu_to_le32(MD_FEATURE_BAD_BLOCKS |
+ MD_FEATURE_REPLACEMENT);
+
/* Since linux kernel v5.4, raid0 always has a layout */
if (has_raid0_layout(sb) && get_linux_version() >= 5004000)
raid0_need_layout = true;
diff --git a/tests/05r1-add-badblocks b/tests/05r1-add-badblocks
new file mode 100644
index 00000000..88b064f2
--- /dev/null
+++ b/tests/05r1-add-badblocks
@@ -0,0 +1,25 @@
+#
+# create a raid1 with a drive and set badblocks for the drive.
+# add a new drive does not cause an error.
+#
+
+# create raid1
+mdadm -CR $md0 -l1 -n2 -e1.0 $dev1 missing
+testdev $md0 1 $mdsize1a 64
+sleep 3
+
+# set badblocks for the drive
+dev1_name=$(basename $dev1)
+echo "10000 100" > /sys/block/md0/md/dev-$dev1_name/bad_blocks
+echo "write_error" > /sys/block/md0/md/dev-$dev1_name/state
+
+# maybe fail but that's ok, as it's only intended to
+# record the bad block in the metadata.
+mkfs.ext3 $md0
+
+# re-add and recovery
+mdadm $md0 -a $dev2
+check recovery
+
+mdadm -S $md0
+
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mdadm: Clear extra flags when initializing metadata
2025-03-10 11:09 ` [PATCH v2] mdadm: Clear extra flags when initializing metadata Wu Guanghao
@ 2025-03-10 14:41 ` Blazej Kucman
2025-03-11 1:10 ` Xiao Ni
2025-03-11 2:23 ` Wu Guanghao
0 siblings, 2 replies; 6+ messages in thread
From: Blazej Kucman @ 2025-03-10 14:41 UTC (permalink / raw)
To: Wu Guanghao; +Cc: Mariusz Tkaczyk, Yu Kuai, xni, ncroxon, linux-raid, liubo254
On Mon, 10 Mar 2025 19:09:36 +0800
Wu Guanghao <wuguanghao3@huawei.com> wrote:
Hi,
Thanks for your patch.
you are only adding a change to native metadata so it would be good to
emphasize this in the title, please change "mdadm:" to "super1:"
There are also a few checkpatch issues,
> When adding a disk to a RAID1 array, the metadata is read from the
> existing member disks for sync. However, only the bad_blocks flag are
> copied, the bad_blocks records are not copied, so the bad_blocks
> records are all zeros. The kernel function super_1_load() detects
> bad_blocks flag and reads the bad_blocks record, then sets the bad
> block using badblocks_set().
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit
description?) #8:
the bad_blocks records are not copied, so the bad_blocks records are
all zeros.
>
> After the kernel commit 1726c7746("badblocks: improve badblocks_set()
> for multiple ranges handling"), if the length of a bad_blocks record
please use SHA-1 ID - first 12 characters and space between ID and
(Tile)
> is 0, it will return a failure. Therefore the device addition will
> fail.
>
> So when adding a new disk, some flags cannot be sync and need to be
> cleared.
>
> Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
> ---
> Changelog:
> v2:
> Add a testcase.
> Clear extra replace flag.
> ---
> super1.c | 4 ++++
> tests/05r1-add-badblocks | 25 +++++++++++++++++++++++++
> 2 files changed, 29 insertions(+)
> create mode 100644 tests/05r1-add-badblocks
>
> diff --git a/super1.c b/super1.c
> index fe3c4c64..f4a29f4f 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -1971,6 +1971,10 @@ static int write_init_super1(struct supertype
> *st) long bm_offset;
> bool raid0_need_layout = false;
>
> + /* Clear extra flags */
> + sb->feature_map &= ~__cpu_to_le32(MD_FEATURE_BAD_BLOCKS |
> + MD_FEATURE_REPLACEMENT);
ERROR: code indent should use tabs where possible
#36: FILE: super1.c:1976:
+ MD_FEATURE_REPLACEMENT);$
WARNING: please, no spaces at the start of a line
#36: FILE: super1.c:1976:
+ MD_FEATURE_REPLACEMENT);$
However, in this case the code will fit on one line, the limit is 100
characters.
> +
> /* Since linux kernel v5.4, raid0 always has a layout */
> if (has_raid0_layout(sb) && get_linux_version() >= 5004000)
> raid0_need_layout = true;
> diff --git a/tests/05r1-add-badblocks b/tests/05r1-add-badblocks
> new file mode 100644
> index 00000000..88b064f2
> --- /dev/null
> +++ b/tests/05r1-add-badblocks
> @@ -0,0 +1,25 @@
> +#
> +# create a raid1 with a drive and set badblocks for the drive.
> +# add a new drive does not cause an error.
> +#
> +
> +# create raid1
> +mdadm -CR $md0 -l1 -n2 -e1.0 $dev1 missing
> +testdev $md0 1 $mdsize1a 64
> +sleep 3
> +
> +# set badblocks for the drive
> +dev1_name=$(basename $dev1)
> +echo "10000 100" > /sys/block/md0/md/dev-$dev1_name/bad_blocks
> +echo "write_error" > /sys/block/md0/md/dev-$dev1_name/state
> +
> +# maybe fail but that's ok, as it's only intended to
> +# record the bad block in the metadata.
> +mkfs.ext3 $md0
> +
> +# re-add and recovery
> +mdadm $md0 -a $dev2
> +check recovery
> +
> +mdadm -S $md0
> +
Since you added the test, would you be able to issue a PR on github
to get the tests running?
Thansk,
Blazej
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mdadm: Clear extra flags when initializing metadata
2025-03-10 14:41 ` Blazej Kucman
@ 2025-03-11 1:10 ` Xiao Ni
2025-03-11 2:08 ` Wu Guanghao
2025-03-11 2:23 ` Wu Guanghao
1 sibling, 1 reply; 6+ messages in thread
From: Xiao Ni @ 2025-03-11 1:10 UTC (permalink / raw)
To: Blazej Kucman
Cc: Wu Guanghao, Mariusz Tkaczyk, Yu Kuai, ncroxon, linux-raid,
liubo254
On Mon, Mar 10, 2025 at 10:42 PM Blazej Kucman
<blazej.kucman@linux.intel.com> wrote:
>
> On Mon, 10 Mar 2025 19:09:36 +0800
> Wu Guanghao <wuguanghao3@huawei.com> wrote:
>
> Hi,
> Thanks for your patch.
>
> you are only adding a change to native metadata so it would be good to
> emphasize this in the title, please change "mdadm:" to "super1:"
>
> There are also a few checkpatch issues,
>
>
> > When adding a disk to a RAID1 array, the metadata is read from the
> > existing member disks for sync. However, only the bad_blocks flag are
> > copied, the bad_blocks records are not copied, so the bad_blocks
> > records are all zeros. The kernel function super_1_load() detects
> > bad_blocks flag and reads the bad_blocks record, then sets the bad
> > block using badblocks_set().
>
> WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit
> description?) #8:
> the bad_blocks records are not copied, so the bad_blocks records are
> all zeros.
>
> >
> > After the kernel commit 1726c7746("badblocks: improve badblocks_set()
> > for multiple ranges handling"), if the length of a bad_blocks record
>
> please use SHA-1 ID - first 12 characters and space between ID and
> (Tile)
>
> > is 0, it will return a failure. Therefore the device addition will
> > fail.
> >
> > So when adding a new disk, some flags cannot be sync and need to be
> > cleared.
> >
> > Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
> > ---
> > Changelog:
> > v2:
> > Add a testcase.
> > Clear extra replace flag.
> > ---
> > super1.c | 4 ++++
> > tests/05r1-add-badblocks | 25 +++++++++++++++++++++++++
> > 2 files changed, 29 insertions(+)
> > create mode 100644 tests/05r1-add-badblocks
> >
> > diff --git a/super1.c b/super1.c
> > index fe3c4c64..f4a29f4f 100644
> > --- a/super1.c
> > +++ b/super1.c
> > @@ -1971,6 +1971,10 @@ static int write_init_super1(struct supertype
> > *st) long bm_offset;
> > bool raid0_need_layout = false;
> >
> > + /* Clear extra flags */
> > + sb->feature_map &= ~__cpu_to_le32(MD_FEATURE_BAD_BLOCKS |
> > + MD_FEATURE_REPLACEMENT);
>
> ERROR: code indent should use tabs where possible
> #36: FILE: super1.c:1976:
> + MD_FEATURE_REPLACEMENT);$
>
> WARNING: please, no spaces at the start of a line
> #36: FILE: super1.c:1976:
> + MD_FEATURE_REPLACEMENT);$
>
> However, in this case the code will fit on one line, the limit is 100
> characters.
>
> > +
> > /* Since linux kernel v5.4, raid0 always has a layout */
> > if (has_raid0_layout(sb) && get_linux_version() >= 5004000)
> > raid0_need_layout = true;
> > diff --git a/tests/05r1-add-badblocks b/tests/05r1-add-badblocks
> > new file mode 100644
> > index 00000000..88b064f2
> > --- /dev/null
> > +++ b/tests/05r1-add-badblocks
> > @@ -0,0 +1,25 @@
> > +#
> > +# create a raid1 with a drive and set badblocks for the drive.
> > +# add a new drive does not cause an error.
> > +#
> > +
> > +# create raid1
> > +mdadm -CR $md0 -l1 -n2 -e1.0 $dev1 missing
> > +testdev $md0 1 $mdsize1a 64
> > +sleep 3
> > +
> > +# set badblocks for the drive
> > +dev1_name=$(basename $dev1)
> > +echo "10000 100" > /sys/block/md0/md/dev-$dev1_name/bad_blocks
> > +echo "write_error" > /sys/block/md0/md/dev-$dev1_name/state
> > +
> > +# maybe fail but that's ok, as it's only intended to
> > +# record the bad block in the metadata.
> > +mkfs.ext3 $md0
> > +
> > +# re-add and recovery
> > +mdadm $md0 -a $dev2
> > +check recovery
> > +
> > +mdadm -S $md0
> > +
>
> Since you added the test, would you be able to issue a PR on github
> to get the tests running?
Hi all
I've been fixing the test failures recently and I'll create a PR for
this patch set. So you can create a PR tomorrow. If you are not
familiar with this, I can help.
Regards
Xiao
>
> Thansk,
> Blazej
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mdadm: Clear extra flags when initializing metadata
2025-03-11 1:10 ` Xiao Ni
@ 2025-03-11 2:08 ` Wu Guanghao
2025-03-11 2:28 ` Xiao Ni
0 siblings, 1 reply; 6+ messages in thread
From: Wu Guanghao @ 2025-03-11 2:08 UTC (permalink / raw)
To: Xiao Ni, Blazej Kucman
Cc: Mariusz Tkaczyk, Yu Kuai, ncroxon, linux-raid, liubo254
在 2025/3/11 9:10, Xiao Ni 写道:
> On Mon, Mar 10, 2025 at 10:42 PM Blazej Kucman
> <blazej.kucman@linux.intel.com> wrote:
>>
>> On Mon, 10 Mar 2025 19:09:36 +0800
>> Wu Guanghao <wuguanghao3@huawei.com> wrote:
>>
>> Hi,
>> Thanks for your patch.
>>
>> you are only adding a change to native metadata so it would be good to
>> emphasize this in the title, please change "mdadm:" to "super1:"
>>
>> There are also a few checkpatch issues,
>>
>>
>>> When adding a disk to a RAID1 array, the metadata is read from the
>>> existing member disks for sync. However, only the bad_blocks flag are
>>> copied, the bad_blocks records are not copied, so the bad_blocks
>>> records are all zeros. The kernel function super_1_load() detects
>>> bad_blocks flag and reads the bad_blocks record, then sets the bad
>>> block using badblocks_set().
>>
>> WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit
>> description?) #8:
>> the bad_blocks records are not copied, so the bad_blocks records are
>> all zeros.
>>
>>>
>>> After the kernel commit 1726c7746("badblocks: improve badblocks_set()
>>> for multiple ranges handling"), if the length of a bad_blocks record
>>
>> please use SHA-1 ID - first 12 characters and space between ID and
>> (Tile)
>>
>>> is 0, it will return a failure. Therefore the device addition will
>>> fail.
>>>
>>> So when adding a new disk, some flags cannot be sync and need to be
>>> cleared.
>>>
>>> Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
>>> ---
>>> Changelog:
>>> v2:
>>> Add a testcase.
>>> Clear extra replace flag.
>>> ---
>>> super1.c | 4 ++++
>>> tests/05r1-add-badblocks | 25 +++++++++++++++++++++++++
>>> 2 files changed, 29 insertions(+)
>>> create mode 100644 tests/05r1-add-badblocks
>>>
>>> diff --git a/super1.c b/super1.c
>>> index fe3c4c64..f4a29f4f 100644
>>> --- a/super1.c
>>> +++ b/super1.c
>>> @@ -1971,6 +1971,10 @@ static int write_init_super1(struct supertype
>>> *st) long bm_offset;
>>> bool raid0_need_layout = false;
>>>
>>> + /* Clear extra flags */
>>> + sb->feature_map &= ~__cpu_to_le32(MD_FEATURE_BAD_BLOCKS |
>>> + MD_FEATURE_REPLACEMENT);
>>
>> ERROR: code indent should use tabs where possible
>> #36: FILE: super1.c:1976:
>> + MD_FEATURE_REPLACEMENT);$
>>
>> WARNING: please, no spaces at the start of a line
>> #36: FILE: super1.c:1976:
>> + MD_FEATURE_REPLACEMENT);$
>>
>> However, in this case the code will fit on one line, the limit is 100
>> characters.
>>
>>> +
>>> /* Since linux kernel v5.4, raid0 always has a layout */
>>> if (has_raid0_layout(sb) && get_linux_version() >= 5004000)
>>> raid0_need_layout = true;
>>> diff --git a/tests/05r1-add-badblocks b/tests/05r1-add-badblocks
>>> new file mode 100644
>>> index 00000000..88b064f2
>>> --- /dev/null
>>> +++ b/tests/05r1-add-badblocks
>>> @@ -0,0 +1,25 @@
>>> +#
>>> +# create a raid1 with a drive and set badblocks for the drive.
>>> +# add a new drive does not cause an error.
>>> +#
>>> +
>>> +# create raid1
>>> +mdadm -CR $md0 -l1 -n2 -e1.0 $dev1 missing
>>> +testdev $md0 1 $mdsize1a 64
>>> +sleep 3
>>> +
>>> +# set badblocks for the drive
>>> +dev1_name=$(basename $dev1)
>>> +echo "10000 100" > /sys/block/md0/md/dev-$dev1_name/bad_blocks
>>> +echo "write_error" > /sys/block/md0/md/dev-$dev1_name/state
>>> +
>>> +# maybe fail but that's ok, as it's only intended to
>>> +# record the bad block in the metadata.
>>> +mkfs.ext3 $md0
>>> +
>>> +# re-add and recovery
>>> +mdadm $md0 -a $dev2
>>> +check recovery
>>> +
>>> +mdadm -S $md0
>>> +
>>
>> Since you added the test, would you be able to issue a PR on github
>> to get the tests running?
>
> Hi all
>
> I've been fixing the test failures recently and I'll create a PR for
> this patch set. So you can create a PR tomorrow. If you are not
> familiar with this, I can help.
>
Is this the repository: https://github.com/md-raid-utilities/mdadm/ ?
If so, I will crerate a PR containing the new changes.
> Regards
> Xiao
>>
>> Thansk,
>> Blazej
>>
>
>
> .
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mdadm: Clear extra flags when initializing metadata
2025-03-10 14:41 ` Blazej Kucman
2025-03-11 1:10 ` Xiao Ni
@ 2025-03-11 2:23 ` Wu Guanghao
1 sibling, 0 replies; 6+ messages in thread
From: Wu Guanghao @ 2025-03-11 2:23 UTC (permalink / raw)
To: Blazej Kucman
Cc: Mariusz Tkaczyk, Yu Kuai, xni, ncroxon, linux-raid, liubo254
在 2025/3/10 22:41, Blazej Kucman 写道:
> On Mon, 10 Mar 2025 19:09:36 +0800
> Wu Guanghao <wuguanghao3@huawei.com> wrote:
>
> Hi,
> Thanks for your patch.
>
> you are only adding a change to native metadata so it would be good to
> emphasize this in the title, please change "mdadm:" to "super1:"
>
> There are also a few checkpatch issues,
>
>
>> When adding a disk to a RAID1 array, the metadata is read from the
>> existing member disks for sync. However, only the bad_blocks flag are
>> copied, the bad_blocks records are not copied, so the bad_blocks
>> records are all zeros. The kernel function super_1_load() detects
>> bad_blocks flag and reads the bad_blocks record, then sets the bad
>> block using badblocks_set().
>
> WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit
> description?) #8:
> the bad_blocks records are not copied, so the bad_blocks records are
> all zeros.
>
>>
>> After the kernel commit 1726c7746("badblocks: improve badblocks_set()
>> for multiple ranges handling"), if the length of a bad_blocks record
>
> please use SHA-1 ID - first 12 characters and space between ID and
> (Tile)
>
>> is 0, it will return a failure. Therefore the device addition will
>> fail.
>>
>> So when adding a new disk, some flags cannot be sync and need to be
>> cleared.
>>
>> Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
>> ---
>> Changelog:
>> v2:
>> Add a testcase.
>> Clear extra replace flag.
>> ---
>> super1.c | 4 ++++
>> tests/05r1-add-badblocks | 25 +++++++++++++++++++++++++
>> 2 files changed, 29 insertions(+)
>> create mode 100644 tests/05r1-add-badblocks
>>
>> diff --git a/super1.c b/super1.c
>> index fe3c4c64..f4a29f4f 100644
>> --- a/super1.c
>> +++ b/super1.c
>> @@ -1971,6 +1971,10 @@ static int write_init_super1(struct supertype
>> *st) long bm_offset;
>> bool raid0_need_layout = false;
>>
>> + /* Clear extra flags */
>> + sb->feature_map &= ~__cpu_to_le32(MD_FEATURE_BAD_BLOCKS |
>> + MD_FEATURE_REPLACEMENT);
>
> ERROR: code indent should use tabs where possible
> #36: FILE: super1.c:1976:
> + MD_FEATURE_REPLACEMENT);$
>
> WARNING: please, no spaces at the start of a line
> #36: FILE: super1.c:1976:
> + MD_FEATURE_REPLACEMENT);$
>
> However, in this case the code will fit on one line, the limit is 100
> characters.
>
Thank you for your feedback. I will modify it in the next version.
>> +
>> /* Since linux kernel v5.4, raid0 always has a layout */
>> if (has_raid0_layout(sb) && get_linux_version() >= 5004000)
>> raid0_need_layout = true;
>> diff --git a/tests/05r1-add-badblocks b/tests/05r1-add-badblocks
>> new file mode 100644
>> index 00000000..88b064f2
>> --- /dev/null
>> +++ b/tests/05r1-add-badblocks
>> @@ -0,0 +1,25 @@
>> +#
>> +# create a raid1 with a drive and set badblocks for the drive.
>> +# add a new drive does not cause an error.
>> +#
>> +
>> +# create raid1
>> +mdadm -CR $md0 -l1 -n2 -e1.0 $dev1 missing
>> +testdev $md0 1 $mdsize1a 64
>> +sleep 3
>> +
>> +# set badblocks for the drive
>> +dev1_name=$(basename $dev1)
>> +echo "10000 100" > /sys/block/md0/md/dev-$dev1_name/bad_blocks
>> +echo "write_error" > /sys/block/md0/md/dev-$dev1_name/state
>> +
>> +# maybe fail but that's ok, as it's only intended to
>> +# record the bad block in the metadata.
>> +mkfs.ext3 $md0
>> +
>> +# re-add and recovery
>> +mdadm $md0 -a $dev2
>> +check recovery
>> +
>> +mdadm -S $md0
>> +
>
> Since you added the test, would you be able to issue a PR on github
> to get the tests running?
>
Sure, I will create a PR and run the testcase.
> Thansk,
> Blazej
>
> .
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mdadm: Clear extra flags when initializing metadata
2025-03-11 2:08 ` Wu Guanghao
@ 2025-03-11 2:28 ` Xiao Ni
0 siblings, 0 replies; 6+ messages in thread
From: Xiao Ni @ 2025-03-11 2:28 UTC (permalink / raw)
To: Wu Guanghao
Cc: Blazej Kucman, Mariusz Tkaczyk, Yu Kuai, ncroxon, linux-raid,
liubo254
On Tue, Mar 11, 2025 at 10:08 AM Wu Guanghao <wuguanghao3@huawei.com> wrote:
>
>
>
> 在 2025/3/11 9:10, Xiao Ni 写道:
> > On Mon, Mar 10, 2025 at 10:42 PM Blazej Kucman
> > <blazej.kucman@linux.intel.com> wrote:
> >>
> >> On Mon, 10 Mar 2025 19:09:36 +0800
> >> Wu Guanghao <wuguanghao3@huawei.com> wrote:
> >>
> >> Hi,
> >> Thanks for your patch.
> >>
> >> you are only adding a change to native metadata so it would be good to
> >> emphasize this in the title, please change "mdadm:" to "super1:"
> >>
> >> There are also a few checkpatch issues,
> >>
> >>
> >>> When adding a disk to a RAID1 array, the metadata is read from the
> >>> existing member disks for sync. However, only the bad_blocks flag are
> >>> copied, the bad_blocks records are not copied, so the bad_blocks
> >>> records are all zeros. The kernel function super_1_load() detects
> >>> bad_blocks flag and reads the bad_blocks record, then sets the bad
> >>> block using badblocks_set().
> >>
> >> WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit
> >> description?) #8:
> >> the bad_blocks records are not copied, so the bad_blocks records are
> >> all zeros.
> >>
> >>>
> >>> After the kernel commit 1726c7746("badblocks: improve badblocks_set()
> >>> for multiple ranges handling"), if the length of a bad_blocks record
> >>
> >> please use SHA-1 ID - first 12 characters and space between ID and
> >> (Tile)
> >>
> >>> is 0, it will return a failure. Therefore the device addition will
> >>> fail.
> >>>
> >>> So when adding a new disk, some flags cannot be sync and need to be
> >>> cleared.
> >>>
> >>> Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
> >>> ---
> >>> Changelog:
> >>> v2:
> >>> Add a testcase.
> >>> Clear extra replace flag.
> >>> ---
> >>> super1.c | 4 ++++
> >>> tests/05r1-add-badblocks | 25 +++++++++++++++++++++++++
> >>> 2 files changed, 29 insertions(+)
> >>> create mode 100644 tests/05r1-add-badblocks
> >>>
> >>> diff --git a/super1.c b/super1.c
> >>> index fe3c4c64..f4a29f4f 100644
> >>> --- a/super1.c
> >>> +++ b/super1.c
> >>> @@ -1971,6 +1971,10 @@ static int write_init_super1(struct supertype
> >>> *st) long bm_offset;
> >>> bool raid0_need_layout = false;
> >>>
> >>> + /* Clear extra flags */
> >>> + sb->feature_map &= ~__cpu_to_le32(MD_FEATURE_BAD_BLOCKS |
> >>> + MD_FEATURE_REPLACEMENT);
> >>
> >> ERROR: code indent should use tabs where possible
> >> #36: FILE: super1.c:1976:
> >> + MD_FEATURE_REPLACEMENT);$
> >>
> >> WARNING: please, no spaces at the start of a line
> >> #36: FILE: super1.c:1976:
> >> + MD_FEATURE_REPLACEMENT);$
> >>
> >> However, in this case the code will fit on one line, the limit is 100
> >> characters.
> >>
> >>> +
> >>> /* Since linux kernel v5.4, raid0 always has a layout */
> >>> if (has_raid0_layout(sb) && get_linux_version() >= 5004000)
> >>> raid0_need_layout = true;
> >>> diff --git a/tests/05r1-add-badblocks b/tests/05r1-add-badblocks
> >>> new file mode 100644
> >>> index 00000000..88b064f2
> >>> --- /dev/null
> >>> +++ b/tests/05r1-add-badblocks
> >>> @@ -0,0 +1,25 @@
> >>> +#
> >>> +# create a raid1 with a drive and set badblocks for the drive.
> >>> +# add a new drive does not cause an error.
> >>> +#
> >>> +
> >>> +# create raid1
> >>> +mdadm -CR $md0 -l1 -n2 -e1.0 $dev1 missing
> >>> +testdev $md0 1 $mdsize1a 64
> >>> +sleep 3
> >>> +
> >>> +# set badblocks for the drive
> >>> +dev1_name=$(basename $dev1)
> >>> +echo "10000 100" > /sys/block/md0/md/dev-$dev1_name/bad_blocks
> >>> +echo "write_error" > /sys/block/md0/md/dev-$dev1_name/state
> >>> +
> >>> +# maybe fail but that's ok, as it's only intended to
> >>> +# record the bad block in the metadata.
> >>> +mkfs.ext3 $md0
> >>> +
> >>> +# re-add and recovery
> >>> +mdadm $md0 -a $dev2
> >>> +check recovery
> >>> +
> >>> +mdadm -S $md0
> >>> +
> >>
> >> Since you added the test, would you be able to issue a PR on github
> >> to get the tests running?
> >
> > Hi all
> >
> > I've been fixing the test failures recently and I'll create a PR for
> > this patch set. So you can create a PR tomorrow. If you are not
> > familiar with this, I can help.
> >
> Is this the repository: https://github.com/md-raid-utilities/mdadm/ ?
> If so, I will crerate a PR containing the new changes.
Yes, you're right.
>
>
> > Regards
> > Xiao
> >>
> >> Thansk,
> >> Blazej
> >>
> >
> >
> > .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-11 2:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <b894d081-eda9-6b28-5fef-75753838a916@huawei.com>
2025-03-10 11:09 ` [PATCH v2] mdadm: Clear extra flags when initializing metadata Wu Guanghao
2025-03-10 14:41 ` Blazej Kucman
2025-03-11 1:10 ` Xiao Ni
2025-03-11 2:08 ` Wu Guanghao
2025-03-11 2:28 ` Xiao Ni
2025-03-11 2:23 ` Wu Guanghao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox