From 581bc3e149f08a3f6ff392e2c0ac34fcbc880571 Mon Sep 17 00:00:00 2001 From: Radek Vykydal Date: Tue, 30 Jul 2024 08:57:46 +0200 Subject: [PATCH] [WIP] 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) TODO: + unify biosboot partition with other stuff (configurable) + look at all roots + test with plain + test with lvm + test with thin lvm + --remove should fail silently? - kickstart support (--erase --reuse --remove) - kickstart tests - reuse fstab mount options (/home is missing compression) + add checks that the device found is unique (including biosboot/efi) - checks for recreate only on plain partition? - create check for usability of the /home reuse (for UI) - update unit tests - luks ? - efi partitions, we have only biosboot so far - update for device_id: https://github.com/rhinstaller/anaconda/pull/5435 - add better mount point assignment kickstart tests, we update code there (fedora, lvm, plain) --- pyanaconda/core/kickstart/commands.py | 2 +- .../modules/common/structures/partitioning.py | 53 ++++++++ .../automatic/automatic_module.py | 4 + .../automatic/automatic_partitioning.py | 127 +++++++++++++++++- .../storage/partitioning/automatic/utils.py | 3 +- .../manual/manual_partitioning.py | 95 ++----------- .../storage/partitioning/manual/utils.py | 124 +++++++++++++++++ 7 files changed, 312 insertions(+), 96 deletions(-) create mode 100644 pyanaconda/modules/storage/partitioning/manual/utils.py diff --git a/pyanaconda/core/kickstart/commands.py b/pyanaconda/core/kickstart/commands.py index da96f35c45b9..b7b45750dbc4 100644 --- a/pyanaconda/core/kickstart/commands.py +++ b/pyanaconda/core/kickstart/commands.py @@ -24,7 +24,7 @@ # Supported kickstart commands. from pykickstart.commands.authselect import F28_Authselect as Authselect -from pykickstart.commands.autopart import F38_AutoPart as AutoPart +from pykickstart.commands.autopart import F41_AutoPart as AutoPart from pykickstart.commands.bootloader import F39_Bootloader as Bootloader from pykickstart.commands.btrfs import F23_BTRFS as BTRFS from pykickstart.commands.cdrom import FC3_Cdrom as Cdrom diff --git a/pyanaconda/modules/common/structures/partitioning.py b/pyanaconda/modules/common/structures/partitioning.py index 20adc6f5c1f8..5184d4588ab8 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._erased_mount_points = [] + self._reused_mount_points = [] + self._removed_mount_points = [] self._hibernation = False self._encrypted = False @@ -111,6 +114,56 @@ def hibernation(self, value: Bool): def excluded_mount_points(self, mount_points: List[Str]): self._excluded_mount_points = mount_points + @property + def erased_mount_points(self) -> List[Str]: + """Erased mount points. + + Erase and reuse existing devices for the mount points. + + For example: / + + :return: a list of mount points + """ + return self._erased_mount_points + + @erased_mount_points.setter + def erased_mount_points(self, mount_points: List[Str]): + self._erased_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_module.py b/pyanaconda/modules/storage/partitioning/automatic/automatic_module.py index e7ac5bff47a7..f7a3026efc61 100644 --- a/pyanaconda/modules/storage/partitioning/automatic/automatic_module.py +++ b/pyanaconda/modules/storage/partitioning/automatic/automatic_module.py @@ -76,6 +76,10 @@ def process_kickstart(self, data): if data.autopart.noswap: request.excluded_mount_points.append("swap") + request.erased_mount_points = data.autopart.erase + request.reused_mount_points = data.autopart.reuse + request.removed_mount_points = data.autopart.remove + request.hibernation = data.autopart.hibernation if data.autopart.encrypted: diff --git a/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py b/pyanaconda/modules/storage/partitioning/automatic/automatic_partitioning.py index 5d8c4da38969..d37a614af200 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,12 +62,94 @@ 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 mountpoint '{}': {}") + .format(mountpoint, + ", ".join([device.name for device in devices]))) + if not devices: + if required: + raise StorageError(_("No devices found for mountpoint '{}'").format(mountpoint)) + else: + return None + + return devices[0] + + def _reused_devices_mountpoints(self, request): + return request.reused_mount_points + request.erased_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 _erase_mountpoint(self, storage, mountpoint): + device = self._get_mountpoint_device(storage, mountpoint) + log.debug("DDDDD erase device %s for mountpoint: %s", device, mountpoint) + reused_devices = self._get_reused_device_names(storage) + # FIXME it would be better to really erase the device + 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("DDDDD remove device %s for mountpoint %s", device, mountpoint) + destroy_device(storage, device) + else: + log.debug("DDDDD device to be removed for mountpoint %s not found", mountpoint) + + def _clear_partitions(self, storage): + super()._clear_partitions(storage) + + log.debug("DDDDD 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? + + # TODO maybe move to _configure_partitioning? Seems safer here. + for mountpoint in self._request.removed_mount_points: + self._remove_mountpoint(storage, mountpoint) + for mountpoint in self._request.erased_mount_points: + self._erase_mountpoint(storage, mountpoint) + + def _schedule_reused_mountpoint(self, storage, mountpoint): + device = self._get_mountpoint_device(storage, mountpoint) + log.debug("DDDDD add mount device request for reused mountpoint: %s device: %s", + mountpoint, device) + device.format.mountpoint = mountpoint + + def _schedule_erased_mountpoint(self, storage, mountpoint): + old_device = self._get_mountpoint_device(storage, mountpoint) + # FIXME: use rather erase for btrfs reformat (no device recreation) + # The device might have been recreated (btrfs) + device = storage.devicetree.resolve_device(old_device.name) + if device: + log.debug("DDDDD add mount device request for erased mountpoint: %s device: %s", + mountpoint, device) + device.format.mountpoint = mountpoint + else: + log.debug("DDDDD device for erased mountpoint %s not found", mountpoint) + def _configure_partitioning(self, storage): """Configure the partitioning. :param storage: an instance of Blivet """ - log.debug("Executing the automatic partitioning.") + + #TODO: check we have unique device for each mountpoint and biosboot/efi parts + # otherwise raise StorageError(_("No unique device for '{}'").format(mp)) # Get the partitioning scheme. scheme = self._request.partitioning_scheme @@ -88,7 +176,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.erased_mount_points: + self._schedule_erased_mountpoint(storage, mountpoint) + @staticmethod def _get_luks_format_args(storage, request): @@ -139,8 +235,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.erased_mount_points) + if (spec.mountpoint or spec.fstype) in skipped: continue # Detect swap. @@ -168,8 +267,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("DDDDD reusing volume group %s", device.vg) + return True + if hasattr(device, "volume"): + log.debug("DDDDD reusing volume %s", device.volume) + 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 @@ -191,8 +302,10 @@ 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 68225d6bc051..4e6522f7e627 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 ac9046051793..bd2a4f6b1f21 100644 --- a/pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py +++ b/pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py @@ -22,9 +22,7 @@ from pyanaconda.core.i18n import _ from pyanaconda.modules.storage.partitioning.automatic.noninteractive_partitioning import \ NonInteractivePartitioningTask -from pyanaconda.modules.storage.partitioning.interactive.utils import destroy_device, \ - generate_device_factory_request -from pyanaconda.modules.storage.partitioning.interactive.add_device import AddDeviceTask +from pyanaconda.modules.storage.partitioning.manual.utils import reformat_device log = get_module_logger(__name__) @@ -74,52 +72,13 @@ 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_spec) - ) - else: - old_fmt = device.format - - if not old_fmt or old_fmt.type is None: - raise StorageError(_("No format on device '{}'").format(device_spec)) - - 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": - depending_subvolumes = device.raw_device.subvolumes - elif device.raw_device.type == "btrfs subvolume": - depending_subvolumes = [sub.name 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 - depending_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_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": @@ -131,41 +90,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 dev_spec: 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_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.resolve_device(dev_spec) - - if device.type == "btrfs volume": - # can't use device factory for just the volume - return self._recreate_btrfs_volume(storage, device) - else: - request = generate_device_factory_request(storage, device) - destroy_device(storage, device) - task = AddDeviceTask(storage, request) - task.run() - device = storage.devicetree.resolve_device(dev_spec) - - return device diff --git a/pyanaconda/modules/storage/partitioning/manual/utils.py b/pyanaconda/modules/storage/partitioning/manual/utils.py new file mode 100644 index 000000000000..fdeaaaf290af --- /dev/null +++ b/pyanaconda/modules/storage/partitioning/manual/utils.py @@ -0,0 +1,124 @@ +# +# 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 _ +from pyanaconda.modules.storage.partitioning.interactive.utils import destroy_device, \ + generate_device_factory_request +from pyanaconda.modules.storage.partitioning.interactive.add_device import AddDeviceTask + + +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={}): + 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": + depending_subvolumes = device.raw_device.subvolumes + elif device.raw_device.type == "btrfs subvolume": + depending_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 depending_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