Merge pull request #1765 from mwhudson/gap-min-size

Use the first fitting gap for ESP, instead of the largest gap

subiquity/common/filesystem/boot.py

@@ -224,8 +224,8 @@ def get_add_part_plan(device, *, spec, args, resize_partition=None):
     # is a bad idea.  So avoid putting any sort of boot stuff on a logical -
     # it's probably a bad idea for all cases.
 
-    gap = gaps.largest_gap(device, in_extended=False)
-    if gap is not None and gap.size >= size and gap.is_usable:
+    gap = gaps.first_gap_with_size(device, size, in_extended=False)
+    if gap is not None:
         create_part_plan.gap = gap.split(size)[0]
         return create_part_plan
     elif resize_partition is not None and not resize_partition.is_logical:

subiquity/common/filesystem/gaps.py

@@ -252,6 +252,14 @@ def largest_gap_size(device, in_extended=None):
     return 0
 
 
+def first_gap_with_size(device, size, *, in_extended=None):
+    for pg in parts_and_gaps(device):
+        if isinstance(pg, Gap) and pg.size >= size and pg.is_usable:
+            if in_extended is None or in_extended == pg.in_extended:
+                return pg
+    return None
+
+
 @functools.singledispatch
 def movable_trailing_partitions_and_gap_size(partition):
     """For a given partition (or LVM logical volume), return the total,

subiquity/common/filesystem/tests/test_gaps.py

@@ -783,3 +783,77 @@ class TestUsable(unittest.TestCase):
         self.assertEqual(
             "TOO_MANY_PRIMARY_PARTS", GapUsable.TOO_MANY_PRIMARY_PARTS.name
         )
+
+
+class TestGapWithSize(GapTestCase):
+    def test_empty_disk(self):
+        d = make_disk(size=10 * MiB)
+        [g1] = gaps.parts_and_gaps(d)
+        self.assertEqual(g1, gaps.first_gap_with_size(d, MiB))
+
+    def test_half_full(self):
+        d = make_disk(size=10 * MiB)
+        make_partition(device=d, size=d.size // 2)
+        [p1, g1] = gaps.parts_and_gaps(d)
+        self.assertEqual(g1, gaps.first_gap_with_size(d, MiB))
+
+    def test_half_full_too_big(self):
+        d = make_disk(size=10 * MiB)
+        make_partition(device=d, size=d.size // 2)
+        [p1, g1] = gaps.parts_and_gaps(d)
+        self.assertIs(None, gaps.first_gap_with_size(d, 10 * MiB))
+
+    def test_one_gap_too_small(self):
+        self.use_alignment_data(
+            PartitionAlignmentData(
+                part_align=10,
+                min_gap_size=1,
+                min_start_offset=10,
+                min_end_offset=10,
+                primary_part_limit=10,
+            )
+        )
+        # 0----10---20---30---40---50---60---70---80---90---100
+        # #####     [ p1 ]                             #####
+        d = make_disk(size=100)
+        make_partition(device=d, size=10, offset=20)
+        [g1, p1, g2] = gaps.parts_and_gaps(d)
+        self.assertEqual(g2, gaps.first_gap_with_size(d, 20))
+
+    def test_unusable(self):
+        self.use_alignment_data(
+            PartitionAlignmentData(
+                part_align=10,
+                min_gap_size=1,
+                min_start_offset=10,
+                min_end_offset=10,
+                primary_part_limit=1,
+            )
+        )
+        # 0----10---20---30---40---50---60---70---80---90---100
+        # #####     [ p1 ]                             #####
+        d = make_disk(size=100)
+        make_partition(device=d, size=10, offset=2)
+        self.assertIs(None, gaps.first_gap_with_size(d, 10))
+
+    def test_in_extended(self):
+        self.use_alignment_data(
+            PartitionAlignmentData(
+                part_align=10,
+                min_gap_size=1,
+                min_start_offset=10,
+                min_end_offset=10,
+                primary_part_limit=10,
+                ebr_space=2,
+            )
+        )
+        # 0----10---20---30---40---50---60---70---80---90---100
+        # ##### g1  [ p1 (extended)          ] g3      #####
+        #            [ p5     ] g2
+        d = make_disk(size=100)
+        make_partition(device=d, size=50, offset=20, flag="extended")
+        make_partition(device=d, size=18, offset=22, flag="logical")
+        [g1, p1, p5, g2, g3] = gaps.parts_and_gaps(d)
+        self.assertEqual(g2, gaps.first_gap_with_size(d, 20))
+        self.assertEqual(g3, gaps.first_gap_with_size(d, 20, in_extended=False))
+        self.assertEqual(g2, gaps.first_gap_with_size(d, 10, in_extended=True))

subiquity/models/tests/test_filesystem.py

@@ -164,10 +164,13 @@ def make_model_and_disk(bootloader=None, **kw):
     return model, make_disk(model, **kw)
 
 
-def make_partition(model, device=None, *, preserve=False, size=None, offset=None, **kw):
+def make_partition(
+    model=None, device=None, *, preserve=False, size=None, offset=None, **kw
+):
     flag = kw.pop("flag", None)
     if device is None:
         device = make_disk(model)
+    model = device._m
     if size is None or offset is None:
         gap = gaps.largest_gap(device)
         if size is None: