On Thu, 2009-08-06 at 11:04 -0400, Peter Jones wrote:
> This identifies that a device is part of a multipath, and builds an
> mpath device for partitioning.
Comments inline...
> ---
> 70-anaconda.rules | 2 +-
> storage/devicelibs/dm.py | 18 +++++
> storage/devices.py | 108 +++++++++++++++++++++++---
> storage/devicetree.py | 173 ++++++++++++++++++++++++++++++++++++------
> storage/errors.py | 3 +
> storage/formats/multipath.py | 88 +++++++++++++++++++++
> storage/udev.py | 46 +++++++++++
> 7 files changed, 401 insertions(+), 37 deletions(-)
> create mode 100644 storage/formats/multipath.py
>
> diff --git a/storage/devicelibs/dm.py b/storage/devicelibs/dm.py
> index 29df126..a4a4ca9 100644
> --- a/storage/devicelibs/dm.py
> +++ b/storage/devicelibs/dm.py
> @@ -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
> +
> +
Why add the above three functions if they don't do anything?
> diff --git a/storage/devices.py b/storage/devices.py
> index 3ca05aa..a866f14 100644
> --- a/storage/devices.py
> +++ b/storage/devices.py
> @@ -96,6 +96,7 @@
> import os
> import math
> import copy
> +import string
If we can avoid this by using the string instances' methods we should do
so.
>
> # device backend modules
> from devicelibs import mdraid
> @@ -416,7 +417,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:
> @@ -446,6 +447,7 @@ class StorageDevice(Device):
> self.minor = numeric_type(minor)
> self.sysfsPath = sysfsPath
> self.exists = exists
> + self.serial = serial
>
> self.protected = False
>
> @@ -735,12 +737,11 @@ class DiskDevice(StorageDevice):
> else:
> self._origPartedDisk = None
>
> -
> @property
> def partedDisk(self):
> if self._partedDisk:
> return self._partedDisk
> -
> +
> log.debug("looking up parted Device: %s" % self.path)
>
> if self.partedDevice:
> @@ -762,14 +763,15 @@ class DiskDevice(StorageDevice):
> # When the device has no partition table but it has a FS, it
> # will be created with label type loop. Treat the same as if
> # the device had no label (cause it really doesn't).
> - if self._partedDisk.type == "loop":
> + if self.partedDisk.type == "loop":
> if self._initcb is not None and self._initcb():
> - self._partedDisk = parted.freshDisk( \
> - device=self.partedDevice, \
> + self._partedDisk = parted.freshDisk(device=self.partedDevice, \
> ty = platform.getPlatform(None).diskType)
> else:
> raise DeviceUserDeniedFormatError("User prefered to not format.")
>
> + return self._partedDisk
> +
It looks like some of the previous patch is being reverted in the above
hunk (self._partedDisk -v- self.partedDisk).
> def __str__(self):
> s = StorageDevice.__str__(self)
> s += (" removable = %(removable)s partedDevice = %(partedDevice)r\n"
> @@ -2731,19 +2733,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, info, 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)
> + info -- the udev info for this device
>
> Keyword Arguments:
>
> @@ -2751,12 +2768,79 @@ 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._info = info
> + self._isUp = False
> + self._pyBlockMultiPath = None
> + self.setupIdentity()
> + DiskDevice.__init__(self, name, format=format, size=size,
> parents=parents, sysfsPath=sysfsPath,
> - exists=exists)
> + initcb=initcb, initlabel=initlabel)
> +
> + def setupIdentity(self):
> + """ Adds identifying remarks to MultipathDevice object.
> +
> + May be overridden by a sub-class for e.g. RDAC handling.
> + """
> + self._serial = self._info['ID_SERIAL_SHORT']
> +
> + @property
> + def identity(self):
> + """ Get identity set with setupIdentityFromInfo()
> +
> + May be overridden by a sub-class for e.g. RDAC handling.
> + """
> + if not hasattr(self, "_serial"):
> + raise RuntimeError, "setupIdentityFromInfo() has not been called."
> + return self._serial
>
> + @property
> + def up(self):
> + return self._isUp
Is this different from status?
> +
> + @property
> + def path(self):
> + """ Device node representing this device. """
> + return "/dev/mapper/%s" % (self.name,)
This should not be needed. StorageDevice already does this, except
without hard-coding /dev/mapper.
> +
> + @property
> + def wwid(self):
> + serial = self.identity
> + ret = []
> + while serial:
> + ret.append(serial[:2])
> + serial = serial[2:]
> + return string.join(ret, ':')
You could avoid the string import above by instead doing this:
return ":".join(ret)
Weird looking, I know.
> +
> + @property
> + def description(self):
> + return "WWID %s" % (self.wwid,)
> +
> + def addParent(self, parent):
> + if self.up:
> + self.teardown()
> + self.parents.append(parent)
> + self.setup()
> + else:
> + self.parents.append(parent)
> +
> + def setup(self, intf=None):
> + if self.up:
> + self.teardown()
> + self._isUp = True
> + parents = []
> + for p in self.parents:
> + parents.append(p.path)
> + self._pyBlockMultiPath = block.device.MultiPath(*parents)
> +
> + def teardown(self, recursive=None):
> + if not self.up:
> + return
> + self._isUp = False
> + self._pyBlockMultiPath = None
>
> class NoDevice(StorageDevice):
> """ A nodev device for nodev filesystems like tmpfs. """
> diff --git a/storage/devicetree.py b/storage/devicetree.py
> index 4ff4727..6030bd4 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
>
> @@ -230,6 +231,7 @@ class DeviceTree(object):
>
> self.__passphrase = passphrase
> self.__luksDevs = {}
> + self.__multipaths = {}
> if luksDict and isinstance(luksDict, dict):
> self.__luksDevs = luksDict
> self._ignoredDisks = []
> @@ -982,16 +984,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
> @@ -1104,6 +1115,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 = {}
> @@ -1193,7 +1205,18 @@ 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)
> + self.addUdevDMDevice(info)
> + 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:
> @@ -1413,6 +1436,44 @@ 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)
> +
> + serial = udev_device_get_serial(info)
> + found = False
> + if self.__multipaths.has_key(serial):
> + mp = self.__multipaths[serial]
> + mp.addParent(device)
> + else:
> + name = generateMultipathDeviceName()
> + devname = "/dev/mapper/%s" % (name,)
> +
> + if self.zeroMbr:
> + cb = lambda: True
> + else:
> + desc = []
> + serialtmp = serial
> + while serialtmp:
> + desc.append(serialtmp[:2])
> + serialtmp = serialtmp[2:]
> + desc = "WWID %s" % (string.join(desc, ':'),)
Again, you can do ":".join(desc) here instead.
> +
> + cb = lambda: questionInitializeDisk(self.intf, devname, desc)
> +
> + 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
> + mp = MultipathDevice(name, info, parents=[device], initcb=cb,
> + initlabel=initlabel)
> + self.__multipaths[serial] = mp
> +
> def handleUdevDMRaidMemberFormat(self, info, device):
> log_method_call(self, name=device.name, type=device.format.type)
> name = udev_device_get_name(info)
> @@ -1504,6 +1565,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:
> @@ -1517,10 +1579,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:
> @@ -1590,6 +1655,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):
> @@ -1708,6 +1775,49 @@ 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'],))
> + 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 devices in non_disk_devices.values():
> + partition_devices += devices
> +
> + return partition_devices + multipath_disks + singlepath_disks
> +
> def populate(self):
> """ Locate all storage devices. """
>
> @@ -1723,27 +1833,32 @@ class DeviceTree(object):
>
> # each iteration scans any devices that have appeared since the
> # previous iteration
> - old_devices = []
> + old_devices = {}
> ignored_devices = []
> + first_iteration = True
> + handled_mpaths = False
> while True:
> devices = []
> new_devices = udev_get_block_devices()
>
> for new_device in new_devices:
> - found = False
> - for old_device in old_devices:
> - if old_device['name'] == new_device['name']:
> - found = True
> - break
> -
> - if not found:
> + if not old_devices.has_key(new_device['name']):
> + old_devices[new_device['name']] = new_device
> devices.append(new_device)
>
> if len(devices) == 0:
> - # nothing is changing -- we are finished building devices
> - break
> -
> - old_devices = new_devices
> + 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
> log.info("devices to scan: %s" % [d['name'] for d in devices])
> for dev in devices:
> self.addUdevDevice(dev)
> @@ -1803,6 +1918,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
> +
> def getDeviceByLabel(self, label):
> if not label:
> return None
> 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 450b54a..adb69e6 100644
> --- a/storage/udev.py
> +++ b/storage/udev.py
> @@ -22,6 +22,7 @@
>
> import os
> import stat
> +import block
>
> import iutil
> from errors import *
> @@ -134,6 +135,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"
It kind of sucks that we have this string both in the format class and
here.
> +
> def udev_device_is_dm(info):
> """ Return True if the device is a device-mapper device. """
> return info.has_key("DM_NAME")
> @@ -158,6 +163,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_SHORT")
> +
> def udev_device_get_sysfs_path(info):
> return info['sysfs_path']
>
> @@ -276,6 +285,43 @@ 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. """
> + 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 is None:
> + return False
> +
> + # this is sort of a lame check, but basically, if diskname gave us "mpath0"
> + # and we start with "mpath" but we're not "mpath0", then we must be
> + # "mpath0" plus some non-numeric crap.
> + if diskname != info["DM_NAME"]:
> + return True
> +
> + return False
> +
> +
> + 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
