curtin-dev team mailing list archive

[Dan Bungert <mp+420764@xxxxxxxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:partition-arbitrary-types into curtin:master.

Commit message:
block/v2: allow setting raw partition_type value

To preserve partition types that do not map to known flag values, we
must be able to capture that original type information and carry it
around in the storage config until used.

If we do not do this, preserved partitions get their type normalized to
another value (usually the equivalent of flags 'linux' or 'boot').

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/420764
-- 
Your team curtin developers is requested to review the proposed merge of ~dbungert/curtin:partition-arbitrary-types into curtin:master.

diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
index 1834343..92f88d0 100644
--- a/curtin/block/schemas.py
+++ b/curtin/block/schemas.py
@@ -304,6 +304,11 @@ PARTITION = {
                  'enum': ['bios_grub', 'boot', 'extended', 'home', 'linux',
                           'logical', 'lvm', 'mbr', 'prep', 'raid', 'swap',
                           '']},
+        'partition_type': {'type': 'string',
+                           'oneOf': [
+                               {'pattern': r'^0x[0-9a-fA-F]{1,2}$'},
+                               {'$ref': '#/definitions/uuid'},
+                               ]},
         'grub_device': {
             'type': ['boolean', 'integer'],
             'minimum': 0,
diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
index a095c8f..34bee7d 100644
--- a/curtin/commands/block_meta_v2.py
+++ b/curtin/commands/block_meta_v2.py
@@ -210,7 +210,7 @@ class DOSPartTable(SFDiskPartTable):
                         prev.start + prev.size,
                         self.one_mib_sectors)
         size = self.bytes2sectors(action['size'])
-        type = FLAG_TO_MBR_TYPE.get(flag)
+        type = action.get('partition_type', FLAG_TO_MBR_TYPE.get(flag))
         if flag == 'boot':
             bootable = True
         else:
diff --git a/curtin/storage_config.py b/curtin/storage_config.py
index 2ede996..e9e8991 100644
--- a/curtin/storage_config.py
+++ b/curtin/storage_config.py
@@ -800,6 +800,8 @@ class BlockdevParser(ProbertParser):
                 entry['size'] *= 512
 
             ptype = blockdev_data.get('ID_PART_ENTRY_TYPE')
+            if ptype is not None:
+                entry['partition_type'] = ptype
             flag_name, _flag_code = ptable_uuid_to_flag_entry(ptype)
 
             if ptable and ptable.get('label') == 'dos':
diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
index 8322062..34ba0d5 100644
--- a/tests/integration/test_block_meta.py
+++ b/tests/integration/test_block_meta.py
@@ -1,12 +1,14 @@
 # This file is part of curtin. See LICENSE file for copyright and license info.
 
-from collections import namedtuple
+import dataclasses
+from dataclasses import dataclass
 import contextlib
 import json
-import sys
-import yaml
 import os
 import re
+import sys
+from typing import Optional
+import yaml
 
 from curtin import block, udev, util
 
@@ -34,7 +36,28 @@ def loop_dev(image, sector_size=512):
         util.subp(['losetup', '--detach', dev])
 
 
-PartData = namedtuple("PartData", ('number', 'offset', 'size'))
+@dataclass(order=True)
+class PartData:
+    number: Optional[int] = None
+    offset: Optional[int] = None
+    size: Optional[int] = None
+    boot: Optional[bool] = None
+    partition_type: Optional[str] = None
+
+    # test cases may initialize the values they care about
+    # test utilities shall initialize all fields
+    def assertFieldsAreNotNone(self):
+        for field in dataclasses.fields(self):
+            assert getattr(self, field.name) is not None
+
+    def __eq__(self, other):
+        for field in dataclasses.fields(self):
+            myval = getattr(self, field.name)
+            otherval = getattr(other, field.name)
+            if myval is not None and otherval is not None \
+                    and myval != otherval:
+                return False
+        return True
 
 
 def _get_ext_size(dev, part_action):
@@ -93,9 +116,21 @@ def _get_extended_partition_size(dev, num):
 
 
 def summarize_partitions(dev):
-    # We don't care about the kname
-    return sorted(
-        [PartData(*d[1:]) for d in block.sysfs_partition_data(dev)])
+    parts = []
+    ptable_json = util.subp(['sfdisk', '-J', dev], capture=True)[0]
+    ptable = json.loads(ptable_json)
+    partitions = ptable['partitiontable']['partitions']
+    for d in block.sysfs_partition_data(dev):
+        nodename = f'/dev/{d[0]}'
+        partition = [part for part in partitions
+                     if part['node'] == nodename][0]
+        ptype = partition['type']
+        boot = partition.get('bootable', False)
+        # We don't care about the kname
+        pd = PartData(*d[1:], partition_type=ptype, boot=boot)
+        pd.assertFieldsAreNotNone()
+        parts.append(pd)
+    return sorted(parts)
 
 
 class StorageConfigBuilder:
@@ -149,6 +184,10 @@ class TestBlockMeta(IntegrationTestCase):
     def setUp(self):
         self.data = self.random_string()
 
+    def assertPartitions(self, *args):
+        with loop_dev(self.img) as dev:
+            self.assertEqual([*args], summarize_partitions(dev))
+
     @contextlib.contextmanager
     def mount(self, dev, partition_cfg):
         mnt_point = self.tmp_dir()
@@ -860,5 +899,30 @@ class TestBlockMeta(IntegrationTestCase):
                 ])
             with self.mount(dev, p1) as mnt_point:
                 for i in range(5, 41, 5):
