← Back to team overview

curtin-dev team mailing list archive

[Merge] ~dbungert/curtin:gpt-parttable-preservation into curtin:master

 

Dan Bungert has proposed merging ~dbungert/curtin:gpt-parttable-preservation into curtin:master.

Commit message:
block/v2: gpt preserve uuid,name,attrs,first-lba

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/426651
-- 
Your team curtin developers is requested to review the proposed merge of ~dbungert/curtin:gpt-parttable-preservation into curtin:master.
diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
index b4838f9..4e885eb 100644
--- a/curtin/commands/block_meta_v2.py
+++ b/curtin/commands/block_meta_v2.py
@@ -1,7 +1,10 @@
 # This file is part of curtin. See LICENSE file for copyright and license info.
 
 import os
-from typing import Optional
+from typing import (
+    List,
+    Optional,
+    )
 
 import attr
 
@@ -25,11 +28,18 @@ from curtin.udev import udevadm_settle
 
 @attr.s(auto_attribs=True)
 class PartTableEntry:
+    # The order listed here matches the order sfdisk represents these fields
+    # when using the --dump argument.
     number: int
     start: int
     size: int
     type: str
     uuid: Optional[str]
+    # name here is the sfdisk term - quoted descriptive text of the partition -
+    # not to be confused with what make_dname() does.
+    # Offered in the partition command as 'partition_name'.
+    name: Optional[str]
+    attrs: Optional[List[str]]
     bootable: bool = False
 
     def render(self):
@@ -38,10 +48,25 @@ class PartTableEntry:
             v = getattr(self, a)
             if v is not None:
                 r += f' {a}={v}'
+        if self.name is not None:
+            r += f' name="{self.name}"'
+        if self.attrs:
+            v = ' '.join(self.attrs)
+            r += f' attrs="{v}"'
         if self.bootable:
             r += ' bootable'
         return r
 
+    def preserve(self, part_info):
+        """for an existing partition,
+        initialize unset values to current values"""
+        for a in 'uuid', 'name':
+            if getattr(self, a) is None:
+                setattr(self, a, part_info.get(a))
+        attrs = part_info.get('attrs')
+        if attrs is not None and self.attrs is None:
+            self.attrs = attrs.split(' ')
+
 
 ONE_MIB_BYTES = 1 << 20
 
@@ -114,8 +139,9 @@ class SFDiskPartTable:
 
     def render(self):
         r = ['label: ' + self.label]
-        if self.label_id:
+        if self.label_id is not None:
             r.extend(['label-id: ' + self.label_id])
+        r.extend(self._headers())
         r.extend([''])
         r.extend([e.render() for e in self.entries])
         return '\n'.join(r)
@@ -138,11 +164,32 @@ class SFDiskPartTable:
         # updated by the kernel.
         udevadm_settle()
 
+    def preserve(self, sfdisk_info):
+        """for an existing disk,
+        initialize unset values to current values"""
+        if sfdisk_info is None:
+            return
+        if self.label_id is None:
+            self.label_id = sfdisk_info.get('id')
+        self._preserve(sfdisk_info)
+
+    def _preserve(self, sfdisk_info):
+        """table-type specific value preservation"""
+        pass
+
+    def _headers(self):
+        """table-type specific headers for render()"""
+        return []
+
 
 class GPTPartTable(SFDiskPartTable):
 
     label = 'gpt'
 
+    def __init__(self, sector_bytes):
+        self.first_lba = None
+        super().__init__(sector_bytes)
+
     def add(self, action):
         number = action.get('number', len(self.entries) + 1)
         if 'offset' in action:
@@ -157,10 +204,26 @@ class GPTPartTable(SFDiskPartTable):
         uuid = action.get('uuid')
         type = action.get('partition_type',
                           FLAG_TO_GUID.get(action.get('flag')))
