From a325fe72862b3af626bb06aa6009f432ff65c091 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Tue, 21 Apr 2020 11:12:43 +1200 Subject: [PATCH] switch to tracking install device by setting grub_device --- subiquity/controllers/filesystem.py | 45 +++++-------- .../controllers/tests/test_filesystem.py | 65 ++++++------------- subiquity/models/filesystem.py | 46 +++++++------ .../views/filesystem/tests/test_filesystem.py | 1 - .../views/filesystem/tests/test_partition.py | 1 + 5 files changed, 60 insertions(+), 98 deletions(-) diff --git a/subiquity/controllers/filesystem.py b/subiquity/controllers/filesystem.py index 293e285e..4ca16644 100644 --- a/subiquity/controllers/filesystem.py +++ b/subiquity/controllers/filesystem.py @@ -169,17 +169,7 @@ class FilesystemController(SubiquityController): elif 'config' in self.ai_data: with self.context.child("applying_autoinstall"): self.model.apply_autoinstall_config(self.ai_data['config']) - grub = self.ai_data.get('grub', {}) - install_devices = grub.get('install_devices') - if install_devices: - device = install_devices[0] - for action in self.model._actions: - if action.id == device: - self.model.grub_install_device = action - break - else: - raise Exception( - "failed to find install_device {!r}".format(device)) + self.model.grub = self.ai_data.get('grub', {}) self.model.swap = self.ai_data.get('swap') def start(self): @@ -433,8 +423,10 @@ class FilesystemController(SubiquityController): self.model.remove_filesystem(fs) delete_format = delete_filesystem - def create_partition(self, device, spec, flag="", wipe=None): - part = self.model.add_partition(device, spec["size"], flag, wipe) + def create_partition(self, device, spec, flag="", wipe=None, + grub_device=None): + part = self.model.add_partition( + device, spec["size"], flag, wipe, grub_device) self.create_filesystem(part, spec) return part @@ -452,7 +444,7 @@ class FilesystemController(SubiquityController): part = self.create_partition( disk, dict(size=part_size, fstype='fat32', mount='/boot/efi'), - flag="boot") + flag="boot", grub_device=True) elif bootloader == Bootloader.PREP: log.debug('_create_boot_partition - adding PReP partition') part = self.create_partition( @@ -460,15 +452,14 @@ class FilesystemController(SubiquityController): dict(size=PREP_GRUB_SIZE_BYTES, fstype=None, mount=None), # must be wiped or grub-install will fail wipe='zero', - flag='prep') - self.model.grub_install_device = part + flag='prep', grub_device=True) elif bootloader == Bootloader.BIOS: log.debug('_create_boot_partition - adding bios_grub partition') part = self.create_partition( disk, dict(size=BIOS_GRUB_SIZE_BYTES, fstype=None, mount=None), flag='bios_grub') - self.model.grub_install_device = disk + disk.grub_device = True return part def create_raid(self, spec): @@ -539,8 +530,7 @@ class FilesystemController(SubiquityController): self.delete(subobj) def reformat(self, disk): - if disk is self.model.grub_install_device: - self.model.grub_install_device = None + disk.grub_device = False for p in list(disk.partitions()): self.delete_partition(p) self.clear(disk) @@ -641,21 +631,25 @@ class FilesystemController(SubiquityController): def make_boot_disk(self, new_boot_disk): boot_partition = None if self.model.bootloader == Bootloader.BIOS: - install_dev = self.model.grub_install_device + install_dev = self.model._one(type="disk", grub_device=True) if install_dev: + install_dev.grub_device = False boot_partition = install_dev._potential_boot_partition() elif self.model.bootloader == Bootloader.UEFI: mount = self.model._mount_for_path("/boot/efi") if mount is not None: boot_partition = mount.device.volume elif self.model.bootloader == Bootloader.PREP: - boot_partition = self.model.grub_install_device + boot_partition = self.model._one( + type="partition", flag="prep", grub_device=True) if boot_partition is not None: if boot_partition.preserve: if self.model.bootloader == Bootloader.PREP: boot_partition.wipe = None + boot_partition.grub_device = False elif self.model.bootloader == Bootloader.UEFI: self.delete_mount(boot_partition.fs().mount()) + boot_partition.grub_device = False else: boot_disk = boot_partition.device full = boot_disk.free_for_partitions == 0 @@ -672,16 +666,17 @@ class FilesystemController(SubiquityController): new_boot_disk.free_for_partitions) if new_boot_disk._has_preexisting_partition(): if self.model.bootloader == Bootloader.BIOS: - self.model.grub_install_device = new_boot_disk + new_boot_disk.grub_device = True elif self.model.bootloader == Bootloader.UEFI: part = new_boot_disk._potential_boot_partition() + part.grub_device = True if part.fs() is None: self.model.add_filesystem(part, 'fat32') self.model.add_mount(part.fs(), '/boot/efi') elif self.model.bootloader == Bootloader.PREP: part = new_boot_disk._potential_boot_partition() part.wipe = 'zero' - self.model.grub_install_device = part + part.grub_device = True else: new_boot_disk.preserve = False self._create_boot_partition(new_boot_disk) @@ -733,8 +728,4 @@ class FilesystemController(SubiquityController): } if 'swap' in rendered: r['swap'] = rendered['swap'] - if self.model.grub_install_device: - r['grub'] = { - 'install_devices': [self.model.grub_install_device.id], - } return r diff --git a/subiquity/controllers/tests/test_filesystem.py b/subiquity/controllers/tests/test_filesystem.py index 6d59c8bb..fa7b354f 100644 --- a/subiquity/controllers/tests/test_filesystem.py +++ b/subiquity/controllers/tests/test_filesystem.py @@ -20,7 +20,6 @@ from subiquity.controllers.filesystem import ( FilesystemController, ) from subiquity.models.tests.test_filesystem import ( - fake_up_blockdata, make_disk, make_model, ) @@ -100,7 +99,7 @@ class TestFilesystemController(unittest.TestCase): controller.make_boot_disk(disk1) self.assertEqual(len(disk1.partitions()), 1) self.assertEqual(disk1.partitions()[0].flag, "bios_grub") - self.assertEqual(controller.model.grub_install_device, disk1) + self.assertTrue(disk1.grub_device) size_before = disk2p1.size controller.make_boot_disk(disk2) @@ -110,7 +109,8 @@ class TestFilesystemController(unittest.TestCase): self.assertEqual( disk2.partitions()[0].size + disk2p1.size, size_before) self.assertEqual(disk2.partitions()[0].flag, "bios_grub") - self.assertEqual(controller.model.grub_install_device, disk2) + self.assertFalse(disk1.grub_device) + self.assertTrue(disk2.grub_device) def test_make_boot_disk_BIOS_existing(self): controller = make_controller(Bootloader.BIOS) @@ -121,14 +121,15 @@ class TestFilesystemController(unittest.TestCase): disk2 = make_disk(controller.model, preserve=False) self.assertEqual(disk1.partitions(), [disk1p1]) - self.assertEqual(controller.model.grub_install_device, None) + self.assertFalse(disk1.grub_device) controller.make_boot_disk(disk1) self.assertEqual(disk1.partitions(), [disk1p1]) - self.assertEqual(controller.model.grub_install_device, disk1) + self.assertTrue(disk1.grub_device) controller.make_boot_disk(disk2) self.assertEqual(disk1.partitions(), [disk1p1]) - self.assertEqual(controller.model.grub_install_device, disk2) + self.assertFalse(disk1.grub_device) + self.assertTrue(disk2.grub_device) def test_make_boot_disk_UEFI(self): controller = make_controller(Bootloader.UEFI) @@ -140,7 +141,7 @@ class TestFilesystemController(unittest.TestCase): controller.make_boot_disk(disk1) self.assertEqual(len(disk1.partitions()), 1) self.assertEqual(disk1.partitions()[0].flag, "boot") - self.assertEqual(controller.model.grub_install_device, None) + self.assertTrue(disk1.partitions()[0].grub_device) efi_mnt = controller.model._mount_for_path("/boot/efi") self.assertEqual(efi_mnt.device.volume, disk1.partitions()[0]) self.assertEqual(disk1.partitions()[0].fs().fstype, "fat32") @@ -153,7 +154,7 @@ class TestFilesystemController(unittest.TestCase): self.assertEqual( disk2.partitions()[0].size + disk2p1.size, size_before) self.assertEqual(disk2.partitions()[0].flag, "boot") - self.assertEqual(controller.model.grub_install_device, None) + self.assertTrue(disk2.partitions()[0].grub_device) efi_mnt = controller.model._mount_for_path("/boot/efi") self.assertEqual(efi_mnt.device.volume, disk2.partitions()[0]) @@ -166,19 +167,20 @@ class TestFilesystemController(unittest.TestCase): disk2 = make_disk(controller.model, preserve=True) self.assertEqual(disk1.partitions(), [disk1p1]) - self.assertEqual(controller.model.grub_install_device, None) + self.assertFalse(disk1p1.grub_device) efi_mnt = controller.model._mount_for_path("/boot/efi") self.assertEqual(efi_mnt, None) controller.make_boot_disk(disk1) self.assertEqual(disk1.partitions(), [disk1p1]) - self.assertEqual(controller.model.grub_install_device, None) + self.assertTrue(disk1p1.grub_device) efi_mnt = controller.model._mount_for_path("/boot/efi") self.assertEqual(efi_mnt.device.volume, disk1p1) self.assertEqual(disk1p1.fs().fstype, "fat32") controller.make_boot_disk(disk2) self.assertEqual(disk1.partitions(), [disk1p1]) - self.assertEqual(controller.model.grub_install_device, None) + self.assertFalse(disk1p1.grub_device) + self.assertTrue(disk2.partitions()[0].grub_device) efi_mnt = controller.model._mount_for_path("/boot/efi") self.assertEqual(efi_mnt.device.volume, disk2.partitions()[0]) @@ -193,9 +195,7 @@ class TestFilesystemController(unittest.TestCase): self.assertEqual(len(disk1.partitions()), 1) self.assertEqual(disk1.partitions()[0].flag, "prep") self.assertEqual(disk1.partitions()[0].wipe, "zero") - self.assertEqual( - controller.model.grub_install_device, - disk1.partitions()[0]) + self.assertTrue(disk1.partitions()[0].grub_device) size_before = disk2p1.size controller.make_boot_disk(disk2) @@ -206,9 +206,7 @@ class TestFilesystemController(unittest.TestCase): disk2.partitions()[0].size + disk2p1.size, size_before) self.assertEqual(disk2.partitions()[0].flag, "prep") self.assertEqual(disk2.partitions()[0].wipe, "zero") - self.assertEqual( - controller.model.grub_install_device, - disk2.partitions()[0]) + self.assertTrue(disk2.partitions()[0].grub_device) def test_make_boot_disk_PREP_existing(self): controller = make_controller(Bootloader.PREP) @@ -219,21 +217,18 @@ class TestFilesystemController(unittest.TestCase): disk2 = make_disk(controller.model, preserve=False) self.assertEqual(disk1.partitions(), [disk1p1]) - self.assertEqual(controller.model.grub_install_device, None) + self.assertFalse(disk1p1.grub_device) controller.make_boot_disk(disk1) self.assertEqual(disk1.partitions(), [disk1p1]) - self.assertEqual(controller.model.grub_install_device, disk1p1) + self.assertTrue(disk1p1.grub_device) self.assertEqual(disk1p1.wipe, 'zero') controller.make_boot_disk(disk2) self.assertEqual(disk1.partitions(), [disk1p1]) self.assertEqual(disk1p1.wipe, None) - self.assertEqual( - controller.model.grub_install_device, disk2.partitions()[0]) self.assertEqual(disk2.partitions()[0].flag, "prep") - self.assertEqual( - controller.model.grub_install_device, - disk2.partitions()[0]) + self.assertFalse(disk1p1.grub_device) + self.assertTrue(disk2.partitions()[0].grub_device) def test_mounting_partition_makes_boot_disk(self): controller = make_controller(Bootloader.UEFI) @@ -248,25 +243,3 @@ class TestFilesystemController(unittest.TestCase): disk1, disk1p2, {'fstype': 'ext4', 'mount': '/'}) efi_mnt = controller.model._mount_for_path("/boot/efi") self.assertEqual(efi_mnt.device.volume, disk1p1) - - def test_autoinstall_grub_devices(self): - controller = make_controller(Bootloader.BIOS) - make_disk(controller.model) - fake_up_blockdata(controller.model) - controller.ai_data = { - 'config': [{'type': 'disk', 'id': 'disk0'}], - 'grub': { - 'install_devices': ['disk0'], - }, - } - controller.convert_autoinstall_config() - new_disk = controller.model._one(type="disk", id="disk0") - self.assertEqual(controller.model.grub_install_device, new_disk) - - def test_make_autoinstall(self): - controller = make_controller(Bootloader.BIOS) - disk = make_disk(controller.model) - fake_up_blockdata(controller.model) - controller.guided_direct(disk) - ai_data = controller.make_autoinstall() - self.assertEqual(ai_data['grub']['install_devices'], [disk.id]) diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index d8242bc9..5bbbbbf1 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -25,7 +25,6 @@ import os import pathlib import platform -from curtin.block import partition_kname from curtin import storage_config from curtin.util import human2bytes @@ -847,16 +846,16 @@ class Disk(_Device): def _can_TOGGLE_BOOT(self): bl = self._m.bootloader if bl == Bootloader.BIOS: - if self._m.grub_install_device is self: + if self.grub_device: return False elif bl == Bootloader.UEFI: m = self._m._mount_for_path('/boot/efi') if m and m.device.volume.device is self: return False elif bl == Bootloader.PREP: - install_dev = self._m.grub_install_device - if install_dev is not None and install_dev.device is self: - return False + for p in self._partitions: + if p.flag == "prep" and p.grub_device: + return False if self._has_preexisting_partition(): return self._can_be_boot_disk() else: @@ -886,6 +885,7 @@ class Partition(_Formattable): flag = attr.ib(default=None) number = attr.ib(default=None) preserve = attr.ib(default=False) + grub_device = attr.ib(default=False) @property def annotations(self): @@ -919,7 +919,7 @@ class Partition(_Formattable): return _("partition {}").format(self._number) def available(self): - if self.flag in ['bios_grub', 'prep']: + if self.flag in ['bios_grub', 'prep'] or self.grub_device: return False if self._constructed_device is not None: return False @@ -1278,8 +1278,8 @@ class FilesystemModel(object): else: self._orig_config = [] self._actions = [] - self.grub_install_device = None self.swap = None + self.grub = None def _make_matchers(self, match): matchers = [] @@ -1545,16 +1545,8 @@ class FilesystemModel(object): } if self.swap is not None: config['swap'] = self.swap - if self.grub_install_device: - dev = self.grub_install_device - if dev.type == "partition": - disk_kname = dev.device.path[5:] # chop off "/dev/" - devpath = "/dev/" + partition_kname(disk_kname, dev._number) - else: - devpath = dev.path - config['grub'] = { - 'install_devices': [devpath], - } + if self.grub is not None: + config['grub'] = self.grub return config def load_probe_data(self, probe_data): @@ -1608,12 +1600,11 @@ class FilesystemModel(object): return self._all(type='lvm_volgroup') def _remove(self, obj): - if obj is self.grub_install_device: - self.grub_install_device = None _remove_backlinks(obj) self._actions.remove(obj) - def add_partition(self, device, size, flag="", wipe=None): + def add_partition(self, device, size, flag="", wipe=None, + grub_device=None): if size > device.free_for_partitions: raise Exception("%s > %s", size, device.free_for_partitions) real_size = align_up(size) @@ -1621,7 +1612,8 @@ class FilesystemModel(object): if device._fs is not None: raise Exception("%s is already formatted" % (device.label,)) p = Partition( - m=self, device=device, size=real_size, flag=flag, wipe=wipe) + m=self, device=device, size=real_size, flag=flag, wipe=wipe, + grub_device=grub_device) if flag in ("boot", "bios_grub", "prep"): device._partitions.insert(0, device._partitions.pop()) device.ptable = device.ptable_for_new_partition() @@ -1725,10 +1717,16 @@ class FilesystemModel(object): # s390x has no such thing if self.bootloader == Bootloader.NONE: return False - elif self.bootloader in [Bootloader.BIOS, Bootloader.PREP]: - return self.grub_install_device is None + elif self.bootloader == Bootloader.BIOS: + return self._one(type='disk', grub_device=True) is None elif self.bootloader == Bootloader.UEFI: - return self._mount_for_path('/boot/efi') is None + for esp in self._all(type='partition', grub_device=True): + if esp.fs() and esp.fs().mount(): + if esp.fs().mount().path == '/boot/efi': + return False + return True + elif self.bootloader == Bootloader.PREP: + return self._one(type='partition', grub_device=True) is None else: raise AssertionError( "unknown bootloader type {}".format(self.bootloader)) diff --git a/subiquity/ui/views/filesystem/tests/test_filesystem.py b/subiquity/ui/views/filesystem/tests/test_filesystem.py index ff3ee643..b8617f17 100644 --- a/subiquity/ui/views/filesystem/tests/test_filesystem.py +++ b/subiquity/ui/views/filesystem/tests/test_filesystem.py @@ -24,7 +24,6 @@ class FilesystemViewTests(unittest.TestCase): controller.ui = mock.Mock() model.bootloader = Bootloader.NONE model.all_devices.return_value = devices - model.grub_install_device = None return FilesystemView(model, controller) def test_simple(self): diff --git a/subiquity/ui/views/filesystem/tests/test_partition.py b/subiquity/ui/views/filesystem/tests/test_partition.py index 673b13b4..b1fe85a4 100644 --- a/subiquity/ui/views/filesystem/tests/test_partition.py +++ b/subiquity/ui/views/filesystem/tests/test_partition.py @@ -215,6 +215,7 @@ class PartitionViewTests(unittest.TestCase): partition = model.add_partition(disk, 512*(2**20), "boot") fs = model.add_filesystem(partition, "fat32") model._orig_config = model._render_actions() + partition.grub_device = True disk.preserve = partition.preserve = fs.preserve = True model.add_mount(fs, '/boot/efi') view, stretchy = make_partition_view(model, disk, partition)