diff --git a/archinstall/lib/disk/device_handler.py b/archinstall/lib/disk/device_handler.py index be6290bb76..7b67ade168 100644 --- a/archinstall/lib/disk/device_handler.py +++ b/archinstall/lib/disk/device_handler.py @@ -13,6 +13,7 @@ get_lsblk_info, linux_root_guid, mount, + swapoff, udev_sync, umount, ) @@ -513,7 +514,14 @@ def umount_all_existing(self, device_path: Path) -> None: # un-mount for existing encrypted partitions if partition.fs_type == FilesystemType.CRYPTO_LUKS: Luks2(partition.path).lock() + elif partition.fs_type == FilesystemType.LINUX_SWAP: + # an active swap area reports '[SWAP]' as its mountpoint; it has to be + # deactivated, unmounting the display marker cannot work + swapoff(partition.path) else: + # a swapfile can live on this filesystem; pass its mountpoints so it is + # deactivated before the unmount instead of failing afterwards + swapoff(partition.path, get_lsblk_info(partition.path).mountpoints) umount(partition.path, recursive=True) def partition( diff --git a/archinstall/lib/disk/luks.py b/archinstall/lib/disk/luks.py index 6289c250ca..2baebbec39 100644 --- a/archinstall/lib/disk/luks.py +++ b/archinstall/lib/disk/luks.py @@ -5,7 +5,7 @@ from types import TracebackType from archinstall.lib.command import SysCommand, SysCommandWorker, run -from archinstall.lib.disk.utils import get_lsblk_info, umount +from archinstall.lib.disk.utils import get_lsblk_info, swapoff, umount from archinstall.lib.exceptions import DiskError, SysCallError from archinstall.lib.log import debug, info from archinstall.lib.models.device import DEFAULT_ITER_TIME @@ -169,15 +169,18 @@ def lock(self) -> None: # For each child (sub-partition/sub-device) for child in lsblk_info.children: - # Unmount the child location - for mountpoint in child.mountpoints: - debug(f'Unmounting {mountpoint}') - umount(mountpoint, recursive=True) + # Deactivate swap first, then unmount by device path. Both helpers take a + # device, so a mountpoint (or lsblk's '[SWAP]' marker) never reaches lsblk. + swapoff(child.path, child.mountpoints) + umount(child.path, recursive=True) # And close it if possible. debug(f'Closing crypt device {child.name}') SysCommand(f'cryptsetup close {child.name}') + if remaining := get_lsblk_info(self.luks_dev_path).children: + raise DiskError(f'Could not close {self.luks_dev_path}: {[c.name for c in remaining]} still active') + def create_keyfile(self, target_path: Path, override: bool = False) -> None: """ Routine to create keyfiles, so it can be moved elsewhere diff --git a/archinstall/lib/disk/utils.py b/archinstall/lib/disk/utils.py index efa1f16515..b936a02f70 100644 --- a/archinstall/lib/disk/utils.py +++ b/archinstall/lib/disk/utils.py @@ -173,6 +173,49 @@ def mount( raise DiskError(f'Could not mount {dev_path}: {command}\n{err.message}') +def active_swaps() -> list[Path]: + """ + Currently active swap areas, as reported by the kernel. + + The first column of /proc/swaps is the area itself, either a block device + (e.g. /dev/dm-0) or a regular file (e.g. /mnt/swapfile). lsblk's '[SWAP]' + marker is a display convention, not state, and must not be used to decide + whether swap is active. + """ + entries: list[Path] = [] + for line in Path('/proc/swaps').read_text().splitlines()[1:]: + if not (fields := line.split()): + continue + entries.append(Path(fields[0])) + + return entries + + +def _swap_areas_of(target: Path, mountpoints: list[Path] | None = None) -> list[Path]: + areas: list[Path] = [] + + for area in active_swaps(): + # /proc/swaps reports resolved names (e.g. /dev/dm-0) while callers hold + # symlinks (e.g. /dev/mapper/root), so compare resolved paths + if area.exists() and target.exists(): + if area.resolve() == target.resolve(): + areas.append(area) + continue + elif area == target: + areas.append(area) + continue + + if area.is_absolute(): + for mountpoint in mountpoints or []: + # lsblk reports '[SWAP]' for an active swap area; that is a display + # marker, not a path, and nothing can live below it + if mountpoint.is_absolute() and area.is_relative_to(mountpoint): + areas.append(area) + break + + return areas + + def umount(mountpoint: Path | str, recursive: bool = False) -> None: lsblk_info = get_lsblk_info(mountpoint) @@ -192,11 +235,50 @@ def umount(mountpoint: Path | str, recursive: bool = False) -> None: def swapon(path: Path) -> None: + """ + Activate the swap area at ``path``, unless the kernel already reports it. + + The layout is mounted into a fresh target, but swap is machine-wide state: + a previous run of the installer (or the running live system) may have left + this very area active, in which case ``swapon`` fails with "Device or + resource busy" and the installation aborts for no reason. + """ + if _swap_areas_of(path): + debug(f'Swap is already active on {path}') + return + try: SysCommand(['swapon', str(path)]) except SysCallError as err: raise DiskError(f'Could not enable swap {path}:\n{err.message}') + if not _swap_areas_of(path): + raise DiskError(f'Swap was not activated on {path}, the kernel reports no swap area for it') + + +def swapoff(path: Path, mountpoints: list[Path] | None = None) -> None: + """ + Deactivate every swap area belonging to ``path``. + + ``mountpoints`` lets the caller pass the mountpoints of ``path`` so that a + swapfile living on it is found as well. A no-op is only correct when the + kernel reports no swap area for ``path``; if anything is still active + afterwards this raises instead of leaving the caller to fail later. + """ + areas = _swap_areas_of(path, mountpoints) + if not areas: + return + + for area in areas: + debug(f'Disabling swap on: {area}') + try: + SysCommand(['swapoff', str(area)]) + except SysCallError as err: + raise DiskError(f'Could not disable swap {area}:\n{err.message}') + + if remaining := _swap_areas_of(path, mountpoints): + raise DiskError(f'Swap is still active on {path}: {[str(a) for a in remaining]}') + def linux_root_guid(arch: str | None) -> PartitionGUID: if arch == 'aarch64': diff --git a/archinstall/lib/models/device.py b/archinstall/lib/models/device.py index b4c069e659..df38b03981 100644 --- a/archinstall/lib/models/device.py +++ b/archinstall/lib/models/device.py @@ -966,7 +966,10 @@ def from_existing_partition(cls, partition_info: _PartitionInfo) -> Self: SubvolumeModification.from_existing_subvol_info(i), ) else: - mountpoint = partition_info.mountpoints[0] if partition_info.mountpoints else None + # lsblk reports '[SWAP]' as the mountpoint of an active swap area. That is a + # display marker, not a path: a swap area has no mountpoint, and treating the + # marker as one leads to mounting the device on /[SWAP]. + mountpoint = next((p for p in partition_info.mountpoints if p.is_absolute()), None) subvol_mods = [] return cls( diff --git a/tests/test_swapoff_existing.py b/tests/test_swapoff_existing.py new file mode 100644 index 0000000000..15cdecde2a --- /dev/null +++ b/tests/test_swapoff_existing.py @@ -0,0 +1,141 @@ +from pathlib import Path +from unittest.mock import MagicMock + +from pytest import MonkeyPatch, raises + +from archinstall.lib.disk.device_handler import DeviceHandler +from archinstall.lib.disk.luks import Luks2 +from archinstall.lib.disk.utils import swapoff, umount +from archinstall.lib.exceptions import DiskError +from archinstall.lib.models.device import FilesystemType + + +def _patch_swaps(monkeypatch: MonkeyPatch, areas: list[Path]) -> list[list[str]]: + """Pretend the kernel reports `areas` as active swap; record swapoff calls.""" + cmds: list[list[str]] = [] + state = list(areas) + + monkeypatch.setattr('archinstall.lib.disk.utils.active_swaps', lambda: list(state)) + + def _swapoff(cmd: list[str]) -> None: + cmds.append(list(cmd)) + state.remove(Path(cmd[-1])) + + monkeypatch.setattr('archinstall.lib.disk.utils.SysCommand', _swapoff) + return cmds + + +def test_swapoff_is_a_noop_when_nothing_is_active(monkeypatch: MonkeyPatch) -> None: + cmds = _patch_swaps(monkeypatch, []) + + swapoff(Path('/dev/sda2')) + + assert cmds == [] + + +def test_swapoff_deactivates_device_and_swapfile(monkeypatch: MonkeyPatch) -> None: + cmds = _patch_swaps(monkeypatch, [Path('/dev/sda2'), Path('/mnt/swapfile'), Path('/dev/sdz9')]) + + swapoff(Path('/dev/sda2')) + swapoff(Path('/dev/sda3'), [Path('/mnt')]) + + # the device itself and the swapfile living on the second one; sdz9 untouched + assert cmds == [['swapoff', '/dev/sda2'], ['swapoff', '/mnt/swapfile']] + + +def test_swapoff_reports_swap_that_refuses_to_go_away(monkeypatch: MonkeyPatch) -> None: + monkeypatch.setattr('archinstall.lib.disk.utils.active_swaps', lambda: [Path('/dev/sda2')]) + monkeypatch.setattr('archinstall.lib.disk.utils.SysCommand', lambda _cmd: None) + + with raises(DiskError) as err: + swapoff(Path('/dev/sda2')) + + assert 'still active' in str(err.value) + + +def test_umount_does_not_silently_ignore_the_swap_marker(monkeypatch: MonkeyPatch) -> None: + """Callers must pass a device; a pseudo mountpoint has to surface as an error.""" + cmds: list[list[str]] = [] + + monkeypatch.setattr('archinstall.lib.disk.utils.SysCommand', lambda cmd: cmds.append(list(cmd))) + monkeypatch.setattr( + 'archinstall.lib.disk.utils.get_lsblk_info', + lambda path: (_ for _ in ()).throw(DiskError(f'Failed to read disk "{path}" with lsblk')), + ) + + with raises(DiskError): + umount(Path('[SWAP]'), recursive=True) + + assert cmds == [] + + +def test_umount_all_existing_dispatches_per_filesystem_type(monkeypatch: MonkeyPatch) -> None: + handler = object.__new__(DeviceHandler) + + swap_part = MagicMock(path=Path('/dev/sda2'), fs_type=FilesystemType.LINUX_SWAP) + crypt_part = MagicMock(path=Path('/dev/sda3'), fs_type=FilesystemType.CRYPTO_LUKS) + ext4_part = MagicMock(path=Path('/dev/sda4'), fs_type=FilesystemType.EXT4) + handler._devices = {Path('/dev/sda'): MagicMock(partition_infos=[swap_part, crypt_part, ext4_part])} + + swapoff_calls: list[tuple[Path, list[Path] | None]] = [] + umount_paths: list[Path] = [] + locked: list[Path] = [] + + monkeypatch.setattr( + 'archinstall.lib.disk.device_handler.swapoff', + lambda path, mountpoints=None: swapoff_calls.append((path, mountpoints)), + ) + monkeypatch.setattr( + 'archinstall.lib.disk.device_handler.umount', + lambda path, **_kwargs: umount_paths.append(path), + ) + monkeypatch.setattr( + 'archinstall.lib.disk.device_handler.Luks2', + lambda path: MagicMock(lock=lambda: locked.append(path)), + ) + monkeypatch.setattr( + 'archinstall.lib.disk.device_handler.get_lsblk_info', + lambda _path: MagicMock(mountpoints=[Path('/mnt')]), + ) + + handler.umount_all_existing(Path('/dev/sda')) + + assert locked == [Path('/dev/sda3')] + assert [p for p, _ in swapoff_calls] == [Path('/dev/sda2'), Path('/dev/sda4')] + # the mounted filesystem gets its mountpoints so a swapfile on it is found + assert swapoff_calls[1][1] == [Path('/mnt')] + assert umount_paths == [Path('/dev/sda4')] + + +def test_luks_lock_unmounts_by_device_not_by_mountpoint(monkeypatch: MonkeyPatch) -> None: + child = MagicMock(path=Path('/dev/mapper/root'), mountpoints=[Path('/mnt/archinstall')]) + child.name = 'root' + closed: list[str] = [] + + umounted: list[Path] = [] + swapped: list[tuple[Path, list[Path] | None]] = [] + + # the mapper's children are gone once it has been closed + monkeypatch.setattr( + 'archinstall.lib.disk.luks.get_lsblk_info', + lambda _path: MagicMock(children=[] if closed else [child]), + ) + monkeypatch.setattr( + 'archinstall.lib.disk.luks.SysCommand', + lambda cmd: closed.append(str(cmd)), + ) + monkeypatch.setattr( + 'archinstall.lib.disk.luks.umount', + lambda path, **_kwargs: umounted.append(path), + ) + monkeypatch.setattr( + 'archinstall.lib.disk.luks.swapoff', + lambda path, mountpoints=None: swapped.append((path, mountpoints)), + ) + + Luks2(Path('/dev/sda3')).lock() + + # swap is deactivated with the mountpoints, and unmounting uses the device path + assert swapped == [(Path('/dev/mapper/root'), [Path('/mnt/archinstall')])] + assert umounted == [Path('/dev/sda3'), Path('/dev/mapper/root')] + assert closed == ['cryptsetup close root']