Merge pull request #718 from mwhudson/resilient-boot

ui for resilient boot

examples/answers-lvm-dmcrypt.yaml

@@ -13,7 +13,7 @@ Mirror:
 Filesystem:
   manual:
     - obj: [disk index 0]
-      action: MAKE_BOOT
+      action: TOGGLE_BOOT
     - &newpart
       obj: [disk index 0]
       action: PARTITION

examples/answers-lvm.yaml

@@ -13,7 +13,7 @@ Mirror:
 Filesystem:
   manual:
     - obj: [disk index 0]
-      action: MAKE_BOOT
+      action: TOGGLE_BOOT
     - &newpart
       obj: [disk index 0]
       action: PARTITION

examples/answers-raid-lvm.yaml

@@ -13,7 +13,7 @@ Mirror:
 Filesystem:
   manual:
     - obj: [disk index 0]
-      action: MAKE_BOOT
+      action: TOGGLE_BOOT
     - &newpart
       obj: [disk index 0]
       action: PARTITION

examples/answers-raid.yaml

@@ -13,7 +13,7 @@ Mirror:
 Filesystem:
   manual:
     - obj: [disk index 0]
-      action: MAKE_BOOT
+      action: TOGGLE_BOOT
     - obj: [disk index 0]
       action: PARTITION
       data:

snapcraft.yaml

@@ -28,7 +28,7 @@ parts:
     plugin: python
     source-type: git
     source: git://git.launchpad.net/curtin
-    source-commit: e967eebc3a427cd62f12f66473ea8430889bc206
+    source-commit: 22b505c98abfbaf50c2ba0f7b0d02c3137566999
     requirements: [requirements.txt]
     organize:
       'lib/python*/site-packages/usr/lib/curtin': 'usr/lib/'

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', {})
+            self.model.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.swap = self.ai_data.get('swap')
 
     def start(self):
@@ -308,9 +298,12 @@ class FilesystemController(SubiquityController):
         log.debug("_answers_action %r", action)
         if 'obj' in action:
             obj = self._action_get(action['obj'])
+            action_name = action['action']
+            if action_name == "MAKE_BOOT":
+                action_name = "TOGGLE_BOOT"
             meth = getattr(
                 self.ui.body.avail_list,
-                "_{}_{}".format(obj.type, action['action']))
+                "_{}_{}".format(obj.type, action_name))
             meth(obj)
             yield
             body = self.ui.body._w
@@ -389,7 +382,7 @@ class FilesystemController(SubiquityController):
             vol = fs.volume
             if vol.type == "partition" and vol.device.type == "disk":
                 if vol.device._can_be_boot_disk():
-                    self.make_boot_disk(vol.device)
+                    self.add_boot_disk(vol.device)
         return mount
 
     def delete_mount(self, mount):
@@ -430,8 +423,10 @@ class FilesystemController(SubiquityController):
         self.model.remove_filesystem(fs)
     delete_format = delete_filesystem
 
-    def create_partition(self, device, spec, flag="", wipe=None):
+    def create_partition(self, device, spec, flag="", wipe=None,
-        part = self.model.add_partition(device, spec["size"], flag, wipe)
+                         grub_device=None):
+        part = self.model.add_partition(
+            device, spec["size"], flag, wipe, grub_device)
         self.create_filesystem(part, spec)
         return part
 
@@ -446,10 +441,11 @@ class FilesystemController(SubiquityController):
             if UEFI_GRUB_SIZE_BYTES*2 >= disk.size:
                 part_size = disk.size // 2
             log.debug('_create_boot_partition - adding EFI partition')
+            spec = dict(size=part_size, fstype='fat32')
+            if self.model._mount_for_path("/boot/efi") is None:
+                spec['mount'] = '/boot/efi'
             part = self.create_partition(
-                disk,
+                disk, spec, flag="boot", grub_device=True)
-                dict(size=part_size, fstype='fat32', mount='/boot/efi'),
-                flag="boot")
         elif bootloader == Bootloader.PREP:
             log.debug('_create_boot_partition - adding PReP partition')
             part = self.create_partition(
@@ -457,15 +453,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')
+                flag='prep', grub_device=True)
-            self.model.grub_install_device = part
         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):
@@ -536,8 +531,7 @@ class FilesystemController(SubiquityController):
             self.delete(subobj)
 
     def reformat(self, disk):
-        if disk is self.model.grub_install_device:
+        disk.grub_device = False
-            self.model.grub_install_device = None
         for p in list(disk.partitions()):
             self.delete_partition(p)
         self.clear(disk)
@@ -562,7 +556,7 @@ class FilesystemController(SubiquityController):
 
         needs_boot = self.model.needs_bootloader_partition()
         log.debug('model needs a bootloader partition? {}'.format(needs_boot))
-        can_be_boot = DeviceAction.MAKE_BOOT in disk.supported_actions
+        can_be_boot = DeviceAction.TOGGLE_BOOT in disk.supported_actions
         if needs_boot and len(disk.partitions()) == 0 and can_be_boot:
             part = self._create_boot_partition(disk)
 
@@ -635,52 +629,91 @@ class FilesystemController(SubiquityController):
         else:
             self.create_volgroup(spec)
 
-    def make_boot_disk(self, new_boot_disk):
+    def _mount_esp(self, part):
-        boot_partition = None
+        if part.fs() is None:
+            self.model.add_filesystem(part, 'fat32')
+        self.model.add_mount(part.fs(), '/boot/efi')
+
+    def remove_boot_disk(self, boot_disk):
         if self.model.bootloader == Bootloader.BIOS:
-            install_dev = self.model.grub_install_device
+            boot_disk.grub_device = False
-            if install_dev:
+            flag = 'bios_grub'
-                boot_partition = install_dev._potential_boot_partition()
         elif self.model.bootloader == Bootloader.UEFI:
