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

Re: [master] Provide a new mpath devicelib interface that does not reorder the devices.



On Mon, 2011-01-17 at 12:54 +0100, Ales Kozumplik wrote:
> This makes DeviceTree.populate()'s task simpler by providing the devices
> in the same order udev reports them so we do not have to iterate through
> the device list so many times (and force skipping some of the devices
> during the first go).
> 
> I also hope the methods' input/output is clearer now.

I might prefer to call it MultipathTopology instead of DeviceTopology,
but otherwise it looks good to me.

Dave

> ---
>  pyanaconda/iw/filter_gui.py            |   24 ++++----
>  pyanaconda/storage/devicelibs/mpath.py |  102 ++++++++++++++++++++++++++++++++
>  pyanaconda/storage/devicetree.py       |   12 +---
>  3 files changed, 119 insertions(+), 19 deletions(-)
> 
> diff --git a/pyanaconda/iw/filter_gui.py b/pyanaconda/iw/filter_gui.py
> index 76f0a72..cca6581 100644
> --- a/pyanaconda/iw/filter_gui.py
> +++ b/pyanaconda/iw/filter_gui.py
> @@ -33,7 +33,8 @@ from pyanaconda.constants import *
>  from iw_gui import *
>  from pyanaconda.storage.devices import devicePathToName
>  from pyanaconda.storage.udev import *
> -from pyanaconda.storage.devicelibs.mpath import *
> +from pyanaconda.storage.devicelibs.mpath\
> +    import DeviceTopology, MultipathConfigWriter
>  from pyanaconda.flags import flags
>  from pyanaconda.storage import iscsi
>  from pyanaconda.storage import fcoe
> @@ -409,7 +410,7 @@ class FilterWindow(InstallWindow):
>          return True
>  
>      def _getFilterDisks(self):
> -        """ Return a list of disks to pass to identifyMultipaths. """
> +        """ Return a list of disks to pass to DeviceTopology. """
>          return filter(lambda d: udev_device_is_disk(d) and \
>                                  not udev_device_is_dm(d) and \
>                                  not udev_device_is_md(d) and \
> @@ -451,17 +452,17 @@ class FilterWindow(InstallWindow):
>          del cfg
>          del mcw
>  
> -        (new_singlepaths, new_mpaths, new_partitions) = identifyMultipaths(new_disks)
> +        topology = DeviceTopology(new_disks)
>          (new_raids, new_nonraids) = self.split_list(lambda d: isRAID(d) and not isCCISS(d),
> -                                                    new_singlepaths)
> +                                                    topology.singlepaths_iter())
>  
>          nonraids = filter(lambda d: d not in self._cachedDevices, new_nonraids)
>          raids = filter(lambda d: d not in self._cachedRaidDevices, new_raids)
>  
>          # The end result of the loop below is that mpaths is a list of lists of
> -        # components, just like new_mpaths.  That's what populate expects.
> +        # components. That's what populate expects.
>          mpaths = []
> -        for mp in new_mpaths:
> +        for mp in topology.multipaths_iter():
>              for d in mp:
>                  # If all components of this multipath device are in the
>                  # cache, skip it.  Otherwise, it's a new device and needs to
> @@ -601,12 +602,13 @@ class FilterWindow(InstallWindow):
>          open("/etc/multipath.conf", "w+").write(cfg)
>          del cfg
>          del mcw
> -        (singlepaths, mpaths, partitions) = identifyMultipaths(disks)
>  
> +        topology = DeviceTopology(disks)
>          # The device list could be really long, so we really only want to
>          # iterate over it the bare minimum of times.  Dividing this list up
>          # now means fewer elements to iterate over later.
> -        singlepaths = filter(lambda info: self._device_size_is_nonzero(info), singlepaths)
> +        singlepaths = filter(lambda info: self._device_size_is_nonzero(info),
> +                             topology.singlepaths_iter())
>          (raids, nonraids) = self.split_list(lambda d: isRAID(d) and not isCCISS(d),
>                                              singlepaths)
>  
> @@ -614,7 +616,7 @@ class FilterWindow(InstallWindow):
>                        self._makeMPath(), self._makeOther(),
>                        self._makeSearch()]
>  
> -        self.populate(nonraids, mpaths, raids)
> +        self.populate(nonraids, topology.multipaths_iter(), raids)
>  
>          # If the "Add Advanced" button is ever clicked, we need to have a list
>          # of what devices previously existed so we know what's new.  Then we
> @@ -627,8 +629,8 @@ class FilterWindow(InstallWindow):
>          # lists, we can't directly store that into the cache.  Instead we want
>          # to flatten it into a single list of all components of all multipaths
>          # and store that.
> -        lst = list(itertools.chain(*mpaths))
> -        self._cachedMPaths = NameCache(lst)
> +        mpath_chain = itertools.chain(*topology.multipaths_iter())
> +        self._cachedMPaths = NameCache(mpath_chain)
>  
>          # Switch to the first notebook page that displays any devices.
>          i = 0
> diff --git a/pyanaconda/storage/devicelibs/mpath.py b/pyanaconda/storage/devicelibs/mpath.py
> index 14d2bda..c73b172 100644
> --- a/pyanaconda/storage/devicelibs/mpath.py
> +++ b/pyanaconda/storage/devicelibs/mpath.py
> @@ -3,6 +3,7 @@ import re
>  
>  from ..udev import *
>  from pyanaconda import iutil
> +from pyanaconda.anaconda_log import log_method_call
>  
>  import logging
>  log = logging.getLogger("storage")
> @@ -225,6 +226,106 @@ def identifyMultipaths(devices):
>      log.info("devices post multipath scan: %s" % s)
>      return (singlepath_disks, multipaths, partition_devices)
>  
> +
> +class DeviceTopology(object):
> +    def __init__(self, devices_list):
> +        self._devices = devices_list
> +        self._nondisks = []
> +        self._singlepaths = []
> +        self._multipaths = [] # mpath members
> +        self._devmap = {}
> +
> +        self._build_topology()
> +
> +    def _build_devmap(self):
> +        self._devmap = {}
> +        for dev in self._devices:
> +            self._devmap[dev['name']] = dev
> +
> +    def _build_mpath_topology(self):
> +        self._mpath_topology = parseMultipathOutput(
> +            iutil.execWithCapture("multipath", ["-d",]))
> +        self._mpath_topology.update(parseMultipathOutput(
> +                iutil.execWithCapture("multipath", ["-ll",])))
> +
> +        delete_keys = []
> +        for (mp, disks) in self._mpath_topology.items():
> +            # single device mpath is not really an mpath, eliminate them:
> +            if len(disks) < 2:
> +                log.info("DeviceTopology: not a multipath: %s" % disks)
> +                delete_keys.append(mp)
> +                continue
> +            # some usb cardreaders use multiple lun's (for different slots) and
> +            # report a fake disk serial which is the same for all the lun's
> +            # (#517603). find those mpaths and eliminate them:
> +            only_non_usbs = [d for d in disks if
> +                             self._devmap[d].get("ID_USB_DRIVER") != "usb-storage"]
> +            if len(only_non_usbs) == 0:
> +                log.info("DeviceToppology: found multi lun usb "
> +                         "mass storage device: %s" % disks)
> +                delete_keys.append(mp)
> +        map(lambda key: self._mpath_topology.pop(key), delete_keys)
> +
> +    def _build_topology(self):
> +        log_method_call(self)
> +        self._build_devmap()
> +        self._build_mpath_topology()
> +
> +        for dev in self._devices:
> +            name = dev['name']
> +            if not udev_device_is_disk(dev):
> +                self._nondisks.append(name)
> +                log.info("DeviceTopology: found non-disk device: %s" % name)
> +                continue
> +            mpath_name = self.multipath_name(name)
> +            if mpath_name:
> +                dev["ID_FS_TYPE"] = "multipath_member"
> +                dev["ID_MPATH_NAME"] = mpath_name
> +                log.info("DeviceTopology: found a multipath member of %s: %s " %
> +                         (mpath_name, name))
> +                continue
> +            # it's a disk and not a multipath member (can be a coalesced
> +            # multipath)
> +            self._singlepaths.append(name)
> +            log.info("DeviceTopology: found singlepath device: %s" % name)
> +
> +    def devices_iter(self):
> +        """ Generator. Yields all disk devices, mpaths members, coalesced mpath
> +            devices and partitions.
> +
> +            This property guarantees the order of the returned devices is the
> +            same as in the device list passed to the object's constructor.
> +        """
> +        for device in self._devices:
> +            yield device
> +
> +    def singlepaths_iter(self):
> +        """ Generator. Yields only the singlepath disks.
> +        """
> +        for name in self._singlepaths:
> +            yield self._devmap[name]
> +
> +    def multipath_name(self, mpath_member_name):
> +        """ If the mpath_member_name is a member of a multipath device return
> +            the name of the device (e.g. mpathc).
> +
> +            Else return None.
> +        """
> +        for (name, members) in self._mpath_topology.items():
> +            if mpath_member_name in members:
> +                return name
> +        return None
> +
> +    def multipaths_iter(self):
> +        """Generator. Yields all the multipath members, in a topology.
> +
> +           Every iteration returns a list of mpath member devices forming a
> +           multipath.
> +        """
> +        for disks in self._mpath_topology.values():
> +            yield [self._devmap[d] for d in disks]
> +
> +
>  class MultipathConfigWriter:
>      def __init__(self):
>          self.blacklist_devices = []
> @@ -303,3 +404,4 @@ blacklist {
>          ret += '}\n'
>  
>          return ret
> +
> diff --git a/pyanaconda/storage/devicetree.py b/pyanaconda/storage/devicetree.py
> index 829159c..f683ef1 100644
> --- a/pyanaconda/storage/devicetree.py
> +++ b/pyanaconda/storage/devicetree.py
> @@ -1712,16 +1712,12 @@ class DeviceTree(object):
>          open("/etc/multipath.conf", "w+").write(cfg)
>          del cfg
>  
> -        devices = udev_get_block_devices()
> -        (singles, mpaths, partitions) = devicelibs.mpath.identifyMultipaths(devices)
> -        devices = singles + reduce(list.__add__, mpaths, []) + partitions
> -        # remember all the devices idenitfyMultipaths() gave us at this point
> +        self.topology = devicelibs.mpath.DeviceTopology(udev_get_block_devices())
> +        log.info("devices to scan: %s" %
> +                 [d['name'] for d in self.topology.devices_iter()])
>          old_devices = {}
> -        for dev in devices:
> +        for dev in self.topology.devices_iter():
>              old_devices[dev['name']] = dev
> -
> -        log.info("devices to scan: %s" % [d['name'] for d in devices])
> -        for dev in devices:
>              self.addUdevDevice(dev)
>  
>          # Having found all the disks, we can now find all the multipaths built



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