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