curtin-dev team mailing list archive

[Olivier Gayot <mp+480694@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/curtin/storage_config.py b/curtin/storage_config.py
> index dae89f4..6e28187 100644
> --- a/curtin/storage_config.py
> +++ b/curtin/storage_config.py
> @@ -470,6 +470,39 @@ class ProbertParser(object):
>          return multipath.is_mpath_partition(
>              blockdev.get('DEVNAME', ''), blockdev)
>  
> +    @staticmethod
> +    def looks_like_ldm_disk(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.

Thanks!

> +
> +            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" but it is ancient and 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
> diff --git a/tests/unittests/test_storage_config.py b/tests/unittests/test_storage_config.py
> index 7b0f68c..fc4b1e1 100644
> --- a/tests/unittests/test_storage_config.py
> +++ b/tests/unittests/test_storage_config.py
> @@ -531,6 +598,35 @@ class TestBlockdevParser(CiTestCase):
>              self.assertDictEqual(expected_dict,
>                                   self.bdevp.asdict(blockdev))
>  
> +    def test_blockdev_asdict_ldm_disk(self):
> +        self.probe_data = _get_data('probert_storage_ldm.json')
> +        self.bdevp = BlockdevParser(self.probe_data)
> +        blockdev = self.bdevp.blockdev_data['/dev/sda']
> +        expected_dict = {
> +            'id': 'disk-sda',
> +            'type': 'disk',
> +            'wwn': '0x5000039fdcf04730',
> +            'serial': 'TOSHIBA_HDWD110_999E6AZFS',
> +            'ptable': 'unsupported',
> +            'path': '/dev/sda',
> +        }
> +        self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
> +
> +    def test_blockdev_asdict_ldm_partition_not_in_ptable(self):

Sure, I'll clarify in the code. Essentially, this code used to raise "Partition entry not found in table". I'm asserting that we now get valid data, even though there's no mention of sda5 in the ptable.

> the last partition is the ldm partition which contains real information about the others

Not quite, I don't think there is such as thing as a "LDM partition". There is a LDM database (which is sort-of an extra partition table) in the last MiB of the disk. 

If my understanding is correct, LDM uses a MSDOS ptable (GPT is also a thing but..) with partition(s) to make the disk "look" full. This way, if we don't realize it's a "dynamic" disk, we are not tempted to use any "free space" - which wouldn't actually be free space.

Now, I don't really understand the difference between a dynamic disk where the ptable shows:
* a single partition spanning the whole disk
* multiple partitions (covering the whole disk)

Maybe it depends how many partitions existed before it got converted to a dynamic disk/LDM. I'm just guessing here.

> +        self.probe_data = _get_data('probert_storage_ldm.json')
> +        self.bdevp = BlockdevParser(self.probe_data)
> +        blockdev = self.bdevp.blockdev_data['/dev/sda5']
> +        expected_dict = {
> +            'id': 'partition-sda5',
> +            'type': 'partition',
> +            'device': 'disk-sda',
> +            'path': '/dev/sda5',
> +            'number': 5,
> +            'offset': 904945664 * 512,
> +            'size': 536869863424,
> +        }
> +        self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
> +
>      def test_blockdev_multipath_disk(self):
>          self.probe_data = _get_data('probert_storage_multipath.json')
>          self.bdevp = BlockdevParser(self.probe_data)


-- 
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.