Ryan Harper has proposed merging ~raharper/curtin:fix/block-meta-get-device-paths-skip-missing into curtin:master.
Commit message: block-meta: skip wipe device paths if not present Curtin extracts the devices from storage config with wipe enabled. These devices may not exist yet (curtin will be creating them) so there's no need to clear them, instead drop them from the list of devices to send to clear-holders. Other changes: multipath: - Clarify multipath logging on device type checking block-meta: - Stop iterating through disk lookup keys after we find one - Use udevadm_settle after parted partition creation before calling kpartx to prevent race with kernel partition update clear-holders: - Log the inputs to clear-holders block: - Log a warning when sys_block_path is called on a non-existent device path. - Log the return value from block.lookup_disk() LP: #1869075 Requested reviews: curtin developers (curtin-dev) Related bugs: Bug #1869075 in curtin: "lvcreate fails with duplicate paths to physical volume" https://bugs.launchpad.net/curtin/+bug/1869075 For more details, see: https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/382718 -- Your team curtin developers is requested to review the proposed merge of ~raharper/curtin:fix/block-meta-get-device-paths-skip-missing into curtin:master.
diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py index 54ee263..a7fe22f 100644 --- a/curtin/block/__init__.py +++ b/curtin/block/__init__.py @@ -143,6 +143,8 @@ def sys_block_path(devname, add=None, strict=True): toks = ['/sys/class/block'] # insert parent dev if devname is partition devname = os.path.normpath(devname) + if devname.startswith('/dev/') and not os.path.exists(devname): + LOG.warning('block.sys_block_path: devname %s does not exist', devname) (parent, partnum) = get_blockdev_for_partition(devname, strict=strict) if partnum: toks.append(path_to_kname(parent)) @@ -906,6 +908,7 @@ def lookup_disk(serial): if not os.path.exists(path): raise ValueError("path '%s' to block device for disk with serial '%s' \ does not exist" % (path, serial_udev)) + LOG.debug('block.lookup_disk() returning path %s', path) return path diff --git a/curtin/block/clear_holders.py b/curtin/block/clear_holders.py index 14ad644..279fad8 100644 --- a/curtin/block/clear_holders.py +++ b/curtin/block/clear_holders.py @@ -605,6 +605,7 @@ def clear_holders(base_paths, try_preserve=False): # handle single path if not isinstance(base_paths, (list, tuple)): base_paths = [base_paths] + LOG.info('Generating device storage trees for path(s): %s', base_paths) # get current holders and plan how to shut them down holder_trees = [gen_holders_tree(path) for path in base_paths] diff --git a/curtin/block/multipath.py b/curtin/block/multipath.py index a488e85..21fc0bd 100644 --- a/curtin/block/multipath.py +++ b/curtin/block/multipath.py @@ -52,35 +52,40 @@ def dmname_to_blkdev_mapping(): def is_mpath_device(devpath, info=None): """ Check if devpath is a multipath device, returns boolean. """ + result = False if not info: info = udev.udevadm_info(devpath) if info.get('DM_UUID', '').startswith('mpath-'): - LOG.debug('%s is multipath device', devpath) - return True + result = True - return False + LOG.debug('%s is multipath device? %s', devpath, result) + return result def is_mpath_member(devpath, info=None): """ Check if a device is a multipath member (a path), returns boolean. """ + result = False try: util.subp(['multipath', '-c', devpath], capture=True) - LOG.debug('%s is multipath device member', devpath) - return True + result = True except util.ProcessExecutionError: - return False + pass + + LOG.debug('%s is multipath device member? %s', devpath, result) + return result def is_mpath_partition(devpath, info=None): """ Check if a device is a multipath partition, returns boolean. """ + result = False if devpath.startswith('/dev/dm-'): if not info: info = udev.udevadm_info(devpath) if 'DM_PART' in udev.udevadm_info(devpath): - LOG.debug("%s is multipath device partition", devpath) - return True + result = True - return False + LOG.debug("%s is multipath device partition? %s", devpath, result) + return result def mpath_partition_to_mpath_id(devpath): diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py index 36d3146..7fbd89f 100644 --- a/curtin/commands/block_meta.py +++ b/curtin/commands/block_meta.py @@ -465,7 +465,9 @@ def get_path_to_storage_volume(volume, storage_config): except ValueError: continue # verify path exists otherwise try the next key - if not os.path.exists(volume_path): + if os.path.exists(volume_path): + break + else: volume_path = None if volume_path is None: @@ -906,8 +908,9 @@ def partition_handler(info, storage_config): # ensure partition exists if multipath.is_mpath_device(disk): + udevadm_settle() # allow partition creation to happen # update device mapper table mapping to mpathX-partN - util.subp(['kpartx', '-v', '-a', '-p-part', disk]) + util.subp(['kpartx', '-v', '-a', '-s', '-p', '-part', disk]) part_path = disk + "-part%s" % partnumber else: part_path = block.dev_path(block.partition_kname(disk_kname, @@ -1697,7 +1700,7 @@ def zfs_handler(info, storage_config): def get_device_paths_from_storage_config(storage_config): """Returns a list of device paths in a storage config which have wipe - config enabled. + config enabled filtering out constructed paths that do not exist. :param: storage_config: Ordered dict of storage configation """ @@ -1706,8 +1709,10 @@ def get_device_paths_from_storage_config(storage_config): if v.get('type') in ['disk', 'partition']: if config.value_as_boolean(v.get('wipe')): try: - dpaths.append( - get_path_to_storage_volume(k, storage_config)) + # skip paths that do not exit, nothing to wipe + dpath = get_path_to_storage_volume(k, storage_config) + if os.path.exists(dpath): + dpaths.append(dpath) except Exception: pass return dpaths diff --git a/examples/tests/multipath-lvm.yaml b/examples/tests/multipath-lvm.yaml index 1215d8b..68c3271 100644 --- a/examples/tests/multipath-lvm.yaml +++ b/examples/tests/multipath-lvm.yaml @@ -69,24 +69,28 @@ storage: name: root_disk multipath: mpatha serial: disk-a + path: /dev/sda grub_device: true - wipe: superblock-recursive + wipe: superblock - id: boot_bios type: partition size: 1MB number: 1 device: main_disk flag: bios_grub + wipe: superblock - id: boot_partition type: partition size: 1GB number: 2 device: main_disk + wipe: superblock - id: main_disk_p3 type: partition number: 3 size: 4GB device: main_disk + wipe: superblock - id: root_vg type: lvm_volgroup name: root_vg diff --git a/examples/tests/multipath.yaml b/examples/tests/multipath.yaml index 4bafe2d..11838d1 100644 --- a/examples/tests/multipath.yaml +++ b/examples/tests/multipath.yaml @@ -11,17 +11,21 @@ storage: name: mpath_a wipe: superblock grub_device: true + multipath: mpatha + path: /dev/sda - id: sda1 type: partition number: 1 size: 3GB device: sda flag: boot + wipe: superblock - id: sda2 type: partition number: 2 size: 1GB device: sda + wipe: superblock - id: sda1_root type: format fstype: ext4 diff --git a/tests/unittests/test_block.py b/tests/unittests/test_block.py index dcdcf51..2b0116b 100644 --- a/tests/unittests/test_block.py +++ b/tests/unittests/test_block.py @@ -658,7 +658,7 @@ class TestSlaveKnames(CiTestCase): # construct side effects to os.path.exists # and os.listdir based on mapping. dirs = [] - exists = [] + exists = [True] if device.startswith('/dev') else [] for (dev, slvs) in cfg.items(): # sys_block_dev checks if dev exists exists.append(True)
-- 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