-        entry = PartTableEntry(number, start, size, type, uuid)
+        name = action.get('partition_name')
+        attrs = action.get('attrs')
+        entry = PartTableEntry(
+            number, start, size, type,
+            uuid=uuid, name=name, attrs=attrs)
         self.entries.append(entry)
         return entry
 
+    def _preserve(self, sfdisk_info):
+        # A certain OS can create a value here that looks a little
+        # questionable, but 'fixing' it is probably worse.
+        if self.first_lba is None:
+            self.first_lba = sfdisk_info.get('firstlba')
+
+    def _headers(self):
+        r = []
+        if self.first_lba is not None:
+            r.extend(['first-lba: ' + str(self.first_lba)])
+        return r
+
 
 class DOSPartTable(SFDiskPartTable):
 
@@ -222,7 +285,8 @@ class DOSPartTable(SFDiskPartTable):
         else:
             bootable = None
         entry = PartTableEntry(
-            number, start, size, type, uuid=None, bootable=bootable)
+            number, start, size, type,
+            uuid=None, name=None, bootable=bootable, attrs=None)
         if flag == 'extended':
             self._extended = entry
         self.entries.append(entry)
@@ -365,14 +429,14 @@ def disk_handler_v2(info, storage_config, handlers):
             part_info = _find_part_info(sfdisk_info, entry.start)
             partition_verify_sfdisk_v2(action, sfdisk_info['label'], part_info,
                                        storage_config, table)
+            entry.preserve(part_info)
             resizes[entry.start] = _prepare_resize(storage_config, action,
                                                    table, part_info)
             preserved_offsets.add(entry.start)
         wipes[entry.start] = _wipe_for_action(action)
 
-    # preserve disk label ids
-    if info.get('preserve') and sfdisk_info is not None:
-        table.label_id = sfdisk_info['id']
+    if info.get('preserve'):
+        table.preserve(sfdisk_info)
 
     for kname, nr, offset, size in block.sysfs_partition_data(disk):
         offset_sectors = table.bytes2sectors(offset)
diff --git a/doc/topics/storage.rst b/doc/topics/storage.rst
index bbff909..13ac2b9 100644
--- a/doc/topics/storage.rst
+++ b/doc/topics/storage.rst
@@ -468,6 +468,32 @@ in ``/dev/disk/by-dname/<name>``.
 For partitions, the udev rule created relies upon disk contents, in this case
 the partition entry UUID.  This will remain in effect unless the underlying disk
 on which the partition resides has the partition table modified or wiped.
+This value differs from the ``partition_name`` field below.
+
+**partition_name** *<name for gpt table partition entry>*
+
+Only applicable with a gpt ``ptable``.
+This value is not the same as the ``name`` field above.
+This field sets the optional freeform ASCII name string on the partition.
+On preserved partitions, if this value is unspecified, the current name will be
+retained.
+
+**uuid**: *<uuid>*
+
+Only applicable with a gpt ``ptable``.
+This field sets the optional UUID value on the partition.
+On preserved partitions, if this value is unspecified, the current UUID will be
+retained.
+
+**attrs**: *<list of strings in sfdisk(8) format>*
+
+Only applicable with a gpt ``ptable``.
+Partition attribute flags may optionally be set.  These flags must be specified
+in the same format that
+`sfdisk(8) <https://manpages.ubuntu.com/manpages/focal/man8/sfdisk.8.html#commands>`_
+expects for the part-attrs argument.
+On preserved partitions, if this value is unspecified, the current attributes
+will be retained.
 
 **multipath**: *<multipath name or serial>*
 
diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
index e542017..ee44bbf 100644
--- a/tests/integration/test_block_meta.py
+++ b/tests/integration/test_block_meta.py
@@ -988,3 +988,134 @@ class TestBlockMeta(IntegrationTestCase):
             PartData(number=1, offset=1 << 20, size=18 << 20))
         with loop_dev(self.img) as dev:
             self.assertEqual(orig_label_id, _get_disk_label_id(dev))
