make get_boot_device_plan_bios v2 compatible

subiquity/common/filesystem/boot.py

@@ -156,11 +156,9 @@ def get_boot_device_plan_bios(device) -> Optional[MakeBootDevicePlan]:
     attr_plan = SetAttrPlan(device, 'grub_device', True)
     if device.ptable == 'msdos':
         return attr_plan
-    if device._has_preexisting_partition():
+    pgs = gaps.parts_and_gaps(device)
-        if device._partitions[0].flag == "bios_grub":
+    if isinstance(pgs[0], Partition) and pgs[0].flag == "bios_grub":
-            return attr_plan
+        return attr_plan
-        else:
-            return None
 
     create_part_plan = CreatePartPlan(
         device=device,
@@ -168,30 +166,41 @@ def get_boot_device_plan_bios(device) -> Optional[MakeBootDevicePlan]:
         spec=dict(size=sizes.BIOS_GRUB_SIZE_BYTES, fstype=None, mount=None),
         args=dict(flag='bios_grub'))
 
-    partitions = device.partitions()
+    movable = []
 
-    if gaps.largest_gap_size(device) >= sizes.BIOS_GRUB_SIZE_BYTES:
+    for pg in pgs:
-        return MultiStepPlan(plans=[
+        if isinstance(pg, gaps.Gap):
-            SlidePlan(
+            if pg.size >= sizes.BIOS_GRUB_SIZE_BYTES:
-                parts=partitions,
+                return MultiStepPlan(plans=[
-                offset_delta=sizes.BIOS_GRUB_SIZE_BYTES),
+                    SlidePlan(
-            create_part_plan,
+                        parts=movable,
-            attr_plan,
+                        offset_delta=sizes.BIOS_GRUB_SIZE_BYTES),
-            ])
+                    create_part_plan,
-    else:
+                    attr_plan,
-        largest_i, largest_part = max(
+                    ])
-            enumerate(partitions),
+            else:
-            key=lambda i_p: i_p[1].size)
+                return None
-        return MultiStepPlan(plans=[
+        elif pg.preserve:
-            SlidePlan(
+            break
-                parts=partitions[:largest_i+1],
+        else:
-                offset_delta=sizes.BIOS_GRUB_SIZE_BYTES),
+            movable.append(pg)
-            ResizePlan(
+
-                part=largest_part,
+    if not movable:
-                size_delta=-sizes.BIOS_GRUB_SIZE_BYTES),
+        return None
-            create_part_plan,
+
-            attr_plan,
+    largest_i, largest_part = max(
-            ])
+        enumerate(movable),
+        key=lambda i_p: i_p[1].size)
+    return MultiStepPlan(plans=[
+        SlidePlan(
+            parts=movable[:largest_i+1],
+            offset_delta=sizes.BIOS_GRUB_SIZE_BYTES),
+        ResizePlan(
+            part=largest_part,
+            size_delta=-sizes.BIOS_GRUB_SIZE_BYTES),
+        create_part_plan,
+        attr_plan,
+        ])
 
 
 def get_add_part_plan(device, *, spec, args):

subiquity/common/filesystem/tests/test_manipulator.py

@@ -18,10 +18,12 @@ import unittest
 
 import attr
 
+import pytest
+
 from subiquity.common.filesystem.actions import (
     DeviceAction,
     )