-            mount = self.model._mount_for_path("/boot/efi")
+            flag = 'boot'
-            if mount is not None:
-                boot_partition = mount.device.volume
         elif self.model.bootloader == Bootloader.PREP:
-            boot_partition = self.model.grub_install_device
+            flag = 'prep'
-        if boot_partition is not None:
+        partitions = [p for p in boot_disk.partitions() if p.flag == flag]
-            if boot_partition.preserve:
+        remount = False
-                if self.model.bootloader == Bootloader.PREP:
+        if boot_disk.preserve:
-                    boot_partition.wipe = None
-                elif self.model.bootloader == Bootloader.UEFI:
-                    self.delete_mount(boot_partition.fs().mount())
-            else:
-                boot_disk = boot_partition.device
-                full = boot_disk.free_for_partitions == 0
-                self.delete_partition(boot_partition)
-                if full:
-                    largest_part = max(
-                        boot_disk.partitions(), key=lambda p: p.size)
-                    largest_part.size += boot_partition.size
-                if new_boot_disk.free_for_partitions < boot_partition.size:
-                    largest_part = max(
-                        new_boot_disk.partitions(), key=lambda p: p.size)
-                    largest_part.size -= (
-                        boot_partition.size -
-                        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
+                return
-            elif self.model.bootloader == Bootloader.UEFI:
+            for p in partitions:
-                part = new_boot_disk._potential_boot_partition()
+                p.grub_device = False
-                if part.fs() is None:
+                if self.model.bootloader == Bootloader.PREP:
-                    self.model.add_filesystem(part, 'fat32')
+                    p.wipe = None
-                self.model.add_mount(part.fs(), '/boot/efi')
+                elif self.model.bootloader == Bootloader.UEFI:
-            elif self.model.bootloader == Bootloader.PREP:
+                    if p.fs():
-                part = new_boot_disk._potential_boot_partition()
+                        if p.fs().mount():
-                part.wipe = 'zero'
+                            self.delete_mount(p.fs().mount())
-                self.model.grub_install_device = part
+                            remount = True
+                        if not p.fs().preserve and p.original_fstype():
+                            self.delete_filesystem(p.fs())
+                            self.model.add_filesystem(
+                                p, p.original_fstype(), preserve=True)
+        else:
+            full = boot_disk.free_for_partitions == 0
+            tot_size = 0
+            for p in partitions:
+                tot_size += p.size
+                if p.fs() and p.fs().mount():
+                    remount = True
+                self.delete_partition(p)
+            if full:
+                largest_part = max(
+                    boot_disk.partitions(), key=lambda p: p.size)
+                largest_part.size += tot_size
+        if self.model.bootloader == Bootloader.UEFI and remount:
+            part = self.model._one(type='partition', grub_device=True)
+            if part:
+                self._mount_esp(part)
+
+    def add_boot_disk(self, new_boot_disk):
+        bootloader = self.model.bootloader
+        if bootloader == Bootloader.PREP:
+            for disk in self.model.all_disks():
+                if disk._is_boot_device():
+                    self.remove_boot_disk(disk)
+        if new_boot_disk._has_preexisting_partition():
+            if bootloader == Bootloader.BIOS:
+                new_boot_disk.grub_device = True
+            elif bootloader == Bootloader.UEFI:
+                should_mount = self.model._mount_for_path('/boot/efi') is None
+                for p in new_boot_disk.partitions():
+                    if p.flag == 'boot':
+                        p.grub_device = True
+                        if should_mount:
+                            self._mount_esp(p)
+                            should_mount = False
+            elif bootloader == Bootloader.PREP:
+                for p in new_boot_disk.partitions():
+                    if p.flag == 'prep':
+                        p.wipe = 'zero'
+                        p.grub_device = True
         else:
             new_boot_disk.preserve = False
+            if bootloader == Bootloader.UEFI:
+                part_size = UEFI_GRUB_SIZE_BYTES
+                if UEFI_GRUB_SIZE_BYTES*2 >= new_boot_disk.size:
+                    part_size = new_boot_disk.size // 2
+            elif bootloader == Bootloader.PREP:
+                part_size = PREP_GRUB_SIZE_BYTES
+            elif bootloader == Bootloader.BIOS:
+                part_size = BIOS_GRUB_SIZE_BYTES
+            if part_size > new_boot_disk.free_for_partitions:
+                largest_part = max(
+                    new_boot_disk.partitions(), key=lambda p: p.size)
+                largest_part.size -= (
+                    part_size - new_boot_disk.free_for_partitions)
             self._create_boot_partition(new_boot_disk)
 
     def guided_direct(self, disk):
@@ -694,8 +727,8 @@ class FilesystemController(SubiquityController):
 
     def guided_lvm(self, disk, lvm_options=None):
         self.reformat(disk)
-        if DeviceAction.MAKE_BOOT in disk.supported_actions:
+        if DeviceAction.TOGGLE_BOOT in disk.supported_actions:
-            self.make_boot_disk(disk)
+            self.add_boot_disk(disk)
         self.create_partition(
             device=disk, spec=dict(
                 size=dehumanize_size('1G'),
@@ -730,8 +763,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

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,
     )
@@ -77,40 +76,48 @@ class TestFilesystemController(unittest.TestCase):
             a for a in controller.model._actions if a.type == 'dm_crypt']
         self.assertEqual(dm_crypts, [])
 
-    def test_can_only_make_boot_once(self):
+    def test_can_only_add_boot_once(self):
         # This is really testing model code but it's much easier to test with a
         # controller around.
         for bl in Bootloader:
             controller, disk = make_controller_and_disk(bl)
-            if DeviceAction.MAKE_BOOT not in disk.supported_actions:
+            if DeviceAction.TOGGLE_BOOT not in disk.supported_actions:
                 continue
-            controller.make_boot_disk(disk)
+            controller.add_boot_disk(disk)
             self.assertFalse(
-                disk._can_MAKE_BOOT,
+                disk._can_TOGGLE_BOOT,
-                "make_boot_disk(disk) did not make _can_MAKE_BOOT false with "
+                "add_boot_disk(disk) did not make _can_TOGGLE_BOOT false "
-                "bootloader {}".format(bl))
+                "with bootloader {}".format(bl))
 
     def test_make_boot_disk_BIOS(self):
         controller = make_controller(Bootloader.BIOS)
         disk1 = make_disk(controller.model, preserve=False)
