curtin-dev team mailing list archive

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

Diff comments:

> 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
> @@ -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:

Yeah I went around the block a few times on which bits to put in helper functions. Its slightly repetitive but the current version seemed more readable to me.

> +        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)

good point.

> +
> +    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))

cands is a set by this point so cands[0] doesn't work. I changed this to something less inscrutable though.

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


-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/415700
Your team curtin developers is subscribed to branch curtin:master.