curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+444326@xxxxxxxxxxxxxxxxxx>]

Michael Hudson-Doyle has proposed merging ~mwhudson/curtin:ptable_uuid_to_flag_entry-retval into curtin:master.

Commit message:
storage_config.ptable_uuid_to_flag_entry: stop returning sgdisk typecode

Every invocation of ptable_uuid_to_flag_entry (including the one in
subiquity) discards the second element of the tuple, so stop returning
it. I also cleaned up the data structures this function works off, which
required some small tweaks in block_meta_v2.



Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/444326
-- 
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:ptable_uuid_to_flag_entry-retval into curtin:master.

diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index b00defe..9499907 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -8,8 +8,10 @@ from curtin.block import (bcache, clear_holders, dasd, iscsi, lvm, mdadm, mkfs,
 from curtin import distro
 from curtin.log import LOG, logged_time
 from curtin.reporter import events
-from curtin.storage_config import (extract_storage_ordered_dict,
-                                   ptable_uuid_to_flag_entry)
+from curtin.storage_config import (
+    extract_storage_ordered_dict,
+    ptable_uuid_to_flag_entry,
+    )
 
 
 from . import populate_one_subcmd
@@ -45,14 +47,14 @@ PTABLES_SUPPORTED = schemas._ptables
 PTABLES_VALID = schemas._ptables_valid
 
 SGDISK_FLAGS = {
+    "bios_grub": 'ef02',
     "boot": 'ef00',
+    "home": '8302',
+    "linux": '8300',
     "lvm": '8e00',
-    "raid": 'fd00',
-    "bios_grub": 'ef02',
     "prep": '4100',
+    "raid": 'fd00',
     "swap": '8200',
-    "home": '8302',
-    "linux": '8300'
 }
 
 MSDOS_FLAGS = {
@@ -900,7 +902,7 @@ def verify_exists(devpath):
 
 
 def get_part_size_bytes(devpath, part_info):
-    (found_type, _code) = ptable_uuid_to_flag_entry(part_info.get('type'))
+    found_type = ptable_uuid_to_flag_entry(part_info.get('type'))
     if found_type == 'extended':
         found_size_bytes = int(part_info['size']) * 512
     else:
@@ -929,14 +931,14 @@ def verify_ptable_flag(devpath, expected_flag, label, part_info):
         if expected_flag == 'boot':
             found_flag = 'boot' if part_info.get('bootable') is True else None
         elif expected_flag == 'extended':
-            (found_flag, _code) = ptable_uuid_to_flag_entry(part_info['type'])
+            found_flag = ptable_uuid_to_flag_entry(part_info['type'])
         elif expected_flag == 'logical':
             (_parent, partnumber) = block.get_blockdev_for_partition(devpath)
             found_flag = 'logical' if int(partnumber) > 4 else None
 
     # gpt and msdos primary partitions look up flag by entry['type']
     if found_flag is None:
-        (found_flag, _code) = ptable_uuid_to_flag_entry(part_info['type'])
+        found_flag = ptable_uuid_to_flag_entry(part_info['type'])
     msg = (
         'Verifying %s partition flag, expecting %s, found %s' % (
          devpath, expected_flag, found_flag))
diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
index 570da83..dbb2b3f 100644
--- a/curtin/commands/block_meta_v2.py
+++ b/curtin/commands/block_meta_v2.py
@@ -21,6 +21,7 @@ from curtin.commands.block_meta import (
 from curtin.log import LOG
 from curtin.storage_config import (
     GPT_GUID_TO_CURTIN_MAP,
+    MBR_TYPE_TO_CURTIN_MAP,
     select_configs,
     )
 from curtin.udev import udevadm_settle
@@ -130,11 +131,10 @@ resizers = {
 
 
 FLAG_TO_GUID = {
-    flag: guid for (guid, (flag, typecode)) in GPT_GUID_TO_CURTIN_MAP.items()
+    flag: guid for (guid, flag) in GPT_GUID_TO_CURTIN_MAP.items()
     }
 FLAG_TO_MBR_TYPE = {
-    flag: typecode[:2].upper() for (guid, (flag, typecode))
-    in GPT_GUID_TO_CURTIN_MAP.items()
+    flag: typecode[2:] for (typecode, flag) in MBR_TYPE_TO_CURTIN_MAP.items()
     }
 FLAG_TO_MBR_TYPE['extended'] = '05'
 
diff --git a/curtin/storage_config.py b/curtin/storage_config.py
index a232e1a..1f0c8ea 100644
--- a/curtin/storage_config.py
+++ b/curtin/storage_config.py
@@ -13,36 +13,37 @@ from curtin import util
 
 # map
 # https://en.wikipedia.org/wiki/GUID_Partition_Table#Partition_type_GUIDs
-# to
-# curtin/commands/block_meta.py:partition_handler()sgdisk_flags/types
+# to values used as "flag" on partition actions.
 GPT_GUID_TO_CURTIN_MAP = {
-    'C12A7328-F81F-11D2-BA4B-00A0C93EC93B': ('boot', 'EF00'),
-    '21686148-6449-6E6F-744E-656564454649': ('bios_grub', 'EF02'),
-    '933AC7E1-2EB4-4F13-B844-0E14E2AEF915': ('home', '8302'),
-    '0FC63DAF-8483-4772-8E79-3D69D8477DE4': ('linux', '8300'),
-    'E6D6D379-F507-44C2-A23C-238F2A3DF928': ('lvm', '8e00'),
-    '024DEE41-33E7-11D3-9D69-0008C781F39F': ('mbr', ''),
-    '9E1A2D38-C612-4316-AA26-8B49521E5A8B': ('prep', '4200'),
-    'A19D880F-05FC-4D3B-A006-743F0F84911E': ('raid', 'fd00'),
-    '0657FD6D-A4AB-43C4-84E5-0933C84B4F4F': ('swap', '8200'),
+    'C12A7328-F81F-11D2-BA4B-00A0C93EC93B': 'boot',
+    '21686148-6449-6E6F-744E-656564454649': 'bios_grub',
+    '933AC7E1-2EB4-4F13-B844-0E14E2AEF915': 'home',
+    '0FC63DAF-8483-4772-8E79-3D69D8477DE4': 'linux',
+    'E6D6D379-F507-44C2-A23C-238F2A3DF928': 'lvm',
+    '024DEE41-33E7-11D3-9D69-0008C781F39F': 'mbr',
+    '9E1A2D38-C612-4316-AA26-8B49521E5A8B': 'prep',
+    'A19D880F-05FC-4D3B-A006-743F0F84911E': 'raid',
+    '0657FD6D-A4AB-43C4-84E5-0933C84B4F4F': 'swap',
 }
 
 # MBR types
 # https://www.win.tue.nl/~aeb/partitions/partition_types-2.html
 # to
-# curtin/commands/block_meta.py:partition_handler()sgdisk_flags/types
+# to values used as "flag" on partition actions.
 MBR_TYPE_TO_CURTIN_MAP = {
-    '0XF': ('extended', 'f'),
-    '0X5': ('extended', 'f'),
-    '0X83': ('linux', '83'),
-    '0X85': ('extended', 'f'),
-    '0XC5': ('extended', 'f'),
+    '0X5':  'extended',
+    '0X82': 'swap',
+    '0X83': 'linux',
+    '0X85': 'extended',
+    '0X8E': 'lvm',
+    '0XC5': 'extended',
+    '0XEF': 'boot',
+    '0XF':  'extended',
+    '0XFD': 'raid',
 }
 
 MBR_BOOT_FLAG = '0x80'
 
-PTABLE_TYPE_MAP = dict(GPT_GUID_TO_CURTIN_MAP, **MBR_TYPE_TO_CURTIN_MAP)
-
 StorageConfig = namedtuple('StorageConfig', ('type', 'schema'))
 STORAGE_CONFIG_TYPES = {
     'bcache': StorageConfig(type='bcache', schema=schemas.BCACHE),
@@ -803,7 +804,7 @@ class BlockdevParser(ProbertParser):
             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)
+            flag_name = ptable_uuid_to_flag_entry(ptype)
 
             if ptable and ptable.get('label') == 'dos':
                 # if the boot flag is set, use this as the flag, logical
@@ -1279,14 +1280,16 @@ class ZfsParser(ProbertParser):
 
 
 def ptable_uuid_to_flag_entry(guid):
-    name = code = None
-    # prefix non-uuid guid values with 0x
-    if guid and '-' not in guid and not guid.upper().startswith('0X'):
-        guid = '0x' + guid
-    if guid and guid.upper() in PTABLE_TYPE_MAP:
-        name, code = PTABLE_TYPE_MAP[guid.upper()]
-
-    return (name, code)
+    if not guid:
+        return None
+    guid = guid.upper()
+    if '-' in guid:
+        return GPT_GUID_TO_CURTIN_MAP.get(guid)
+    else:
+        # prefix non-uuid guid values with 0x
+        if not guid.startswith('0X'):
+            guid = '0X' + guid
+        return MBR_TYPE_TO_CURTIN_MAP.get(guid)
 
 
 def extract_storage_config(probe_data, strict=False):
diff --git a/tests/unittests/test_storage_config.py b/tests/unittests/test_storage_config.py
index a538ece..2973193 100644
--- a/tests/unittests/test_storage_config.py
+++ b/tests/unittests/test_storage_config.py
@@ -257,24 +257,21 @@ class TestBlockdevParser(CiTestCase):
         """ BlockdevParser maps ptable UUIDs to boot flags. """
         boot_guids = ['C12A7328-F81F-11D2-BA4B-00A0C93EC93B',
                       'c12a7328-f81f-11d2-ba4b-00a0c93ec93b']
-        expected_tuple = ('boot', 'EF00')
+        expected_flag = 'boot'
         for guid in boot_guids:
-            self.assertEqual(expected_tuple,
-                             ptable_uuid_to_flag_entry(guid))
+            self.assertEqual(expected_flag, ptable_uuid_to_flag_entry(guid))
 
     # XXX: Parameterize me
     def test_blockdev_ptable_uuid_flag_invalid(self):
-        """ BlockdevParser returns (None, None) for invalid uuids. """
+        """ BlockdevParser returns None for invalid uuids. """
         for invalid in [None, '', {}, []]:
-            self.assertEqual((None, None),
-                             ptable_uuid_to_flag_entry(invalid))
+            self.assertEqual(None, ptable_uuid_to_flag_entry(invalid))
 
     # XXX: Parameterize me
     def test_blockdev_ptable_uuid_flag_unknown_uuid(self):
-        """ BlockdevParser returns (None, None) for unknown uuids. """
+        """ BlockdevParser returns None for unknown uuids. """
         for unknown in [self.random_string(), self.random_string()]:
-            self.assertEqual((None, None),
-                             ptable_uuid_to_flag_entry(unknown))
+            self.assertEqual(None, ptable_uuid_to_flag_entry(unknown))
 
     def test_get_unique_ids(self):
         """ BlockdevParser extracts uniq udev ID_ values. """