-                    with open(f'{mnt_point}/{str(i)}', 'rb') as fp:
+                    with open(f'{mnt_point}/{i}', 'rb') as fp:
                         self.assertEqual(bytes([i]) * (2 << 20), fp.read())
+
+    def test_dos_parttype(self):
+        # msdos partition table partitions shall retain their type
+        # create initial situation similar to this
+        # Device     Boot     Start       End   Sectors  Size Id Type
+        # /dev/sda1  *         2048    104447    102400   50M  7 HPFS/NTFS/exFA
+        # /dev/sda2          104448 208668781 208564334 99.5G  7 HPFS/NTFS/exFA
+        # /dev/sda3       208670720 209711103   1040384  508M 27 Hidden NTFS Wi
+        self.img = self.tmp_path('image.img')
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=self.img, size='200M', ptable='msdos')
+        config.add_part(size=50 << 20, offset=1 << 20, number=1,
+                        fstype='ntfs', flag='boot', partition_type='0x7')
+        config.add_part(size=100 << 20, offset=51 << 20, number=2,
+                        fstype='ntfs', partition_type='0x7')
+        config.add_part(size=48 << 20, offset=151 << 20, number=3,
+                        fstype='ntfs', partition_type='0x27')
+        self.run_bm(config.render())
+        self.assertPartitions(
+            PartData(number=1, offset=1 << 20, size=50 << 20,
+                     partition_type='7', boot=True),
+            PartData(number=2, offset=51 << 20, size=100 << 20,
+                     partition_type='7', boot=False),
+            PartData(number=3, offset=151 << 20, size=48 << 20,
+                     partition_type='27', boot=False))
diff --git a/tests/unittests/test_storage_config.py b/tests/unittests/test_storage_config.py
index d6b0a36..df48a4d 100644
--- a/tests/unittests/test_storage_config.py
+++ b/tests/unittests/test_storage_config.py
@@ -350,6 +350,7 @@ class TestBlockdevParser(CiTestCase):
             'offset': 1048576,
             'size': 499122176,
             'flag': 'linux',
+            'partition_type': '0fc63daf-8483-4772-8e79-3d69d8477de4',
         }
         self.assertDictEqual(expected_dict,
                              self.bdevp.asdict(blockdev))
@@ -394,6 +395,7 @@ class TestBlockdevParser(CiTestCase):
             'path': '/dev/vda1',
             'number': 1,
             'size': 784334848,
+            'partition_type': '0x0',
         }
         self.assertDictEqual(expected_dict, self.bdevp.asdict(test_value))
 
@@ -427,7 +429,7 @@ class TestBlockdevParser(CiTestCase):
         self.probe_data = _get_data('probert_storage_lvm.json')
         self.bdevp = BlockdevParser(self.probe_data)
         blockdev = self.bdevp.blockdev_data['/dev/vda2']
-        expected_dict = {
+        base_expected_dict = {
             'id': 'partition-vda2',
             'type': 'partition',
             'path': '/dev/vda2',
@@ -439,6 +441,8 @@ class TestBlockdevParser(CiTestCase):
         }
         for ext_part_entry in ['0xf', '0x5', '0x85', '0xc5']:
             blockdev['ID_PART_ENTRY_TYPE'] = ext_part_entry
+            expected_dict = base_expected_dict.copy()
+            expected_dict['partition_type'] = ext_part_entry
             self.assertDictEqual(expected_dict,
                                  self.bdevp.asdict(blockdev))
 
@@ -455,6 +459,7 @@ class TestBlockdevParser(CiTestCase):
             'offset': 3223322624,
             'size': 2147483648,
             'flag': 'logical',
+            'partition_type': '0x83',
         }
         self.assertDictEqual(expected_dict,
                              self.bdevp.asdict(blockdev))
@@ -473,6 +478,7 @@ class TestBlockdevParser(CiTestCase):
             'offset': 1048576,
             'size': 536870912,
             'flag': 'boot',
+            'partition_type': '0xb',
         }
         self.assertDictEqual(expected_dict,
                              self.bdevp.asdict(blockdev))
@@ -491,6 +497,7 @@ class TestBlockdevParser(CiTestCase):
             'offset': 3223322624,
             'size': 2147483648,
             'flag': 'boot',
+            'partition_type': '0x83',
         }
         self.assertDictEqual(expected_dict,
                              self.bdevp.asdict(blockdev))
@@ -551,6 +558,7 @@ class TestBlockdevParser(CiTestCase):
             'offset': 2097152,
             'size': 10734272512,
             'type': 'partition',
+            'partition_type': '0fc63daf-8483-4772-8e79-3d69d8477de4',
             }
         self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
 
@@ -1018,11 +1026,17 @@ class TestExtractStorageConfig(CiTestCase):
                           'raidlevel': 'raid1', 'name': 'md1',
                           'devices': ['partition-vdb1', 'partition-vdc1'],
                           'spare_devices': []}, raids[0])
-        self.assertEqual({'id': 'raid-md1p1', 'type': 'partition',
-                          'path': '/dev/md1p1',
-                          'size': 4285530112, 'flag': 'linux', 'number': 1,
-                          'device': 'raid-md1', 'offset': 1048576},
-                         raid_partitions[0])
+        self.assertEqual({
+            'id': 'raid-md1p1',
+            'type': 'partition',
+            'path': '/dev/md1p1',
+            'size': 4285530112,
+            'flag': 'linux',
+            'number': 1,
+            'partition_type': '0fc63daf-8483-4772-8e79-3d69d8477de4',
+            'device': 'raid-md1',
+            'offset': 1048576},
+            raid_partitions[0])
 
     @skipUnlessJsonSchema()
     def test_find_extended_partition(self):