+
+    def test_gpt_uuid_persistent(self):
+        # A persistent partition with an unspecified uuid shall keep the uuid
+        self.img = self.tmp_path('image.img')
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=self.img, size='20M', ptable='gpt')
+        config.add_part(number=1, offset=1 << 20, size=18 << 20)
+        self.run_bm(config.render())
+        pd = PartData(number=1, offset=1 << 20, size=18 << 20)
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            expected_uuid = sfdisk_info['partitions'][0]['uuid']
+
+        config.set_preserve()
+        self.run_bm(config.render())
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            actual_uuid = sfdisk_info['partitions'][0]['uuid']
+            self.assertEqual(expected_uuid, actual_uuid)
+
+    def test_gpt_set_name(self):
+        self.img = self.tmp_path('image.img')
+        name = self.random_string() + ' ' + self.random_string()
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=self.img, size='20M', ptable='gpt')
+        config.add_part(number=1, offset=1 << 20, size=18 << 20,
+                        partition_name=name)
+        self.run_bm(config.render())
+        pd = PartData(number=1, offset=1 << 20, size=18 << 20)
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            actual_name = sfdisk_info['partitions'][0]['name']
+        self.assertEqual(name, actual_name)
+
+    def test_gpt_name_persistent(self):
+        self.img = self.tmp_path('image.img')
+        name = self.random_string()
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=self.img, size='20M', ptable='gpt')
+        p1 = config.add_part(number=1, offset=1 << 20, size=18 << 20,
+                             partition_name=name)
+        self.run_bm(config.render())
+        pd = PartData(number=1, offset=1 << 20, size=18 << 20)
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            actual_name = sfdisk_info['partitions'][0]['name']
+        self.assertEqual(name, actual_name)
+
+        del p1['partition_name']
+        config.set_preserve()
+        self.run_bm(config.render())
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            actual_name = sfdisk_info['partitions'][0]['name']
+        self.assertEqual(name, actual_name)
+
+    def test_gpt_set_single_attr(self):
+        self.img = self.tmp_path('image.img')
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=self.img, size='20M', ptable='gpt')
+        attrs = ['GUID:63']
+        config.add_part(number=1, offset=1 << 20, size=18 << 20,
+                        attrs=attrs)
+        self.run_bm(config.render())
+        pd = PartData(number=1, offset=1 << 20, size=18 << 20)
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            attrs_str = sfdisk_info['partitions'][0]['attrs']
+            actual_attrs = set(attrs_str.split(' '))
+        self.assertEqual(set(attrs), actual_attrs)
+
+    def test_gpt_set_multi_attr(self):
+        self.img = self.tmp_path('image.img')
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=self.img, size='20M', ptable='gpt')
+        attrs = ['GUID:63', 'RequiredPartition']
+        config.add_part(number=1, offset=1 << 20, size=18 << 20,
+                        attrs=attrs)
+        self.run_bm(config.render())
+        pd = PartData(number=1, offset=1 << 20, size=18 << 20)
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            attrs_str = sfdisk_info['partitions'][0]['attrs']
+            actual_attrs = set(attrs_str.split(' '))
+        self.assertEqual(set(attrs), actual_attrs)
+
+    def test_gpt_attrs_persistent(self):
+        self.img = self.tmp_path('image.img')
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=self.img, size='20M', ptable='gpt')
+        attrs = ['GUID:63']
+        p1 = config.add_part(number=1, offset=1 << 20, size=18 << 20,
+                             attrs=attrs)
+        self.run_bm(config.render())
+        pd = PartData(number=1, offset=1 << 20, size=18 << 20)
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            attrs_str = sfdisk_info['partitions'][0]['attrs']
+            actual_attrs = set(attrs_str.split(' '))
+        self.assertEqual(set(attrs), actual_attrs)
+
+        del p1['attrs']
+        config.set_preserve()
+        self.run_bm(config.render())
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            attrs_str = sfdisk_info['partitions'][0]['attrs']
+            actual_attrs = set(attrs_str.split(' '))
+        self.assertEqual(set(attrs), actual_attrs)
+
+    def test_gpt_first_lba_persistent(self):
+        self.img = self.tmp_path('image.img')
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=self.img, size='20M', ptable='gpt')
+        config.add_part(number=1, offset=1 << 20, size=18 << 20)
+        self.run_bm(config.render())
+        pd = PartData(number=1, offset=1 << 20, size=18 << 20)
+        self.assertPartitions(pd)
+        with loop_dev(self.img) as dev:
+            sfdisk_info = block.sfdisk_info(dev)
+            first_lba = sfdisk_info['firstlba']
+        self.assertEqual(2048, first_lba)
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 9185d4e..963d043 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -2713,6 +2713,85 @@ label: gpt
 1:  start=2048 size=18432 type={ptype}'''
         self.assertEqual(expected, table.render())
 
+    def test_gpt_name(self):
+        name = self.random_string()
+        table = block_meta_v2.GPTPartTable(512)
+        table.add(dict(number=1, offset=1 << 20, size=9 << 20, flag='boot',
+                       partition_name=name))
+        type_id = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
+        expected = f'''\
+label: gpt
+
+1:  start=2048 size=18432 type={type_id} name="{name}"'''
+        self.assertEqual(expected, table.render())
+
+    def test_gpt_name_spaces(self):
+        name = self.random_string() + " " + self.random_string()
+        table = block_meta_v2.GPTPartTable(512)
+        table.add(dict(number=1, offset=1 << 20, size=9 << 20, flag='boot',
+                       partition_name=name))
+        type_id = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
+        expected = f'''\
+label: gpt
+
+1:  start=2048 size=18432 type={type_id} name="{name}"'''
+        self.assertEqual(expected, table.render())
+
+    def test_gpt_attrs_none(self):
+        table = block_meta_v2.GPTPartTable(512)
+        table.add(dict(number=1, offset=1 << 20, size=9 << 20, flag='boot',
+                       attrs=None))
+        type_id = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
+        expected = f'''\
+label: gpt
+
+1:  start=2048 size=18432 type={type_id}'''
+        self.assertEqual(expected, table.render())
+
+    def test_gpt_attrs_empty(self):
+        table = block_meta_v2.GPTPartTable(512)
+        table.add(dict(number=1, offset=1 << 20, size=9 << 20, flag='boot',
+                       attrs=[]))
+        type_id = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
+        expected = f'''\
+label: gpt
+
+1:  start=2048 size=18432 type={type_id}'''
+        self.assertEqual(expected, table.render())
+
+    def test_gpt_attrs_required(self):
+        table = block_meta_v2.GPTPartTable(512)
+        table.add(dict(number=1, offset=1 << 20, size=9 << 20, flag='boot',
+                       attrs=['RequiredPartition']))
+        type_id = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
+        expected = f'''\
+label: gpt
+
+1:  start=2048 size=18432 type={type_id} attrs="RequiredPartition"'''
+        self.assertEqual(expected, table.render())
+
+    def test_gpt_attrs_bit(self):
+        table = block_meta_v2.GPTPartTable(512)
+        table.add(dict(number=1, offset=1 << 20, size=9 << 20, flag='boot',
+                       attrs=['GUID:51']))
+        type_id = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
+        expected = f'''\
+label: gpt
+
+1:  start=2048 size=18432 type={type_id} attrs="GUID:51"'''
+        self.assertEqual(expected, table.render())
+
+    def test_gpt_attrs_multi(self):
+        table = block_meta_v2.GPTPartTable(512)
+        table.add(dict(number=1, offset=1 << 20, size=9 << 20, flag='boot',
+                       attrs=['RequiredPartition', 'GUID:51']))
+        type_id = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
+        expected = f'''\
+label: gpt
+
+1:  start=2048 size=18432 type={type_id} attrs="RequiredPartition GUID:51"'''
+        self.assertEqual(expected, table.render())
+
     def test_dos_basic(self):
         table = block_meta_v2.DOSPartTable(512)
         expected = '''\
