On 07/24/2009 01:26 PM, David Lehman wrote:
> 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.
Yeah, okay.
>
>> +
>> + # 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/.
Well, as I said in my reply to dcantrell, this code is really just
a stop-gap and will be going away.
>> +
>> + 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?
I don't really know how that /helps/ anything -- If somebody
passes in None, they're going to get all the StorageDevice objects
with no serial number. Which basically means CD drives.
>
>> 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).
Yep. Although currently this code is merely there for "completeness";
it's not actually used.
>
>> +
>> + @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.
Yeah, okay.
>
>> +
>> +
>> + 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):
I'll send an updated patch in a bit.
--
Peter
I was born not knowing and have had only a little time to
change that here and there.
-- Feynman
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
