Re: [PATCH 07/14] Move identifyMultipaths from DeviceTree to devicelibs.

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

 



Hi,

comments inline.

On 12/01/2009 09:15 PM, Chris Lumens wrote:
The return type has also changed.  It now returns a tuple of a list of regular
devices, a list of multipath sets, and a list of partition devices.  This is
necessary so the filtering UI can use the same knowledge that was in the
DeviceTree object to know which devices are part of a multipath set and which
are just standalone disks.
---
  storage/devicelibs/mpath.py |   81 +++++++++++++++++++++++++++++
  storage/devicetree.py       |  117 +++++++++----------------------------------
  2 files changed, 105 insertions(+), 93 deletions(-)

diff --git a/storage/devicelibs/mpath.py b/storage/devicelibs/mpath.py
index 4a1c417..67c60f0 100644
--- a/storage/devicelibs/mpath.py
+++ b/storage/devicelibs/mpath.py
@@ -1,3 +1,84 @@
+from ..udev import *
+
+def identifyMultipaths(devices):
+    # this function does a couple of things
+    # 1) identifies multipath disks
+    # 2) sets their ID_FS_TYPE to multipath_member
+    # 3) removes the individual members of an mpath's partitions
+    # sample input with multipath pairs [sda,sdc] and [sdb,sdd]
+    # [sr0, sda, sda1, sdb, sda2, sdc, sdd, sdc1, sdc2, sde, sde1]
+    # sample output:
+    # [sr0, sda, sdb, sdc, sdd, sde, sde1]
+
+    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)) or \
+                (not d.has_key('ID_SERIAL_SHORT')):
+            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 = []
+    multipaths = []
+    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:
+            # 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)
+            all_usb = True
+            for d in disks:
+                if d.get("ID_USB_DRIVER") != "usb-storage":
+                    all_usb = False
+                    break
+            if all_usb:
+                log.info("adding multi lun usb mass storage device to singlepath_disks: %s" %
+                         [disk['name'] for disk in disks])
+                singlepath_disks.extend(disks)
+                continue
+
+            for d in disks:
+                log.info("adding %s to multipath_disks" % (d['name'],))
+                d["ID_FS_TYPE"] = "multipath_member"
+
+            multipaths.append(disks)
+            log.info("found multipath set: [%s]" % [d['name'] for d in disks])
+
+    for mpath in multipaths:
+        for serial in [d['ID_SERIAL_SHORT'] for d in mpath]:
+            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 devs in non_disk_devices.values():
+        partition_devices += devs
+
+    # this is the list of devices we want to keep from the original
+    # device list, but we want to maintain its original order.
+    singlepath_disks = filter(lambda d: d in devices, singlepath_disks)
+    #multipaths = filter(lambda d: d in devices, multipaths)
+    partition_devices = filter(lambda d: d in devices, partition_devices)
+

This might be me not parsing the python code above correctly, but
I don't think this does what you want it do, it does not put the devices
inside singlepath_disks / partition_devices in devices order.