+
+        controller.add_boot_disk(disk1)
+        self.assertEqual(len(disk1.partitions()), 1)
+        self.assertEqual(disk1.partitions()[0].flag, "bios_grub")
+        self.assertTrue(disk1.grub_device)
+
+        controller.remove_boot_disk(disk1)
+        self.assertEqual(len(disk1.partitions()), 0)
+        self.assertFalse(disk1.grub_device)
+
         disk2 = make_disk(controller.model, preserve=False)
         disk2p1 = controller.model.add_partition(
             disk2, size=disk2.free_for_partitions)
-
-        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)
-
         size_before = disk2p1.size
-        controller.make_boot_disk(disk2)
+        controller.add_boot_disk(disk2)
-        self.assertEqual(len(disk1.partitions()), 0)
         self.assertEqual(len(disk2.partitions()), 2)
         self.assertEqual(disk2.partitions()[1], disk2p1)
         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.assertTrue(disk2.grub_device)
+
+        controller.remove_boot_disk(disk2)
+        self.assertEqual(disk2.partitions(), [disk2p1])
+        self.assertEqual(disk2p1.size, size_before)
+        self.assertFalse(disk2.grub_device)
 
     def test_make_boot_disk_BIOS_existing(self):
         controller = make_controller(Bootloader.BIOS)
@@ -118,17 +125,24 @@ class TestFilesystemController(unittest.TestCase):
         disk1p1 = controller.model.add_partition(
             disk1, size=1 << 20, flag="bios_grub")
         disk1p1.preserve = True
-        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)
+        controller.add_boot_disk(disk1)
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, disk1)
+        self.assertTrue(disk1.grub_device)
+        controller.remove_boot_disk(disk1)
+        self.assertEqual(disk1.partitions(), [disk1p1])
+        self.assertFalse(disk1.grub_device)
 
-        controller.make_boot_disk(disk2)
+    def assertIsMountedAtBootEFI(self, device):
-        self.assertEqual(disk1.partitions(), [disk1p1])
+        efi_mnts = device._m._all(type="mount", path="/boot/efi")
-        self.assertEqual(controller.model.grub_install_device, disk2)
+        self.assertEqual(len(efi_mnts), 1)
+        self.assertEqual(efi_mnts[0].device.volume, device)
+
+    def assertNotMounted(self, device):
+        if device.fs():
+            self.assertIs(device.fs().mount(), None)
 
     def test_make_boot_disk_UEFI(self):
         controller = make_controller(Bootloader.UEFI)
@@ -137,25 +151,32 @@ class TestFilesystemController(unittest.TestCase):
         disk2p1 = controller.model.add_partition(
             disk2, size=disk2.free_for_partitions)
 
-        controller.make_boot_disk(disk1)
+        controller.add_boot_disk(disk1)
         self.assertEqual(len(disk1.partitions()), 1)
-        self.assertEqual(disk1.partitions()[0].flag, "boot")
+        disk1esp = disk1.partitions()[0]
-        self.assertEqual(controller.model.grub_install_device, None)
+        self.assertEqual(disk1esp.flag, "boot")
-        efi_mnt = controller.model._mount_for_path("/boot/efi")
+        self.assertEqual(disk1esp.fs().fstype, "fat32")
-        self.assertEqual(efi_mnt.device.volume, disk1.partitions()[0])
+        self.assertTrue(disk1esp.grub_device)
-        self.assertEqual(disk1.partitions()[0].fs().fstype, "fat32")
+        self.assertIsMountedAtBootEFI(disk1esp)
 
         size_before = disk2p1.size
-        controller.make_boot_disk(disk2)
+        controller.add_boot_disk(disk2)
-        self.assertEqual(len(disk1.partitions()), 0)
         self.assertEqual(len(disk2.partitions()), 2)
         self.assertEqual(disk2.partitions()[1], disk2p1)
-        self.assertEqual(
+        disk2esp = disk2.partitions()[0]
-            disk2.partitions()[0].size + disk2p1.size, size_before)
+        self.assertEqual(disk2esp.size + disk2p1.size, size_before)
-        self.assertEqual(disk2.partitions()[0].flag, "boot")
+        self.assertEqual(disk2esp.flag, "boot")
-        self.assertEqual(controller.model.grub_install_device, None)
+        self.assertTrue(disk2esp.grub_device)
-        efi_mnt = controller.model._mount_for_path("/boot/efi")
+        self.assertIsMountedAtBootEFI(disk1esp)
-        self.assertEqual(efi_mnt.device.volume, disk2.partitions()[0])
+        self.assertNotMounted(disk2esp)
+
+        controller.remove_boot_disk(disk1)
+        self.assertIsMountedAtBootEFI(disk2esp)
+        self.assertEqual(len(disk1.partitions()), 0)
+
+        controller.remove_boot_disk(disk2)
+        self.assertEqual(len(disk2.partitions()), 1)
+        self.assertEqual(disk2p1.size, size_before)
 
     def test_make_boot_disk_UEFI_existing(self):
         controller = make_controller(Bootloader.UEFI)
