curtin-dev team mailing list archive

[Olivier Gayot <mp+480694@xxxxxxxxxxxxxxxxxx>]

Olivier Gayot has proposed merging ~ogayot/curtin:ldm into curtin:master.

Commit message:
storage-config: don't crash when dealing with LDM or Windows dynamic disks

The Linux kernel has (partial) support for Windows dynamic disks (aka. LDM)
built on MSDOS ptables. If the kernel is built with CONFIG_LDM_PARTITION (which
is true in Ubuntu), it will detect partitions that are not listed in the
partition table itself, but in the LDM database, stored in the last MiB of the
disk.

In such a scenario, when Subiquity invokes curtin's storage-config using data
collected by probert, curtin fails with:

  RuntimeError: "couldn't find partition entry in table".

And this is expected since the partitions are listed in the LDM database (which
curtin can't read) and not in the partition table.

Instead of raising an error in this scenario, we now look if the disk seems to
have a LDM layout, and if it does, link the partition to the disk but mark the
ptable as "unsupported" instead of "msdos".

This allows subiquity to do its thing, knowing that a disk has an "unsupported"
layout.

Requested reviews:
  curtin developers (curtin-dev)

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

diff --git a/curtin/storage_config.py b/curtin/storage_config.py
index dae89f4..2ed4d63 100644
--- a/curtin/storage_config.py
+++ b/curtin/storage_config.py
@@ -470,6 +470,37 @@ class ProbertParser(object):
         return multipath.is_mpath_partition(
             blockdev.get('DEVNAME', ''), blockdev)
 
+    def looks_like_ldm_disk(self, blockdev) -> bool:
+        """ Tell if the disk looks like a Windows dynamic disk or LDM (aka.
+            Logical Disk Manager).
+
+            Here we consider that a disk is a dynamic disk if it contains a DOS
+            partition table will all partitions having type 0x42.
+
+            The Linux kernel and possibly libldm (currently in universe) do
+            extra checks to determine if a disk is a dynamic disk.
+
+            Here we only scratch the surface (thus the verb "looks_like" rather
+            than "is") so it is better to only use this function if we already
+            suspect the disk could be a LDM disk.
+        """
+        found_one = False
+        try:
+            ptable = blockdev["partitiontable"]
+            # Currently, the Linux kernel only supports dynamic disks on dos
+            # partition tables.
+            if ptable["label"] != "dos":
+                return False
+            for part in ptable["partitions"]:
+                # This used to be "SFS" Windows dynamic disks use it too.
+                if part.get("type") != "42":
+                    return False
+                found_one = True
+        except KeyError:
+            return False
+
+        return found_one
+
     def blockdev_to_id(self, blockdev):
         """ Examine a blockdev dictionary and return a tuple of curtin
             storage type and name that can be used as a value for
@@ -759,6 +790,9 @@ class BlockdevParser(ProbertParser):
                 else:
                     entry['ptable'] = schemas._ptable_unsupported
 
+            if self.looks_like_ldm_disk(blockdev_data):
+                entry['ptable'] = schemas._ptable_unsupported
+
             match = re.fullmatch(r'/dev/(?P<ctrler>nvme\d+)n\d', devname)
             if match is not None:
                 entry['nvme_controller'] = f'nvme-controller-{match["ctrler"]}'
@@ -797,8 +831,20 @@ class BlockdevParser(ProbertParser):
                         break
 
                 if part is None:
-                    raise RuntimeError(
-                        "Couldn't find partition entry in table")
+                    # Could not find the partition in the partition table.
+
+                    # It is expected for LDM (or Windows dynamic disks).
+                    # The Linux kernel can discover partitions from the "LDM
+                    # database" which is stored in the last 1MiB of the disk.
+                    if self.looks_like_ldm_disk(parent_blockdev):
+                        part = {
+                            'start': attrs['start'],
+                            'size': attrs['size'],
+                            'on-dynamic-disk': True,
+                        }
+                    else:
+                        raise RuntimeError(
+                            "Couldn't find partition entry in table")
             else:
                 part = attrs
 
@@ -813,7 +859,7 @@ class BlockdevParser(ProbertParser):
                 entry['offset'] = offset_val
 
             entry['size'] = int(part['size'])
-            if ptable:
+            if ptable and not part.get("on-dynamic-disk"):
                 entry['size'] *= logical_sector_size
 
             if blockdev_data.get('ID_PART_TABLE_TYPE') == 'gpt':
@@ -833,7 +879,7 @@ class BlockdevParser(ProbertParser):
                 ptype_flag = blockdev_data.get('ID_PART_ENTRY_FLAGS')
                 if ptype_flag in [MBR_BOOT_FLAG]:
                     flag_name = 'boot'
-                else:
+                elif not part.get("on-dynamic-disk"):
                     # logical partitions are not tagged in data, however
                     # the partition number > 4 (ie, not primary nor extended)
                     if entry['number'] > 4: