Conversation
mwilck
force-pushed
the
last-fail-fix
branch
from
September 18, 2026 22:07
d89ce7b to
00d150f
Compare
Since commit fc6fd40 ("Manage: Block unsafe member failing"), mdadm has a safeguard against failing the last leg of an error by accident. Add tests for this feature. The tests involving partitions as component devices and using set-A / set-B are currently broken. They will be fixed with the next commit. Signed-off-by: Martin Wilck <mwilck@suse.com>
devnm2devid() works only for disk devices (/sys/block/$DEVNAME/dev), but not for partitions (sys/block/$DISK/$PART/dev). This causes is_remove_safe() to falsely return true if the last leg of a mirror that is failed is a partition. The safeguard that commit fc6fd40 ("Manage: Block unsafe member failing") was supposed to provide is not effective in these cases. It also fails for "--fail set-X" in RAID10 setups. Moreover, in Manage_subdevs(), we already determine the device correctly, there is no need to determine it again; we just need to pass the rdev to is_remove_safe(). Fixes: fc6fd40 ("Manage: Block unsafe member failing") Suggested-by: Shinkichi Yamazaki <shinkichi.yamazaki@suse.com> Signed-off-by: Martin Wilck <mwilck@suse.com>
mwilck
force-pushed
the
last-fail-fix
branch
from
September 21, 2026 16:34
00d150f to
2f89420
Compare
Collaborator
Author
|
Modified; based directly on main now, not on #303. |
mtkaczyk
reviewed
Sep 21, 2026
| @@ -0,0 +1,39 @@ | |||
| # create a RAID1 from partitions, try to fail both legs. | |||
Member
There was a problem hiding this comment.
Kernel failfast feature is something else. We have name conflict I think.
| SEP=p | ||
| fi | ||
|
|
||
| mdadm -CR $md0 -l1 -n2 --assume-clean $dev2 $dev3 |
Member
There was a problem hiding this comment.
R1 and R10 covered, what about r5 or any from raid456?
| mdadm -S "$md0" | ||
|
|
||
| for d in $dev0 $dev1 $dev2 $dev3; do | ||
| sfdisk $d <<EOF |
Member
There was a problem hiding this comment.
two cases are covered here:
- bug for partitions
- mdadm owned faulty state verification
Codex told me:
That exact partition-based example appears in [test line 195. Each expanded --disks= argument is
collected into the device array at test line 346) then assigned to $dev0 through $dev13 in
tests/func.sh line 262
I found exactly one test under tests/ that creates partitions: [tests/07autodetect line 3.
It creates two partitionable MD arrays, runs sfdisk on those MD devices, and then uses ${mdp0}p1 and ${mdp1}p1.
It does not repartition the physical devices passed through --disks; the partition table is created inside
the MD arrays. No other test script uses sfdisk, fdisk, parted, partprobe, or kpartx.
I would prefer to not include partitions directly assuming that this defect will not appear in this form anymore and we should focus on executing this whole test suite on partitions and block devices as a two separate jobs.
Executing on partitions is very important case for MD stacked device. I know we have no working CI yet, but this should be our direction.
If I missed something, please let me know.
| * Return: True if array will be operational, false otherwise. | ||
| */ | ||
| bool is_remove_safe(mdu_array_info_t *array, const int fd, char *devname, const int verbose) | ||
| static bool is_remove_safe(mdu_array_info_t *array, const int fd, dev_t devid, const int verbose) |
Member
There was a problem hiding this comment.
description is not updated. When, updating please include why correct devid is important to be used here. You can mention the bug you discovered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(This goes on top of #303, which it currently includes)fc6fd40 ("Manage: Block unsafe member failing") was meant as a safeguard against users shooting themselves into the foot by removing the last up-to-date leg of a RAID mirror. But it works only if the RAID members are disks, not for partitions, and fails for
set-A/set-Boperations, too.Add a test for this feature (which fails in the first place), and a fix that makes the test succeed.