Also note that this is not necessary. udev not always give us devices in
a nice logical order, so sometimes we scan devices before we have scanned
there parents (I've fixed several BIOS RAID related bugs caused by this),
so the devicetree.py scanning code is setup to handle parents not being scanned
yet and in that case force scanning the parent first.

Long story short, there is no reason to maintain the devices order, so this
can be dropped.

+    mpathStr = "["
+    for mpath in multipaths:
+        mpathStr += str([d['name'] for d in mpath])
+    mpathStr += "]"
+
+    s = "(%s, %s, %s)" % ([d['name'] for d in singlepath_disks], \
+                          mpathStr, \
+                          [d['name'] for d in partition_devices])
+    log.info("devices post multipath scan: %s" % s)
+    return (singlepath_disks, multipaths, partition_devices)

  class MultipathConfigWriter:
      def __init__(self):
diff --git a/storage/devicetree.py b/storage/devicetree.py
index a5a59e7..ab71f12 100644
--- a/storage/devicetree.py
+++ b/storage/devicetree.py
@@ -1894,82 +1894,6 @@ class DeviceTree(object):
          for leaf in self.leaves:
              leafInconsistencies(leaf)

-    def identifyMultipaths(self, devices):
-        # this function does a couple of things
-        # 1) identifies multipath disks
-        # 2) sets their ID_FS_TYPE to multipath_member
-        # 3) removes the individual members of an mpath's partitions
-        # sample input with multipath pairs [sda,sdc] and [sdb,sdd]
-        # [sr0, sda, sda1, sdb, sda2, sdc, sdd, sdc1, sdc2, sde, sde1]
-        # sample output:
-        # [sr0, sda, sdb, sdc, sdd, sde, sde1]
-
-        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)) or \
-                    (not d.has_key('ID_SERIAL_SHORT')):
-                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:
-                # 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)
-                all_usb = True
-                for d in disks:
-                    if d.get("ID_USB_DRIVER") != "usb-storage":
-                        all_usb = False
-                        break
-                if all_usb:
-                    log.info("adding multi lun usb mass storage device to singlepath_disks: %s" %
-                             [disk['name'] for disk in disks])
-                    singlepath_disks.extend(disks)
-                    continue
-
-                multipath_members = {}
-                for d in disks:
-                    log.info("adding %s to multipath_disks" % (d['name'],))
-                    d["ID_FS_TYPE"] = "multipath_member"
-                    multipath_disks.append(d)
-
-                    multipath_members[d['name']] = { 'info': d,
-                                                     'found': False }
-                    log.info("found multipath set: [%s]" % [d['name'] for d in disks])
-
-        for serial in [d['ID_SERIAL_SHORT'] 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 devs in non_disk_devices.values():
-            partition_devices += devs
-
-        # this is the list of devices we want to keep from the original
-        # device list, but we want to maintain its original order.
-        okdevs = singlepath_disks + multipath_disks + partition_devices
-        names = [d['name'] for d in okdevs]
-        retdevs = []
-        for dev in devices:
-            if dev['name'] in names:
-                retdevs.append(dev)
-        log.info("devices post multipath scan: %s" % [d['name'] for d in retdevs])
-        return retdevs
-
      def populate(self):
          """ Locate all storage devices. """

@@ -1993,12 +1917,28 @@ class DeviceTree(object):
                       % (livetarget,))
              self.protectedDevNames.append(livetarget)

-        # each iteration scans any devices that have appeared since the
-        # previous iteration
+        # First iteration - let's just look for disks.
          old_devices = {}
-        ignored_devices = []
-        first_iteration = True
-        handled_mpaths = False
+
+        devices = udev_get_block_devices()
+        for dev in devices:
+            old_devices[dev['name']] = dev
+
+        (singles, mpaths, partitions) = devicelibs.mpath.identifyMultipaths(devices)
+        devices = singles + reduce(list.__add__, mpaths, []) + partitions
+        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
+        # upon them.
+        for mp in self.__multipaths.values():
+            log.info("adding mpath device %s" % mp.name)
+            mp.setup()
+            self._addDevice(mp)
+
+        # Now, loop and scan for devices that have appeared since the two above
+        # blocks or since previous iterations.
          while True:
              devices = []
              new_devices = udev_get_block_devices()
@@ -2009,18 +1949,9 @@ class DeviceTree(object):
                      devices.append(new_device)

              if len(devices) == 0:
-                if handled_mpaths:
-                    # nothing is changing -- we are finished building devices
-                    break
-                for mp in self.__multipaths.values():
-                    log.info("adding mpath device %s" % (mp.name,))
-                    mp.setup()
-                    self._addDevice(mp)
-                handled_mpaths = True
-
-            if first_iteration:
-                devices = self.identifyMultipaths(devices)
-                first_iteration = False
+                # nothing is changing -- we are finished building devices
+                break
+
              log.info("devices to scan: %s" % [d['name'] for d in devices])
              for dev in devices:
                  self.addUdevDevice(dev)

Other then the one remark, this looks good.

Regards,

Hans

_______________________________________________
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