Handle active swap partitions when opening an existing disk - #4745
Open
VykosMolt wants to merge 2 commits into
Open
Handle active swap partitions when opening an existing disk#4745VykosMolt wants to merge 2 commits into
VykosMolt wants to merge 2 commits into
Conversation
lsblk reports an active swap area by printing `[SWAP]` where a mountpoint
would go, in both `mountpoint` and `mountpoints`. `LsblkInfo` parsed that
straight into `Path('[SWAP]')`, so a swap partition looked like it was
mounted at a folder of that name. That is why `[SWAP]` shows up as a
mountpoint in the partition list, and it is what later gets handed to
`umount`.
`[SWAP]` is the only bracketed value lsblk emits, so match it exactly rather
than stripping anything bracketed: a real mountpoint may legitimately contain
brackets.
`umount_all_existing()` ran `umount` against every partition that was not LUKS, including swap. Swap is not mounted at a folder, so the call failed and the partition stayed busy, which is what breaks opening the installer a second time on a disk with a legacy swap partition. Route swap partitions to a new `swapoff()` helper alongside `swapon()`. swapoff fails if it is pointed at something that is not currently in use as swap, so the helper asks `swapon --show` first and does nothing if the path is not there. Both sides are resolved because swap can be switched on through a link such as /dev/disk/by-uuid/... A real failure raises `DiskError`, matching `swapon()`. Taking that list from swapon rather than from lsblk keeps the helper usable for anything that can be swap, including encrypted and LVM devices and swap files. lsblk cannot describe a swap file at all.
Collaborator
|
This seems to be a purely AI generated PR, we don't have a AI policy (yet) so I'll leave these comments for it
|
This was referenced Sep 1, 2026
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
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.
Fixes #4235
Install once with a normal swap partition, then open the installer again on
that disk. Swap shows up in the partition list as if it were mounted at a
folder called
[SWAP], and tearing the layout down fails.Both come from the same place. When a partition is in use as swap, lsblk
prints
[SWAP]where it would normally print a mountpoint, in bothmountpointandmountpoints.LsblkInfoturned that intoPath('[SWAP]'),so as far as the rest of the code was concerned the partition was mounted at a
folder of that name.
umount()then saw a non-empty list of mountpoints,skipped the early return it has for unmounted partitions, and ran
umount -R '[SWAP]'.Two changes.
LsblkInfono longer treats[SWAP]as a path. It is matched exactly ratherthan stripping anything in brackets, because
[SWAP]is the only bracketedvalue lsblk prints and a real folder is allowed brackets in its name. That
also keeps it out of the partition list, which I think matches what you meant
by "could simply be skipped".
umount_all_existing()now sends swap partitions to a newswapoff()helpernext to
swapon(), rather than trying to unmount them. Swap is not mounted ata folder, so there is nothing to unmount; it has to be switched off or the
partition stays busy and the disk cannot be repartitioned.
swapoffitself errors if you point it at something that is not currently inuse as swap, so the helper asks
swapon --showwhat is in use before doinganything. That makes it safe to call on a partition whose swap is already off,
which matters because the installer cannot tell the two apart. Both paths are
resolved before comparing, since swap can be switched on through a link like
/dev/disk/by-uuid/...while swapon reports the device it points at.I took that list from swapon rather than from lsblk on purpose. A swap area is
not always a block device, and
get_lsblk_info()raises on a swap file:Going through swapon means the helper works for partitions, encrypted and LVM
devices, and swap files alike, so it should be reusable for the teardown work
in #4536 and for the swap file in #4729 without either needing its own copy.
Real failures raise
DiskError, matchingswapon(), so nothing is swallowed.Nine tests cover both fields carrying
[SWAP], swap that is not currently on,ordinary mountpoints, a real mountpoint with brackets in it, the symlink case,
and both failure paths. Seven of them fail on master.
One known gap: LUKS-encrypted swap still will not tear down, because
cryptsetup closefinds the mapper busy. That is broken on master today tooand it needs a LUKS setup to test properly, so I have left it rather than
guess at it.
One limitation worth stating:
swapon --show --rawprints names unquoted, so aswap path containing whitespace would be ambiguous. Nothing archinstall creates
looks like that, but it is a real bound rather than a non-issue.
Claude Opus was used for codebase archaeology, implementation assistance, and
test review. I reviewed and tested the changes myself.