From 931c5a8fdf048b98e02d76ceba6f63e07b79271d Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 4 Dec 2023 13:25:45 +0100 Subject: [PATCH 1/2] storage: add a new more generic API for mount point constraints The API should provide also information whether a mount point is required or recommended. --- .../modules/common/structures/storage.py | 32 ++++++++++-- .../modules/storage/devicetree/viewer.py | 50 +++++++++++++++++-- .../storage/devicetree/viewer_interface.py | 20 ++++++-- .../storage/test_module_device_tree.py | 30 ++++++++++- 4 files changed, 119 insertions(+), 13 deletions(-) diff --git a/pyanaconda/modules/common/structures/storage.py b/pyanaconda/modules/common/structures/storage.py index 6a84a6323be..c6242459a77 100644 --- a/pyanaconda/modules/common/structures/storage.py +++ b/pyanaconda/modules/common/structures/storage.py @@ -450,16 +450,16 @@ def get_root_device(self): return self.mount_points.get("/") -class RequiredMountPointData(DBusData): - """Constrains (filesystem and device types allowed) for mount points required - for the installed system - """ +class MountPointConstraintsData(DBusData): + """Constrains (filesystem and device types allowed) for mount points""" def __init__(self): self._mount_point = "" self._required_filesystem_type = "" self._encryption_allowed = False self._logical_volume_allowed = False + self._required = False + self._recommended = False @property def mount_point(self) -> Str: @@ -508,3 +508,27 @@ def logical_volume_allowed(self) -> Bool: @logical_volume_allowed.setter def logical_volume_allowed(self, logical_volume_allowed: Bool): self._logical_volume_allowed = logical_volume_allowed + + @property + def required(self) -> Bool: + """Whether this mount point is required + + :return: bool + """ + return self._required + + @required.setter + def required(self, required: Bool): + self._required = required + + @property + def recommended(self) -> Bool: + """Whether this mount point is recommended + + :return: bool + """ + return self._recommended + + @recommended.setter + def recommended(self, recommended: Bool): + self._recommended = recommended diff --git a/pyanaconda/modules/storage/devicetree/viewer.py b/pyanaconda/modules/storage/devicetree/viewer.py index 8b5798d0cb3..254ce9f7579 100644 --- a/pyanaconda/modules/storage/devicetree/viewer.py +++ b/pyanaconda/modules/storage/devicetree/viewer.py @@ -26,7 +26,7 @@ from pyanaconda.core.i18n import _ from pyanaconda.modules.common.errors.storage import UnknownDeviceError from pyanaconda.modules.common.structures.storage import DeviceData, DeviceActionData, \ - DeviceFormatData, OSData, RequiredMountPointData + DeviceFormatData, OSData, MountPointConstraintsData from pyanaconda.modules.storage.devicetree.utils import get_required_device_size, \ get_supported_filesystems from pyanaconda.modules.storage.platform import platform @@ -462,13 +462,55 @@ def _get_os_data(self, root): } return data + def _get_mount_point_constraints_data(self, spec): + """Get the mount point data. + + :param spec: an instance of PartSpec + :return: an instance of MountPointConstraintsData + """ + data = MountPointConstraintsData() + data.mount_point = spec.mountpoint or "" + data.required_filesystem_type = spec.fstype or "" + data.encryption_allowed = spec.encrypted + data.logical_volume_allowed = spec.lv + + return data + + def get_mount_point_constraints(self): + """Get list of constraints on mountpoints for the current platform + + Also provides hints if the partition is required or recommended. + + This includes mount points required to boot (e.g. /boot/efi, /boot) + and the / partition which is always considered to be required. + + :return: a list of mount points with its constraints + """ + + constraints = [] + + # Root partition is required + root_partition = PartSpec(mountpoint="/", lv=True, thin=True, encrypted=True) + root_constraint = self._get_mount_point_constraints_data(root_partition) + root_constraint.required = True + constraints.append(root_constraint) + + # Platform partitions are required except for /boot partiotion which is recommended + for p in platform.partitions: + if p.mountpoint: + constraint = self._get_mount_point_constraints_data(p) + constraint.required = True + constraints.append(constraint) + + return constraints + def _get_platform_mount_point_data(self, spec): """Get the mount point data. :param spec: an instance of PartSpec - :return: an instance of RequiredMountPointData + :return: an instance of MountPointConstraintsData """ - data = RequiredMountPointData() + data = MountPointConstraintsData() data.mount_point = spec.mountpoint or "" data.required_filesystem_type = spec.fstype or "" data.encryption_allowed = spec.encrypted @@ -482,7 +524,7 @@ def get_required_mount_points(self): This includes mount points required to boot (e.g. /boot and /boot/efi) and the / partition which is always considered to be required. - :return: a list of mount points + :return: a list of mount points with its constraints """ root_partition = PartSpec(mountpoint="/", lv=True, thin=True, encrypted=True) ret = list(map(self._get_platform_mount_point_data, diff --git a/pyanaconda/modules/storage/devicetree/viewer_interface.py b/pyanaconda/modules/storage/devicetree/viewer_interface.py index 9a0eee051d7..bdb9de91e97 100644 --- a/pyanaconda/modules/storage/devicetree/viewer_interface.py +++ b/pyanaconda/modules/storage/devicetree/viewer_interface.py @@ -22,7 +22,7 @@ from dasbus.typing import * # pylint: disable=wildcard-import from pyanaconda.modules.common.constants.interfaces import DEVICE_TREE_VIEWER from pyanaconda.modules.common.structures.storage import DeviceData, DeviceActionData, \ - DeviceFormatData, OSData, RequiredMountPointData + DeviceFormatData, OSData, MountPointConstraintsData __all__ = ["DeviceTreeViewerInterface"] @@ -193,13 +193,27 @@ def GetExistingSystems(self) -> List[Structure]: """ return OSData.to_structure_list(self.implementation.get_existing_systems()) + # FIXME: remove the replaced API + def GetMountPointConstraints(self) -> List[Structure]: + """Get list of constraints on mountpoints for the current platform + + Also provides hints if the partition is required or recommended. + + This includes mount points required to boot (e.g. /boot/efi, /boot) + and the / partition which is always considered to be required. + + :return: a list of mount points with its constraints + """ + return MountPointConstraintsData.to_structure_list( + self.implementation.get_mount_point_constraints()) + def GetRequiredMountPoints(self) -> List[Structure]: """Get list of required mount points for the current platform This includes mount points required to boot (e.g. /boot and /boot/efi) and the / partition which is always considered to be required. - :return: a list of mount points + :return: a list of mount points with its constraints """ - return RequiredMountPointData.to_structure_list( + return MountPointConstraintsData.to_structure_list( self.implementation.get_required_mount_points()) diff --git a/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py b/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py index 5af938c640d..58187110a9d 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py +++ b/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py @@ -36,7 +36,8 @@ from dasbus.typing import * # pylint: disable=wildcard-import from pyanaconda.core.kernel import KernelArguments from pyanaconda.modules.common.errors.storage import UnknownDeviceError, MountFilesystemError -from pyanaconda.modules.common.structures.storage import DeviceFormatData, RequiredMountPointData +from pyanaconda.modules.common.structures.storage import DeviceFormatData, \ + MountPointConstraintsData from pyanaconda.modules.storage.devicetree import DeviceTreeModule, create_storage, utils from pyanaconda.modules.storage.devicetree.devicetree_interface import DeviceTreeInterface from pyanaconda.modules.storage.devicetree.populate import FindDevicesTask @@ -829,13 +830,38 @@ def test_set_device_mount_options(self): self.interface.SetDeviceMountOptions("dev1", "") assert dev1.format.options == "defaults" + def test_get_mount_point_constraints(self): + """Test GetMountPointConstraints.""" + result = self.interface.GetMountPointConstraints() + assert isinstance(result, list) + assert len(result) == 2 + + result = MountPointConstraintsData.from_structure_list( + self.interface.GetMountPointConstraints() + ) + for mp in result: + assert mp.mount_point is not None + assert mp.required_filesystem_type is not None + + # we are always adding / so it's a good candidate for testing + root = next(r for r in result if r.mount_point == "/") + assert root is not None + assert root.encryption_allowed is True + assert root.logical_volume_allowed is True + assert root.mount_point == "/" + assert root.required_filesystem_type == "" + assert root.required is True + assert root.recommended is False + def test_get_required_mount_points(self): """Test GetRequiredMountPoints.""" result = self.interface.GetRequiredMountPoints() assert isinstance(result, list) assert len(result) != 0 - result = RequiredMountPointData.from_structure_list(self.interface.GetRequiredMountPoints()) + result = MountPointConstraintsData.from_structure_list( + self.interface.GetRequiredMountPoints() + ) for mp in result: assert mp.mount_point is not None assert mp.required_filesystem_type is not None From 0e28a7d5173d4ccb29a23cac1fd095b9912bea88 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Fri, 15 Dec 2023 09:43:22 +0100 Subject: [PATCH 2/2] storage: do not add /boot among required partitions In general it is recommended to have a separate /boot, but not strictly required. In some cases it is required but we need to add API for that, because it depends mainly on the filesystem of the root partition. So now the requirement regards mainly biosboot and efi partitions, which is dependent on the platform. --- pyanaconda/modules/storage/devicetree/viewer.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pyanaconda/modules/storage/devicetree/viewer.py b/pyanaconda/modules/storage/devicetree/viewer.py index 254ce9f7579..f313a0aeef1 100644 --- a/pyanaconda/modules/storage/devicetree/viewer.py +++ b/pyanaconda/modules/storage/devicetree/viewer.py @@ -484,6 +484,9 @@ def get_mount_point_constraints(self): This includes mount points required to boot (e.g. /boot/efi, /boot) and the / partition which is always considered to be required. + /boot is not required in general but can be required in some cases, + depending on the filesystem on the root partition (ie crypted root). + :return: a list of mount points with its constraints """ @@ -499,7 +502,10 @@ def get_mount_point_constraints(self): for p in platform.partitions: if p.mountpoint: constraint = self._get_mount_point_constraints_data(p) - constraint.required = True + if p.mountpoint == "/boot": + constraint.recommended = True + else: + constraint.required = True constraints.append(constraint) return constraints