@@ -166,21 +187,18 @@ class TestFilesystemController(unittest.TestCase):
         disk2 = make_disk(controller.model, preserve=True)
 
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, None)
         efi_mnt = controller.model._mount_for_path("/boot/efi")
         self.assertEqual(efi_mnt, None)
-        controller.make_boot_disk(disk1)
+        controller.add_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.assertIsMountedAtBootEFI(disk1p1)
-        self.assertEqual(efi_mnt.device.volume, disk1p1)
         self.assertEqual(disk1p1.fs().fstype, "fat32")
 
-        controller.make_boot_disk(disk2)
+        controller.add_boot_disk(disk2)
         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.assertIsMountedAtBootEFI(disk1p1)
-        self.assertEqual(efi_mnt.device.volume, disk2.partitions()[0])
 
     def test_make_boot_disk_PREP(self):
         controller = make_controller(Bootloader.PREP)
@@ -189,16 +207,14 @@ class TestFilesystemController(unittest.TestCase):
         disk2p1 = controller.model.add_partition(
             disk2, size=disk2.free_for_partitions)
 
-        controller.make_boot_disk(disk1)
+        controller.add_boot_disk(disk1)
         self.assertEqual(len(disk1.partitions()), 1)
         self.assertEqual(disk1.partitions()[0].flag, "prep")
         self.assertEqual(disk1.partitions()[0].wipe, "zero")
-        self.assertEqual(
+        self.assertTrue(disk1.partitions()[0].grub_device)
-            controller.model.grub_install_device,
-            disk1.partitions()[0])
 
         size_before = disk2p1.size
-        controller.make_boot_disk(disk2)
+        controller.add_boot_disk(disk2)
         self.assertEqual(len(disk1.partitions()), 0)
         self.assertEqual(len(disk2.partitions()), 2)
         self.assertEqual(disk2.partitions()[1], disk2p1)
@@ -206,9 +222,6 @@ 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])
 
     def test_make_boot_disk_PREP_existing(self):
         controller = make_controller(Bootloader.PREP)
@@ -216,24 +229,18 @@ class TestFilesystemController(unittest.TestCase):
         disk1p1 = controller.model.add_partition(
             disk1, size=8 << 20, flag="prep")
         disk1p1.preserve = True
-        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)
+        controller.add_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)
+        controller.remove_boot_disk(disk1)
         self.assertEqual(disk1.partitions(), [disk1p1])
+        self.assertFalse(disk1p1.grub_device)
         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])
 
     def test_mounting_partition_makes_boot_disk(self):
         controller = make_controller(Bootloader.UEFI)
@@ -248,25 +255,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])

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
 
@@ -409,7 +408,7 @@ class DeviceAction(enum.Enum):
     FORMAT = _("Format")
     REMOVE = _("Remove from RAID/LVM")
     DELETE = _("Delete")
-    MAKE_BOOT = _("Make Boot Device")
+    TOGGLE_BOOT = _("Make Boot Device")
 
 
 def _generic_can_EDIT(obj):
@@ -516,7 +515,7 @@ class _Formattable(ABC):
                 m = fs.mount()
                 if m:
                     r.append(_("mounted at {path}").format(path=m.path))
-                else:
+                elif getattr(self, 'flag', None) != "boot":
                     r.append(_("not mounted"))
             elif fs.preserve:
                 if fs.mount() is None:
@@ -777,32 +776,22 @@ class Disk(_Device):
     def dasd(self):
         return self._m._one(type='dasd', device_id=self.device_id)
 
-    def _potential_boot_partition(self):
-        if self._m.bootloader == Bootloader.NONE:
-            return None
-        if not self._partitions:
-            return None
-        if self._m.bootloader == Bootloader.BIOS:
-            if self._partitions[0].flag == "bios_grub":
-                return self._partitions[0]
-            else:
-                return None
-        flag = {
-            Bootloader.UEFI: "boot",
-            Bootloader.PREP: "prep",
-            }[self._m.bootloader]
-        for p in self._partitions:
-            # XXX should check not extended in the UEFI case too (until we fix
-            # that bug)
-            if p.flag == flag:
-                return p
-        return None
-
     def _can_be_boot_disk(self):
-        if self._m.bootloader == Bootloader.BIOS and self.ptable == "msdos":
+        bl = self._m.bootloader
-            return True
+        if self._has_preexisting_partition():
+            if bl == Bootloader.BIOS:
+                if self.ptable == "msdos":
+                    return True
+                else:
+                    return self._partitions[0].flag == "bios_grub"
+            else:
+                flag = {Bootloader.UEFI: "boot", Bootloader.PREP: "prep"}[bl]
+                for p in self._partitions:
+                    if p.flag == flag:
+                        return True
+                return False
         else:
-            return self._potential_boot_partition() is not None
+            return True
 
     @property
     def supported_actions(self):
@@ -814,7 +803,7 @@ class Disk(_Device):
             DeviceAction.REMOVE,
             ]
         if self._m.bootloader != Bootloader.NONE:
-            actions.append(DeviceAction.MAKE_BOOT)
+            actions.append(DeviceAction.TOGGLE_BOOT)
         return actions
 
     _can_INFO = True
@@ -843,24 +832,29 @@ class Disk(_Device):
         self._constructed_device is None)
     _can_REMOVE = property(_generic_can_REMOVE)
 
-    @property
+    def _is_boot_device(self):
-    def _can_MAKE_BOOT(self):
         bl = self._m.bootloader
