Skip to content

fix: skip deleted partitions when wiping signatures and encrypting - #4853

Open
Kolgrim33 wants to merge 1 commit into
archlinux:masterfrom
Kolgrim33:fix/manual-partitioning-delete-wipefs
Open

Kolgrim33 wants to merge 1 commit into
archlinux:masterfrom
Kolgrim33:fix/manual-partitioning-delete-wipefs

Conversation

@Kolgrim33

Copy link
Copy Markdown

PR Description:

When using manual partitioning without wiping the whole disk (wipe=False), deleting an existing partition sets its status to ModificationStatus.DELETE while retaining its dev_path. Because PartitionModification.exists() only checks status == ModificationStatus.EXIST, filtering with [p for p in modification.partitions if not p.exists()] also includes DELETE entries.

After disk.commit() removes the deleted partition's device node, the signature-wipe loop in DeviceHandler.partition() ran wipefs --all against the removed device node, crashing the installation with SysCallError.

Similarly, FilesystemHandler._encrypt_partitions() and select_partitions_to_encrypt() filtered with not p.exists() instead of p.is_create_or_modify(), and _encrypt_partitions() reset enc_mods = {} inside the device_modifications loop.

This PR:

  • Checks part_mod.is_create_or_modify() before running wipefs --all in DeviceHandler.partition().
  • Uses p.is_create_or_modify() in FilesystemHandler._encrypt_partitions() (matching _format_partitions()) and select_partitions_to_encrypt().
  • Removes the redundant enc_mods = {} reset inside the device_modifications loop in _encrypt_partitions().
  • Adds unit tests in tests/test_disk_partitioning.py.

Fixes #4849

Tests and Checks

  • I have tested the code!

When using manual partitioning without wiping the whole disk, deleting
an existing partition marks it with ModificationStatus.DELETE while
keeping its previous dev_path. Because PartitionModification.exists()
only checks for ModificationStatus.EXIST, [p for p in partitions if not
p.exists()] also includes deleted partitions.

After disk.commit() removes the deleted partition's device node, the
signature-wipe loop in DeviceHandler.partition() attempted to run
'wipefs --all' against the removed device node, crashing the installer
with a SysCallError.

Similarly, FilesystemHandler._encrypt_partitions() and
select_partitions_to_encrypt() used 'not p.exists()' instead of
'p.is_create_or_modify()', and _encrypt_partitions() re-initialized
enc_mods inside the device_modifications loop.

Filter for is_create_or_modify() when wiping signatures, validating/
encrypting partitions, and selecting partitions to encrypt, and preserve
enc_mods across device modifications.

Fixes archlinux#4849
@Kolgrim33
Kolgrim33 requested a review from Torxed as a code owner October 25, 2026 16:45

@xander-lin xander-lin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approve — the patch itself is correct, minimal, and well tested.

To be explicit about scope: this does not fully fix the delete-partition
failure mode.
With this patch applied (42a647c), the same scenario still
crashes hard — it just crashes one stage later. Stage 1 (wipefs) now passes
thanks to the new guard; stage 2 (mount) dies on the same tombstone entry. For
a user who deletes a swap partition, or assigns a mountpoint and then deletes
it, the install still fails; only the error message changes. Verified repro on
this branch below.

What I verified

  • Reproduced the exact failure from #4849 on a real loop device (manual partitioning, no wipe, delete the highest-numbered partition): master dies at wipefs --all /dev/loop0p2 (No such file or directory), this branch completes stage 1 cleanly. The patch does what it claims.
  • is_create_or_modify() and part_mod.dev_path is the right guard (see root cause below).
  • The _encrypt_partitions rework is a real second fix (multi-disk LVM-on-LUKS was losing enc_mods to the in-loop reset) and also removes the _validate_partitions false positive for DELETE entries (fs_type=None).
  • Full test suite passes; the new tests are meaningful.
  • And then the flow dies in stage 2: the mount stage has the exact same blind spot (details below).

Root cause: the skip contract is per-stage, and the mount stage doesn't have it

A PartitionModification with status=DELETE is a tombstone: it must stay
in the device modification (the partitioner needs its dev_path to remove the
partition), but every downstream stage must skip it — after partition()
commits, that entry's dev_path points at a node that no longer exists (if the
GPT number gets recycled, at an unrelated partition).

