From 2abdfb5a4880ad7d1f5156668187903670f26b54 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Thu, 17 Aug 2023 14:31:31 +1200 Subject: [PATCH 1/5] add gap_with_size function to find a gap of a given size --- subiquity/common/filesystem/gaps.py | 7 +++ .../common/filesystem/tests/test_gaps.py | 52 +++++++++++++++++++ subiquity/models/tests/test_filesystem.py | 5 +- 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/subiquity/common/filesystem/gaps.py b/subiquity/common/filesystem/gaps.py index 736b4240..c27f80ec 100644 --- a/subiquity/common/filesystem/gaps.py +++ b/subiquity/common/filesystem/gaps.py @@ -252,6 +252,13 @@ def largest_gap_size(device, in_extended=None): return 0 +def gap_with_size(device, size): + for pg in parts_and_gaps(device): + if isinstance(pg, Gap) and pg.size >= size and pg.is_usable: + 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, diff --git a/subiquity/common/filesystem/tests/test_gaps.py b/subiquity/common/filesystem/tests/test_gaps.py index 6277dfd8..6ca9dcb4 100644 --- a/subiquity/common/filesystem/tests/test_gaps.py +++ b/subiquity/common/filesystem/tests/test_gaps.py @@ -783,3 +783,55 @@ 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.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.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.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 * MiB) + make_partition(device=d, size=10 * MiB, offset=10 * MiB) + [g1, p1, g2] = gaps.parts_and_gaps(d) + self.assertEqual(g2, gaps.gap_with_size(d, 20 * MiB)) + + 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 * MiB) + make_partition(device=d, size=10 * MiB, offset=10 * MiB) + self.assertIs(None, gaps.gap_with_size(d, 10 * MiB)) diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index e0762cec..4688f615 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/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: From f338cfed6449a31b74ae583554f7b4e679cb1104 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Thu, 17 Aug 2023 15:21:49 +1200 Subject: [PATCH 2/5] add "in_extended" awareness to gap_with_size --- subiquity/common/filesystem/gaps.py | 5 ++- .../common/filesystem/tests/test_gaps.py | 38 +++++++++++++++---- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/subiquity/common/filesystem/gaps.py b/subiquity/common/filesystem/gaps.py index c27f80ec..b925db8a 100644 --- a/subiquity/common/filesystem/gaps.py +++ b/subiquity/common/filesystem/gaps.py @@ -252,10 +252,11 @@ def largest_gap_size(device, in_extended=None): return 0 -def gap_with_size(device, size): +def 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: - return pg + if in_extended is None or in_extended == pg.in_extended: + return pg return None diff --git a/subiquity/common/filesystem/tests/test_gaps.py b/subiquity/common/filesystem/tests/test_gaps.py index 6ca9dcb4..34c9e31b 100644 --- a/subiquity/common/filesystem/tests/test_gaps.py +++ b/subiquity/common/filesystem/tests/test_gaps.py @@ -814,11 +814,11 @@ class TestGapWithSize(GapTestCase): ) ) # 0----10---20---30---40---50---60---70---80---90---100 - # ##### [ p1 ] ##### - d = make_disk(size=100 * MiB) - make_partition(device=d, size=10 * MiB, offset=10 * MiB) + # ##### [ 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.gap_with_size(d, 20 * MiB)) + self.assertEqual(g2, gaps.gap_with_size(d, 20)) def test_unusable(self): self.use_alignment_data( @@ -831,7 +831,29 @@ class TestGapWithSize(GapTestCase): ) ) # 0----10---20---30---40---50---60---70---80---90---100 - # ##### [ p1 ] ##### - d = make_disk(size=100 * MiB) - make_partition(device=d, size=10 * MiB, offset=10 * MiB) - self.assertIs(None, gaps.gap_with_size(d, 10 * MiB)) + # ##### [ p1 ] ##### + d = make_disk(size=100) + make_partition(device=d, size=10, offset=2) + self.assertIs(None, gaps.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.gap_with_size(d, 20)) + self.assertEqual(g3, gaps.gap_with_size(d, 20, in_extended=False)) + self.assertEqual(g2, gaps.gap_with_size(d, 10, in_extended=True)) From 7367a373ece3c1cbc4c19c04b5b8d2df8d9d7060 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Thu, 17 Aug 2023 15:23:43 +1200 Subject: [PATCH 3/5] Use the first fitting gap for ESP, instead of the largest gap --- subiquity/common/filesystem/boot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subiquity/common/filesystem/boot.py b/subiquity/common/filesystem/boot.py index 998d3754..c5c7b92e 100644 --- a/subiquity/common/filesystem/boot.py +++ b/subiquity/common/filesystem/boot.py @@ -224,7 +224,7 @@ 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) + gap = gaps.gap_with_size(device, size, in_extended=False) if gap is not None and gap.size >= size and gap.is_usable: create_part_plan.gap = gap.split(size)[0] return create_part_plan From fc06fc0488f22575da3436fd930b7bb9f246b1c1 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Fri, 18 Aug 2023 15:27:29 +1200 Subject: [PATCH 4/5] remove redundant logic --- subiquity/common/filesystem/boot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subiquity/common/filesystem/boot.py b/subiquity/common/filesystem/boot.py index c5c7b92e..9fecf1de 100644 --- a/subiquity/common/filesystem/boot.py +++ b/subiquity/common/filesystem/boot.py @@ -225,7 +225,7 @@ def get_add_part_plan(device, *, spec, args, resize_partition=None): # it's probably a bad idea for all cases. gap = gaps.gap_with_size(device, size, in_extended=False) - if gap is not None and gap.size >= size and gap.is_usable: + 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: From 778516a75c8ce24600d88b96a9715ef76084794c Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Fri, 18 Aug 2023 15:29:20 +1200 Subject: [PATCH 5/5] rename new function to first_gap_with_size --- subiquity/common/filesystem/boot.py | 2 +- subiquity/common/filesystem/gaps.py | 2 +- subiquity/common/filesystem/tests/test_gaps.py | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/subiquity/common/filesystem/boot.py b/subiquity/common/filesystem/boot.py index 9fecf1de..77098114 100644 --- a/subiquity/common/filesystem/boot.py +++ b/subiquity/common/filesystem/boot.py @@ -224,7 +224,7 @@ 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.gap_with_size(device, size, in_extended=False) + 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 diff --git a/subiquity/common/filesystem/gaps.py b/subiquity/common/filesystem/gaps.py index b925db8a..6994277c 100644 --- a/subiquity/common/filesystem/gaps.py +++ b/subiquity/common/filesystem/gaps.py @@ -252,7 +252,7 @@ def largest_gap_size(device, in_extended=None): return 0 -def gap_with_size(device, size, *, in_extended=None): +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: diff --git a/subiquity/common/filesystem/tests/test_gaps.py b/subiquity/common/filesystem/tests/test_gaps.py index 34c9e31b..129e729c 100644 --- a/subiquity/common/filesystem/tests/test_gaps.py +++ b/subiquity/common/filesystem/tests/test_gaps.py @@ -789,19 +789,19 @@ class TestGapWithSize(GapTestCase): def test_empty_disk(self): d = make_disk(size=10 * MiB) [g1] = gaps.parts_and_gaps(d) - self.assertEqual(g1, gaps.gap_with_size(d, MiB)) + 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.gap_with_size(d, MiB)) + 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.gap_with_size(d, 10 * MiB)) + self.assertIs(None, gaps.first_gap_with_size(d, 10 * MiB)) def test_one_gap_too_small(self): self.use_alignment_data( @@ -818,7 +818,7 @@ class TestGapWithSize(GapTestCase): 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.gap_with_size(d, 20)) + self.assertEqual(g2, gaps.first_gap_with_size(d, 20)) def test_unusable(self): self.use_alignment_data( @@ -834,7 +834,7 @@ class TestGapWithSize(GapTestCase): # ##### [ p1 ] ##### d = make_disk(size=100) make_partition(device=d, size=10, offset=2) - self.assertIs(None, gaps.gap_with_size(d, 10)) + self.assertIs(None, gaps.first_gap_with_size(d, 10)) def test_in_extended(self): self.use_alignment_data( @@ -854,6 +854,6 @@ class TestGapWithSize(GapTestCase): 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.gap_with_size(d, 20)) - self.assertEqual(g3, gaps.gap_with_size(d, 20, in_extended=False)) - self.assertEqual(g2, gaps.gap_with_size(d, 10, in_extended=True)) + 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))