Re: [rhel6-branch 1/5] Use dm subsystem functions to identify dmraid,mpath partitions.

[David Lehman <dlehman redhat com>]

My only comment is that the label for the friendly mpath names might be
more useful if it mentioned the benefit of using non-friendly names.
Otherwise, ack.

Dave


On Thu, 2011-06-16 at 15:39 +0200, Ales Kozumplik wrote:
> From: David Lehman <dlehman redhat com>
> 
> Reimplements udev_device_is_dmraid_partition and
> udev_device_is_multipath_partition so they no longer require a
> DeviceTree instance.
> 
> Related: rhbz#709653
> ---
>  storage/devicetree.py |    8 ++++----
>  storage/udev.py       |   43 ++++++++++---------------------------------
>  2 files changed, 14 insertions(+), 37 deletions(-)
> 
> diff --git a/storage/devicetree.py b/storage/devicetree.py
> index 3cfab88..773667b 100644
> --- a/storage/devicetree.py
> +++ b/storage/devicetree.py
> @@ -1027,12 +1027,12 @@ class DeviceTree(object):
>              device = self.getDeviceByName(name)
>  
>              if device is None:
> -                if udev_device_is_multipath_partition(info, self):
> -                    diskname = udev_device_get_multipath_partition_disk(info)
> +                if udev_device_is_multipath_partition(info):
> +                    diskname = udev_device_get_dm_partition_disk(info)
>                      disk = self.getDeviceByName(diskname)
>                      return self.addUdevPartitionDevice(info, disk=disk)
> -                elif udev_device_is_dmraid_partition(info, self):
> -                    diskname = udev_device_get_dmraid_partition_disk(info)
> +                elif udev_device_is_dmraid_partition(info):
> +                    diskname = udev_device_get_dm_partition_disk(info)
>                      disk = self.getDeviceByName(diskname)
>                      return self.addUdevPartitionDevice(info, disk=disk)
>  
> diff --git a/storage/udev.py b/storage/udev.py
> index b1383b5..de91c6d 100644
> --- a/storage/udev.py
> +++ b/storage/udev.py
> @@ -457,7 +457,7 @@ def udev_device_is_biosraid(info):
>  
>      return False
>  
> -def udev_device_get_dmraid_partition_disk(info):
> +def udev_device_get_dm_partition_disk(info):
>      try:
>          p_index = info["DM_NAME"].rindex("p")
>      except (KeyError, AttributeError, ValueError):
> @@ -468,43 +468,20 @@ def udev_device_get_dmraid_partition_disk(info):
>  
>      return info["DM_NAME"][:p_index]
>  
> -def udev_device_is_dmraid_partition(info, devicetree):
> -    diskname = udev_device_get_dmraid_partition_disk(info)
> -    dmraid_devices = devicetree.getDevicesByType("dm-raid array")
> -
> -    for device in dmraid_devices:
> -        if diskname == device.name:
> -            return True
> -
> -    return False
> -
> -def udev_device_is_multipath_partition(info, devicetree):
> -    """ Return True if the device is a partition of a multipath device. """
> -    if not udev_device_is_dm(info):
> -        return False
> -    if not info["DM_NAME"].startswith("mpath"):
> +def udev_device_is_dmraid_partition(info):
> +    if not udev_device_is_dm_raid(info):
>          return False
> -    diskname = udev_device_get_dmraid_partition_disk(info)
> -    if diskname is None:
> -        return False
> -
> -    # this is sort of a lame check, but basically, if diskname gave us "mpath0"
> -    # and we start with "mpath" but we're not "mpath0", then we must be
> -    # "mpath0" plus some non-numeric crap.
> -    if diskname != info["DM_NAME"]:
> -        return True
>  
> -    return False
> +    diskname = udev_device_get_dm_partition_disk(info)
> +    return diskname not in ("", None)
>  
> -def udev_device_get_multipath_partition_disk(info):
> +def udev_device_is_multipath_partition(info):
>      """ Return True if the device is a partition of a multipath device. """
> -    # XXX PJFIX This whole function is crap.
> -    if not udev_device_is_dm(info):
> -        return False
> -    if not info["DM_NAME"].startswith("mpath"):
> +    if not udev_device_is_dm_mpath(info):
>          return False
> -    diskname = udev_device_get_dmraid_partition_disk(info)
> -    return diskname
> +
> +    diskname = udev_device_get_dm_partition_disk(info)
> +    return diskname not in ("", None)
>  
>  def udev_device_is_multipath_member(info):
>      """ Return True if the device is part of a multipath. """