Re: [PATCH] Recognize mpath devices when we see them.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2009-07-23 at 17:04 -0400, Peter Jones wrote:
> This identifies that a device is part of a multipath, and builds an
> mpath device for partitioning.

Several comments inline...

> ---
>  70-anaconda.rules            |    2 +-
>  storage/devicelibs/dm.py     |   18 ++++
>  storage/devices.py           |   33 ++++++--
>  storage/devicetree.py        |  188 +++++++++++++++++++++++++++++++++++++++---
>  storage/errors.py            |    3 +
>  storage/formats/multipath.py |   88 ++++++++++++++++++++
>  storage/udev.py              |   41 +++++++++
>  7 files changed, 352 insertions(+), 21 deletions(-)
>  create mode 100644 storage/formats/multipath.py
> 
> diff --git a/70-anaconda.rules b/70-anaconda.rules
> index 65d3141..a42cd57 100644
> --- a/70-anaconda.rules
> +++ b/70-anaconda.rules
> @@ -8,7 +8,7 @@ IMPORT{program}="$env{ANACBIN}/blkid -o udev -p $tempnode"
>  
>  KERNEL!="dm-*", GOTO="anaconda_mdraid"
>  
> -IMPORT{program}="$env{ANACBIN}/dmsetup info -c --nameprefixes --unquoted --rows --noheadings -o name,uuid,suspended,readonly,major,minor,open,tables_loaded -j%M -m%m"
> +IMPORT{program}="$env{ANACBIN}/dmsetup info -c --nameprefixes --unquoted --rows --noheadings -o name,uuid,suspended,readonly,major,minor,open,tables_loaded,names_using_dev -j%M -m%m"
>  ENV{DM_NAME}!="?*", GOTO="anaconda_end"
>  
>  SYMLINK+="disk/by-id/dm-name-$env{DM_NAME}"
> diff --git a/storage/devicelibs/dm.py b/storage/devicelibs/dm.py
> index 29df126..8d24467 100644
> --- a/storage/devicelibs/dm.py
> +++ b/storage/devicelibs/dm.py
> @@ -67,6 +67,14 @@ def dm_node_from_name(map_name):
>      log.debug("dm_node_from_name(%s) returning '%s'" % (map_name, dm_node))
>      return dm_node
>  
> +def dm_is_multipath(major, minor):
> +    for map in block.dm.maps():
> +        dev = map.dev
> +        if dev.major == int(major) and dev.minor == int(minor):
> +            for table in map.table:
> +                if " multipath " in table.params:
> +                    return True
> +
>  def _get_backing_devnums_from_map(map_name):
>      ret = []
>      buf = iutil.execWithCapture("dmsetup",
> @@ -105,3 +113,13 @@ def get_backing_devs_from_name(map_name):
>      slave_devs = os.listdir("/sys/block/virtual/%s" % dm_node)
>      return slave_devs
>  
> +def dm_is_multipath_disk(map_name):
> +    return False
> +
> +def dm_get_mutlipath_partition_disk(map_name):
> +    return None
> +
> +def dm_is_multipath_partition(map_name):
> +    return False
> +
> +
> diff --git a/storage/devices.py b/storage/devices.py
> index a4e996a..586ecc9 100644
> --- a/storage/devices.py
> +++ b/storage/devices.py
> @@ -416,7 +416,7 @@ class StorageDevice(Device):
>  
>      def __init__(self, device, format=None,
>                   size=None, major=None, minor=None,
> -                 sysfsPath='', parents=None, exists=None):
> +                 sysfsPath='', parents=None, exists=None, serial=None):
>          """ Create a StorageDevice instance.
>  
>              Arguments:
> @@ -447,6 +447,7 @@ class StorageDevice(Device):
>          self.minor = numeric_type(minor)
>          self.sysfsPath = sysfsPath
>          self.exists = exists
> +        self.serial = serial
>  
>          self.protected = False
>  
> @@ -2718,19 +2719,34 @@ class DMRaidArrayDevice(DiskDevice):
>          # information about it
>          self._size = self.currentSize
>  
> +class _MultipathDeviceNameGenerator:
> +    def __init__(self):
> +        self.number = 0
> +    def get(self):
> +        ret = self.number
> +        self.number += 1
> +        return ret
> +_multipathDeviceNameGenerator = _MultipathDeviceNameGenerator()
>  
> -class MultipathDevice(DMDevice):
> +def generateMultipathDeviceName():
> +    number = _multipathDeviceNameGenerator.get()
> +    return "mpath%s" % (number, )
> +
> +class MultipathDevice(DiskDevice):
>      """ A multipath device """
>      _type = "dm-multipath"
>      _packages = ["device-mapper-multipath"]
> +    _devDir = "/dev/mapper"
>  
> -    def __init__(self, name, format=None, size=None,
> -                 exists=None, parents=None, sysfsPath=''):
> +    def __init__(self, name, mpath, format=None, size=None,
> +                 parents=None, sysfsPath='', initcb=None,
> +                 initlabel=None):
>          """ Create a MultipathDevice instance.
>  
>              Arguments:
>  
>                  name -- the device name (generally a device node's basename)
> +                mpath -- the pyblock mpath device
>  
>              Keyword Arguments:
>  
> @@ -2738,12 +2754,13 @@ class MultipathDevice(DMDevice):
>                  size -- the device's size
>                  format -- a DeviceFormat instance
>                  parents -- a list of the backing devices (Device instances)
> -                exists -- indicates whether this is an existing device
> +                initcb -- the call back to be used when initiating disk.
> +                initlabel -- whether to start with a fresh disklabel
>          """
> -        DMDevice.__init__(self, name, format=format, size=size,
> +        self.mpath = mpath
> +        DiskDevice.__init__(self, name, format=format, size=size,
>                            parents=parents, sysfsPath=sysfsPath,
> -                          exists=exists)
> -
> +                          initcb=initcb, initlabel=initlabel)
>  
>  class NoDevice(StorageDevice):
>      """ A nodev device for nodev filesystems like tmpfs. """
> diff --git a/storage/devicetree.py b/storage/devicetree.py
> index 71d7a48..335c3ec 100644
> --- a/storage/devicetree.py
> +++ b/storage/devicetree.py
> @@ -31,6 +31,7 @@ from partitioning import shouldClear
>  from pykickstart.constants import *
>  import formats
>  import devicelibs.mdraid
> +import devicelibs.dm
>  from udev import *
>  from iutil import log_method_call
>  
> @@ -226,6 +227,7 @@ class DeviceTree(object):
>  
>          self.__passphrase = passphrase
>          self.__luksDevs = {}
> +        self.__multipaths = []
>          if luksDict and isinstance(luksDict, dict):
>              self.__luksDevs = luksDict
>          self._ignoredDisks = []
> @@ -978,16 +980,25 @@ class DeviceTree(object):
>              # try to get the device again now that we've got all the slaves
>              device = self.getDeviceByName(name)
>  
> -            if device is None and \
> -                    udev_device_is_dmraid_partition(info, self):
> -                diskname = udev_device_get_dmraid_partition_disk(info)
> -                disk = self.getDeviceByName(diskname)
> -                device = PartitionDevice(name, sysfsPath=sysfs_path,
> -                                         major=udev_device_get_major(info),
> -                                         minor=udev_device_get_minor(info),
> -                                         exists=True, parents=[disk])
> -                # DWL FIXME: call self.addUdevPartitionDevice here instead
> -                self._addDevice(device)
> +            if device is None:
> +                if udev_device_is_multipath_partition(info, self):
> +                    diskname = udev_device_get_multipath_partition_disk(info)
> +                    disk = self.getDeviceByName(diskname)
> +                    device = PartitionDevice(name, sysfsPath=sysfs_path,
> +                                             major=udev_device_get_major(info),
> +                                             minor=udev_device_get_minor(info),
> +                                             exists=True, parents=[disk])
> +                elif udev_device_is_dmraid_partition(info, self):
> +                    diskname = udev_device_get_dmraid_partition_disk(info)
> +                    disk = self.getDeviceByName(diskname)
> +                    device = PartitionDevice(name, sysfsPath=sysfs_path,
> +                                             major=udev_device_get_major(info),
> +                                             minor=udev_device_get_minor(info),
> +                                             exists=True, parents=[disk])
> +                if not device is None:
> +                    # DWL FIXME: call self.addUdevPartitionDevice here instead
> +                    self._addDevice(device)
> +
>  
>              # if we get here, we found all of the slave devices and
>              # something must be wrong -- if all of the slaves are in
> @@ -1100,6 +1111,7 @@ class DeviceTree(object):
>          log_method_call(self, name=name)
>          uuid = udev_device_get_uuid(info)
>          sysfs_path = udev_device_get_sysfs_path(info)
> +        serial = udev_device_get_serial(info)
>          device = None
>  
>          kwargs = {}
> @@ -1187,7 +1199,17 @@ class DeviceTree(object):
>          #
>          # The first step is to either look up or create the device
>          #
> -        if udev_device_is_dm(info):
> +        if udev_device_is_multipath_member(info):
> +            device = StorageDevice(name,
> +                            major=udev_device_get_major(info),
> +                            minor=udev_device_get_minor(info),
> +                            sysfsPath=sysfs_path, exists=True,
> +                            serial=udev_device_get_serial(info))
> +            self._addDevice(device)
> +        elif udev_device_is_dm(info) and \
> +               devicelibs.dm.dm_is_multipath(info["DM_MAJOR"], info["DM_MINOR"]):
> +            log.debug("%s is a multipath device" % name)
> +        elif udev_device_is_dm(info):
>              log.debug("%s is a device-mapper device" % name)
>              # try to look up the device
>              if device is None and uuid:
> @@ -1407,6 +1429,68 @@ class DeviceTree(object):
>              md_array._addDevice(device)
>              self._addDevice(md_array)
>  
> +    def handleMultipathMemberFormat(self, info, device):
> +        log_method_call(self, name=device.name, type=device.format.type)
> +
> +        def _get_multipath(multipaths, serial):
> +            for mp in multipaths:
> +                for name, vals in mp.items():
> +                    if vals['info']['ID_SERIAL'] == serial:
> +                        return mp
> +
> +        def _multipath_is_complete(multipath):
> +            complete = True
> +            for name, vals in multipath.items():
> +                if vals['found'] == False:
> +                    complete = False
> +                    break
> +            return complete
> +
> +        def _multipath_mark_device_found(multipath, device):
> +            multipath[device.name]['found'] = True
> +
> +        serial = udev_device_get_serial(info)
> +        multipath = _get_multipath(self.__multipaths, serial)
> +        _multipath_mark_device_found(multipath, device)
> +        if _multipath_is_complete(multipath):
> +            members = self.getDevicesBySerial(serial)

All of the utility functions above seem like stuff that should be either
somewhere in storage/devicelibs/ or, better yet, inside MultipathDevice.

> +
> +            # this whole block is lame and needs fixed -- we already have
> +            # identified the multipath device, we don't need pyblock's probe,
> +            # merely to assemble.  -- pjfix
> +            import block as _block
> +            _bdModulePath = ":/tmp/updates/bdevid/:/mnt/source/RHupdates/bdevid/"
> +            _block.setBdevidPath(_block.getBdevidPath() + _bdModulePath)
> +            _block.load("scsi")

It would be great if this could be somewhere else, like in
storage/devicelibs/.

> +
> +            probeDevices = []
> +            for d in members:
> +                probeDevices.append("/dev/%s" % (d.name,))
> +
> +            mpaths = _block.getMPaths(probeDevices)
> +            name = generateMultipathDeviceName()
> +            if self.zeroMbr:
> +                cb = lambda: True
> +            else:
> +                cb = lambda: questionInitializeDisk(self.intf, name)
> +            initlabel = False
> +            if not self.clearPartDisks or \
> +                    rs.name in self.clearPartDisks:
> +                initlabel = self.reinitializeDisks
> +                for protected in self.protectedDevNames:
> +                    disk_name = re.sub(r'p\d+$', '', protected)
> +                    if disk_name != protected and \
> +                            disk_name == name:
> +                        initlabel = False
> +                        break
> +            mpath = MultipathDevice(name,
> +                                    mpath=mpaths[0],
> +                                    #parents=probeDevices,
> +                                    initcb=cb,
> +                                    initlabel=initlabel)
> +            log.info("adding mpath device %s [%s]" % (name, probeDevices))
> +            self._addDevice(mpath)
> +
>      def handleUdevDMRaidMemberFormat(self, info, device):
>          log_method_call(self, name=device.name, type=device.format.type)
>          name = udev_device_get_name(info)
> @@ -1498,6 +1582,7 @@ class DeviceTree(object):
>          uuid = udev_device_get_uuid(info)
>          label = udev_device_get_label(info)
>          format_type = udev_device_get_format(info)
> +        serial = udev_device_get_serial(info)
>  
>          format = None
>          if (not device) or (not format_type) or device.format.type:
> @@ -1511,10 +1596,13 @@ class DeviceTree(object):
>          kwargs = {"uuid": uuid,
>                    "label": label,
>                    "device": device.path,
> +                  "serial": serial,
>                    "exists": True}
>  
>          # set up type-specific arguments for the format constructor
> -        if format_type == "crypto_LUKS":
> +        if format_type == "multipath_member":
> +            kwargs["multipath_members"] = self.getDevicesBySerial(serial)
> +        elif format_type == "crypto_LUKS":
>              # luks/dmcrypt
>              kwargs["name"] = "luks-%s" % uuid
>          elif format_type in formats.mdraid.MDRaidMember._udevTypes:
> @@ -1584,6 +1672,8 @@ class DeviceTree(object):
>              self.handleUdevDMRaidMemberFormat(info, device)
>          elif device.format.type == "lvmpv":
>              self.handleUdevLVMPVFormat(info, device)
> +        elif device.format.type == "multipath_member":
> +            self.handleMultipathMemberFormat(info, device)
>  
>      def _handleInconsistencies(self):
>          def reinitializeVG(vg):
> @@ -1702,6 +1792,50 @@ class DeviceTree(object):
>                          (disk.name, disk.format, format))
>                  disk.format = format
>  
> +    def identifyMultipaths(self, devices):
> +        log.info("devices to scan for multipath: %s" % [d['name'] for d in devices])
> +        serials = {}
> +        non_disk_devices = {}
> +        for d in devices:
> +            serial = udev_device_get_serial(d)
> +            if not udev_device_is_disk(d):
> +                non_disk_devices.setdefault(serial, [])
> +                non_disk_devices[serial].append(d)
> +                log.info("adding %s to non_disk_device list" % (d['name'],))
> +                continue
> +
> +            serials.setdefault(serial, [])
> +            serials[serial].append(d)
> +
> +        singlepath_disks = []
> +        multipath_disks = []
> +        for serial, disks in serials.items():
> +            if len(disks) == 1:
> +                log.info("adding %s to singlepath_disks" % (disks[0]['name'],))
> +                singlepath_disks.append(disks[0])
> +            else:
> +                multipath_members = {}
> +                for d in disks:
> +                    log.info("adding %s to multipath_disks" % (d['name'],))
> +                    multipath_disks.append(d)
> +                    d["ID_FS_TYPE"] = "multipath_member"
> +
> +                    multipath_members[d['name']] = { 'info': d,
> +                                                     'found': False }
> +                    self.__multipaths.append(multipath_members)
> +                    log.info("found multipath set: [%s]" % [d['name'] for d in disks])
> +
> +        for serial in [d['ID_SERIAL'] for d in multipath_disks]:
> +            if non_disk_devices.has_key(serial):
> +                    log.info("filtering out non disk devices [%s]" % [d['name'] for d in non_disk_devices[serial]])
> +                    del non_disk_devices[serial]
> +
> +        partition_devices = []
> +        for devices in non_disk_devices.values():
> +            partition_devices += devices
> +
> +        return partition_devices + multipath_disks + singlepath_disks
> +
>      def populate(self):
>          """ Locate all storage devices. """
>  
> @@ -1719,6 +1853,7 @@ class DeviceTree(object):
>          # previous iteration
>          old_devices = []
>          ignored_devices = []
> +        first_iteration = True
>          while True:
>              devices = []
>              new_devices = udev_get_block_devices()
> @@ -1738,6 +1873,9 @@ class DeviceTree(object):
>                  break
>  
>              old_devices = new_devices
> +            if first_iteration:
> +                devices = self.identifyMultipaths(devices)
> +                first_iteration = False
>              log.info("devices to scan: %s" % [d['name'] for d in devices])
>              for dev in devices:
>                  self.addUdevDevice(dev)
> @@ -1797,6 +1935,16 @@ class DeviceTree(object):
>  
>          return found
>  
> +    def getDevicesBySerial(self, serial):
> +        devices = []
> +        for device in self._devices:
> +            if not hasattr(device, "serial"):
> +                log.warning("device %s has no serial attr" % device.name)
> +                continue
> +            if device.serial == serial:
> +                devices.append(device)
> +        return devices
> +

Should the above make sure serial is set to something?

>      def getDeviceByLabel(self, label):
>          if not label:
>              return None
> @@ -1904,6 +2052,22 @@ class DeviceTree(object):
>          return uuids
>  
>      @property
> +    def serials(self):
> +        """ Dict with serial keys and Device values. """
> +        serials = {}
> +        for dev in self._devices:
> +            try:
> +                serial = dev.serial
> +            except AttributeError:
> +                serial = None
> +
> +            if serial:
> +                serials[serial] = dev
> +
> +        return serials
> +

Aren't there going to be multiple devices that share a serial in the
case of multipath? If so, the dict values should be lists (or
something).

> +
> +    @property
>      def labels(self):
>          """ Dict with label keys and Device values.
>  
> diff --git a/storage/errors.py b/storage/errors.py
> index 9dd121c..e0175fc 100644
> --- a/storage/errors.py
> +++ b/storage/errors.py
> @@ -64,6 +64,9 @@ class FormatTeardownError(DeviceFormatError):
>  class DMRaidMemberError(DeviceFormatError):
>      pass
>  
> +class MultipathMemberError(DeviceFormatError):
> +    pass
> +
>  class FSError(DeviceFormatError):
>      pass
>  
> diff --git a/storage/formats/multipath.py b/storage/formats/multipath.py
> new file mode 100644
> index 0000000..cad2552
> --- /dev/null
> +++ b/storage/formats/multipath.py
> @@ -0,0 +1,88 @@
> +# multipath.py
> +# multipath device formats
> +#
> +# Copyright (C) 2009  Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +# Any Red Hat trademarks that are incorporated in the source code or
> +# documentation are not subject to the GNU General Public License and
> +# may only be used or replicated with the express permission of
> +# Red Hat, Inc.
> +#
> +# Red Hat Author(s): Peter Jones <pjones@xxxxxxxxxx>
> +#
> +
> +from iutil import log_method_call
> +from ..errors import *
> +from . import DeviceFormat, register_device_format
> +
> +import gettext
> +_ = lambda x: gettext.ldgettext("anaconda", x)
> +
> +import logging
> +log = logging.getLogger("storage")
> +
> +class MultipathMember(DeviceFormat):
> +    """ A multipath member disk. """
> +    _type = "multipath_member"
> +    _name = "multipath member device"
> +    _udev_types = ["multipath_member"]
> +    _formattable = False                # can be formatted
> +    _supported = True                   # is supported
> +    _linuxNative = False                # for clearpart
> +    _packages = ["device-mapper-multipath"] # required packages
> +    _resizable = False                  # can be resized
> +    _bootable = False                   # can be used as boot
> +    _maxSize = 0                        # maximum size in MB
> +    _minSize = 0                        # minimum size in MB
> +
> +    def __init__(self, *args, **kwargs):
> +        """ Create a DeviceFormat instance.
> +
> +            Keyword Arguments:
> +
> +                device -- path to the underlying device
> +                uuid -- this format's UUID
> +                exists -- indicates whether this is an existing format
> +
> +            On initialization this format is like DeviceFormat
> +
> +        """
> +        log_method_call(self, *args, **kwargs)
> +        DeviceFormat.__init__(self, *args, **kwargs)
> +
> +        # Initialize the attribute that will hold the block object.
> +        self._member = None
> +
> +    @property
> +    def member(self):
> +        return self._member
> +
> +    @member.setter
> +    def member(self, member):
> +        self._member = member
> +
> +    def create(self, *args, **kwargs):
> +        log_method_call(self, device=self.device,
> +                        type=self.type, status=self.status)
> +        raise MultipathMemberError("creation of multipath members is non-sense")
> +
> +    def destroy(self, *args, **kwargs):
> +        log_method_call(self, device=self.device,
> +                        type=self.type, status=self.status)
> +        raise MultipathMemberError("destruction of multipath members is non-sense")
> +
> +register_device_format(MultipathMember)
> +
> diff --git a/storage/udev.py b/storage/udev.py
> index e97d5d5..ba08f75 100644
> --- a/storage/udev.py
> +++ b/storage/udev.py
> @@ -22,6 +22,7 @@
>  
>  import os
>  import stat
> +import block
>  
>  import iutil
>  from errors import *
> @@ -214,6 +215,10 @@ def udev_device_get_label(udev_info):
>      """ Get the label from the device's format as reported by udev. """
>      return udev_info.get("ID_FS_LABEL")
>  
> +def udev_device_is_multipath_member(info):
> +    """ Return True if the device is part of a multipath. """
> +    return info.get("ID_FS_TYPE") == "multipath_member"
> +
>  def udev_device_is_dm(info):
>      """ Return True if the device is a device-mapper device. """
>      return info.has_key("DM_NAME")
> @@ -238,6 +243,10 @@ def udev_device_is_partition(info):
>      has_start = os.path.exists("/sys/%s/start" % info['sysfs_path'])
>      return info.get("DEVTYPE") == "partition" or has_start
>  
> +def udev_device_get_serial(udev_info):
> +    """ Get the serial number/UUID from the device as reported by udev. """
> +    return udev_info.get("ID_SERIAL")
> +
>  def udev_device_get_sysfs_path(info):
>      return info['sysfs_path']
>  
> @@ -356,6 +365,38 @@ def udev_device_is_dmraid_partition(info, devicetree):
>  
>      return False
>  
> +def udev_device_is_multipath_partition(info, devicetree):
> +    """ 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"):
> +        return False
> +    diskname = udev_device_get_dmraid_partition_disk(info)
> +    if diskname != info["DM_NAME"]:
> +        return True
> +
> +    return False

Looks like you could at least drop the unused code below and stop
passing in the device tree here.

> +    
> +    
> +    mpath_devices = devicetree.getDevicesByType("multipath")
> +
> +    for device in mpath_devices:
> +        if diskname == device.name:
> +            return True
> +
> +    return False
> +
> +def udev_device_get_multipath_partition_disk(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"):
> +        return False
> +    diskname = udev_device_get_dmraid_partition_disk(info)
> +    return diskname
> +
>  # iscsi disks have ID_PATH in the form of:
>  # ip-${iscsi_address}:${iscsi_port}-iscsi-${iscsi_tgtname}-lun-${lun}
>  def udev_device_is_iscsi(info):

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux