fix: unmount the whole subtree below a mapper in Luks2.lock()

archinstall/lib/disk/luks.py

@@ -8,11 +8,21 @@
 from archinstall.lib.disk.utils import get_lsblk_info, umount
 from archinstall.lib.exceptions import DiskError, SysCallError
 from archinstall.lib.log import debug, info
-from archinstall.lib.models.device import DEFAULT_ITER_TIME
+from archinstall.lib.models.device import DEFAULT_ITER_TIME, LsblkInfo
 from archinstall.lib.models.users import Password
 from archinstall.lib.utils.util import generate_password
 
 
+def _lsblk_subtree(info: LsblkInfo) -> list[LsblkInfo]:
+	"""The device and everything stacked below it, parents before children."""
+	nodes = [info]
+
+	for child in info.children:
+		nodes.extend(_lsblk_subtree(child))
+
+	return nodes
+
+
 @dataclass
 class Luks2:
 	luks_dev_path: Path
@@ -169,10 +179,13 @@ 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)
+			# Everything mounted below the child has to go before the child itself: on
+			# LUKS-on-LVM the mountpoints live on the logical volumes, which are
+			# grandchildren of the mapper, so looking one level down left them mounted
+			# and `cryptsetup close` then found the device busy.
+			for node in reversed(_lsblk_subtree(child)):
+				debug(f'Unmounting {node.name}')
+				umount(node.path, recursive=True)
 
 			# And close it if possible.
 			debug(f'Closing crypt device {child.name}')

tests/test_luks_lock_subtree.py

@@ -0,0 +1,93 @@
+from collections.abc import Callable
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+from archinstall.lib.disk.luks import Luks2, _lsblk_subtree
+from archinstall.lib.models.device import LsblkInfo
+
+
+def _node(name: str, children: list[LsblkInfo] | None = None) -> LsblkInfo:
+	info = MagicMock(spec=LsblkInfo)
+	info.name = name
+	info.path = Path(f'/dev/{name}')
+	info.mountpoints = []
+	info.fsroots = []
+	info.children = children or []
+	return info
+
+
+def _recorder(sink: list[Path]) -> Callable[..., None]:
+	def record(path: Path, **_kwargs: object) -> None:
+		sink.append(Path(path))
+
+	return record
+
+
+def _collector(sink: list[str]) -> Callable[..., None]:
+	def collect(command: str) -> None:
+		sink.append(command)
+
+	return collect
+
+
+def test_subtree_includes_grandchildren_parents_first() -> None:
+	"""The subtree of a mapper has to reach the logical volumes under it."""
+	grandchild_a = _node('vg-root')
+	grandchild_b = _node('vg-home')
+	child = _node('vg', [grandchild_a, grandchild_b])
+
+	nodes = _lsblk_subtree(child)
+
+	assert [n.name for n in nodes] == ['vg', 'vg-root', 'vg-home']
+
+
+def test_lock_unmounts_grandchild_mountpoints() -> None:
+	"""On LUKS-on-LVM the mountpoints live on the logical volumes, two levels down.
+
+	`cryptsetup close` refuses to close a mapper that still has something mounted
+	below it, so the whole subtree has to be unmounted, children before parents.
+	"""
+	root_lv = _node('vg-root')
+	root_lv.mountpoints = [Path('/mnt/archinstall')]
+	home_lv = _node('vg-home')
+	home_lv.mountpoints = [Path('/mnt/archinstall/home')]
+	vg = _node('vg', [root_lv, home_lv])
+
+	lsblk_root = MagicMock(spec=LsblkInfo)
+	lsblk_root.children = [vg]
+
+	luks = object.__new__(Luks2)
+	luks.luks_dev_path = Path('/dev/sda1')
+
+	umounted: list[Path] = []
+
+	with (
+		patch('archinstall.lib.disk.luks.get_lsblk_info', return_value=lsblk_root),
+		patch('archinstall.lib.disk.luks.umount', side_effect=_recorder(umounted)),
+		patch('archinstall.lib.disk.luks.SysCommand'),
+	):
+		luks.lock()
+
+	assert Path('/dev/vg-home') in umounted, 'the logical volumes must be unmounted'
+	assert Path('/dev/vg-root') in umounted
+	assert umounted.index(Path('/dev/vg-home')) < umounted.index(Path('/dev/vg'))
+
+
+def test_lock_still_closes_the_mapper() -> None:
+	child = _node('vg')
+	lsblk_root = MagicMock(spec=LsblkInfo)
+	lsblk_root.children = [child]
+
+	luks = object.__new__(Luks2)
+	luks.luks_dev_path = Path('/dev/sda1')
+
+	closed: list[str] = []
+
+	with (
+		patch('archinstall.lib.disk.luks.get_lsblk_info', return_value=lsblk_root),
+		patch('archinstall.lib.disk.luks.umount'),
+		patch('archinstall.lib.disk.luks.SysCommand', side_effect=_collector(closed)),
+	):
+		luks.lock()
+
+	assert any('cryptsetup close vg' in command for command in closed), closed