curtin-dev team mailing list archive

[Michael Hudson-Doyle <mwhudsonlp@xxxxxxxxxxx>]

Thanks for the comments, please take another look

Diff comments:

> diff --git a/curtin/storage_config.py b/curtin/storage_config.py
> index 81b7385..ebe7bb9 100644
> --- a/curtin/storage_config.py
> +++ b/curtin/storage_config.py
> @@ -1047,16 +1046,25 @@ class RaidParser(ProbertParser):
>          # FIXME, need to handle rich md_name values, rather than mdX
>          # LP: #1803933
>          raidname = os.path.basename(devname)
> -        return {'type': 'raid',
> -                'id': 'raid-%s' % raidname,
> -                'name': raidname,
> -                'raidlevel': raid_data.get('raidlevel'),
> -                'devices': sorted([
> -                    self.blockdev_to_id(self.blockdev_data[dev])
> -                    for dev in raid_data.get('devices')]),
> -                'spare_devices': sorted([
> -                    self.blockdev_to_id(self.blockdev_data[dev])
> -                    for dev in raid_data.get('spare_devices')])}
> +
> +        action = {
> +            'type': 'raid',
> +            'id': self.blockdev_to_id(raid_data),
> +            'name': raidname,
> +            'raidlevel': raid_data.get('raidlevel'),
> +            }
> +
> +        if 'container' in raid_data:
> +            action['container'] = self.blockdev_byid_to_devname(
> +                raid_data['container'])
> +        else:
> +            action['metadata'] = raid_data.get("MD_METADATA")

MD_METADATA doesn't appear in the output for a volume in an imsm container (I guess because it's not like a volume in a container gets to choose its metadata format?) but I think you're right that it's not curtin's place to rely on/enforce this distinction.

> +            for k in 'devices', 'spare_devices':
> +                action[k] = sorted([
> +                    self.blockdev_byid_to_devname(dev)
> +                    for dev in raid_data.get(k, [])])
> +
> +        return action
>  
>      def parse(self):
>          """parse probert 'raid' data format.
> diff --git a/tests/unittests/test_storage_config.py b/tests/unittests/test_storage_config.py
> index 4c0e272..eb8ff4c 100644
> --- a/tests/unittests/test_storage_config.py
> +++ b/tests/unittests/test_storage_config.py

Yes, should have done that on my own initiative really.

> @@ -668,6 +668,7 @@ class TestRaidParser(CiTestCase):
>              'type': 'raid',
>              'id': 'raid-md0',
>              'name': 'md0',
> +            'metadata': '1.2',
>              'raidlevel': 'raid5',
>              'devices': ['disk-vde', 'disk-vdf', 'disk-vdg'],
>              'spare_devices': [],
> @@ -942,7 +943,7 @@ class TestExtractStorageConfig(CiTestCase):
>                             cfg['id'].startswith('raid')]
>          self.assertEqual(1, len(raids))
>          self.assertEqual(1, len(raid_partitions))
> -        self.assertEqual({'id': 'raid-md1', 'type': 'raid',
> +        self.assertEqual({'id': 'raid-md1', 'type': 'raid', 'metadata': '1.2',
>                            'raidlevel': 'raid1', 'name': 'md1',
>                            'devices': ['partition-vdb1', 'partition-vdc1'],
>                            'spare_devices': []}, raids[0])


-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/404051
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:convert-imsm into curtin:master.