Merge ~mwhudson/curtin:default-first-lba into curtin:master

Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Dan Bungert
Approved revision: 02bc28d88d3556a292f9d0dad9368ba0f67b128b
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~mwhudson/curtin:default-first-lba
Merge into: curtin:master
Diff against target: 53 lines (+29/-2)
2 files modified
curtin/commands/block_meta_v2.py (+8/-2)
tests/integration/test_block_meta.py (+21/-0)
Reviewer Review Type Date Requested Status
Dan Bungert Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+433618@code.launchpad.net

Commit message

block_meta_v2: set first-lba if needed when creating a new GPT

sfdisk sets first-lba to 2048 by default, which fails if one of
the partitions has an start smaller than that. Having a
partition start so early in the disk is a bit strange but it
works if you are preserving a partition (because we preserve
first-lba) so it should probably work when creating one too.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Dan Bungert (dbungert) wrote :

This looks great. Would you add a test covering this case?
I did see sub-2048 values when playing with gpt preservation so this is quite worth doing.

~mwhudson/curtin:default-first-lba updated
02bc28d... by Michael Hudson-Doyle

add integration test

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Good point, added.

Revision history for this message
Dan Bungert (dbungert) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
2index 348bf89..3c6e1a5 100644
3--- a/curtin/commands/block_meta_v2.py
4+++ b/curtin/commands/block_meta_v2.py
5@@ -227,8 +227,14 @@ class GPTPartTable(SFDiskPartTable):
6
7 def _headers(self):
8 r = []
9- if self.first_lba is not None:
10- r.extend(['first-lba: ' + str(self.first_lba)])
11+ first_lba = self.first_lba
12+ if first_lba is None:
13+ min_start = min(
14+ [entry.start for entry in self.entries], default=2048)
15+ if min_start < 2048:
16+ first_lba = min_start
17+ if first_lba is not None:
18+ r.extend(['first-lba: ' + str(first_lba)])
19 if self.last_lba is not None:
20 r.extend(['last-lba: ' + str(self.last_lba)])
21 if self.table_length is not None:
22diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
23index 5f7434f..ff3877a 100644
24--- a/tests/integration/test_block_meta.py
25+++ b/tests/integration/test_block_meta.py
26@@ -350,6 +350,27 @@ class TestBlockMeta(IntegrationTestCase):
27 def test_specified_offsets_msdos_v2(self):
28 self._test_specified_offsets('msdos', 2)
29
30+ def test_minimum_gpt_offset_v2(self):
31+ # The default first-lba for a GPT is 2048 (i.e. one megabyte
32+ # into the disk) but it is possible to create a GPT with a
33+ # value as low as 34 and curtin shouldn't get in the way of
34+ # you doing that.
35+ psize = 20 << 20
36+ offset = 34*512
37+ img = self.tmp_path('image.img')
38+ config = StorageConfigBuilder(version=2)
39+ config.add_image(path=img, size='100M', ptable='gpt')
40+ config.add_part(size=psize, number=1, offset=offset)
41+ self.run_bm(config.render())
42+
43+ with loop_dev(img) as dev:
44+ self.assertEqual(
45+ summarize_partitions(dev), [
46+ PartData(number=1, offset=offset, size=psize),
47+ ])
48+ config.set_preserve()
49+ self.run_bm(config.render())
50+
51 def _test_non_default_numbering(self, ptable, version):
52 psize = 40 << 20
53 img = self.tmp_path('image.img')

Subscribers

People subscribed via source and target branches