From faea101e129033b5b76c14deb1a24c2c6e9d3ba4 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Mon, 31 May 2021 15:45:12 +1200 Subject: [PATCH] try to refactor some boot device code in preparation for imsm --- po/POTFILES.in | 3 + subiquity/common/filesystem/actions.py | 10 +-- subiquity/common/filesystem/boot.py | 70 +++++++++++++++++++++ subiquity/common/filesystem/manipulator.py | 16 ++--- subiquity/models/filesystem.py | 24 ------- subiquity/server/controllers/filesystem.py | 7 ++- subiquity/ui/views/filesystem/filesystem.py | 11 ++-- 7 files changed, 93 insertions(+), 48 deletions(-) create mode 100644 subiquity/common/filesystem/boot.py diff --git a/po/POTFILES.in b/po/POTFILES.in index 77047529..29032463 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -33,6 +33,7 @@ subiquity/common/api/tests/test_endtoend.py subiquity/common/api/tests/test_server.py subiquity/common/errorreport.py subiquity/common/filesystem/actions.py +subiquity/common/filesystem/boot.py subiquity/common/filesystem/__init__.py subiquity/common/filesystem/labels.py subiquity/common/filesystem/manipulator.py @@ -106,6 +107,7 @@ subiquity/__main__.py subiquity/models/filesystem.py subiquity/models/identity.py subiquity/models/__init__.py +subiquity/models/kernel.py subiquity/models/keyboard.py subiquity/models/locale.py subiquity/models/mirror.py @@ -126,6 +128,7 @@ subiquity/server/controllers/filesystem.py subiquity/server/controllers/identity.py subiquity/server/controllers/__init__.py subiquity/server/controllers/install.py +subiquity/server/controllers/kernel.py subiquity/server/controllers/keyboard.py subiquity/server/controllers/locale.py subiquity/server/controllers/mirror.py diff --git a/subiquity/common/filesystem/actions.py b/subiquity/common/filesystem/actions.py index 0f6a55b6..572348db 100644 --- a/subiquity/common/filesystem/actions.py +++ b/subiquity/common/filesystem/actions.py @@ -18,7 +18,7 @@ import functools from subiquitycore.gettext38 import pgettext -from subiquity.common.filesystem import labels +from subiquity.common.filesystem import boot, labels from subiquity.models.filesystem import ( Bootloader, Disk, @@ -341,12 +341,12 @@ _can_toggle_boot = make_checker(DeviceAction.TOGGLE_BOOT) @_can_toggle_boot.register(Disk) def _can_toggle_boot_disk(disk): - if disk._is_boot_device(): - for disk2 in disk._m.all_disks(): - if disk2 is not disk and disk2._is_boot_device(): + if boot.is_boot_device(disk): + for disk2 in boot.all_boot_devices(disk._m): + if disk2 is not disk: return True return False elif disk._fs is not None or disk._constructed_device is not None: return False else: - return disk._can_be_boot_disk() + return boot.can_be_boot_device(disk) diff --git a/subiquity/common/filesystem/boot.py b/subiquity/common/filesystem/boot.py new file mode 100644 index 00000000..ce9e24db --- /dev/null +++ b/subiquity/common/filesystem/boot.py @@ -0,0 +1,70 @@ +# Copyright 2021 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +import functools + +from subiquity.models.filesystem import ( + Disk, + Bootloader, + ) + + +@functools.singledispatch +def is_boot_device(device): + """Is `device` a boot device?""" + return False + + +@is_boot_device.register(Disk) +def _is_boot_device_disk(disk): + bl = disk._m.bootloader + if bl == Bootloader.NONE: + return False + elif bl == Bootloader.BIOS: + return disk.grub_device + elif bl in [Bootloader.PREP, Bootloader.UEFI]: + return any(p.grub_device for p in disk._partitions) + + +@functools.singledispatch +def can_be_boot_device(device, *, with_reformatting=False): + """Can `device` be made into a boot device? + + If with_reformatting=True, return true if the device can be made + into a boot device after reformatting. + """ + return False + + +@can_be_boot_device.register(Disk) +def _can_be_boot_device_disk(disk, *, with_reformatting=False): + bl = disk._m.bootloader + if disk._has_preexisting_partition() and not with_reformatting: + if bl == Bootloader.BIOS: + if disk.ptable == "msdos": + return True + else: + return disk._partitions[0].flag == "bios_grub" + elif bl == Bootloader.UEFI: + return any(p.is_esp for p in disk._partitions) + elif bl == Bootloader.PREP: + return any(p.flag == "prep" for p in disk._partitions) + else: + return True + + +def all_boot_devices(model): + """Return all current boot devices for `model`.""" + return [disk for disk in model.all_disks() if is_boot_device(disk)] diff --git a/subiquity/common/filesystem/manipulator.py b/subiquity/common/filesystem/manipulator.py index 4b98c237..5bba4a1a 100644 --- a/subiquity/common/filesystem/manipulator.py +++ b/subiquity/common/filesystem/manipulator.py @@ -15,9 +15,7 @@ import logging -from subiquity.common.filesystem.actions import ( - DeviceAction, - ) +from subiquity.common.filesystem import boot from subiquity.common.types import Bootloader from subiquity.models.filesystem import ( align_up, @@ -40,9 +38,8 @@ class FilesystemManipulator: mount = self.model.add_mount(fs, spec['mount']) if self.model.needs_bootloader_partition(): vol = fs.volume - if vol.type == "partition" and vol.device.type == "disk": - if vol.device._can_be_boot_disk(): - self.add_boot_disk(vol.device) + if vol.type == "partition" and boot.can_be_boot_device(vol.device): + self.add_boot_disk(vol.device) return mount def delete_mount(self, mount): @@ -216,7 +213,7 @@ class FilesystemManipulator: needs_boot = self.model.needs_bootloader_partition() log.debug('model needs a bootloader partition? {}'.format(needs_boot)) - can_be_boot = DeviceAction.TOGGLE_BOOT in DeviceAction.supported(disk) + can_be_boot = boot.can_be_boot_device(disk) if needs_boot and len(disk.partitions()) == 0 and can_be_boot: part = self._create_boot_partition(disk) @@ -338,9 +335,8 @@ class FilesystemManipulator: def add_boot_disk(self, new_boot_disk): bootloader = self.model.bootloader if not self.supports_resilient_boot: - for disk in self.model.all_disks(): - if disk._is_boot_device(): - self.remove_boot_disk(disk) + for disk in boot.all_boot_devices(self.model): + self.remove_boot_disk(disk) if new_boot_disk._has_preexisting_partition(): if bootloader == Bootloader.BIOS: new_boot_disk.grub_device = True diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 76b579ec..f252c514 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -615,30 +615,6 @@ class Disk(_Device): def dasd(self): return self._m._one(type='dasd', device_id=self.device_id) - def _can_be_boot_disk(self): - bl = self._m.bootloader - if self._has_preexisting_partition(): - if bl == Bootloader.BIOS: - if self.ptable == "msdos": - return True - else: - return self._partitions[0].flag == "bios_grub" - elif bl == Bootloader.UEFI: - return any(p.is_esp for p in self._partitions) - elif bl == Bootloader.PREP: - return any(p.flag == "prep" for p in self._partitions) - else: - return True - - def _is_boot_device(self): - bl = self._m.bootloader - if bl == Bootloader.NONE: - return False - elif bl == Bootloader.BIOS: - return self.grub_device - elif bl in [Bootloader.PREP, Bootloader.UEFI]: - return any(p.grub_device for p in self._partitions) - @property def ok_for_raid(self): if self._fs is not None: diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 586d1ebe..a2e1d711 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -40,7 +40,7 @@ from subiquity.common.errorreport import ErrorReportKind from subiquity.common.filesystem.actions import ( DeviceAction, ) -from subiquity.common.filesystem import labels +from subiquity.common.filesystem import boot, labels from subiquity.common.filesystem.manipulator import FilesystemManipulator from subiquity.common.types import ( Bootloader, @@ -226,8 +226,9 @@ class FilesystemController(SubiquityController, FilesystemManipulator): status=ProbeStatus.DONE, error_report=self.full_probe_error(), disks=[ - labels.for_client(d, min_size=min_size) - for d in self.model._all(type='disk') + labels.for_client(device, min_size=min_size) + for device in self.model._actions + if boot.can_be_boot_device(device, with_reformatting=True) ]) async def guided_POST(self, choice: Optional[GuidedChoice]) \ diff --git a/subiquity/ui/views/filesystem/filesystem.py b/subiquity/ui/views/filesystem/filesystem.py index 26b0d9d5..72a98fab 100644 --- a/subiquity/ui/views/filesystem/filesystem.py +++ b/subiquity/ui/views/filesystem/filesystem.py @@ -63,7 +63,7 @@ from subiquitycore.view import BaseView from subiquity.common.filesystem.actions import ( DeviceAction, ) -from subiquity.common.filesystem import labels +from subiquity.common.filesystem import boot, labels from subiquity.models.filesystem import ( humanize_size, ) @@ -286,7 +286,7 @@ class DeviceList(WidgetWrap): self.parent.refresh_model_inputs() def _disk_TOGGLE_BOOT(self, disk): - if disk._is_boot_device(): + if boot.is_boot_device(disk): self.parent.controller.remove_boot_disk(disk) else: self.parent.controller.add_boot_disk(disk) @@ -329,13 +329,12 @@ class DeviceList(WidgetWrap): ptype=device.ptable_for_new_partition().upper()) def _label_TOGGLE_BOOT(self, action, device): - if device._is_boot_device(): + if boot.is_boot_device(device): return _("Stop Using As Boot Device") else: if self.parent.controller.supports_resilient_boot: - for other in self.parent.model.all_disks(): - if other._is_boot_device(): - return _("Add As Another Boot Device") + if boot.all_boot_devices(self.parent.model): + return _("Add As Another Boot Device") return _("Use As Boot Device") def _action_menu_for_device(self, device):