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