That skip contract is implicit and re-implemented per stage:

stage skips DELETE?
_format_partitions (is_create_or_modify filter) yes
wipefs loop (device_handler.partition) yes, thanks to this PR
Installer._mount_partition no

The TUI delete action only flips the status (partitioning_menu.py:361):
fs_type, dev_path, mountpoint and btrfs subvolumes all stay on the
tombstone. So at mount time _mount_partition() walks into one of three
branches that all act on the stale entry — and the sort in
_mount_partition_layout() (installer.py:305, mountpoint or Path('/')) puts
mountpoint-less entries (i.e. every tombstone) first, so the failure fires
before any real mount:

branch line (this branch) first-run trigger
if part_mod.mountpoint: installer.py:377 user assigns a mountpoint, then deletes the partition
elif part_mod.fs_type == BTRFS: installer.py:385 delete a btrfs partition whose mounted subvols were copied into the blueprint by from_existing_partition
elif part_mod.is_swap(): installer.py:394 delete a swap partition without recreating it (user goes swapless)

Full repro (verified on this branch; swap-branch variant)

Needs root + a spare loop device. Stage 1 passes (the PR guard works), stage 2
dies on the tombstone:

truncate -s 1G /tmp/repro.img
losetup /dev/loop0 /tmp/repro.img   # adjust to a free loop dev
sfdisk -q /dev/loop0 <<'EOF'
label: gpt
start=1MiB, size=400MiB, type=linux-swap
start=401MiB, size=500MiB
EOF
losetup -d /dev/loop0
losetup -P /dev/loop0 /tmp/repro.img   # re-attach with partition scan
mkswap /dev/loop0p1 && mkfs.ext4 -F /dev/loop0p2
# run as root; crashes at swapon(/dev/loop0p1): No such file or directory
from pathlib import Path

from archinstall.lib.disk.device_handler import device_handler
from archinstall.lib.disk.filesystem import FilesystemHandler
from archinstall.lib.installer import Installer
from archinstall.lib.models.device import (
	DeviceModification, DiskLayoutConfiguration, DiskLayoutType,
	ModificationStatus, PartitionModification,
)

device_handler.load_devices()
bd = device_handler.get_device(Path('/dev/loop0'))

parts = []
for info in sorted(bd.partition_infos, key=lambda i: i.partn or 0):
	pm = PartitionModification.from_existing_partition(info)  # TUI-faithful
	if pm.dev_path == Path('/dev/loop0p1'):
		pm.status = ModificationStatus.DELETE   # exactly what the TUI does,
		pm.mountpoint = None                    # and only that (partitioning_menu.py:361)
	parts.append(pm)
if not any(p.status == ModificationStatus.DELETE for p in parts):
	raise SystemExit('expected p1 in blueprint; check loop dev setup')
mod = DeviceModification(bd, wipe=False, partitions=parts)
config = DiskLayoutConfiguration(config_type=DiskLayoutType.Manual, device_modifications=[mod])

mnt = Path('/tmp/repro-mnt'); mnt.mkdir(exist_ok=True)
FilesystemHandler(config).perform_filesystem_operations()  # stage 1: OK on this branch
Installer(mnt, config).mount_ordered_layout()              # stage 2: DiskError

Expected on this branch: DiskError: Could not enable swap /dev/loop0p1 ... No such file or directory — the partition was correctly removed in stage 1,
then mounted from its tombstone entry in stage 2. On master, the same script
already fails in stage 1 at wipefs (#4849).

Suggested follow-up (not required for this PR)

Enforce the skip contract once, centrally, instead of per-branch — e.g. in
_mount_partition_layout() (ModificationStatus added to the
models.device import):

not_pv_part_mods = [p for p in mod.partitions if p not in pvs and p.status != ModificationStatus.DELETE]

That one line closes all three branches. Note that the swap branch itself has
to stay: genfstab derives swap fstab entries from /proc/swaps, so the
partition must be activated before fstab generation — the missing part is only
the DELETE skip. Happy to follow up with an issue + fix PR for the mount stage
if that's out of scope for this one.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Manual partitioning without wipe: deleting an existing partition crashes installer (wipefs runs on deleted partition path)

3 participants