From d9718101c10989ff643e2481044923369a9e939c Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 4 Dec 2023 13:25:45 +0100 Subject: [PATCH 1/3] storage: do not add /boot among required partitions In general 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. TODO: update tests --- pyanaconda/modules/storage/devicetree/viewer.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pyanaconda/modules/storage/devicetree/viewer.py b/pyanaconda/modules/storage/devicetree/viewer.py index f183ad708c4..762ae684e10 100644 --- a/pyanaconda/modules/storage/devicetree/viewer.py +++ b/pyanaconda/modules/storage/devicetree/viewer.py @@ -493,6 +493,10 @@ def get_required_mount_points(self): :return: a list of mount points """ root_partition = PartSpec(mountpoint="/", lv=True, thin=True, encrypted=True) + # FIXME in general /boot is not required, just recommended. Depending + # on the filesystem on the root partition it may be required (ie + # crypted root). ret = list(map(self._get_platform_mount_point_data, - [p for p in platform.partitions if p.mountpoint] + [root_partition])) + [p for p in platform.partitions if p.mountpoint and p.mountpoint != "/boot"] + + [root_partition])) return ret From 49828c243a0d5674672e9f4b4bf51e9a878bcbe0 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 4 Dec 2023 13:49:54 +0100 Subject: [PATCH 2/3] storage: provide list of required mount points --- .../modules/common/structures/storage.py | 6 ++-- .../modules/storage/devicetree/viewer.py | 31 ++++++++++++++----- .../storage/devicetree/viewer_interface.py | 23 +++++++++++--- .../storage/test_module_device_tree.py | 17 ++++++++-- 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/pyanaconda/modules/common/structures/storage.py b/pyanaconda/modules/common/structures/storage.py index 6db09ec0e2c..7795e1de766 100644 --- a/pyanaconda/modules/common/structures/storage.py +++ b/pyanaconda/modules/common/structures/storage.py @@ -455,10 +455,8 @@ 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 = "" diff --git a/pyanaconda/modules/storage/devicetree/viewer.py b/pyanaconda/modules/storage/devicetree/viewer.py index 762ae684e10..5070cf4d250 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 @@ -474,9 +474,9 @@ 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 @@ -487,16 +487,33 @@ def _get_platform_mount_point_data(self, spec): def get_required_mount_points(self): """Get list of required mount points for the current platform - This includes mount points required to boot (e.g. /boot and /boot/efi) + This includes mount points required to boot (e.g. /boot/efi) and the / partition which is always considered to be required. - :return: a list of mount points + FIXME in general /boot is not required, just recommended. Depending on + the filesystem on the root partition it may be required (ie crypted + root). + + :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, + [p for p in platform.partitions if p.mountpoint and p.mountpoint != "/boot"] + + [root_partition])) + return ret + + def get_recommended_mount_points(self): + """Get list of recommended mount points for the current platform + + Currently it contains only /boot partition if it is default the for + platform. + + :return: a list of mount points with its constraints + """ # FIXME in general /boot is not required, just recommended. Depending # on the filesystem on the root partition it may be required (ie # crypted root). + recommended = ["/boot"] ret = list(map(self._get_platform_mount_point_data, - [p for p in platform.partitions if p.mountpoint and p.mountpoint != "/boot"] - + [root_partition])) + [p for p in platform.partitions if p.mountpoint in recommended])) return ret diff --git a/pyanaconda/modules/storage/devicetree/viewer_interface.py b/pyanaconda/modules/storage/devicetree/viewer_interface.py index 9a0eee051d7..4773adead19 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"] @@ -196,10 +196,25 @@ def GetExistingSystems(self) -> List[Structure]: 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) + This includes mount points required to boot (e.g. /boot/efi) and the / partition which is always considered to be required. - :return: a list of mount points + FIXME in general /boot is not required, just recommended. Depending on + the filesystem on the root partition it may be required (ie crypted + root). + + :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()) + + def GetRecommendedMountPoints(self) -> List[Structure]: + """Get list of recommended mount points for the current platform + + Currently it contains only /boot partition if it is default the for + platform. + + :return: a list of mount points with its constraints + """ + return MountPointConstraintsData.to_structure_list( + self.implementation.get_recommended_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 21180e86ada..db818334fc7 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,7 @@ 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 @@ -855,7 +855,7 @@ def test_get_required_mount_points(self): 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 @@ -868,6 +868,19 @@ def test_get_required_mount_points(self): assert root.mount_point == "/" assert root.required_filesystem_type == "" + def test_get_recommended_mount_points(self): + """Test GetRecommendedMountPoints.""" + result = self.interface.GetRecommendedMountPoints() + assert isinstance(result, list) + assert len(result) == 1 + + result = MountPointConstraintsData.from_structure_list(self.interface.GetRecommendedMountPoints()) + boot = result[0] + assert boot is not None + assert boot.encryption_allowed is False + assert boot.logical_volume_allowed is False + assert boot.mount_point == "/boot" + assert boot.required_filesystem_type == "" class DeviceTreeTasksTestCase(unittest.TestCase): """Test the storage tasks.""" From 602318dac6b37859230930cff084bc5943b92e26 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Fri, 8 Dec 2023 10:53:45 +0100 Subject: [PATCH 3/3] storage: add a new more generic API for mount point constraints Instead of adding GetRecommendedMountPoints to GetRequiredMountpoints replace the both with a bit more generic GetMountPointConstraints that would add recommended/required flag to the constraints. --- .../modules/common/structures/storage.py | 26 ++++++++++ .../modules/storage/devicetree/viewer.py | 48 +++++++++++++++++++ .../storage/devicetree/viewer_interface.py | 14 ++++++ 3 files changed, 88 insertions(+) diff --git a/pyanaconda/modules/common/structures/storage.py b/pyanaconda/modules/common/structures/storage.py index 7795e1de766..58ad366c611 100644 --- a/pyanaconda/modules/common/structures/storage.py +++ b/pyanaconda/modules/common/structures/storage.py @@ -463,6 +463,8 @@ def __init__(self): 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: @@ -511,3 +513,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 5070cf4d250..cc2b9d4c3db 100644 --- a/pyanaconda/modules/storage/devicetree/viewer.py +++ b/pyanaconda/modules/storage/devicetree/viewer.py @@ -470,6 +470,54 @@ 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. + + FIXME /boot 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 + """ + + 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) + if p.mountpoint == "/boot": + constraint.recommended = True + else: + constraint.required = True + constraints.append(constraint) + + return constraints + def _get_platform_mount_point_data(self, spec): """Get the mount point data. diff --git a/pyanaconda/modules/storage/devicetree/viewer_interface.py b/pyanaconda/modules/storage/devicetree/viewer_interface.py index 4773adead19..416d994507a 100644 --- a/pyanaconda/modules/storage/devicetree/viewer_interface.py +++ b/pyanaconda/modules/storage/devicetree/viewer_interface.py @@ -193,6 +193,20 @@ 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