-        if bl == Bootloader.BIOS:
+        if bl == Bootloader.NONE:
-            if self._m.grub_install_device is self:
+            return False
-                return False
+        elif bl == Bootloader.BIOS:
-        elif bl == Bootloader.UEFI:
+            return self.grub_device
-            m = self._m._mount_for_path('/boot/efi')
+        elif bl in [Bootloader.PREP, Bootloader.UEFI]:
-            if m and m.device.volume.device is self:
+            for p in self._partitions:
-                return False
+                if p.grub_device:
-        elif bl == Bootloader.PREP:
+                    return True
-            install_dev = self._m.grub_install_device
+            return False
-            if install_dev is not None and install_dev.device is self:
+
-                return False
+    @property
-        if self._has_preexisting_partition():
+    def _can_TOGGLE_BOOT(self):
-            return self._can_be_boot_disk()
+        if self._is_boot_device():
+            for disk in self._m.all_disks():
+                if disk is not self and disk._is_boot_device():
+                    return True
+            return False
+        elif self._fs is not None or self._constructed_device is not None:
+            return False
         else:
-            return self._fs is None and self._constructed_device is None
+            return self._can_be_boot_disk()
 
     @property
     def ok_for_raid(self):
@@ -886,15 +880,31 @@ 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):
         r = super().annotations
         if self.flag == "prep":
             r.append("PReP")
+            if self.preserve:
+                if self.grub_device:
+                    r.append("configured")
+                else:
+                    r.append("unconfigured")
         elif self.flag == "boot":
-            r.append("ESP")
+            if self.fs() and self.fs().mount():
+                r.append("primary ESP")
+            elif self.grub_device:
+                r.append("backup ESP")
+            else:
+                r.append("unused ESP")
         elif self.flag == "bios_grub":
+            if self.preserve:
+                if self.device.grub_device:
+                    r.append("configured")
+                else:
+                    r.append("unconfigured")
             r.append("bios_grub")
         elif self.flag == "extended":
             r.append("extended")
@@ -919,7 +929,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 +1288,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 +1555,8 @@ class FilesystemModel(object):
             }
         if self.swap is not None:
             config['swap'] = self.swap
-        if self.grub_install_device:
+        if self.grub is not None:
-            dev = self.grub_install_device
+            config['grub'] = self.grub
-            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],
-                }
         return config
 
     def load_probe_data(self, probe_data):
@@ -1608,12 +1610,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 +1622,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 +1727,16 @@ class FilesystemModel(object):
         # s390x has no such thing
         if self.bootloader == Bootloader.NONE:
             return False
-        elif self.bootloader in [Bootloader.BIOS, Bootloader.PREP]:
+        elif self.bootloader == Bootloader.BIOS:
-            return self.grub_install_device is None
+            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))

subiquity/models/tests/test_filesystem.py

@@ -216,17 +216,50 @@ class TestFilesystemModel(unittest.TestCase):
         self.assertEqual(disk.annotations, [])
 
     def test_partition_annotations(self):
-        model, disk = make_model_and_disk()
+        model = make_model()
-        part = model.add_partition(disk, size=disk.free_for_partitions)
+        part = make_partition(model)
         self.assertEqual(part.annotations, ['new'])
         part.preserve = True
         self.assertEqual(part.annotations, ['existing'])
-        part.flag = "boot"
+
-        self.assertEqual(part.annotations, ['existing', 'ESP'])
+        model = make_model()
-        part.flag = "prep"
+        part = make_partition(model, flag="bios_grub")
-        self.assertEqual(part.annotations, ['existing', 'PReP'])
+        self.assertEqual(
-        part.flag = "bios_grub"
+            part.annotations, ['new', 'bios_grub'])
-        self.assertEqual(part.annotations, ['existing', 'bios_grub'])
+        part.preserve = True
+        self.assertEqual(
+            part.annotations, ['existing', 'unconfigured', 'bios_grub'])
+        part.device.grub_device = True
+        self.assertEqual(
+            part.annotations, ['existing', 'configured', 'bios_grub'])
+
+        model = make_model()
+        part = make_partition(model, flag="boot", grub_device=True)
+        self.assertEqual(part.annotations, ['new', 'backup ESP'])
+        fs = model.add_filesystem(part, fstype="fat32")
+        model.add_mount(fs, "/boot/efi")
+        self.assertEqual(part.annotations, ['new', 'primary ESP'])
+
+        model = make_model()
+        part = make_partition(model, flag="boot", preserve=True)
+        self.assertEqual(part.annotations, ['existing', 'unused ESP'])
+        part.grub_device = True
+        self.assertEqual(part.annotations, ['existing', 'backup ESP'])
+        fs = model.add_filesystem(part, fstype="fat32")
+        model.add_mount(fs, "/boot/efi")
+        self.assertEqual(part.annotations, ['existing', 'primary ESP'])
+
+        model = make_model()
+        part = make_partition(model, flag="prep", grub_device=True)
+        self.assertEqual(part.annotations, ['new', 'PReP'])
+
+        model = make_model()
+        part = make_partition(model, flag="prep", preserve=True)
+        self.assertEqual(
+            part.annotations, ['existing', 'PReP', 'unconfigured'])
+        part.grub_device = True
+        self.assertEqual(
+            part.annotations, ['existing', 'PReP', 'configured'])
 
     def test_vg_default_annotations(self):
         model, disk = make_model_and_disk()
@@ -443,81 +476,81 @@ class TestFilesystemModel(unittest.TestCase):
         model, disk = make_model_and_disk()
         self.assertActionNotSupported(disk, DeviceAction.DELETE)
 
-    def test_disk_action_MAKE_BOOT_NONE(self):
+    def test_disk_action_TOGGLE_BOOT_NONE(self):
         model, disk = make_model_and_disk(Bootloader.NONE)
-        self.assertActionNotSupported(disk, DeviceAction.MAKE_BOOT)
+        self.assertActionNotSupported(disk, DeviceAction.TOGGLE_BOOT)
 
