Skip to content

disk-deactivate: wipe raid arrays before stopping them #1248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kmein wants to merge 2 commits into
base: master
Choose a base branch
from
+131 −0
Open

disk-deactivate: wipe raid arrays before stopping them #1248

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
669ca49
disk-deactivate: wipe raid arrays before stopping them
kmein Apr 8, 2026
dffa200
add canary test for btrfs on mdadm wipes
kmein Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions disk-deactivate/disk-deactivate.jq
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def deactivate:
]
elif (.type | contains("raid")) then
[
"wipefs --all -f \(.path | shellquote)",
"mdadm --stop \(.name | shellquote)"
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mdadm --stop is invoked with .name, but lsblk’s name field is typically not an absolute device path. This can make the stop command fail (and it also differs from other places in the repo that stop arrays via /dev/md/<name>). Prefer stopping the array via .path (or otherwise ensure an absolute /dev/... path is passed).

Suggested change
"mdadm --stop \(.name | shellquote)"
"mdadm --stop \(.path | shellquote)"

Copilot uses AI. Check for mistakes.
Comment on lines 61 to 64
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fixes a subtle teardown ordering bug (wipe md device signatures before stopping the array), but there’s no automated regression test ensuring destroyFormatMount actually forces a re-mkfs for filesystems on mdadm arrays (especially btrfs with backup superblocks). Consider adding a NixOS test that provisions btrfs-on-mdadm, runs destroy+recreate, and asserts the old btrfs signature/data is gone (i.e., mkfs is not skipped).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

@kmein kmein Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a NixOS test that ensures the expected behaviour. It does not use diskoLib.testLib.makeDiskoTest because that function does not test the unhappy-path possibility of files being left over from previous iterations.

]
else
Expand Down
Loading
⚡