curtin-dev team mailing list archive

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

Michael Hudson-Doyle has proposed merging ~mwhudson/curtin:FR-2656-attach-device-to-action into curtin:master.

Commit message:
storage_config: attach 'path' key to all actions that refer to a block device

Disks and partitions already get this, add it to raid, logical volume,
dm_crypt and bcache actions.

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/429401

This is a bit of an RFC and basically the easiest way to do this. It adds a bunch of attributes that will be accepted by the schema but ignored on the input, which is potentially confusing. We could add a key like 'devpath' or even '_devpath' instead, we could even programmatically mangle the schemas so that it's not valid on input. What do you think?

(This needs doc updates but that can wait until we decide on the above point)
-- 
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:FR-2656-attach-device-to-action into curtin:master.

diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
index 92f88d0..3278331 100644
--- a/curtin/block/schemas.py
+++ b/curtin/block/schemas.py
@@ -67,6 +67,7 @@ BCACHE = {
             'type': ['string'],
             'enum': ['writethrough', 'writeback', 'writearound', 'none'],
         },
+        'path': {'type': 'string', 'pattern': _path_dev},
     },
 }
 DASD = {
@@ -163,6 +164,7 @@ DM_CRYPT = {
         'volume': {'$ref': '#/definitions/ref_id'},
         'key': {'$ref': '#/definitions/id'},
         'keyfile': {'$ref': '#/definitions/id'},
+        'path': {'type': 'string', 'pattern': _path_dev},
         'preserve': {'$ref': '#/definitions/preserve'},
         'type': {'const': 'dm_crypt'},
     },
@@ -213,6 +215,7 @@ LVM_PARTITION = {
         'size': {'$ref': '#/definitions/size'},  # XXX: This is not used
         'type': {'const': 'lvm_partition'},
         'volgroup': {'$ref': '#/definitions/ref_id'},
+        'path': {'type': 'string', 'pattern': _path_dev},
     },
 }
 LVM_VOLGROUP = {
@@ -286,8 +289,7 @@ PARTITION = {
         'multipath': {'type': 'string'},
         # Permit path to device as output.
         # This value is ignored for input.
-        'path': {'type': 'string',
-                 'pattern': _path_dev},
+        'path': {'type': 'string', 'pattern': _path_dev},
         'name': {'$ref': '#/definitions/name'},
         'offset': {'$ref': '#/definitions/size'},  # XXX: This is not used
         'resize': {'type': 'boolean'},
@@ -335,6 +337,7 @@ RAID = {
         'name': {'$ref': '#/definitions/name'},
         'mdname': {'$ref': '#/definitions/name'},  # XXX: Docs need updating
         'metadata': {'type': ['string', 'number']},
+        'path': {'type': 'string', 'pattern': _path_dev},
         'preserve': {'$ref': '#/definitions/preserve'},
         'ptable': {'$ref': '#/definitions/ptable'},
         'wipe': {'$ref': '#/definitions/wipe'},
diff --git a/curtin/storage_config.py b/curtin/storage_config.py
index e9e8991..9c522d8 100644
--- a/curtin/storage_config.py
+++ b/curtin/storage_config.py
@@ -589,10 +589,11 @@ class BcacheParser(ProbertParser):
         backing_device = backing_data.get('blockdev')
         cache_device = _find_cache_device(backing_data, self.caching)
         cache_mode = _cache_mode(backing_data)
-        bcache_name = os.path.basename(_find_bcache_devname(backing_uuid,
-                                       backing_data, self.blockdev_data))
+        devname = _find_bcache_devname(backing_uuid,
+                                       backing_data, self.blockdev_data)
+        bcache_name = os.path.basename(devname)
         bcache_entry = {'type': 'bcache', 'id': 'disk-%s' % bcache_name,
-                        'name': bcache_name}
+                        'name': bcache_name, 'path': devname}
 
         if cache_mode:
             bcache_entry['cache_mode'] = cache_mode
@@ -911,6 +912,8 @@ class LvmParser(ProbertParser):
         return {'type': 'lvm_partition',
                 'id': 'lvm-partition-%s' % lv_config['name'],
                 'name': lv_config['name'],
+                'path': self.lookup_devname('/dev/{}/{}'.format(
+                    lv_config['volgroup'], lv_config['name'])),
                 'size': lv_config['size'],
                 'volgroup': 'lvm-volgroup-%s' % lv_config['volgroup']}
 
@@ -1020,6 +1023,7 @@ class DmcryptParser(ProbertParser):
 
         return {'type': 'dm_crypt',
                 'id': 'dmcrypt-%s' % crypt_name,
+                'path': bdev,
                 'volume': bdev_id,
                 'key': '',
                 'dm_name': crypt_name}
@@ -1058,6 +1062,7 @@ class RaidParser(ProbertParser):
             'type': 'raid',
             'id': self.blockdev_to_id(raid_data),
             'name': raidname,
+            'path': devname,
             'raidlevel': raid_data.get('raidlevel'),
             }
 
diff --git a/tests/unittests/test_storage_config.py b/tests/unittests/test_storage_config.py
index df48a4d..e4c005e 100644
--- a/tests/unittests/test_storage_config.py
+++ b/tests/unittests/test_storage_config.py
@@ -202,6 +202,7 @@ class TestBcacheParser(CiTestCase):
             'type': 'bcache',
             'id': 'disk-bcache0',
             'name': 'bcache0',
+            'path': '/dev/bcache0',
             'backing_device': 'partition-sda3',
             'cache_device': 'partition-nvme0n1p1',
             'cache_mode': 'writeback',
@@ -221,6 +222,7 @@ class TestBcacheParser(CiTestCase):
             'type': 'bcache',
             'id': 'disk-bcache0',
             'name': 'bcache0',
+            'path': '/dev/bcache0',
             'backing_device': 'partition-sda3',
             'cache_mode': 'writeback',
         }
@@ -674,6 +676,7 @@ class TestLvmParser(CiTestCase):
         expected_dict = {
             'type': 'lvm_partition',
             'name': 'my-storage',
+            'path': '/dev/dm-2',
             'id': 'lvm-partition-my-storage',
             'size': '1073741824B',
             'volgroup': 'lvm-volgroup-ubuntu-vg',
@@ -720,6 +723,7 @@ class TestRaidParser(CiTestCase):
             'type': 'raid',
             'id': 'raid-md0',
             'name': 'md0',
+            'path': '/dev/md0',
             'metadata': '1.2',
             'raidlevel': 'raid5',
             'devices': ['disk-vde', 'disk-vdf', 'disk-vdg'],
@@ -744,6 +748,7 @@ class TestRaidParser(CiTestCase):
             'type': 'raid',
             'id': 'raid-md127',
             'name': 'md127',
+            'path': '/dev/md127',
             'metadata': 'imsm',
             'raidlevel': 'container',
             'devices': ['disk-nvme0n1', 'disk-nvme1n1'],
@@ -758,6 +763,7 @@ class TestRaidParser(CiTestCase):
             'type': 'raid',
             'id': 'raid-md126',
             'name': 'md126',
+            'path': '/dev/md126',
             'raidlevel': 'raid0',
             'container': 'raid-md127',
             }
@@ -851,6 +857,7 @@ class TestDmCryptParser(CiTestCase):
             'type': 'dm_crypt',
             'id': 'dmcrypt-dmcrypt0',
             'dm_name': devname,
+            'path': '/dev/dm-2',
             'key': '',
             'volume': 'lvm-partition-lv3',
         }