在 2025/3/11 9:10, Xiao Ni 写道:
> On Mon, Mar 10, 2025 at 10:42 PM Blazej Kucman
> <blazej.kucman@xxxxxxxxxxxxxxx> wrote:
>>
>> On Mon, 10 Mar 2025 19:09:36 +0800
>> Wu Guanghao <wuguanghao3@xxxxxxxxxx> 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@xxxxxxxxxx>
>>> ---
>>> 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
>>
>
>
> .
