Michael Hudson-Doyle has proposed merging
~mwhudson/curtin:v2-disk-identification into curtin:master.
Commit message:
block_meta: all fields on a disk action must match with v2 config
This is for https://bugs.launchpad.net/subiquity/+bug/1929213 and
similar reports.
Requested reviews:
curtin developers (curtin-dev)
For more details, see:
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/415700
--
Your team curtin developers is requested to review the proposed merge of
~mwhudson/curtin:v2-disk-identification into curtin:master.
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index f3f19dc..42325c1 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -13,8 +13,13 @@ from curtin.storage_config import (extract_storage_ordered_dict,
from . import populate_one_subcmd
-from curtin.udev import (compose_udev_equality, udevadm_settle,
- udevadm_trigger, udevadm_info)
+from curtin.udev import (
+ compose_udev_equality,
+ udev_all_block_device_properties,
+ udevadm_info,
+ udevadm_settle,
+ udevadm_trigger,
+ )
import glob
import os
@@ -24,6 +29,7 @@ import sys
import tempfile
import time
+
FstabData = namedtuple(
"FstabData", ('spec', 'path', 'fstype', 'options', 'freq', 'passno',
'device'))
@@ -428,6 +434,112 @@ def get_poolname(info, storage_config):
return poolname
+def v1_get_path_to_disk(vol):
+ volume_path = None
+ for disk_key in ['wwn', 'serial', 'device_id', 'path']:
+ vol_value = vol.get(disk_key)
+ try:
+ if not vol_value:
+ continue
+ if disk_key in ['wwn', 'serial']:
+ volume_path = block.lookup_disk(vol_value)
+ elif disk_key == 'path':
+ if vol_value.startswith('iscsi:'):
+ i = iscsi.ensure_disk_connected(vol_value)
+ volume_path = os.path.realpath(i.devdisk_path)
+ else:
+ # resolve any symlinks to the dev_kname so
+ # sys/class/block access is valid. ie, there are no
+ # udev generated values in sysfs
+ volume_path = os.path.realpath(vol_value)
+ # convert /dev/sdX to /dev/mapper/mpathX value
+ if multipath.is_mpath_member(volume_path):
+ volume_path = '/dev/mapper/' + (
+ multipath.get_mpath_id_from_device(volume_path))
+ elif disk_key == 'device_id':
+ dasd_device = dasd.DasdDevice(vol_value)
+ volume_path = dasd_device.devname
+ except ValueError:
+ continue
+ # verify path exists otherwise try the next key
+ if os.path.exists(volume_path):
+ break
+ else:
+ volume_path = None
+
+ if volume_path is None:
+ raise ValueError("Failed to find storage volume id='%s' config: %s"
+ % (vol['id'], vol))
+
+ return volume_path
+
+
+def v2_get_path_to_disk(vol):
+ all_disks = [
+ dev for dev in udev_all_block_device_properties()
+ if 'DM_PART' not in dev and 'PARTN' not in dev
+ ]
+
+ def disks_matching(key, value):
+ return [
+ dev['DEVNAME']
+ for dev in all_disks
+ if key in dev and dev[key] == value
+ ]
+
+ def disk_by_path(path):
+ for dev in all_disks:
+ if dev['DEVNAME'] == path or path in dev['DEVLINKS'].split():
+ return dev['DEVNAME']
+
+ cands = []
+ if 'wwn' in vol:
+ for key in 'DM_WWN', 'ID_WWN':
+ devs = disks_matching(key, vol['wwn'])
+ if devs:
+ break
+ cands.append(set(devs))
+ if 'serial' in vol:
+ for key in 'DM_SERIAL', 'ID_SERIAL':
+ devs = disks_matching(key, vol['serial'])
+ if devs:
+ break
+ cands.append(set(devs))
+ if 'device_id' in vol:
+ dasd_device = dasd.DasdDevice(vol['device_id'])
+ cands.append(set([dasd_device.devname]))
+ volume_path = dasd_device.devname
+ # verify path exists otherwise try the next key
+ if os.path.exists(volume_path):
+ cands.append(set([volume_path]))
+ if 'path' in vol:
+ path = vol['path']
+ if path.startswith('iscsi:'):
+ i = iscsi.ensure_disk_connected(path)
+ path = i.devdisk_path
+ if multipath.is_mpath_member(path):
+ # if path points to a multipath member, convert that to point
+ # at the multipath device
+ path = '/dev/mapper/' + multipath.get_mpath_id_from_device(path)
+ path = disk_by_path(path)
+ if path is not None:
+ cands.append(set([path]))
+ else:
+ cands.append(set())
+
+ cands = set.intersection(*cands)
+
+ if len(cands) == 0:
+ raise ValueError("Failed to find storage volume id='%s' config: %s"
+ % (vol['id'], vol))
+ if len(cands) > 1:
+ raise ValueError(
+ "Storage volume id='%s' config: %s identified multiple devices"
+ % (vol['id'], vol))
+
+ return next(iter(cands))
+
+
def get_path_to_storage_volume(volume, storage_config):
# Get path to block device for volume. Volume param should refer to id of
# volume in storage config
@@ -454,41 +566,10 @@ def get_path_to_storage_volume(volume, storage_config):
elif vol.get('type') == "disk":
# Get path to block device for disk. Device_id param should refer
# to id of device in storage config
- volume_path = None
- for disk_key in ['wwn', 'serial', 'device_id', 'path']:
- vol_value = vol.get(disk_key)
- try:
- if not vol_value:
- continue
- if disk_key in ['wwn', 'serial']:
- volume_path = block.lookup_disk(vol_value)
- elif disk_key == 'path':
- if vol_value.startswith('iscsi:'):
- i = iscsi.ensure_disk_connected(vol_value)
- volume_path = os.path.realpath(i.devdisk_path)
- else:
- # resolve any symlinks to the dev_kname so
- # sys/class/block access is valid. ie, there are no
- # udev generated values in sysfs
- volume_path = os.path.realpath(vol_value)
- # convert /dev/sdX to /dev/mapper/mpathX value
- if multipath.is_mpath_member(volume_path):
- volume_path = '/dev/mapper/' + (
- multipath.get_mpath_id_from_device(volume_path))
- elif disk_key == 'device_id':
- dasd_device = dasd.DasdDevice(vol_value)
- volume_path = dasd_device.devname
- except ValueError:
- continue
- # verify path exists otherwise try the next key
- if os.path.exists(volume_path):
- break
- else:
- volume_path = None
-
- if volume_path is None:
- raise ValueError("Failed to find storage volume id='%s' config: %s"
- % (vol['id'], vol))
+ if getattr(storage_config, 'version', 1) > 1:
+ volume_path = v2_get_path_to_disk(vol)
+ else:
+ volume_path = v1_get_path_to_disk(vol)
elif vol.get('type') == "lvm_partition":
# For lvm partitions, a directory in /dev/ should be present with the
@@ -2017,8 +2098,8 @@ def meta_custom(args):
storage_config_dict = extract_storage_ordered_dict(cfg)
- version = cfg['storage']['version']
- if version > 1:
+ storage_config_dict.version = cfg['storage']['version']
+ if storage_config_dict.version > 1:
from curtin.commands.block_meta_v2 import (
disk_handler_v2,
partition_handler_v2,
diff --git a/curtin/udev.py b/curtin/udev.py
index 2b7a46e..11db908 100644
--- a/curtin/udev.py
+++ b/curtin/udev.py
@@ -129,4 +129,13 @@ def udevadm_info(path=None):
return info
+def udev_all_block_device_properties():
+ import pyudev
+ props = []
+ c = pyudev.context()
+ for device in c.list_devices(subsystem='block'):
+ props.append(dict(device.properties))
+ return props
+
+
# vi: ts=4 expandtab syntax=python
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 3e22792..48653e6 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -23,6 +23,10 @@ class TestGetPathToStorageVolume(CiTestCase):
self.add_patch(basepath + 'devsync', 'm_devsync')
self.add_patch(basepath + 'util.subp', 'm_subp')
self.add_patch(basepath + 'multipath.is_mpath_member', 'm_mp')
+ self.add_patch(
+ basepath + 'multipath.get_mpath_id_from_device', 'm_get_mpath_id')
+ self.add_patch(
+ basepath + 'udev_all_block_device_properties', 'm_udev_all')
def test_block_lookup_called_with_disk_wwn(self):
volume = 'mydisk'
@@ -57,6 +61,7 @@ class TestGetPathToStorageVolume(CiTestCase):
self.assertEqual(expected_calls, self.m_lookup.call_args_list)
def test_fallback_to_path_when_lookup_wwn_serial_fail(self):
+ self.m_mp.return_value = False
volume = 'mydisk'
wwn = self.random_string()
serial = self.random_string()
@@ -75,6 +80,7 @@ class TestGetPathToStorageVolume(CiTestCase):
self.assertEqual(path, result)
def test_block_lookup_not_called_with_wwn_or_serial_keys(self):
+ self.m_mp.return_value = False
volume = 'mydisk'
path = "/%s/%s" % (self.random_string(), self.random_string())
cfg = {'id': volume, 'type': 'disk', 'path': path}
@@ -106,6 +112,202 @@ class TestGetPathToStorageVolume(CiTestCase):
self.assertEqual(expected_calls, self.m_lookup.call_args_list)
self.m_exists.assert_has_calls([call(path)])
+ def test_v2_match_serial(self):
+ serial = self.random_string()
+ devname = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname, 'ID_SERIAL': serial},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'serial': serial}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_match_serial_skips_partition(self):
+ serial = self.random_string()
+ devname = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname, 'ID_SERIAL': serial},
+ {'DEVNAME': devname+'p1', 'ID_SERIAL': serial, 'PARTN': "1"},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'serial': serial}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_match_serial_prefer_mpath(self):
+ serial = self.random_string()
+ devname1 = self.random_string()
+ devname2 = self.random_string()
+ devname_dm = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname1, 'ID_SERIAL': serial},
+ {'DEVNAME': devname2, 'ID_SERIAL': serial},
+ {'DEVNAME': devname_dm, 'DM_SERIAL': serial},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'serial': serial}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname_dm,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_match_serial_mpath_skip_partition(self):
+ serial = self.random_string()
+ devname1 = self.random_string()
+ devname2 = self.random_string()
+ devname_dm = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname1, 'ID_SERIAL': serial},
+ {'DEVNAME': devname2, 'ID_SERIAL': serial},
+ {'DEVNAME': devname_dm, 'DM_SERIAL': serial},
+ {'DEVNAME': devname_dm+'p1', 'DM_SERIAL': serial, 'DM_PART': '1'},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'serial': serial}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname_dm,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_match_wwn(self):
+ wwn = self.random_string()
+ devname = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname, 'ID_WWN': wwn},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'wwn': wwn}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_match_wwn_prefer_mpath(self):
+ wwn = self.random_string()
+ devname1 = self.random_string()
+ devname2 = self.random_string()
+ devname_dm = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname1, 'ID_WWN': wwn},
+ {'DEVNAME': devname2, 'ID_WWN': wwn},
+ {'DEVNAME': devname_dm, 'DM_WWN': wwn},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'wwn': wwn}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname_dm,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_match_serial_and_wwn(self):
+ serial = self.random_string()
+ wwn = self.random_string()
+ devname = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname, 'ID_SERIAL': serial, 'ID_WWN': wwn},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'serial': serial, 'wwn': wwn}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_different_serial_same_wwn(self):
+ serial1 = self.random_string()
+ serial2 = self.random_string()
+ wwn = self.random_string()
+ devname1 = self.random_string()
+ devname2 = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname1, 'ID_SERIAL': serial1, 'ID_WWN': wwn},
+ {'DEVNAME': devname2, 'ID_SERIAL': serial2, 'ID_WWN': wwn},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'serial': serial2, 'wwn': wwn}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname2,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_fails_duplicate_wwn(self):
+ serial1 = self.random_string()
+ serial2 = self.random_string()
+ wwn = self.random_string()
+ devname1 = self.random_string()
+ devname2 = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname1, 'ID_SERIAL': serial1, 'ID_WWN': wwn},
+ {'DEVNAME': devname2, 'ID_SERIAL': serial2, 'ID_WWN': wwn},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'wwn': wwn}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ with self.assertRaises(ValueError):
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg)
+
+ def test_v2_match_path(self):
+ self.m_mp.return_value = False
+ devname = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'path': devname}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_match_path_link(self):
+ self.m_mp.return_value = False
+ devname = self.random_string()
+ link1 = self.random_string()
+ link2 = self.random_string()
+ links = link1 + ' ' + link2
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname, 'DEVLINKS': links},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'path': link1}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
+ def test_v2_match_path_prefers_multipath(self):
+ self.m_mp.return_value = True
+ self.m_get_mpath_id.return_value = 'mpatha'
+ devname1 = self.random_string()
+ devname2 = self.random_string()
+ self.m_udev_all.return_value = [
+ {'DEVNAME': devname1, 'DEVLINKS': ''},
+ {'DEVNAME': devname2, 'DEVLINKS': '/dev/mapper/mpatha'},
+ ]
+ disk_id = self.random_string()
+ cfg = {'id': disk_id, 'type': 'disk', 'path': devname1}
+ s_cfg = OrderedDict({disk_id: cfg})
+ s_cfg.version = 2
+ self.assertEqual(
+ devname2,
+ block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
class TestBlockMetaSimple(CiTestCase):
def setUp(self):
--
Mailing list: https://launchpad.net/~curtin-dev
Post to : curtin-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~curtin-dev
More help : https://help.launchpad.net/ListHelp