-from subiquity.common.filesystem import gaps, sizes
+from subiquity.common.filesystem import boot, gaps, sizes
 from subiquity.common.filesystem.manipulator import FilesystemManipulator
 from subiquity.models.tests.test_filesystem import (
     make_disk,
@@ -35,9 +37,10 @@ from subiquity.models.filesystem import (
     )
 
 
-def make_manipulator(bootloader=None):
+def make_manipulator(bootloader=None, storage_version=1):
     manipulator = FilesystemManipulator()
     manipulator.model = make_model(bootloader)
+    manipulator.model.storage_version = storage_version
     manipulator.supports_resilient_boot = True
     return manipulator
 
@@ -125,7 +128,8 @@ class TestFilesystemManipulator(unittest.TestCase):
     def assertIsBootDisk(self, manipulator, disk):
         if manipulator.model.bootloader == Bootloader.BIOS:
             self.assertTrue(disk.grub_device)
-            self.assertEqual(disk.partitions()[0].flag, "bios_grub")
+            if disk.ptable == 'gpt':
+                self.assertEqual(disk.partitions()[0].flag, "bios_grub")
         elif manipulator.model.bootloader == Bootloader.UEFI:
             for part in disk.partitions():
                 if part.flag == "boot" and part.grub_device:
@@ -379,12 +383,13 @@ class TestFilesystemManipulator(unittest.TestCase):
             manipulator.add_boot_disk(disk)
         self.assertIsBootDisk(manipulator, disk)
 
-    def DONT_test_add_boot_BIOS_preserved(self):  # needs v2 partitioning
+    def test_add_boot_BIOS_preserved(self):
-        manipulator = make_manipulator(Bootloader.BIOS)
+        manipulator = make_manipulator(Bootloader.BIOS, 2)
         disk = make_disk(manipulator.model, preserve=True)
         half_size = gaps.largest_gap_size(disk)//2
         part = make_partition(
-            manipulator.model, disk, size=half_size, offset=half_size)
+            manipulator.model, disk, size=half_size, offset=half_size,
+            preserve=True)
         with self.assertPartitionOperations(
                 disk,
                 Create(
@@ -393,6 +398,59 @@ class TestFilesystemManipulator(unittest.TestCase):
                 Unchanged(part=part),
                 ):
             manipulator.add_boot_disk(disk)
+        self.assertIsBootDisk(manipulator, disk)
+
+    def test_add_boot_BIOS_new_and_preserved(self):
+        manipulator = make_manipulator(Bootloader.BIOS, 2)
+        # 2002MiB so that the space available for partitioning (2000MiB)
+        # divided by 4 is an whole number of megabytes.
+        disk = make_disk(manipulator.model, preserve=True, size=2002*MiB)
+        avail = gaps.largest_gap_size(disk)
+        p1_new = make_partition(
+            manipulator.model, disk, size=avail//4)
+        p2_preserved = make_partition(
+            manipulator.model, disk, size=3*avail//4, preserve=True)
+        with self.assertPartitionOperations(
+                disk,
+                Create(
+                    offset=disk.alignment_data().min_start_offset,
+                    size=sizes.BIOS_GRUB_SIZE_BYTES),
+                MoveResize(
+                    part=p1_new,
+                    offset=sizes.BIOS_GRUB_SIZE_BYTES,
+                    size=-sizes.BIOS_GRUB_SIZE_BYTES),
+                Unchanged(part=p2_preserved),
+                ):
+            manipulator.add_boot_disk(disk)
+        self.assertIsBootDisk(manipulator, disk)
+
+    def _test_no_add_boot_BIOS_preserved_at_start(self, version):
+        manipulator = make_manipulator(Bootloader.BIOS, version)
+        disk = make_disk(manipulator.model, preserve=True)
+        half_size = gaps.largest_gap_size(disk)//2
+        make_partition(
+            manipulator.model, disk, size=half_size,
+            preserve=True)
+        self.assertFalse(boot.can_be_boot_device(disk))
+
+    def test_no_add_boot_BIOS_preserved_at_start_v1(self):
+        self._test_no_add_boot_BIOS_preserved_at_start(1)
+
+    def test_no_add_boot_BIOS_preserved_at_start_v2(self):
+        self._test_no_add_boot_BIOS_preserved_at_start(2)
+
+    def test_add_boot_BIOS_msdos(self):
+        manipulator = make_manipulator(Bootloader.BIOS)
+        disk = make_disk(manipulator.model, preserve=True, ptable='msdos')
+        part = make_partition(
+            manipulator.model, disk, size=gaps.largest_gap_size(disk),
+            preserve=True)
+        with self.assertPartitionOperations(
+                disk,
+                Unchanged(part=part),
+                ):
+            manipulator.add_boot_disk(disk)
+        self.assertIsBootDisk(manipulator, disk)
 
     def _test_add_boot_empty(self, manipulator, disk, size):
         with self.assertPartitionOperations(