@@ -2740,6 +2819,83 @@ label: dos
 1:  start=2048 size=18432 type={ptype} bootable'''
         self.assertEqual(expected, table.render())
 
+    def test_preserve_labelid_gpt(self):
+        table = block_meta_v2.GPTPartTable(512)
+        self.assertIsNone(table.label_id)
+        label_id = str(random_uuid())
+        sfdisk_info = {'id': label_id}
+        table.preserve(sfdisk_info)
+        self.assertEqual(label_id, table.label_id)
+
+    def test_override_labelid_gpt(self):
+        table = block_meta_v2.GPTPartTable(512)
+        self.assertIsNone(table.label_id)
+        table.label_id = label_id = str(random_uuid())
+        sfdisk_info = {'id': str(random_uuid())}
+        table.preserve(sfdisk_info)
+        self.assertEqual(label_id, table.label_id)
+
+    def test_preserve_labelid_msdos(self):
+        table = block_meta_v2.DOSPartTable(512)
+        self.assertIsNone(table.label_id)
+        label_id = '0x12345678'
+        sfdisk_info = {'id': label_id}
+        table.preserve(sfdisk_info)
+        self.assertEqual(label_id, table.label_id)
+
+    def test_override_labelid_msdos(self):
+        table = block_meta_v2.GPTPartTable(512)
+        self.assertIsNone(table.label_id)
+        table.label_id = label_id = '0x12345678'
+        sfdisk_info = {'id': '0x88888888'}
+        table.preserve(sfdisk_info)
+        self.assertEqual(label_id, table.label_id)
+
+    def test_preserve_firstlba(self):
+        table = block_meta_v2.GPTPartTable(512)
+        self.assertIsNone(table.first_lba)
+        first_lba = 1234
+        sfdisk_info = {'firstlba': first_lba}
+        table.preserve(sfdisk_info)
+        self.assertEqual(first_lba, table.first_lba)
+
+    def test_override_firstlba(self):
+        table = block_meta_v2.GPTPartTable(512)
+        self.assertIsNone(table.first_lba)
+        table.first_lba = first_lba = 1234
+        sfdisk_info = {'firstlba': 8888}
+        table.preserve(sfdisk_info)
+        self.assertEqual(first_lba, table.first_lba)
+
+    def test_dos_entry_render(self):
+        pte = block_meta_v2.PartTableEntry(
+                number=1, start=2, size=3, type='04', bootable=True,
+                uuid=None, name=None, attrs=None)
+        expected = '1:  start=2 size=3 type=04 bootable'
+        self.assertEqual(expected, pte.render())
+
+    def test_gpt_entry_render(self):
+        uuid = str(random_uuid())
+        pte = block_meta_v2.PartTableEntry(
+                number=1, start=2, size=3, type='04', bootable=True,
+                uuid=uuid, name='name',
+                attrs=['stuff', 'things'])
+        expected = f'1:  start=2 size=3 type=04 uuid={uuid} ' + \
+            'name="name" attrs="stuff things" bootable'
+        self.assertEqual(expected, pte.render())
+
+    def test_gpt_entry_preserve(self):
+        uuid = str(random_uuid())
+        name = self.random_string()
+        attrs = f'{self.random_string()} {self.random_string()}'
+        pte = block_meta_v2.PartTableEntry(
+                number=1, start=2, size=3, type='04', bootable=False,
+                uuid=None, name=None, attrs=None)
+        pte.preserve({'uuid': uuid, 'name': name, 'attrs': attrs})
+        expected = f'1:  start=2 size=3 type=04 uuid={uuid} ' + \
+            f'name="{name}" attrs="{attrs}"'
+        self.assertEqual(expected, pte.render())
+
 
 class TestPartitionNeedsResize(CiTestCase):
 

Follow ups