From 2f01f6394ced610c8eaf7433ab4f72222db0e5bf Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Tue, 30 Jul 2024 08:57:46 +0200 Subject: [PATCH 1/8] home reuse: add support for /home reuse to automatic partitioning We use 'destroy' only for plain partition mountpoints as we can't destroy lv or btrfs subvolume - it would not free space for the new one (and its vg or btrfs volume). For lvs and btrfs subvols we reuse 'reformat' from mount point assignment (real erase would be perhaps better, even for mount point assignment as recreating the device complicates the logic) --- .../modules/common/structures/partitioning.py | 53 ++++++++ .../automatic/automatic_partitioning.py | 122 +++++++++++++++++- .../storage/partitioning/automatic/utils.py | 3 +- .../manual/manual_partitioning.py | 103 ++------------- .../storage/partitioning/manual/utils.py | 122 ++++++++++++++++++ .../test_module_part_automatic.py | 3 + 6 files changed, 305 insertions(+), 101 deletions(-) create mode 100644 pyanaconda/modules/storage/partitioning/manual/utils.py diff --git a/pyanaconda/modules/common/structures/partitioning.py b/pyanaconda/modules/common/structures/partitioning.py index 0ffef6bce41..0116dd96ee9 100644 --- a/pyanaconda/modules/common/structures/partitioning.py +++ b/pyanaconda/modules/common/structures/partitioning.py @@ -32,6 +32,9 @@ def __init__(self): self._partitioning_scheme = conf.storage.default_scheme self._file_system_type = "" self._excluded_mount_points = [] + self._reformatted_mount_points = [] + self._reused_mount_points = [] + self._removed_mount_points = [] self._hibernation = False self._encrypted = False @@ -113,6 +116,56 @@ def hibernation(self, value: Bool): def excluded_mount_points(self, mount_points: List[Str]): self._excluded_mount_points = mount_points + @property + def reformatted_mount_points(self) -> List[Str]: + """Reformatted mount points. + + Reformat and reuse existing devices for the mount points. + + For example: / + + :return: a list of mount points + """ + return self._reformatted_mount_points + + @reformatted_mount_points.setter + def reformatted_mount_points(self, mount_points: List[Str]): + self._reformatted_mount_points = mount_points + + @property + def reused_mount_points(self) -> List[Str]: + """Reused mount points. + + Reuse existing devices for the mount points. + + For example: /home + + :return: a list of mount points + """ + return self._reused_mount_points + + @reused_mount_points.setter + def reused_mount_points(self, mount_points: List[Str]): + self._reused_mount_points = mount_points + + @property + def removed_mount_points(self) -> List[Str]: + """Removed mount points. + + Destroy the devices for the mount points if they exist. + + Supported only for plain partition mount points + + For example: /boot + + :return: a list of mount points + """ + return self._removed_mount_points + + @removed_mount_points.setter + def removed_mount_points(self, mount_points: List[Str]): + self._removed_mount_points = mount_points + @property def encrypted(self) -> Bool: """Should devices be encrypted? diff --git a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py index 507e96c96bc..9b3b01225ee 100644 --- a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py +++ b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py @@ -17,16 +17,22 @@ # from blivet.partitioning import do_partitioning, grow_lvm from blivet.static_data import luks_data +from blivet.errors import StorageError from pyanaconda.anaconda_loggers import get_module_logger +from pyanaconda.core.i18n import _ from pyanaconda.modules.common.structures.partitioning import PartitioningRequest from pyanaconda.modules.storage.partitioning.automatic.noninteractive_partitioning import \ NonInteractivePartitioningTask +from pyanaconda.modules.storage.partitioning.manual.utils import \ + reformat_device +from pyanaconda.modules.storage.partitioning.interactive.utils import destroy_device from pyanaconda.modules.storage.partitioning.automatic.utils import get_candidate_disks, \ schedule_implicit_partitions, schedule_volumes, schedule_partitions, get_pbkdf_args, \ get_default_partitioning, get_part_spec, get_disks_for_implicit_partitions from pyanaconda.core.storage import suggest_swap_size + log = get_module_logger(__name__) __all__ = ["AutomaticPartitioningTask"] @@ -56,6 +62,83 @@ def _get_initialization_config(self): config.clear_non_existent = True return config + def _get_mountpoint_device(self, storage, mountpoint, required=True): + devices = [] + # TODO add support for EFI + if mountpoint == "biosboot": + for device in storage.devices: + if device.format.type == "biosboot": + devices.append(device) + else: + for root in storage.roots: + if mountpoint in root.mounts: + devices.append(root.mounts[mountpoint]) + if len(devices) > 1: + raise StorageError(_("Multiple devices found for mount point '{}': {}") + .format(mountpoint, + ", ".join([device.name for device in devices]))) + if not devices: + if required: + raise StorageError(_("No devices found for mount point '{}'").format(mountpoint)) + else: + return None + + return devices[0] + + def _reused_devices_mountpoints(self, request): + return request.reused_mount_points + request.reformatted_mount_points + + def _get_reused_device_names(self, storage): + reused_devices = {} + for mountpoint in self._reused_devices_mountpoints(self._request): + device = self._get_mountpoint_device(storage, mountpoint) + reused_devices[device.name] = mountpoint + return reused_devices + + def _reformat_mountpoint(self, storage, mountpoint): + device = self._get_mountpoint_device(storage, mountpoint) + log.debug("reformat device %s for mountpoint: %s", device, mountpoint) + reused_devices = self._get_reused_device_names(storage) + reformat_device(storage, device, dependencies=reused_devices) + + def _remove_mountpoint(self, storage, mountpoint): + device = self._get_mountpoint_device(storage, mountpoint, required=False) + if device: + log.debug("remove device %s for mountpoint %s", device, mountpoint) + destroy_device(storage, device) + else: + log.debug("device to be removed for mountpoint %s not found", mountpoint) + + def _clear_partitions(self, storage): + super()._clear_partitions(storage) + + log.debug("storage.roots.mounts %s", [root.mounts for root in storage.roots]) + + # TODO check that partitioning scheme matches - do it earlier in the + # check but also here? + + for mountpoint in self._request.removed_mount_points: + self._remove_mountpoint(storage, mountpoint) + for mountpoint in self._request.reformatted_mount_points: + self._reformat_mountpoint(storage, mountpoint) + + def _schedule_reused_mountpoint(self, storage, mountpoint): + device = self._get_mountpoint_device(storage, mountpoint) + log.debug("add mount device request for reused mountpoint: %s device: %s", + mountpoint, device) + device.format.mountpoint = mountpoint + + def _schedule_reformatted_mountpoint(self, storage, mountpoint): + old_device = self._get_mountpoint_device(storage, mountpoint) + # The device might have been recreated (btrfs) + device = storage.devicetree.resolve_device(old_device.name) + if device: + log.debug("add mount device request for reformatted mountpoint: %s device: %s", + mountpoint, device) + device.format.mountpoint = mountpoint + else: + log.debug("device for reformatted mountpoint %s not found", mountpoint) + def _configure_partitioning(self, storage): """Configure the partitioning. @@ -88,7 +171,15 @@ def _configure_partitioning(self, storage): requests = self._get_partitioning(storage, scheme, self._request) # Do the autopart. - self._do_autopart(storage, scheme, requests, encrypted, luks_format_args) + create_implicit_partitions = not self._implicit_partitions_reused(storage, self._request) + self._do_autopart(storage, scheme, requests, encrypted, luks_format_args, + create_implicit_partitions) + + for mountpoint in self._request.reused_mount_points: + self._schedule_reused_mountpoint(storage, mountpoint) + for mountpoint in self._request.reformatted_mount_points: + self._schedule_reformatted_mountpoint(storage, mountpoint) + @staticmethod def _get_luks_format_args(storage, request): @@ -140,8 +231,11 @@ def _get_partitioning(storage, scheme, request: PartitioningRequest): if spec.schemes and scheme not in spec.schemes: continue - # Skip excluded mount points. - if (spec.mountpoint or spec.fstype) in request.excluded_mount_points: + # Skip excluded or reused mount points. + skipped = request.excluded_mount_points + skipped.extend(request.reused_mount_points) + skipped.extend(request.reformatted_mount_points) + if (spec.mountpoint or spec.fstype) in skipped: continue # Detect swap. @@ -169,8 +263,20 @@ def _get_partitioning(storage, scheme, request: PartitioningRequest): return specs + def _implicit_partitions_reused(self, storage, request): + for mountpoint in self._reused_devices_mountpoints(request): + device = self._get_mountpoint_device(storage, mountpoint) + if hasattr(device, "vg"): + log.debug("reusing volume group %s for %s", device.vg, mountpoint) + return True + if hasattr(device, "volume"): + log.debug("reusing volume %s for %s", device.volume, mountpoint) + return True + return False + @staticmethod - def _do_autopart(storage, scheme, requests, encrypted=False, luks_fmt_args=None): + def _do_autopart(storage, scheme, requests, encrypted=False, luks_fmt_args=None, + create_implicit_partitions=True): """Perform automatic partitioning. :param storage: an instance of Blivet @@ -192,8 +298,12 @@ def _do_autopart(storage, scheme, requests, encrypted=False, luks_fmt_args=None) log.debug("candidate disks: %s", [d.name for d in disks]) # Schedule implicit partitions. - extra_disks = get_disks_for_implicit_partitions(disks, scheme, requests) - devs = schedule_implicit_partitions(storage, extra_disks, scheme, encrypted, luks_fmt_args) + devs = [] + if create_implicit_partitions: + extra_disks = get_disks_for_implicit_partitions(disks, scheme, requests) + devs = schedule_implicit_partitions( + storage, extra_disks, scheme, encrypted, luks_fmt_args + ) # Schedule requested partitions. devs = schedule_partitions(storage, disks, devs, scheme, requests, encrypted, luks_fmt_args) diff --git a/pyanaconda/modules/storage/partitioning/automatic/utils.py b/pyanaconda/modules/storage/partitioning/automatic/utils.py index 68225d6bc05..4e6522f7e62 100644 --- a/pyanaconda/modules/storage/partitioning/automatic/utils.py +++ b/pyanaconda/modules/storage/partitioning/automatic/utils.py @@ -422,7 +422,8 @@ def schedule_partitions(storage, disks, implicit_devices, scheme, requests, encr parents=dev) storage.create_device(luks_dev) - if scheme in (AUTOPART_TYPE_LVM, AUTOPART_TYPE_LVM_THINP, AUTOPART_TYPE_BTRFS): + if scheme in (AUTOPART_TYPE_LVM, AUTOPART_TYPE_LVM_THINP, AUTOPART_TYPE_BTRFS) and \ + implicit_devices: # doing LVM/BTRFS -- make sure the newly created partition fits in some # free space together with one of the implicitly requested partitions smallest_implicit = sorted(implicit_devices, key=lambda d: d.size)[0] diff --git a/pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py b/pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py index bc412270230..5f7e94e4dc0 100644 --- a/pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py +++ b/pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py @@ -16,12 +16,12 @@ # Red Hat, Inc. # from blivet.errors import StorageError -from blivet.formats import get_format from pyanaconda.anaconda_loggers import get_module_logger from pyanaconda.core.i18n import _ from pyanaconda.modules.storage.partitioning.automatic.noninteractive_partitioning import \ NonInteractivePartitioningTask +from pyanaconda.modules.storage.partitioning.manual.utils import reformat_device log = get_module_logger(__name__) @@ -77,52 +77,14 @@ def _setup_mount_point(self, storage, mount_data): ) if reformat: - if format_type: - fmt = get_format(format_type) - - if not fmt: - raise StorageError( - _("Unknown or invalid format '{}' specified for " - "device '{}'").format(format_type, device.name) - ) - else: - old_fmt = device.format - - if not old_fmt or old_fmt.type is None: - raise StorageError(_("No format on device '{}'").format(device.name)) - - fmt = get_format(old_fmt.type) - - if device.raw_device.type in ("btrfs volume", "btrfs subvolume"): - # 'Format', or rather clear the device by recreating it - - # recreating @device will remove all nested subvolumes of it, we cannot allow - # using these nested subvolumes for other MountPointRequest without also - # re-creating them - if device.raw_device.type == "btrfs volume": - dep_subvolumes = device.raw_device.subvolumes - elif device.raw_device.type == "btrfs subvolume": - dep_subvolumes = [sub.device_id for sub in device.raw_device.volume.subvolumes - if sub.depends_on(device.raw_device)] - problem_subvolumes = [req for req in self._requests if (req.mount_point - and not req.reformat - and req.device_spec in - dep_subvolumes)] - if problem_subvolumes: - err = (_("{} mounted as {}").format(dep.device_spec, - dep.mount_point) for dep in problem_subvolumes) - raise StorageError( - _("Reformatting the '{}' subvolume will remove the following nested " - "subvolumes which cannot be reused: {}").format(device.raw_device.name, - ", ".join(err))) - device = self._recreate_btrfs_device(storage, device_spec) - mount_data.mount_options = device.format.options - else: - storage.format_device(device, fmt) - - # make sure swaps end up in /etc/fstab - if fmt.type == "swap": - storage.add_fstab_swap(device) + requested_devices = dict(((req.device_spec, req.mount_point) + for req in self._requests)) + device, mount_options = reformat_device(storage, + device, + format_type, + dependencies=requested_devices) + if mount_options is not None: + mount_data.mount_options = mount_options # add "mounted" swaps to fstab if device.format.type == "swap" and mount_point == "swap": @@ -134,50 +96,3 @@ def _setup_mount_point(self, storage, mount_data): device.format.create_options = mount_data.format_options device.format.options = mount_data.mount_options - - def _recreate_btrfs_volume(self, storage, device): - """Recreate a btrfs volume device by destroying and adding it. - - :param storage: an instance of the Blivet's storage object - :param device: a BtrfsVolumeDevice to recreate - """ - if device.children: - raise StorageError( - _("Cannot reformat Btrfs volume '{}' with " - "existing subvolumes").format(device.name)) - storage.destroy_device(device) - for parent in device.parents: - storage.format_device(parent, get_format("btrfs")) - new_btrfs = storage.new_btrfs(parents=device.parents[:], - name=device.name) - storage.create_device(new_btrfs) - return new_btrfs - - def _recreate_btrfs_subvolume(self, storage, device): - """Recreate a btrfs subvolume device by destroying and adding it. - - :param storage: an instance of the Blivet's storage object - :param device: a BtrfsSubVolumeDevice to recreate - """ - storage.recursive_remove(device) - new_btrfs = storage.new_btrfs(parents=device.parents[:], - name=device.name, - subvol=True) - storage.create_device(new_btrfs) - return new_btrfs - - def _recreate_btrfs_device(self, storage, dev_spec): - """Recreate a device by destroying and adding it. - - :param storage: an instance of the Blivet's storage object - :param dev_spec: a string describing a block device to be recreated - """ - device = storage.devicetree.get_device_by_device_id(dev_spec) - - if device.type == "btrfs volume": - # can't use device factory for just the volume - return self._recreate_btrfs_volume(storage, device) - elif device.type == "btrfs subvolume": - # using the factory for subvolumes in some cases removes - # the volume too, we don't want that - return self._recreate_btrfs_subvolume(storage, device) diff --git a/pyanaconda/modules/storage/partitioning/manual/utils.py b/pyanaconda/modules/storage/partitioning/manual/utils.py new file mode 100644 index 00000000000..b1ae7d886ab --- /dev/null +++ b/pyanaconda/modules/storage/partitioning/manual/utils.py @@ -0,0 +1,122 @@ +# +# Utilities for the manual partitioning module +# +# Copyright (C) 2024 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# + +from blivet.errors import StorageError +from blivet.formats import get_format + +from pyanaconda.core.i18n import _ + + +def _recreate_btrfs_volume(storage, device): + """Recreate a btrfs volume device by destroying and adding it. + + :param storage: an instance of the Blivet's storage object + :param device: a BtrfsVolumeDevice to recreate + """ + if device.children: + raise StorageError( + _("Cannot reformat Btrfs volume '{}' with " + "existing subvolumes").format(device.name)) + storage.destroy_device(device) + for parent in device.parents: + storage.format_device(parent, get_format("btrfs")) + new_btrfs = storage.new_btrfs(parents=device.parents[:], + name=device.name) + storage.create_device(new_btrfs) + return new_btrfs + + +def _recreate_btrfs_subvolume(storage, device): + """Recreate a btrfs subvolume device by destroying and adding it. + :param storage: an instance of the Blivet's storage object + :param device: a BtrfsSubVolumeDevice to recreate + """ + storage.recursive_remove(device) + new_btrfs = storage.new_btrfs(parents=device.parents[:], + name=device.name, + subvol=True) + storage.create_device(new_btrfs) + return new_btrfs + + +def recreate_btrfs_device(storage, device): + """Recreate a device by destroying and adding it. + + :param storage: an instance of the Blivet's storage object + :param device: a block device to be recreated + """ + if device.type == "btrfs volume": + # can't use device factory for just the volume + return _recreate_btrfs_volume(storage, device) + elif device.type == "btrfs subvolume": + # using the factory for subvolumes in some cases removes + # the volume too, we don't want that + return _recreate_btrfs_subvolume(storage, device) + + +def reformat_device(storage, device, format_type=None, dependencies=None): + dependencies = dependencies or {} + mount_options = None + if format_type: + fmt = get_format(format_type) + + if not fmt: + raise StorageError( + _("Unknown or invalid format '{}' specified for " + "device '{}'").format(format_type, device.name) + ) + else: + old_fmt = device.format + + if not old_fmt or old_fmt.type is None: + raise StorageError(_("No format on device '{}'").format(device.name)) + + fmt = get_format(old_fmt.type) + + if device.raw_device.type in ("btrfs volume", "btrfs subvolume"): + # 'Format', or rather clear the device by recreating it + + # recreating @device will remove all nested subvolumes of it, + # so guard the list of dependencies + if device.raw_device.type == "btrfs volume": + dep_subvolumes = device.raw_device.subvolumes + elif device.raw_device.type == "btrfs subvolume": + dep_subvolumes = [sub.name for sub in device.raw_device.volume.subvolumes + if sub.depends_on(device.raw_device)] + problem_subvolumes = [(device_name, mountpoint) + for device_name, mountpoint in dependencies.items() + if device_name in dep_subvolumes] + + if problem_subvolumes: + err = (_("{} mounted as {}").format(*dep) for dep in problem_subvolumes) + raise StorageError( + _("Reformatting the '{}' subvolume will remove the following nested " + "subvolumes which cannot be reused: {}").format(device.raw_device.name, + ", ".join(err))) + device = recreate_btrfs_device(storage, device) + mount_options = device.format.options + else: + storage.format_device(device, fmt) + + # make sure swaps end up in /etc/fstab + if fmt.type == "swap": + storage.add_fstab_swap(device) + + return device, mount_options diff --git a/tests/unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_automatic.py b/tests/unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_automatic.py index 79aa2b16bc3..03a06dba09f 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_automatic.py +++ b/tests/unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_automatic.py @@ -91,6 +91,9 @@ def test_request_property(self): 'partitioning-scheme': get_variant(Int, AUTOPART_TYPE_LVM_THINP), 'file-system-type': get_variant(Str, 'ext4'), 'excluded-mount-points': get_variant(List[Str], ['/home', '/boot', 'swap']), + 'reformatted-mount-points': get_variant(List[Str], ['/']), + 'removed-mount-points': get_variant(List[Str], ['/boot', 'bootloader']), + 'reused-mount-points': get_variant(List[Str], ['/home']), 'hibernation': get_variant(Bool, False), 'encrypted': get_variant(Bool, True), 'passphrase': get_variant(Str, '123456'), From 5153979fe205ff15c475d2609bdeaa5fab64d5eb Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Wed, 28 Aug 2024 15:11:59 +0200 Subject: [PATCH 2/8] home reuse: update existing OSs when applying partitioning --- .../storage/partitioning/automatic/automatic_partitioning.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py index 9b3b01225ee..a3276e2533c 100644 --- a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py +++ b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py @@ -26,6 +26,7 @@ NonInteractivePartitioningTask from pyanaconda.modules.storage.partitioning.manual.utils import \ reformat_device +from pyanaconda.modules.storage.devicetree.root import find_existing_installations from pyanaconda.modules.storage.partitioning.interactive.utils import destroy_device from pyanaconda.modules.storage.partitioning.automatic.utils import get_candidate_disks, \ schedule_implicit_partitions, schedule_volumes, schedule_partitions, get_pbkdf_args, \ @@ -112,6 +113,8 @@ def _remove_mountpoint(self, storage, mountpoint): def _clear_partitions(self, storage): super()._clear_partitions(storage) + # Make sure disk selection is taken into account when finding installations + storage.roots = find_existing_installations(storage.devicetree) log.debug("storage.roots.mounts %s", [root.mounts for root in storage.roots]) # TODO check that partitioning scheme matches - do it earlier in the From c5375020cd6058008c264acc0e066c0e1ebe57e1 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Thu, 29 Aug 2024 18:25:23 +0200 Subject: [PATCH 3/8] home reuse: remove bootloader partitions implicitly --- .../automatic/automatic_partitioning.py | 45 +++++++++++++++---- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py index a3276e2533c..fdae5321568 100644 --- a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py +++ b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py @@ -31,6 +31,7 @@ from pyanaconda.modules.storage.partitioning.automatic.utils import get_candidate_disks, \ schedule_implicit_partitions, schedule_volumes, schedule_partitions, get_pbkdf_args, \ get_default_partitioning, get_part_spec, get_disks_for_implicit_partitions +from pyanaconda.modules.storage.platform import platform from pyanaconda.core.storage import suggest_swap_size @@ -65,15 +66,9 @@ def _get_initialization_config(self): def _get_mountpoint_device(self, storage, mountpoint, required=True): devices = [] - # TODO add support for EFI - if mountpoint == "biosboot": - for device in storage.devices: - if device.format.type == "biosboot": - devices.append(device) - else: - for root in storage.roots: - if mountpoint in root.mounts: - devices.append(root.mounts[mountpoint]) + for root in storage.roots: + if mountpoint in root.mounts: + devices.append(root.mounts[mountpoint]) if len(devices) > 1: raise StorageError(_("Multiple devices found for mount point '{}': {}") .format(mountpoint, @@ -110,6 +105,37 @@ def _remove_mountpoint(self, storage, mountpoint): else: log.debug("device to be removed for mountpoint %s not found", mountpoint) + def _remove_bootloader_partitions(self, storage, required=True): + bootloader_types = ["efi", "biosboot", "appleboot", "prepboot"] + bootloader_parts = [part for part in platform.partitions + if part.fstype in bootloader_types] + if len(bootloader_parts) > 1: + raise StorageError(_("Multiple boot loader partitions required: %s"), bootloader_parts) + if not bootloader_parts: + log.debug("No bootloader partition required") + return False + part_type = bootloader_parts[0].fstype + + devices = [] + for device in storage.devices: + if device.format.type == part_type: + devices.append(device) + if len(devices) > 1: + raise StorageError(_("Multiple devices found for boot loader partition '{}': {}") + .format(part_type, + ", ".join([device.name for device in devices]))) + if not devices: + if required: + raise StorageError(_("No devices found for boot loader partition '{}'") + .format(part_type)) + else: + log.debug("No devices found for bootloader partition %s", part_type) + return False + device = devices[0] + log.debug("remove device %s for bootloader partition %s", device, part_type) + destroy_device(storage, device) + return True + def _clear_partitions(self, storage): super()._clear_partitions(storage) @@ -120,6 +146,7 @@ def _clear_partitions(self, storage): # TODO check that partitioning scheme matches - do it earlier in the # check but also here? + self._remove_bootloader_partitions(storage) for mountpoint in self._request.removed_mount_points: self._remove_mountpoint(storage, mountpoint) for mountpoint in self._request.reformatted_mount_points: From 5becb545194f40fcce65e8d5881464218f31041e Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Fri, 30 Aug 2024 14:20:21 +0200 Subject: [PATCH 4/8] home reuse: require removing of bootloader partition explicitly But handle efi vs biosboot etc automagically. --- .../partitioning/automatic/automatic_partitioning.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py index fdae5321568..07a0bd8a316 100644 --- a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py +++ b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py @@ -146,9 +146,11 @@ def _clear_partitions(self, storage): # TODO check that partitioning scheme matches - do it earlier in the # check but also here? - self._remove_bootloader_partitions(storage) for mountpoint in self._request.removed_mount_points: - self._remove_mountpoint(storage, mountpoint) + if mountpoint == "bootloader": + self._remove_bootloader_partitions(storage) + else: + self._remove_mountpoint(storage, mountpoint) for mountpoint in self._request.reformatted_mount_points: self._reformat_mountpoint(storage, mountpoint) From 4af3a1f76c69774b646e42562a45b2aa4ade1a40 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Wed, 18 Sep 2024 14:38:52 +0200 Subject: [PATCH 5/8] home reuse: check autopartitioning scheme against reused mountpoints --- .../automatic/automatic_partitioning.py | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py index 07a0bd8a316..8a1ed282ac6 100644 --- a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py +++ b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py @@ -33,6 +33,8 @@ get_default_partitioning, get_part_spec, get_disks_for_implicit_partitions from pyanaconda.modules.storage.platform import platform from pyanaconda.core.storage import suggest_swap_size +from pykickstart.constants import AUTOPART_TYPE_BTRFS, AUTOPART_TYPE_LVM, \ + AUTOPART_TYPE_LVM_THINP, AUTOPART_TYPE_PLAIN log = get_module_logger(__name__) @@ -143,8 +145,8 @@ def _clear_partitions(self, storage): storage.roots = find_existing_installations(storage.devicetree) log.debug("storage.roots.mounts %s", [root.mounts for root in storage.roots]) - # TODO check that partitioning scheme matches - do it earlier in the - # check but also here? + # Check that partitioning scheme matches + self._check_reused_scheme(storage, self._request) for mountpoint in self._request.removed_mount_points: if mountpoint == "bootloader": @@ -154,6 +156,21 @@ def _clear_partitions(self, storage): for mountpoint in self._request.reformatted_mount_points: self._reformat_mountpoint(storage, mountpoint) + def _check_reused_scheme(self, storage, request): + scheme = request.partitioning_scheme + required_home_device_type = { + AUTOPART_TYPE_BTRFS: "btrfs subvolume", + AUTOPART_TYPE_LVM: "lvmlv", + AUTOPART_TYPE_LVM_THINP: "lvmthinlv", + AUTOPART_TYPE_PLAIN: "partition", + } + for mountpoint in request.reused_mount_points: + device = self._get_mountpoint_device(storage, mountpoint) + if device.type != required_home_device_type[scheme]: + raise StorageError(_("Reused device type '{}' of mount point '{}' does not " + "match the required automatic partitioning scheme.") + .format(device.type, mountpoint)) + def _schedule_reused_mountpoint(self, storage, mountpoint): device = self._get_mountpoint_device(storage, mountpoint) log.debug("add mount device request for reused mountpoint: %s device: %s", From 3c14c0c27c5b5f7da60be9e13c483b3d9b6ef094 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 23 Sep 2024 11:39:11 +0200 Subject: [PATCH 6/8] home reuse: reuse mount options of reused mountpoins --- pyanaconda/modules/storage/devicetree/root.py | 29 ++++++++++++--- .../automatic/automatic_partitioning.py | 14 ++++++- .../modules/storage/test_model.py | 37 +++++++++++++++++++ 3 files changed, 73 insertions(+), 7 deletions(-) diff --git a/pyanaconda/modules/storage/devicetree/root.py b/pyanaconda/modules/storage/devicetree/root.py index da97baed016..05fe5cefb69 100644 --- a/pyanaconda/modules/storage/devicetree/root.py +++ b/pyanaconda/modules/storage/devicetree/root.py @@ -124,7 +124,7 @@ def _find_existing_installations(devicetree): continue architecture, product, version = get_release_string(chroot=sysroot) - (mounts, devices) = _parse_fstab(devicetree, chroot=sysroot) + (mounts, devices, mountopts) = _parse_fstab(devicetree, chroot=sysroot) blivet_util.umount(mountpoint=sysroot) if not mounts and not devices: @@ -137,6 +137,7 @@ def _find_existing_installations(devicetree): arch=architecture, devices=devices, mounts=mounts, + mountopts=mountopts, )) return roots @@ -249,16 +250,17 @@ def _parse_fstab(devicetree, chroot): :param devicetree: a device tree :param chroot: a path to the target OS installation - :return: a tuple of a mount dict and a device list + :return: a tuple of a mount dict, device list, mount options """ mounts = {} devices = [] + mountopts = {} path = "%s/etc/fstab" % chroot if not os.access(path, os.R_OK): # XXX should we raise an exception instead? log.info("cannot open %s for read", path) - return mounts, devices + return mounts, devices, mountopts blkid_tab = BlkidTab(chroot=chroot) try: @@ -306,17 +308,18 @@ def _parse_fstab(devicetree, chroot): if fstype != "swap": mounts[mountpoint] = device + mountopts[mountpoint] = options devices.append(device) - return mounts, devices + return mounts, devices, mountopts class Root(object): """A root represents an existing OS installation.""" def __init__(self, name=None, product=None, version=None, arch=None, devices=None, - mounts=None): + mounts=None, mountopts=None): """Create a new OS representation. :param name: a name of the OS or None @@ -325,6 +328,7 @@ def __init__(self, name=None, product=None, version=None, arch=None, devices=Non :param arch: a machine's architecture or None :param devices: a list of all devices :param mounts: a dictionary of mount points and devices + :param mountopts: a dictionary of mount points and its mount options """ self._name = name self._product = product @@ -332,6 +336,7 @@ def __init__(self, name=None, product=None, version=None, arch=None, devices=Non self._arch = arch self._devices = devices or [] self._mounts = mounts or {} + self._mountopts = mountopts or {} @property def name(self): @@ -377,6 +382,14 @@ def mounts(self): """ return self._mounts + @property + def mountopts(self): + """Mount point options of mount points defined by the OS. + + :return: a dictionary of mount points and their mount options + """ + return self._mountopts + def copy(self, storage): """Create a copy with devices of the given storage model. @@ -392,6 +405,12 @@ def _get_mount(i): m, d = i[0], _get_device(i[1]) return (m, d) if m and d else None + def _get_mount_opt(i): + m, d = i[0], _get_device(i[1]) + return (m, self._mountopts[m]) if m and d and m in self._mountopts else None + new_root._devices = list(filter(None, map(_get_device, new_root._devices))) new_root._mounts = dict(filter(None, map(_get_mount, new_root._mounts.items()))) + new_root._mountopts = dict(filter(None, map(_get_mount_opt, + new_root._mounts.items()))) return new_root diff --git a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py index 8a1ed282ac6..8a47b1487b3 100644 --- a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py +++ b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py @@ -83,6 +83,12 @@ def _get_mountpoint_device(self, storage, mountpoint, required=True): return devices[0] + def _get_mountpoint_options(self, storage, mountpoint): + for root in storage.roots: + if mountpoint in root.mountopts: + return root.mountopts[mountpoint] + return None + def _reused_devices_mountpoints(self, request): return request.reused_mount_points + request.reformatted_mount_points @@ -173,9 +179,13 @@ def _check_reused_scheme(self, storage, request): def _schedule_reused_mountpoint(self, storage, mountpoint): device = self._get_mountpoint_device(storage, mountpoint) - log.debug("add mount device request for reused mountpoint: %s device: %s", - mountpoint, device) + mountopts = self._get_mountpoint_options(storage, mountpoint) + log.debug("add mount device request for reused mountpoint: %s device: %s " + "with mountopts: %s", + mountpoint, device, mountopts) device.format.mountpoint = mountpoint + if mountopts: + device.format.options = mountopts def _schedule_reformatted_mountpoint(self, storage, mountpoint): old_device = self._get_mountpoint_device(storage, mountpoint) diff --git a/tests/unit_tests/pyanaconda_tests/modules/storage/test_model.py b/tests/unit_tests/pyanaconda_tests/modules/storage/test_model.py index 63bcf3cc87c..f375f34ffd7 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/storage/test_model.py +++ b/tests/unit_tests/pyanaconda_tests/modules/storage/test_model.py @@ -149,3 +149,40 @@ def test_copy_roots(self): assert len(root2_copy.mounts) == 2 assert "/" in root2_copy.mounts assert "/home" in root2_copy.mounts + + def test_copy_mountopts(self): + """Test the copy of mount options.""" + dev1 = StorageDevice("dev1") + self._add_device(dev1) + + dev2 = StorageDevice("dev2") + self._add_device(dev2) + + dev3 = StorageDevice("dev3") + self._add_device(dev3) + + root1 = Root( + name="Linux 1", + devices=[dev2], + mounts={"/": dev2}, + ) + self.storage.roots.append(root1) + + root2 = Root( + name="Linux 2", + devices=[dev1, dev3], + mounts={"/": dev1, "/home": dev3}, + mountopts={"/home": "opt1"} + ) + self.storage.roots.append(root2) + + storage_copy = self.storage.copy() + assert len(storage_copy.roots) == 2 + + root1_copy = storage_copy.roots[0] + assert root1_copy.name == "Linux 1" + assert len(root1_copy.mountopts) == 0 + + root2_copy = storage_copy.roots[1] + assert root2_copy.name == "Linux 2" + assert len(root2_copy.mountopts) == 1 From 5012db1e07af07e8d69dea8e7f740706ec655f2a Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Fri, 4 Oct 2024 14:40:30 +0200 Subject: [PATCH 7/8] home reuse: define static and class methods --- .../automatic/automatic_partitioning.py | 84 +++++++++++-------- 1 file changed, 51 insertions(+), 33 deletions(-) diff --git a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py index 8a47b1487b3..5b12686b143 100644 --- a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py +++ b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py @@ -66,7 +66,8 @@ def _get_initialization_config(self): config.clear_non_existent = True return config - def _get_mountpoint_device(self, storage, mountpoint, required=True): + @staticmethod + def _get_mountpoint_device(storage, mountpoint, required=True): devices = [] for root in storage.roots: if mountpoint in root.mounts: @@ -83,37 +84,43 @@ def _get_mountpoint_device(self, storage, mountpoint, required=True): return devices[0] - def _get_mountpoint_options(self, storage, mountpoint): + @staticmethod + def _get_mountpoint_options(storage, mountpoint): for root in storage.roots: if mountpoint in root.mountopts: return root.mountopts[mountpoint] return None - def _reused_devices_mountpoints(self, request): + @staticmethod + def _reused_devices_mountpoints(request): return request.reused_mount_points + request.reformatted_mount_points - def _get_reused_device_names(self, storage): + @classmethod + def _get_reused_device_names(cls, storage, request): reused_devices = {} - for mountpoint in self._reused_devices_mountpoints(self._request): - device = self._get_mountpoint_device(storage, mountpoint) + for mountpoint in cls._reused_devices_mountpoints(request): + device = cls._get_mountpoint_device(storage, mountpoint) reused_devices[device.name] = mountpoint return reused_devices - def _reformat_mountpoint(self, storage, mountpoint): - device = self._get_mountpoint_device(storage, mountpoint) + @classmethod + def _reformat_mountpoint(cls, storage, mountpoint, request): + device = cls._get_mountpoint_device(storage, mountpoint) log.debug("reformat device %s for mountpoint: %s", device, mountpoint) - reused_devices = self._get_reused_device_names(storage) + reused_devices = cls._get_reused_device_names(storage, request) reformat_device(storage, device, dependencies=reused_devices) - def _remove_mountpoint(self, storage, mountpoint): - device = self._get_mountpoint_device(storage, mountpoint, required=False) + @classmethod + def _remove_mountpoint(cls, storage, mountpoint): + device = cls._get_mountpoint_device(storage, mountpoint, required=False) if device: log.debug("remove device %s for mountpoint %s", device, mountpoint) destroy_device(storage, device) else: log.debug("device to be removed for mountpoint %s not found", mountpoint) - def _remove_bootloader_partitions(self, storage, required=True): + @staticmethod + def _remove_bootloader_partitions(storage, required=True): bootloader_types = ["efi", "biosboot", "appleboot", "prepboot"] bootloader_parts = [part for part in platform.partitions if part.fstype in bootloader_types] @@ -154,15 +161,10 @@ def _clear_partitions(self, storage): # Check that partitioning scheme matches self._check_reused_scheme(storage, self._request) - for mountpoint in self._request.removed_mount_points: - if mountpoint == "bootloader": - self._remove_bootloader_partitions(storage) - else: - self._remove_mountpoint(storage, mountpoint) - for mountpoint in self._request.reformatted_mount_points: - self._reformat_mountpoint(storage, mountpoint) + self._clear_existing_mountpoints(storage, self._request) - def _check_reused_scheme(self, storage, request): + @classmethod + def _check_reused_scheme(cls, storage, request): scheme = request.partitioning_scheme required_home_device_type = { AUTOPART_TYPE_BTRFS: "btrfs subvolume", @@ -171,15 +173,26 @@ def _check_reused_scheme(self, storage, request): AUTOPART_TYPE_PLAIN: "partition", } for mountpoint in request.reused_mount_points: - device = self._get_mountpoint_device(storage, mountpoint) + device = cls._get_mountpoint_device(storage, mountpoint) if device.type != required_home_device_type[scheme]: raise StorageError(_("Reused device type '{}' of mount point '{}' does not " "match the required automatic partitioning scheme.") .format(device.type, mountpoint)) - def _schedule_reused_mountpoint(self, storage, mountpoint): - device = self._get_mountpoint_device(storage, mountpoint) - mountopts = self._get_mountpoint_options(storage, mountpoint) + @classmethod + def _clear_existing_mountpoints(cls, storage, request): + for mountpoint in request.removed_mount_points: + if mountpoint == "bootloader": + cls._remove_bootloader_partitions(storage) + else: + cls._remove_mountpoint(storage, mountpoint) + for mountpoint in request.reformatted_mount_points: + cls._reformat_mountpoint(storage, mountpoint, request) + + @classmethod + def _schedule_reused_mountpoint(cls, storage, mountpoint): + device = cls._get_mountpoint_device(storage, mountpoint) + mountopts = cls._get_mountpoint_options(storage, mountpoint) log.debug("add mount device request for reused mountpoint: %s device: %s " "with mountopts: %s", mountpoint, device, mountopts) @@ -187,8 +200,9 @@ def _schedule_reused_mountpoint(self, storage, mountpoint): if mountopts: device.format.options = mountopts - def _schedule_reformatted_mountpoint(self, storage, mountpoint): - old_device = self._get_mountpoint_device(storage, mountpoint) + @classmethod + def _schedule_reformatted_mountpoint(cls, storage, mountpoint): + old_device = cls._get_mountpoint_device(storage, mountpoint) # The device might have been recreated (btrfs) device = storage.devicetree.resolve_device(old_device.name) if device: @@ -234,11 +248,14 @@ def _configure_partitioning(self, storage): self._do_autopart(storage, scheme, requests, encrypted, luks_format_args, create_implicit_partitions) - for mountpoint in self._request.reused_mount_points: - self._schedule_reused_mountpoint(storage, mountpoint) - for mountpoint in self._request.reformatted_mount_points: - self._schedule_reformatted_mountpoint(storage, mountpoint) + self._schedule_existing_mountpoints(storage, self._request) + @classmethod + def _schedule_existing_mountpoints(cls, storage, request): + for mountpoint in request.reused_mount_points: + cls._schedule_reused_mountpoint(storage, mountpoint) + for mountpoint in request.reformatted_mount_points: + cls._schedule_reformatted_mountpoint(storage, mountpoint) @staticmethod def _get_luks_format_args(storage, request): @@ -322,9 +339,10 @@ def _get_partitioning(storage, scheme, request: PartitioningRequest): return specs - def _implicit_partitions_reused(self, storage, request): - for mountpoint in self._reused_devices_mountpoints(request): - device = self._get_mountpoint_device(storage, mountpoint) + @classmethod + def _implicit_partitions_reused(cls, storage, request): + for mountpoint in cls._reused_devices_mountpoints(request): + device = cls._get_mountpoint_device(storage, mountpoint) if hasattr(device, "vg"): log.debug("reusing volume group %s for %s", device.vg, mountpoint) return True From e67f34afb7da8ae4dc7569b3cefc387cd833ca38 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Mon, 7 Oct 2024 13:39:16 +0200 Subject: [PATCH 8/8] home reuse: add unit tests --- .../test_module_part_automatic.py | 531 +++++++++++++++++- 1 file changed, 530 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_automatic.py b/tests/unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_automatic.py index 03a06dba09f..112c4f99db3 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_automatic.py +++ b/tests/unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_automatic.py @@ -20,10 +20,11 @@ import unittest import pytest -from unittest.mock import Mock, patch, ANY +from unittest.mock import Mock, patch, ANY, call from blivet.formats.luks import LUKS2PBKDFArgs from blivet.size import Size +from blivet.errors import StorageError from pyanaconda.modules.common.structures.validation import ValidationReport from pyanaconda.modules.storage.partitioning.automatic.resizable_module import \ @@ -415,6 +416,534 @@ def test_get_partitioning_btrfs_only(self, platform, mocked_conf): assert ["/"] == [spec.mountpoint for spec in requests] +class AutomaticPartitioningTaskReuseTestCase(unittest.TestCase): + """Test the automatic partitioning task mountpoint reuse functionality.""" + + @patch('pyanaconda.modules.storage.partitioning.automatic.automatic_partitioning.destroy_device') + @patch('pyanaconda.modules.storage.partitioning.automatic.automatic_partitioning.platform') + def test_remove_bootloder_partitions(self, platform, destroy_device): + storage = Mock() + + # Test platfrorm bootloader partitions + + # Test no bootoloaer partition required + platform.partitions = [ + PartSpec( + mountpoint="/boot", + size=Size("1GiB"), + lv=False, + ), + ] + assert AutomaticPartitioningTask._remove_bootloader_partitions(storage) is False + + # Test multiple bootloader partitions required + platform.partitions = [ + PartSpec( + fstype="biosboot", + size=Size("1MiB"), + ), + PartSpec( + mountpoint="/boot/efi", + fstype="efi", + size=Size("500MiB"), + max_size=Size("600MiB"), + grow=True, + ) + ] + with pytest.raises(StorageError): + AutomaticPartitioningTask._remove_bootloader_partitions(storage) + + # biosboot + platform.partitions = [ + # boot partition is ignored + PartSpec( + mountpoint="/boot", + size=Size("1GiB"), + lv=False + ), + PartSpec( + fstype="biosboot", + size=Size("1MiB") + ), + ] + biosboot_device = Mock(format=Mock(type="biosboot")) + storage.devices = [ + biosboot_device, + Mock(format=Mock(type="xfs")), + ] + + assert AutomaticPartitioningTask._remove_bootloader_partitions(storage) is True + destroy_device.assert_called_with(storage, biosboot_device) + + # biosboot, two found + biosboot_device1 = Mock(format=Mock(type="biosboot")) + biosboot_device2 = Mock(format=Mock(type="biosboot")) + biosboot_device1.name = "bd1" + biosboot_device2.name = "bd2" + storage.devices = [ + biosboot_device1, + biosboot_device2, + Mock(format=Mock(type="xfs")), + ] + with pytest.raises(StorageError): + AutomaticPartitioningTask._remove_bootloader_partitions(storage) + + # bootloader part not found + storage.devices = [ + Mock(format=Mock(type="xfs")), + ] + with pytest.raises(StorageError): + AutomaticPartitioningTask._remove_bootloader_partitions(storage) + assert AutomaticPartitioningTask._remove_bootloader_partitions( + storage, required=False + ) is False + + # prepboot + platform.partitions = [ + PartSpec( + fstype="prepboot", + size=Size("4MiB") + ), + ] + biosboot_device = Mock(format=Mock(type="prepboot")) + storage.devices = [ + biosboot_device, + Mock(format=Mock(type="xfs")), + ] + assert AutomaticPartitioningTask._remove_bootloader_partitions(storage) is True + destroy_device.assert_called_with(storage, biosboot_device) + + # appleboot + platform.partitions = [ + PartSpec( + fstype="appleboot", + size=Size("1MiB") + ), + ] + biosboot_device = Mock(format=Mock(type="appleboot")) + storage.devices = [ + biosboot_device, + Mock(format=Mock(type="xfs")), + ] + assert AutomaticPartitioningTask._remove_bootloader_partitions(storage) is True + destroy_device.assert_called_with(storage, biosboot_device) + + # prepboot + platform.partitions = [ + PartSpec( + mountpoint="/boot/efi", + fstype="efi", + size=Size("500MiB"), + max_size=Size("600MiB"), + grow=True, + ) + ] + biosboot_device = Mock(format=Mock(type="efi")) + storage.devices = [ + biosboot_device, + Mock(format=Mock(type="xfs")), + ] + assert AutomaticPartitioningTask._remove_bootloader_partitions(storage) is True + destroy_device.assert_called_with(storage, biosboot_device) + + def test_get_mountpoint_device(self): + storage = Mock() + + # device found + home_device = Mock() + storage.roots = [ + Mock(mounts={ + "/home": home_device, + "/": Mock(), + }), + ] + assert AutomaticPartitioningTask._get_mountpoint_device( + storage, "/home" + ) == home_device + + # device not found + storage.roots = [ + Mock(mounts={ + "/": Mock(), + }), + ] + with pytest.raises(StorageError): + AutomaticPartitioningTask._get_mountpoint_device(storage, "/home") + assert AutomaticPartitioningTask._get_mountpoint_device( + storage, "/home", required=False + ) is None + + # multiple devices found + home_device1 = Mock() + home_device2 = Mock() + home_device1.name = "device1_name" + home_device2.name = "device2_name" + storage.roots = [ + Mock(mounts={ + "/home": home_device1, + "/": Mock(), + }), + Mock(mounts={ + "/home": home_device2, + }), + ] + with pytest.raises(StorageError): + AutomaticPartitioningTask._get_mountpoint_device(storage, "/home") + + def test_get_mountpoint_options(self): + storage = Mock() + home_opts = "subvol=home,compress=zstd:1" + storage.roots = [ + Mock(mountopts={ + "/home": home_opts + }), + ] + assert AutomaticPartitioningTask._get_mountpoint_options( + storage, "/home" + ) == home_opts + assert AutomaticPartitioningTask._get_mountpoint_options( + storage, "/" + ) is None + + def test_get_reused_device_names(self): + request = Mock( + reused_mount_points=["/home"], + reformatted_mount_points=["/"] + ) + storage = Mock() + device1 = Mock() + device1.name = "home" + device2 = Mock() + device2.name = "root" + storage.roots = [ + Mock(mounts={ + "/home": device1, + "/": device2, + }), + ] + assert AutomaticPartitioningTask._get_reused_device_names( + storage, request + ) == { + "home": "/home", + "root": "/", + } + + def test_check_reused_scheme(self): + request = Mock( + partitioning_scheme=AUTOPART_TYPE_BTRFS, + reused_mount_points=["/home"], + ) + storage = Mock() + storage.roots = [ + Mock(mounts={ + "/home": Mock(type="btrfs subvolume") + }), + ] + AutomaticPartitioningTask._check_reused_scheme(storage, request) + + # all reused mountpoints must have the type based on the scheme + request.reused_mount_points = ["/home", "/data"] + storage.roots = [ + Mock(mounts={ + "/home": Mock(type="btrfs subvolume"), + "/data": Mock(type="partition"), + }), + ] + with pytest.raises(StorageError): + AutomaticPartitioningTask._check_reused_scheme(storage, request) + + def _get_mocked_storage_w_existing_system(self, + bootloader_type="efi", + root_device_type="btrfs subvolume", + root_format_type="btrfs", + separate_boot=True, + ): + storage = Mock() + + bootloader_device = Mock(format=Mock(type=bootloader_type), type="partition") + bootloader_device.name = "vda1" + root_device = Mock(format=Mock(type=root_format_type), type=root_device_type) + root_device.name = "root" + home_device = Mock(format=Mock(type=root_format_type), type=root_device_type) + home_device.name = "home" + + storage.devices = [ + bootloader_device, + root_device, + home_device, + ] + if separate_boot: + boot_device = Mock(format=Mock(type="ext4"), type="partition") + boot_device.name = "vda2" + storage.devices.append(boot_device) + else: + boot_device = None + + storage.roots = [ + Mock(mounts={ + "/home": home_device, + "/": root_device, + }), + ] + if separate_boot: + storage.roots[0].mounts["/boot"] = boot_device + + return storage, bootloader_device, boot_device, root_device, home_device + + @patch('pyanaconda.modules.storage.partitioning.automatic.automatic_partitioning.reformat_device') + @patch('pyanaconda.modules.storage.partitioning.automatic.automatic_partitioning.destroy_device') + @patch('pyanaconda.modules.storage.partitioning.automatic.automatic_partitioning.platform') + def test_clear_existing_mountpoints(self, platform, destroy_device, reformat_device): + # Existing btrfs with efi + + platform.partitions = [ + PartSpec( + mountpoint="/boot/efi", + fstype="efi", + size=Size("500MiB"), + max_size=Size("600MiB"), + grow=True, + ), + PartSpec( + mountpoint="/boot", + size=Size("1GiB"), + lv=False + ), + ] + + storage, bootloader_device, boot_device, root_device, _home_device = \ + self._get_mocked_storage_w_existing_system() + + request = Mock( + partitioning_scheme=AUTOPART_TYPE_BTRFS, + reused_mount_points=["/home"], + removed_mount_points=["/boot", "bootloader"], + reformatted_mount_points=["/"], + ) + + expected_reused_devices = { + "home": "/home", + "root": "/", + } + AutomaticPartitioningTask._clear_existing_mountpoints(storage, request) + destroy_device.assert_has_calls([ + call(storage, bootloader_device), + call(storage, boot_device), + ], any_order=True) + reformat_device.assert_called_with(storage, root_device, + dependencies=expected_reused_devices) + + # missing mountpoint to be removed (/boot) is ignored + storage, bootloader_device, _boot_device, _root_device, _home_device = \ + self._get_mocked_storage_w_existing_system(separate_boot=False) + destroy_device.reset_mock() + AutomaticPartitioningTask._clear_existing_mountpoints(storage, request) + destroy_device.assert_called_once_with(storage, bootloader_device) + + # missing mountpoint to be reformatted (/data) prevents the reuse + storage, _bootloader_device, _boot_device, _root_device, _home_device = \ + self._get_mocked_storage_w_existing_system() + + request = Mock( + partitioning_scheme=AUTOPART_TYPE_BTRFS, + reused_mount_points=["/home"], + removed_mount_points=["/boot", "bootloader"], + reformatted_mount_points=["/", "/data"], + ) + with pytest.raises(StorageError): + AutomaticPartitioningTask._clear_existing_mountpoints(storage, request) + + # Existing plain with biosboot + + platform.partitions = [ + PartSpec( + fstype="biosboot", + size=Size("1MiB"), + ), + PartSpec( + mountpoint="/boot", + size=Size("1GiB"), + lv=False + ), + ] + storage, bootloader_device, boot_device, _root_device, _home_device = \ + self._get_mocked_storage_w_existing_system( + root_device_type="partition", + root_format_type="xfs", + bootloader_type="biosboot" + ) + + request = Mock( + partitioning_scheme=AUTOPART_TYPE_PLAIN, + reused_mount_points=["/home"], + removed_mount_points=["/boot", "bootloader", "/"], + reformatted_mount_points=[], + ) + + expected_reused_devices = { + "home": "/home", + } + destroy_device.reset_mock() + reformat_device.reset_mock() + AutomaticPartitioningTask._clear_existing_mountpoints(storage, request) + destroy_device.assert_has_calls([ + call(storage, bootloader_device), + call(storage, boot_device), + ], any_order=True) + reformat_device.assert_not_called() + + # Existing lvm with efi without separate /boot + + platform.partitions = [ + PartSpec( + mountpoint="/boot/efi", + fstype="efi", + size=Size("500MiB"), + max_size=Size("600MiB"), + grow=True, + ), + PartSpec( + mountpoint="/boot", + size=Size("1GiB"), + lv=False + ), + ] + + storage, bootloader_device, _boot_device, root_device, _home_device = \ + self._get_mocked_storage_w_existing_system( + root_device_type="lvmlv", + root_format_type=None, + separate_boot=False, + ) + + request = Mock( + partitioning_scheme=AUTOPART_TYPE_LVM, + reused_mount_points=["/home"], + removed_mount_points=["/boot", "bootloader"], + reformatted_mount_points=["/"], + ) + + expected_reused_devices = { + "home": "/home", + "root": "/", + } + destroy_device.reset_mock() + reformat_device.reset_mock() + AutomaticPartitioningTask._clear_existing_mountpoints(storage, request) + destroy_device.assert_called_once_with(storage, bootloader_device) + reformat_device.assert_called_with(storage, root_device, + dependencies=expected_reused_devices) + + def test_schedule_existing_mountpoints(self): + # Existing btrfs with efi + + storage, _bootloader_device, _boot_device, _root_device, home_device = \ + self._get_mocked_storage_w_existing_system() + + home_opts = "subvol=home,compress=zstd:1" + storage.roots[0].mountopts = { + "/home": home_opts + } + + request = Mock( + partitioning_scheme=AUTOPART_TYPE_BTRFS, + reused_mount_points=["/home"], + removed_mount_points=["/boot", "bootloader"], + reformatted_mount_points=["/"], + ) + + reformatted_device = Mock() + storage.devicetree.resolve_device.return_value = reformatted_device + + AutomaticPartitioningTask._schedule_existing_mountpoints(storage, request) + assert home_device.format.mountpoint == "/home" + assert home_device.format.options == home_opts + assert reformatted_device.format.mountpoint == "/" + + # missing mountpoint to be reused (/home) prevents the reuse + storage.roots[0].mounts.pop("/home") + with pytest.raises(StorageError): + AutomaticPartitioningTask._schedule_existing_mountpoints(storage, request) + + # Existing plain with biosboot + + storage, _bootloader_device, _boot_device, _root_device, home_device = \ + self._get_mocked_storage_w_existing_system( + root_device_type="partition", + root_format_type="xfs", + bootloader_type="biosboot" + ) + + home_opts = "defaults" + storage.roots[0].mountopts = { + "/home": home_opts + } + + request = Mock( + partitioning_scheme=AUTOPART_TYPE_PLAIN, + reused_mount_points=["/home"], + removed_mount_points=["/boot", "bootloader", "/"], + reformatted_mount_points=[], + ) + + AutomaticPartitioningTask._schedule_existing_mountpoints(storage, request) + assert home_device.format.mountpoint == "/home" + assert home_device.format.options == home_opts + + # Existing lvm with efi without separate /boot + + storage, _bootloader_device, _boot_device, _root_device, home_device = \ + self._get_mocked_storage_w_existing_system( + root_device_type="lvmlv", + root_format_type=None, + separate_boot=False, + ) + + home_opts = "defaults" + storage.roots[0].mountopts = { + "/home": home_opts + } + + request = Mock( + partitioning_scheme=AUTOPART_TYPE_LVM, + reused_mount_points=["/home"], + removed_mount_points=["/boot", "bootloader"], + reformatted_mount_points=["/"], + ) + + reformatted_device = Mock() + storage.devicetree.resolve_device.return_value = reformatted_device + + AutomaticPartitioningTask._schedule_existing_mountpoints(storage, request) + assert home_device.format.mountpoint == "/home" + assert home_device.format.options == home_opts + assert reformatted_device.format.mountpoint == "/" + + def test_implicit_partitions_reused(self): + storage, _bootloader_device, _boot_device, root_device, home_device = \ + self._get_mocked_storage_w_existing_system() + + request = Mock( + partitioning_scheme=AUTOPART_TYPE_BTRFS, + reused_mount_points=["/home"], + removed_mount_points=["/boot", "bootloader"], + reformatted_mount_points=["/"], + ) + + # Make sure there is no 'vg' or 'volume' attribute + home_device.mock_add_spec(['format', 'name']) + root_device.mock_add_spec(['format', 'name']) + assert AutomaticPartitioningTask._implicit_partitions_reused(storage, request) is False + + # / is on volume group + root_device.mock_add_spec(['format', 'name', 'vg']) + assert AutomaticPartitioningTask._implicit_partitions_reused(storage, request) is True + + # / is on btrfs + root_device.mock_add_spec(['format', 'name', 'volume']) + assert AutomaticPartitioningTask._implicit_partitions_reused(storage, request) is True + + class AutomaticPartitioningUtilsTestCase(unittest.TestCase): """Test the automatic partitioning utils."""