-    def test_disk_action_MAKE_BOOT_BIOS(self):
+    def test_disk_action_TOGGLE_BOOT_BIOS(self):
         model = make_model(Bootloader.BIOS)
         # Disks with msdos partition tables can always be the BIOS boot disk.
         dos_disk = make_disk(model, ptable='msdos', preserve=True)
-        self.assertActionPossible(dos_disk, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(dos_disk, DeviceAction.TOGGLE_BOOT)
         # Even if they have existing partitions
         make_partition(
             model, dos_disk, size=dos_disk.free_for_partitions, preserve=True)
-        self.assertActionPossible(dos_disk, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(dos_disk, DeviceAction.TOGGLE_BOOT)
         # (we never create dos partition tables so no need to test
         # preserve=False case).
 
         # GPT disks with new partition tables can always be the BIOS boot disk
         gpt_disk = make_disk(model, ptable='gpt', preserve=False)
-        self.assertActionPossible(gpt_disk, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(gpt_disk, DeviceAction.TOGGLE_BOOT)
         # Even if they are filled with partitions (we resize partitions to fit)
         make_partition(model, gpt_disk, size=dos_disk.free_for_partitions)
-        self.assertActionPossible(gpt_disk, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(gpt_disk, DeviceAction.TOGGLE_BOOT)
 
         # GPT disks with existing partition tables but no partitions can be the
         # BIOS boot disk (in general we ignore existing empty partition tables)
         gpt_disk2 = make_disk(model, ptable='gpt', preserve=True)
-        self.assertActionPossible(gpt_disk2, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(gpt_disk2, DeviceAction.TOGGLE_BOOT)
         # If there is an existing *partition* though, it cannot be the boot
         # disk
         make_partition(model, gpt_disk2, preserve=True)
-        self.assertActionNotPossible(gpt_disk2, DeviceAction.MAKE_BOOT)
+        self.assertActionNotPossible(gpt_disk2, DeviceAction.TOGGLE_BOOT)
         # Unless there is already a bios_grub partition we can reuse
         gpt_disk3 = make_disk(model, ptable='gpt', preserve=True)
         make_partition(
             model, gpt_disk3, flag="bios_grub", preserve=True)
         make_partition(
             model, gpt_disk3, preserve=True)
-        self.assertActionPossible(gpt_disk3, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(gpt_disk3, DeviceAction.TOGGLE_BOOT)
         # Edge case city: the bios_grub partition has to be first
         gpt_disk4 = make_disk(model, ptable='gpt', preserve=True)
         make_partition(
             model, gpt_disk4, preserve=True)
         make_partition(
             model, gpt_disk4, flag="bios_grub", preserve=True)
-        self.assertActionNotPossible(gpt_disk4, DeviceAction.MAKE_BOOT)
+        self.assertActionNotPossible(gpt_disk4, DeviceAction.TOGGLE_BOOT)
 
-    def _test_MAKE_BOOT_boot_partition(self, bl, flag):
+    def _test_TOGGLE_BOOT_boot_partition(self, bl, flag):
-        # The logic for when MAKE_BOOT is enabled for both UEFI and PREP
+        # The logic for when TOGGLE_BOOT is enabled for both UEFI and PREP
         # bootloaders turns out to be the same, modulo the special flag that
         # has to be present on a partition.
         model = make_model(bl)
         # A disk with a new partition table can always be the UEFI/PREP boot
         # disk.
         new_disk = make_disk(model, preserve=False)
-        self.assertActionPossible(new_disk, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(new_disk, DeviceAction.TOGGLE_BOOT)
         # Even if they are filled with partitions (we resize partitions to fit)
         make_partition(model, new_disk, size=new_disk.free_for_partitions)
-        self.assertActionPossible(new_disk, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(new_disk, DeviceAction.TOGGLE_BOOT)
 
         # A disk with an existing but empty partitions can also be the
         # UEFI/PREP boot disk.
         old_disk = make_disk(model, preserve=True)
-        self.assertActionPossible(old_disk, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(old_disk, DeviceAction.TOGGLE_BOOT)
         # If there is an existing partition though, it cannot.
         make_partition(model, old_disk, preserve=True)
-        self.assertActionNotPossible(old_disk, DeviceAction.MAKE_BOOT)
+        self.assertActionNotPossible(old_disk, DeviceAction.TOGGLE_BOOT)
         # If there is an existing ESP/PReP partition though, fine!
         make_partition(model, old_disk, flag=flag, preserve=True)
-        self.assertActionPossible(old_disk, DeviceAction.MAKE_BOOT)
+        self.assertActionPossible(old_disk, DeviceAction.TOGGLE_BOOT)
 
-    def test_disk_action_MAKE_BOOT_UEFI(self):
+    def test_disk_action_TOGGLE_BOOT_UEFI(self):
-        self._test_MAKE_BOOT_boot_partition(Bootloader.UEFI, "boot")
+        self._test_TOGGLE_BOOT_boot_partition(Bootloader.UEFI, "boot")
 
-    def test_disk_action_MAKE_BOOT_PREP(self):
+    def test_disk_action_TOGGLE_BOOT_PREP(self):
-        self._test_MAKE_BOOT_boot_partition(Bootloader.PREP, "prep")
+        self._test_TOGGLE_BOOT_boot_partition(Bootloader.PREP, "prep")
 
     def test_partition_action_INFO(self):
         model, part = make_model_and_partition()
@@ -578,9 +611,9 @@ class TestFilesystemModel(unittest.TestCase):
         disk2p1 = make_partition(model, disk2, preserve=True)
         self.assertActionNotPossible(disk2p1, DeviceAction.DELETE)
 
-    def test_partition_action_MAKE_BOOT(self):
+    def test_partition_action_TOGGLE_BOOT(self):
         model, part = make_model_and_partition()
-        self.assertActionNotSupported(part, DeviceAction.MAKE_BOOT)
+        self.assertActionNotSupported(part, DeviceAction.TOGGLE_BOOT)
 
     def test_raid_action_INFO(self):
         model, raid = make_model_and_raid()
@@ -684,9 +717,9 @@ class TestFilesystemModel(unittest.TestCase):
         model.add_volgroup('vg0', {raid2})
         self.assertActionNotPossible(raid2, DeviceAction.DELETE)
 
-    def test_raid_action_MAKE_BOOT(self):
+    def test_raid_action_TOGGLE_BOOT(self):
         model, raid = make_model_and_raid()
-        self.assertActionNotSupported(raid, DeviceAction.MAKE_BOOT)
+        self.assertActionNotSupported(raid, DeviceAction.TOGGLE_BOOT)
 
     def test_vg_action_INFO(self):
         model, vg = make_model_and_vg()
@@ -741,9 +774,9 @@ class TestFilesystemModel(unittest.TestCase):
         model.add_mount(fs, '/')
         self.assertActionNotPossible(vg, DeviceAction.DELETE)
 
-    def test_vg_action_MAKE_BOOT(self):
+    def test_vg_action_TOGGLE_BOOT(self):
         model, vg = make_model_and_vg()
-        self.assertActionNotSupported(vg, DeviceAction.MAKE_BOOT)
+        self.assertActionNotSupported(vg, DeviceAction.TOGGLE_BOOT)
 
     def test_lv_action_INFO(self):
         model, lv = make_model_and_lv()
@@ -785,9 +818,9 @@ class TestFilesystemModel(unittest.TestCase):
         lv2.preserve = lv2.volgroup.preserve = True
         self.assertActionNotPossible(lv2, DeviceAction.DELETE)
 
-    def test_lv_action_MAKE_BOOT(self):
+    def test_lv_action_TOGGLE_BOOT(self):
         model, lv = make_model_and_lv()
-        self.assertActionNotSupported(lv, DeviceAction.MAKE_BOOT)
+        self.assertActionNotSupported(lv, DeviceAction.TOGGLE_BOOT)
 
 
 def fake_up_blockdata(model):

subiquity/ui/views/filesystem/filesystem.py

@@ -61,6 +61,7 @@ from subiquitycore.ui.utils import (
 from subiquitycore.view import BaseView
 
 from subiquity.models.filesystem import (
+    Bootloader,
     DeviceAction,
     humanize_size,
     )
@@ -282,8 +283,11 @@ class DeviceList(WidgetWrap):
         disk._constructed_device = None
         self.parent.refresh_model_inputs()
 
-    def _disk_MAKE_BOOT(self, disk):
+    def _disk_TOGGLE_BOOT(self, disk):
-        self.parent.controller.make_boot_disk(disk)
+        if disk._is_boot_device():
+            self.parent.controller.remove_boot_disk(disk)
+        else:
+            self.parent.controller.add_boot_disk(disk)
         self.parent.refresh_model_inputs()
 
     _partition_EDIT = _stretchy_shower(
@@ -311,16 +315,33 @@ class DeviceList(WidgetWrap):
         log.debug('_action %s %s', action, device.id)
         meth(device)
 
+    def _label_REMOVE(self, action, device):
+        cd = device.constructed_device()
+        if cd:
+            return _("Remove from {}").format(cd.desc())
+        else:
+            return _(action.value)
+
+    def _label_PARTITION(self, action, device):
+        return _("Add {} Partition").format(
+            device.ptable_for_new_partition().upper())
+
+    def _label_TOGGLE_BOOT(self, action, device):
+        if device._is_boot_device():
+            return _("Stop Using As Boot Device")
+        else:
+            if self.parent.model.bootloader != Bootloader.PREP:
+                for other in self.parent.model.all_disks():
+                    if other._is_boot_device():
+                        return _("Add As Another Boot Device")
+            return _("Use As Boot Device")
+
     def _action_menu_for_device(self, device):
         device_actions = []
         for action in device.supported_actions:
-            label = _(action.value)
+            label_meth = getattr(
-            if action == DeviceAction.REMOVE and device.constructed_device():
+                self, '_label_{}'.format(action.name), lambda a, d: _(a.value))
-                cd = device.constructed_device()
+            label = label_meth(action, device)
-                label = _("Remove from {}").format(cd.desc())
-            if action == DeviceAction.PARTITION:
-                label = _("Add {} Partition").format(
-                    device.ptable_for_new_partition().upper())
             enabled, whynot = device.action_possible(action)
             if whynot:
                 assert not enabled

subiquity/ui/views/filesystem/partition.py

@@ -35,6 +35,7 @@ from subiquitycore.ui.interactive import StringEditor
 from subiquitycore.ui.selector import Option, Selector
 from subiquitycore.ui.container import Pile
 from subiquitycore.ui.stretchy import Stretchy
+from subiquitycore.ui.utils import rewrap
 
 from subiquity.models.filesystem import (
     align_up,
@@ -270,34 +271,65 @@ class PartitionForm(Form):
         return r
 
 
-bios_grub_partition_description = _(
+bios_grub_partition_description = _("""\
-    "Required bootloader partition\n"
+Bootloader partition
-    "\n"
-    "GRUB will be installed onto the target disk's MBR.\n"
-    "\n"
-    "However, on a disk with a GPT partition table, there is not enough space "
-    "after the MBR for GRUB to store its second-stage core.img, so a small "
-    "unformatted partition is needed at the start of the disk. It will not "
-    "contain a filesystem and will not be mounted, and cannot be edited here.")
 
-boot_partition_description = _(
+{middle}
-    "Required bootloader partition\n"
-    "\n"
-    'This is the ESP / "EFI system partition" required by UEFI. Grub will be '
-    'installed onto this partition, which must be formatted as fat32.')
 
-boot_partition_description_size = _(
+However, on a disk with a GPT partition table, there is not enough
-    ' The only aspect of this partition that can be edited is the size.')
+space after the MBR for GRUB to store its second-stage core.img, so a
+small unformatted partition is needed at the start of the disk. It
+will not contain a filesystem and will not be mounted, and cannot be
+edited here.
+""")
 
-boot_partition_description_reformat = _(
+unconfigured_bios_grub_partition_middle = _("""\
-    ' You can choose whether to use the existing filesystem on this '
+If this disk is selected as a boot device, GRUB will be installed onto
-    'partition or reformat it.')
+the target disk's MBR.""")
 
-prep_partition_description = _(
+configured_bios_grub_partition_middle = _("""\
-    "Required bootloader partition\n"
+As this disk has been selected as a boot device, GRUB will be
-    "\n"
+installed onto the target disk's MBR.""")
-    'This is the PReP partion which is required on POWER. Grub will be '
+
-    'installed onto this partition.')
+unconfigured_boot_partition_description = _("""\
+Bootloader partition
+
+This is an ESP / "EFI system partition" as required by UEFI. If this
+disk is selected as a boot device, Grub will be installed onto this
+partition, which must be formatted as fat32.
+""")
+
+configured_boot_partition_description = _("""\
+Bootloader partition
+
+This is an ESP / "EFI system partition" as required by UEFI. As this
+disk has been selected as a boot device, Grub will be installed onto
+this partition, which must be formatted as fat32.
+""")
+
+boot_partition_description_size = _("""\
+The only aspect of this partition that can be edited is the size.
+""")
+
+boot_partition_description_reformat = _("""\
+You can choose whether to use the existing filesystem on this
+partition or reformat it.
+""")
+
+unconfigured_prep_partition_description = _("""\
+Required bootloader partition
+
+This is the PReP partion which is required on POWER. If this disk is
+selected as a boot device, Grub will be installed onto this partition.
+""")
+
+configured_prep_partition_description = _("""\
+Required bootloader partition
+
+This is the PReP partion which is required on POWER. As this disk has
+been selected as a boot device, Grub will be installed onto this
+partition.
+""")
 
 
 def initial_data_for_fs(fs):
@@ -335,9 +367,11 @@ class PartitionStretchy(Stretchy):
         else:
             lvm_names = None
         if self.partition:
-            if self.partition.flag in ["bios_grub", "prep"]:
+            if partition.flag in ["bios_grub", "prep"]:
                 label = None
                 initial['mount'] = None
+            elif partition.flag == "boot" and not partition.grub_device:
+                label = None
             else:
                 label = _("Save")
             initial['size'] = humanize_size(self.partition.size)
@@ -397,6 +431,8 @@ class PartitionStretchy(Stretchy):
                         self.form.fstype.widget.index = 0
                     else:
                         self.form.fstype.widget.index = 2
+                    if not self.partition.grub_device:
+                        self.form.fstype.enabled = False
                     self.form.mount.enabled = False
                 else:
                     opts = [Option(("fat32", True))]
@@ -419,24 +455,37 @@ class PartitionStretchy(Stretchy):
         focus_index = 0
         if partition is not None:
             if self.partition.flag == "boot":
-                desc = boot_partition_description
+                if self.partition.grub_device:
-                if self.partition.preserve:
+                    desc = _(configured_boot_partition_description)
-                    desc += boot_partition_description_reformat
+                    if self.partition.preserve:
+                        desc += _(boot_partition_description_reformat)
+                    else:
+                        desc += _(boot_partition_description_size)
                 else:
-                    desc += boot_partition_description_size
+                    focus_index = 2
+                    desc = _(unconfigured_boot_partition_description)
                 rows.extend([
-                    Text(_(desc)),
+                    Text(rewrap(desc)),
                     Text(""),
                 ])
             elif self.partition.flag == "bios_grub":
+                if self.partition.device.grub_device:
+                    middle = _(configured_bios_grub_partition_middle)
+                else:
+                    middle = _(unconfigured_bios_grub_partition_middle)
+                desc = _(bios_grub_partition_description).format(middle=middle)
                 rows.extend([
-                    Text(_(bios_grub_partition_description)),
+                    Text(rewrap(desc)),
                     Text(""),
                 ])
                 focus_index = 2
             elif self.partition.flag == "prep":
+                if self.partition.grub_device:
+                    desc = _(configured_prep_partition_description)
+                else:
+                    desc = _(unconfigured_prep_partition_description)
                 rows.extend([
-                    Text(_(prep_partition_description)),
+                    Text(rewrap(desc)),
                     Text(""),
                 ])
                 focus_index = 2

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):

subiquity/ui/views/filesystem/tests/test_partition.py

@@ -196,14 +196,13 @@ class PartitionViewTests(unittest.TestCase):
         disk.preserve = partition.preserve = fs.preserve = True
         view, stretchy = make_partition_view(model, disk, partition)
 
-        self.assertTrue(stretchy.form.fstype.enabled)
+        self.assertFalse(stretchy.form.fstype.enabled)
         self.assertEqual(stretchy.form.fstype.value, None)
         self.assertFalse(stretchy.form.mount.enabled)
         self.assertEqual(stretchy.form.mount.value, None)
 
         view_helpers.click(stretchy.form.done_btn.base_widget)
         expected_data = {
-            'fstype': None,
             'mount': None,
             'use_swap': False,
             }
@@ -215,6 +214,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)