This is nice in that we can drop some custom code, but python-pyudev also provides classes that make it really easy to listen for uevents.
David Lehman (5): Replace our pyudev with the package python-pyudev. Cache MD metadata while populating the devicetree. Remove an obsolete block related to unpredictable md device names. Keep lvm and md metadata separate from udev info. Adapt multipath detection code to external pyudev module.
blivet/deviceaction.py | 4 +- blivet/devices.py | 95 +++++++-------------- blivet/devicetree.py | 147 +++++++++++++++----------------- blivet/pyudev.py | 226 ------------------------------------------------- blivet/udev.py | 122 +++++++++----------------- blivet/util.py | 2 +- python-blivet.spec | 1 + tests/udev_test.py | 91 +------------------- 8 files changed, 141 insertions(+), 547 deletions(-) delete mode 100644 blivet/pyudev.py
There are two major differences in how we use the module. Each device is represented by a pyudev.Device instance, which is based on collections.abc.Mapping. That means we can no longer change the data to do things like set a "multipath_member" format type or inject LVM or MD metadata.
The other difference is that sysfs paths now include the "/sys", which is great since it's always mounted in the same place. --- blivet/deviceaction.py | 4 +- blivet/devices.py | 95 +++++++-------------- blivet/devicetree.py | 49 +++++------ blivet/pyudev.py | 226 ------------------------------------------------- blivet/udev.py | 122 +++++++++----------------- blivet/util.py | 2 +- python-blivet.spec | 1 + tests/udev_test.py | 91 +------------------- 8 files changed, 99 insertions(+), 491 deletions(-) delete mode 100644 blivet/pyudev.py
diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py index db4c7a7..d2818c1 100644 --- a/blivet/deviceaction.py +++ b/blivet/deviceaction.py @@ -553,9 +553,7 @@ class ActionCreateFormat(DeviceAction): self.device.format.create(device=self.device.path, options=self.device.formatArgs) # Get the UUID now that the format is created - udev.settle() - self.device.updateSysfsPath() - info = udev.get_block_device(self.device.sysfsPath) + info = udev.get_device(self.device.sysfsPath) # only do this if the format has a device known to udev # (the format might not have a normal device at all) if info: diff --git a/blivet/devices.py b/blivet/devices.py index 8ab01da..dd1adef 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -27,6 +27,7 @@ import tempfile import abc from decimal import Decimal import re +import pyudev
from six import add_metaclass
@@ -107,7 +108,7 @@ def deviceNameToDiskByPath(deviceName=None): return ""
ret = None - for dev in udev.get_block_devices(): + for dev in udev.get_devices(): if udev.device_get_name(dev) == deviceName: ret = udev.device_get_by_path(dev) break @@ -508,7 +509,6 @@ class StorageDevice(Device):
_type = "blivet" _devDir = "/dev" - sysfsBlockDir = "class/block" _formatImmutable = False _partitionable = False _isDisk = False @@ -751,11 +751,22 @@ class StorageDevice(Device):
def updateSysfsPath(self): """ Update this device's sysfs path. """ - log_method_call(self, self.name, status=self.status) - sysfsName = self.name.replace("/", "!") - path = os.path.join("/sys", self.sysfsBlockDir, sysfsName) - self.sysfsPath = os.path.realpath(path)[4:] - log.debug("%s sysfsPath set to %s", self.name, self.sysfsPath) + # We're using os.path.exists as a stand-in for status. We can't use + # the status property directly because MDRaidArrayDevice.status calls + # this method. + log_method_call(self, self.name, status=os.path.exists(self.path)) + if not self.exists: + raise errors.DeviceError("device has not been created", self.name) + + try: + udev_device = pyudev.Device.from_device_file(udev.global_udev, + self.path) + except pyudev.DeviceNotFoundError as e: + log.error("failed to update sysfs path for %s: %s" % (self.name, e)) + self.sysfsPath = '' + else: + self.sysfsPath = udev_device.sys_path + log.debug("%s sysfsPath set to %s" % (self.name, self.sysfsPath))
@property def formatArgs(self): @@ -780,7 +791,7 @@ class StorageDevice(Device): log.debug("not sending change uevent for inactive device") return
- path = os.path.normpath("/sys/%s" % self.sysfsPath) + path = os.path.normpath(self.sysfsPath) try: util.notify_kernel(path, action="change") except (ValueError, IOError) as e: @@ -1055,7 +1066,7 @@ class StorageDevice(Device):
@property def removable(self): - devpath = os.path.normpath("/sys/%s" % self.sysfsPath) + devpath = os.path.normpath(self.sysfsPath) remfile = os.path.normpath("%s/removable" % devpath) return (self.sysfsPath and os.path.exists(devpath) and os.access(remfile, os.R_OK) and @@ -1575,23 +1586,6 @@ class PartitionDevice(StorageDevice): weight = property(lambda d: d._getWeight(), lambda d,w: d._setWeight(w))
- def updateSysfsPath(self): - """ Update this device's sysfs path. """ - log_method_call(self, self.name, status=self.status) - if not self.parents: - self.sysfsPath = '' - - elif isinstance(self.parents[0], DMDevice): - dm_node = dm.dm_node_from_name(self.name) - path = os.path.join("/sys", self.sysfsBlockDir, dm_node) - self.sysfsPath = os.path.realpath(path)[4:] - elif isinstance(self.parents[0], MDRaidArrayDevice): - md_node = mdraid.md_node_from_name(self.name) - path = os.path.join("/sys", self.sysfsBlockDir, md_node) - self.sysfsPath = os.path.realpath(path)[4:] - else: - StorageDevice.updateSysfsPath(self) - def _setName(self, value): self._name = value # actual name setting is done by parted
@@ -2069,19 +2063,6 @@ class DMDevice(StorageDevice): None) return (match.live_table and not match.suspended) if match else False
- def updateSysfsPath(self): - """ Update this device's sysfs path. """ - log_method_call(self, self.name, status=self.status) - if not self.exists: - raise errors.DeviceError("device has not been created", self.name) - - if self.status: - dm_node = self.getDMNode() - path = os.path.join("/sys", self.sysfsBlockDir, dm_node) - self.sysfsPath = os.path.realpath(path)[4:] - else: - self.sysfsPath = '' - #def getTargetType(self): # return dm.getDmTarget(name=self.name)
@@ -3825,22 +3806,6 @@ class MDRaidArrayDevice(ContainerDevice):
spares = property(_getSpares, _setSpares)
- def updateSysfsPath(self): - """ Update this device's sysfs path. """ - # don't include self.status here since this method is called by - # MDRaidArrayDevice.status - log_method_call(self, self.name) - if not self.exists: - raise errors.DeviceError("device has not been created", self.name) - - # We don't use self.status here because self.status requires a valid - # sysfs path to function correctly. - if os.path.exists(self.path): - md_node = mdraid.md_node_from_name(self.name) - self.sysfsPath = "/devices/virtual/block/%s" % md_node - else: - self.sysfsPath = '' - def _addParent(self, member): super(MDRaidArrayDevice, self)._addParent(member)
@@ -3901,7 +3866,7 @@ class MDRaidArrayDevice(ContainerDevice): self.updateSysfsPath()
# make sure the active array is the one we expect - info = udev.get_block_device(self.sysfsPath) + info = udev.get_device(self.sysfsPath) uuid = udev.device_get_md_uuid(info) if uuid and uuid != self.uuid: log.warning("md array %s is active, but has UUID %s -- not %s", @@ -3909,7 +3874,7 @@ class MDRaidArrayDevice(ContainerDevice): self.sysfsPath = "" return status
- state_file = "/sys/%s/md/array_state" % self.sysfsPath + state_file = "%s/md/array_state" % self.sysfsPath try: state = open(state_file).read().strip() if state in ("clean", "active", "active-idle", "readonly", "read-auto"): @@ -3939,7 +3904,7 @@ class MDRaidArrayDevice(ContainerDevice): def degraded(self): """ Return True if the array is running in degraded mode. """ rc = False - degraded_file = "/sys/%s/md/degraded" % self.sysfsPath + degraded_file = "%s/md/degraded" % self.sysfsPath if os.access(degraded_file, os.R_OK): val = open(degraded_file).read().strip() if val == "1": @@ -3997,7 +3962,7 @@ class MDRaidArrayDevice(ContainerDevice): # mdadm reuses minors indiscriminantly when there is no mdadm.conf, so # we need to clear the sysfs path now so our status method continues to # give valid results - self.updateSysfsPath() + self.sysfsPath = ''
def teardown(self, recursive=None): """ Close, or tear down, a device. """ @@ -4039,15 +4004,13 @@ class MDRaidArrayDevice(ContainerDevice):
def _postCreate(self): # this is critical since our status method requires a valid sysfs path - md_node = mdraid.md_node_from_name(self.name) - self.sysfsPath = "/devices/virtual/block/%s" % md_node - self.exists = True # I think we can remove this. - + self.exists = True # this is needed to run updateSysfsPath + self.updateSysfsPath() StorageDevice._postCreate(self)
# update our uuid attribute with the new array's UUID - info = udev.get_block_device(self.sysfsPath) - self.uuid = udev.device_get_md_uuid(info) + info = mdraid.mddetail(self.path) + self.uuid = info.get("UUID") for member in self.devices: member.format.mdUuid = self.uuid
@@ -5267,7 +5230,7 @@ class BTRFSVolumeDevice(BTRFSDevice, ContainerDevice):
def _postCreate(self): super(BTRFSVolumeDevice, self)._postCreate() - info = udev.get_block_device(self.sysfsPath) + info = udev.get_device(self.sysfsPath) if not info: log.error("failed to get updated udev info for new btrfs volume") else: diff --git a/blivet/devicetree.py b/blivet/devicetree.py index bf90fb5..80ddbb1 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -660,7 +660,7 @@ class DeviceTree(object):
if self.udevDeviceIsDisk(info): # Ignore any readonly disks - if util.get_sysfs_attr(info["sysfs_path"], 'ro') == '1': + if util.get_sysfs_attr(udev.device_get_sysfs_path(info), 'ro') == '1': log.debug("Ignoring read only device %s", name) # FIXME: We have to handle this better, ie: not ignore these. self.addIgnoredDisk(name) @@ -706,10 +706,10 @@ class DeviceTree(object): if not device: # initiate detection of all PVs and hope that it leads to us having # the VG and LVs in the tree - for pv_name in os.listdir("/sys" + sysfs_path + "/slaves"): - link = os.readlink("/sys" + sysfs_path + "/slaves/" + pv_name) + for pv_name in os.listdir(sysfs_path + "/slaves"): + link = os.readlink(sysfs_path + "/slaves/" + pv_name) pv_sysfs_path = os.path.normpath(sysfs_path + '/slaves/' + link) - pv_info = udev.get_block_device(pv_sysfs_path) + pv_info = udev.get_device(pv_sysfs_path) self.addUdevDevice(pv_info)
vg_name = udev.device_get_lv_vg_name(info) @@ -752,7 +752,7 @@ class DeviceTree(object): if device is None: # we couldn't find it, so create it # first, get a list of the slave devs and look them up - slave_dir = os.path.normpath("/sys/%s/slaves" % sysfs_path) + slave_dir = os.path.normpath("%s/slaves" % sysfs_path) slave_names = os.listdir(slave_dir) for slave_name in slave_names: # if it's a dm-X name, resolve it to a map name first @@ -762,7 +762,7 @@ class DeviceTree(object): dev_name = slave_name.replace("!", "/") # handles cciss slave_dev = self.getDeviceByName(dev_name) path = os.path.normpath("%s/%s" % (slave_dir, slave_name)) - new_info = udev.get_block_device(os.path.realpath(path)[4:]) + new_info = udev.get_device(os.path.realpath(path)) if not slave_dev: # we haven't scanned the slave yet, so do it now if new_info: @@ -822,7 +822,7 @@ class DeviceTree(object): # TODO: look for this device by dm-uuid?
# first, get a list of the slave devs and look them up - slave_dir = os.path.normpath("/sys/%s/slaves" % sysfs_path) + slave_dir = os.path.normpath("%s/slaves" % sysfs_path) slave_names = os.listdir(slave_dir) for slave_name in slave_names: # if it's a dm-X name, resolve it to a map name first @@ -832,7 +832,7 @@ class DeviceTree(object): dev_name = slave_name.replace("!", "/") # handles cciss slave_dev = self.getDeviceByName(dev_name) path = os.path.normpath("%s/%s" % (slave_dir, slave_name)) - new_info = udev.get_block_device(os.path.realpath(path)[4:]) + new_info = udev.get_device(os.path.realpath(path)) if not slave_dev: # we haven't scanned the slave yet, so do it now if new_info: @@ -865,7 +865,7 @@ class DeviceTree(object): log_method_call(self, name=name) sysfs_path = udev.device_get_sysfs_path(info)
- slave_dir = os.path.normpath("/sys/%s/slaves" % sysfs_path) + slave_dir = os.path.normpath("%s/slaves" % sysfs_path) slave_names = os.listdir(slave_dir) for slave_name in slave_names: # if it's a dm-X name, resolve it to a map name @@ -877,7 +877,7 @@ class DeviceTree(object): if not slave_dev: # we haven't scanned the slave yet, so do it now path = os.path.normpath("%s/%s" % (slave_dir, slave_name)) - new_info = udev.get_block_device(os.path.realpath(path)[4:]) + new_info = udev.get_device(os.path.realpath(path)) if new_info: self.addUdevDevice(new_info) if self.getDeviceByName(dev_name) is None: @@ -936,7 +936,7 @@ class DeviceTree(object):
if disk is None: # create a device instance for the disk - new_info = udev.get_block_device(os.path.dirname(sysfs_path)) + new_info = udev.get_device(os.path.dirname(sysfs_path)) if new_info: self.addUdevDevice(new_info) disk = self.getDeviceByName(disk_name) @@ -1030,7 +1030,7 @@ class DeviceTree(object): if not container: parentSysName = mdraid.md_node_from_name(parentName) container_sysfs = "/class/block/" + parentSysName - container_info = udev.get_block_device(container_sysfs) + container_info = udev.get_device(container_sysfs) if not container_info: log.error("failed to find md container %s at %s", parentName, container_sysfs) @@ -1102,7 +1102,7 @@ class DeviceTree(object): name = udev.device_get_name(info) log_method_call(self, name=name) sysfs_path = udev.device_get_sysfs_path(info) - sys_file = "/sys/%s/loop/backing_file" % sysfs_path + sys_file = "%s/loop/backing_file" % sysfs_path backing_file = open(sys_file).read().strip() file_device = self.getDeviceByName(backing_file) if not file_device: @@ -1122,7 +1122,7 @@ class DeviceTree(object):
def addUdevDevice(self, info): name = udev.device_get_name(info) - log_method_call(self, name=name, info=pprint.pformat(info)) + log_method_call(self, name=name, info=pprint.pformat(dict(info))) uuid = udev.device_get_uuid(info) sysfs_path = udev.device_get_sysfs_path(info)
@@ -1490,7 +1490,7 @@ class DeviceTree(object):
if lv_device.status: lv_device.updateSysfsPath() - lv_info = udev.get_block_device(lv_device.sysfsPath) + lv_info = udev.get_device(lv_device.sysfsPath) if not lv_info: log.error("failed to get udev data for lv %s", lv_device.name) return @@ -1587,7 +1587,7 @@ class DeviceTree(object):
# check the list of devices udev knows about to see if the array # this device belongs to is already active - for dev in udev.get_block_devices(): + for dev in udev.get_devices(): if not udev.device_is_md(dev): continue
@@ -1690,13 +1690,13 @@ class DeviceTree(object): self._addDevice(dm_array)
# Wait for udev to scan the just created nodes, to avoid a race - # with the udev.get_block_device() call below. + # with the udev.get_device() call below. udev.settle()
# Get the DMRaidArrayDevice a DiskLabel format *now*, in case # its partitions get scanned before it does. dm_array.updateSysfsPath() - dm_array_info = udev.get_block_device(dm_array.sysfsPath) + dm_array_info = udev.get_device(dm_array.sysfsPath) self.handleUdevDiskLabelFormat(dm_array_info, dm_array)
# Use the rs's object on the device. @@ -1897,7 +1897,7 @@ class DeviceTree(object): def updateDeviceFormat(self, device): log.info("updating format of device: %s", device) try: - util.notify_kernel("/sys%s" % device.sysfsPath) + util.notify_kernel(device.sysfsPath) except (ValueError, IOError) as e: log.warning("failed to notify kernel of change: %s", e)
@@ -2040,7 +2040,7 @@ class DeviceTree(object): self._addDevice(filedev) self._addDevice(loopdev) self._addDevice(dmdev) - info = udev.get_block_device(dmdev.sysfsPath) + info = udev.get_device(dmdev.sysfsPath) self.addUdevDevice(info)
def backupConfigs(self, restore=False): @@ -2154,18 +2154,19 @@ class DeviceTree(object): # blocks or since previous iterations. while True: devices = [] - new_devices = udev.get_block_devices() + new_devices = udev.get_devices()
for new_device in new_devices: - if new_device['name'] not in old_devices: - old_devices[new_device['name']] = new_device + new_name = udev.device_get_name(new_device) + if not old_devices.has_key(new_name): + old_devices[new_name] = new_device devices.append(new_device)
if len(devices) == 0: # nothing is changing -- we are finished building devices break
- log.info("devices to scan: %s", [d['name'] for d in devices]) + log.info("devices to scan: %s", [udev.device_get_name(d) for d in devices]) for dev in devices: self.addUdevDevice(dev)
diff --git a/blivet/pyudev.py b/blivet/pyudev.py deleted file mode 100644 index fa0846f..0000000 --- a/blivet/pyudev.py +++ /dev/null @@ -1,226 +0,0 @@ -from __future__ import print_function - -import sys -import os -import fnmatch -from ctypes import CDLL, c_char_p, c_int, c_void_p - - -# XXX this one may need some tweaking... -def find_library(name, somajor=0): - env = os.environ.get("LD_LIBRARY_PATH") - common = ["/lib64", "/lib"] - - if env: - libdirs = env.split(":") + common - else: - libdirs = common - - for ldir in filter(os.path.isdir, libdirs): - files = fnmatch.filter(os.listdir(ldir), "lib%s.so.%d" % (name, somajor)) - if files: - return next(os.path.join(ldir, lfile) for lfile in files) - - return None - -def get_library(): - name = "udev" - somajor = 1 - lib = find_library(name=name, somajor=somajor) - - if not lib or not os.path.exists(lib): - raise ImportError("No library named %s.%d" % (name, somajor)) - return lib - -# load the udev library -libudev = CDLL(get_library()) - - -# create aliases for needed functions and set the return types where needed -libudev_udev_new = libudev.udev_new -libudev_udev_new.argtypes = [] -libudev_udev_new.restype = c_void_p -libudev_udev_unref = libudev.udev_unref -libudev_udev_unref.argtypes = [ c_void_p ] - -libudev_udev_device_new_from_syspath = libudev.udev_device_new_from_syspath -libudev_udev_device_new_from_syspath.restype = c_void_p -libudev_udev_device_new_from_syspath.argtypes = [ c_void_p, c_char_p ] -libudev_udev_device_unref = libudev.udev_device_unref -libudev_udev_device_unref.argtypes = [ c_void_p ] - -libudev_udev_device_get_syspath = libudev.udev_device_get_syspath -libudev_udev_device_get_syspath.restype = c_char_p -libudev_udev_device_get_syspath.argtypes = [ c_void_p ] -libudev_udev_device_get_sysname = libudev.udev_device_get_sysname -libudev_udev_device_get_sysname.restype = c_char_p -libudev_udev_device_get_sysname.argtypes = [ c_void_p ] -libudev_udev_device_get_devpath = libudev.udev_device_get_devpath -libudev_udev_device_get_devpath.restype = c_char_p -libudev_udev_device_get_devpath.argtypes = [ c_void_p ] -libudev_udev_device_get_devtype = libudev.udev_device_get_devtype -libudev_udev_device_get_devtype.restype = c_char_p -libudev_udev_device_get_devtype.argtypes = [ c_void_p ] -libudev_udev_device_get_devnode = libudev.udev_device_get_devnode -libudev_udev_device_get_devnode.restype = c_char_p -libudev_udev_device_get_devnode.argtypes = [ c_void_p ] -libudev_udev_device_get_subsystem = libudev.udev_device_get_subsystem -libudev_udev_device_get_subsystem.restype = c_char_p -libudev_udev_device_get_subsystem.argtypes = [ c_void_p ] -libudev_udev_device_get_sysnum = libudev.udev_device_get_sysnum -libudev_udev_device_get_sysnum.restype = c_char_p -libudev_udev_device_get_sysnum.argtypes = [ c_void_p ] - -libudev_udev_device_get_properties_list_entry = libudev.udev_device_get_properties_list_entry -libudev_udev_device_get_properties_list_entry.restype = c_void_p -libudev_udev_device_get_properties_list_entry.argtypes = [ c_void_p ] -libudev_udev_list_entry_get_next = libudev.udev_list_entry_get_next -libudev_udev_list_entry_get_next.restype = c_void_p -libudev_udev_list_entry_get_next.argtypes = [ c_void_p ] - -libudev_udev_list_entry_get_name = libudev.udev_list_entry_get_name -libudev_udev_list_entry_get_name.restype = c_char_p -libudev_udev_list_entry_get_name.argtypes = [ c_void_p ] -libudev_udev_list_entry_get_value = libudev.udev_list_entry_get_value -libudev_udev_list_entry_get_value.restype = c_char_p -libudev_udev_list_entry_get_value.argtypes = [ c_void_p ] - -libudev_udev_enumerate_new = libudev.udev_enumerate_new -libudev_udev_enumerate_new.restype = c_void_p -libudev_udev_enumerate_new.argtypes = [ c_void_p ] -libudev_udev_enumerate_unref = libudev.udev_enumerate_unref -libudev_udev_enumerate_unref.argtypes = [ c_void_p ] - -libudev_udev_enumerate_add_match_subsystem = libudev.udev_enumerate_add_match_subsystem -libudev_udev_enumerate_add_match_subsystem.restype = c_int -libudev_udev_enumerate_add_match_subsystem.argtypes = [ c_void_p, c_char_p ] -libudev_udev_enumerate_scan_devices = libudev.udev_enumerate_scan_devices -libudev_udev_enumerate_scan_devices.restype = c_int -libudev_udev_enumerate_scan_devices.argtypes = [ c_void_p ] -libudev_udev_enumerate_get_list_entry = libudev.udev_enumerate_get_list_entry -libudev_udev_enumerate_get_list_entry.restype = c_void_p -libudev_udev_enumerate_get_list_entry.argtypes = [ c_void_p ] - -libudev_udev_device_get_devlinks_list_entry = libudev.udev_device_get_devlinks_list_entry -libudev_udev_device_get_devlinks_list_entry.restype = c_void_p -libudev_udev_device_get_devlinks_list_entry.argtypes = [ c_void_p ] - - -class UdevDevice(dict): - - def __init__(self, udev, sysfs_path): - dict.__init__(self) - - # create new udev device from syspath - udev_device = libudev_udev_device_new_from_syspath(udev, sysfs_path) - if not udev_device: - # device does not exist - return - - # set syspath and sysname properties - self.syspath = libudev_udev_device_get_syspath(udev_device) - self.sysname = libudev_udev_device_get_sysname(udev_device) - - # get the devlinks list - devlinks = [] - devlinks_entry = libudev_udev_device_get_devlinks_list_entry(udev_device) - - while devlinks_entry: - path = libudev_udev_list_entry_get_name(devlinks_entry) - devlinks.append(path) - - devlinks_entry = libudev_udev_list_entry_get_next(devlinks_entry) - - # add devlinks list to the dictionary - self["symlinks"] = devlinks - - # get the first property entry - property_entry = libudev_udev_device_get_properties_list_entry(udev_device) - - while property_entry: - name = libudev_udev_list_entry_get_name(property_entry) - value = libudev_udev_list_entry_get_value(property_entry) - - # lvm outputs values for multiple lvs in one line - # we want to split them and make a list - # if the first lv's value is empty we end up with a value starting - # with name=, prepend a space that our split does the right thing - if value.startswith("%s=" % name): - value = " " + value - - if value.count(" %s=" % name): - value = value.split(" %s=" % name) - - self[name] = value - - # get next property entry - property_entry = libudev_udev_list_entry_get_next(property_entry) - - # set additional properties - self.devpath = libudev_udev_device_get_devpath(udev_device) - self.subsystem = libudev_udev_device_get_subsystem(udev_device) - self.devtype = libudev_udev_device_get_devtype(udev_device) - self.sysnum = libudev_udev_device_get_sysnum(udev_device) - self.devnode = libudev_udev_device_get_devnode(udev_device) - - # cleanup - libudev_udev_device_unref(udev_device) - - -class Udev(object): - - def __init__(self): - self.udev = libudev_udev_new() - - def create_device(self, sysfs_path): - return UdevDevice(self.udev, sysfs_path) - - def enumerate_devices(self, subsystem=None): - context = libudev_udev_enumerate_new(self.udev) - - # add the match subsystem - if subsystem is not None: - rc = libudev_udev_enumerate_add_match_subsystem(context, subsystem) - if not rc == 0: - print("error: unable to add the match subsystem", file=sys.stderr) - libudev_udev_enumerate_unref(context) - return [] - - # scan the devices - rc = libudev_udev_enumerate_scan_devices(context) - if not rc == 0: - print("error: unable to enumerate the devices", file=sys.stderr) - libudev_udev_enumerate_unref(context) - return [] - - # create the list of sysfs paths - sysfs_paths = [] - - # get the first list entry - list_entry = libudev_udev_enumerate_get_list_entry(context) - - while list_entry: - sysfs_path = libudev_udev_list_entry_get_name(list_entry) - sysfs_paths.append(sysfs_path) - - # get next list entry - list_entry = libudev_udev_list_entry_get_next(list_entry) - - # cleanup - libudev_udev_enumerate_unref(context) - - return sysfs_paths - - def scan_devices(self, sysfs_paths=None): - if sysfs_paths is None: - sysfs_paths = self.enumerate_devices() - - for sysfs_path in sysfs_paths: - device = self.create_device(sysfs_path) - - if device: - yield device - - def unref(self): - libudev_udev_unref(self.udev) - self.udev = None diff --git a/blivet/udev.py b/blivet/udev.py index b66e30b..b87295b 100644 --- a/blivet/udev.py +++ b/blivet/udev.py @@ -27,45 +27,28 @@ import re from . import util from .size import Size
-from . import pyudev -global_udev = pyudev.Udev() +import pyudev +global_udev = pyudev.Context()
import logging log = logging.getLogger("blivet")
-def enumerate_devices(deviceClass="block"): - devices = global_udev.enumerate_devices(subsystem=deviceClass) - return [path[4:] for path in devices] - def get_device(sysfs_path): - if not os.path.exists("/sys%s" % sysfs_path): - log.debug("%s does not exist", sysfs_path) - return None - - # XXX we remove the /sys part when enumerating devices, - # so we have to prepend it when creating the device - dev = global_udev.create_device("/sys" + sysfs_path) - - if dev: - dev["name"] = dev.sysname - dev["sysfs_path"] = sysfs_path - - # now add in the contents of the uevent file since they're handy - dev = parse_uevent_file(dev) + try: + dev = pyudev.Device.from_sys_path(global_udev, sysfs_path) + except pyudev.DeviceNotFoundError as e: + log.error(e) + dev = None
return dev
-def get_devices(deviceClass="block"): +def get_devices(subsystem="block"): settle() - entries = [] - for path in enumerate_devices(deviceClass): - entry = get_device(path) - if entry: - entries.append(entry) - return entries + return [d for d in global_udev.list_devices(subsystem=subsystem) + if not __is_blacklisted_blockdev(d.sys_name)]
def parse_uevent_file(dev): - path = os.path.normpath("/sys/%s/uevent" % dev['sysfs_path']) + path = os.path.normpath("%s/uevent" % device_get_sysfs_path(dev)) if not os.access(path, os.R_OK): return dev
@@ -103,7 +86,7 @@ def resolve_devspec(devspec): from . import devices
ret = None - for dev in get_block_devices(): + for dev in get_devices(): if devspec.startswith("LABEL="): if device_get_label(dev) == devspec[6:]: ret = dev @@ -120,7 +103,7 @@ def resolve_devspec(devspec): if not spec.startswith("/dev/"): spec = os.path.normpath("/dev/" + spec)
- for link in dev["symlinks"]: + for link in device_get_symlinks(dev): if spec == link: ret = dev break @@ -135,37 +118,18 @@ def resolve_glob(glob): if not glob: return ret
- for dev in get_block_devices(): + for dev in get_devices(): name = device_get_name(dev)
if fnmatch.fnmatch(name, glob): ret.append(name) else: - for link in dev["symlinks"]: + for link in device_get_symlinks(dev): if fnmatch.fnmatch(link, glob): ret.append(name)
return ret
-def get_block_devices(): - settle() - entries = [] - for path in enumerate_block_devices(): - entry = get_block_device(path) - if entry: - if entry["name"].startswith("md"): - # mdraid is really braindead, when a device is stopped - # it is no longer usefull in anyway (and we should not - # probe it) yet it still sticks around, see bug rh523387 - state = None - state_file = "/sys/%s/md/array_state" % entry["sysfs_path"] - if os.access(state_file, os.R_OK): - state = open(state_file).read().strip() - if state == "clear": - continue - entries.append(entry) - return entries - def __is_blacklisted_blockdev(dev_name): """Is this a blockdev we never want for an install?""" if dev_name.startswith("ram") or dev_name.startswith("fd"): @@ -180,17 +144,6 @@ def __is_blacklisted_blockdev(dev_name):
return False
-def enumerate_block_devices(): - return [d for d in enumerate_devices(deviceClass="block") if not __is_blacklisted_blockdev(os.path.basename(d))] - -def get_block_device(sysfs_path): - dev = get_device(sysfs_path) - if not dev or 'name' not in dev: - return None - else: - return dev - - # These are functions for retrieving specific pieces of information from # udev database entries. def device_get_name(udev_info): @@ -198,7 +151,7 @@ def device_get_name(udev_info): if "DM_NAME" in udev_info: name = udev_info["DM_NAME"] else: - name = udev_info["name"] + name = udev_info.sys_name
return name
@@ -236,7 +189,7 @@ def device_is_md(info):
# The udev information keeps shifting around. Only md arrays have a # /sys/class/block/<name>/md/ subdirectory. - md_dir = "/sys" + device_get_sysfs_path(info) + "/md" + md_dir = device_get_sysfs_path(info) + "/md" return os.path.exists(md_dir)
def device_is_cciss(info): @@ -256,7 +209,7 @@ def device_is_zfcp(info): if info.get("DEVTYPE") != "disk": return False
- subsystem = "/sys" + info.get("sysfs_path") + subsystem = device_get_sysfs_path(info)
while True: topdir = os.path.realpath(os.path.dirname(subsystem)) @@ -284,7 +237,7 @@ def device_get_zfcp_attribute(info, attr=None): log.debug("device_get_zfcp_attribute() called with attr=None") return None
- attribute = "/sys%s/device/%s" % (info.get("sysfs_path"), attr,) + attribute = "%s/device/%s" % (device_get_sysfs_path(info), attr,) attribute = os.path.realpath(attribute)
if not os.path.isfile(attribute): @@ -295,14 +248,14 @@ def device_get_zfcp_attribute(info, attr=None):
def device_get_dasd_bus_id(info): """ Return the CCW bus ID of the dasd device. """ - return info.get("sysfs_path").split("/")[-3] + return device_get_sysfs_path(info).split("/")[-3]
def device_get_dasd_flag(info, flag=None): """ Return the specified flag for the dasd device. """ if flag is None: return None
- path = "/sys" + info.get("sysfs_path") + "/device/" + flag + path = device_get_sysfs_path(info) + "/device/" + flag if not os.path.isfile(path): return None
@@ -318,17 +271,17 @@ def device_is_disk(info): """ Return True is the device is a disk. """ if device_is_cdrom(info): return False - has_range = os.path.exists("/sys/%s/range" % info['sysfs_path']) + has_range = os.path.exists("%s/range" % device_get_sysfs_path(info)) return info.get("DEVTYPE") == "disk" or has_range
def device_is_partition(info): - has_start = os.path.exists("/sys/%s/start" % info['sysfs_path']) + has_start = os.path.exists("%s/start" % device_get_sysfs_path(info)) return info.get("DEVTYPE") == "partition" or has_start
def device_is_loop(info): """ Return True if the device is a configured loop device. """ return (device_get_name(info).startswith("loop") and - os.path.isdir("/sys/%s/loop" % info['sysfs_path'])) + os.path.isdir("%s/loop" % device_get_sysfs_path(info)))
def device_get_serial(udev_info): """ Get the serial number/UUID from the device as reported by udev. """ @@ -356,7 +309,7 @@ def device_get_path(info): return info["ID_PATH"]
def device_get_symlinks(info): - return info.get("symlinks", []) + return info.get("DEVLINKS", [])
def device_get_by_path(info): for link in device_get_symlinks(info): @@ -366,7 +319,7 @@ def device_get_by_path(info): return device_get_name(info)
def device_get_sysfs_path(info): - return info['sysfs_path'] + return info.sys_path
def device_get_major(info): return int(info["MAJOR"]) @@ -675,7 +628,7 @@ def device_get_iscsi_session(info): # The position of sessionX part depends on device # (e.g. offload vs. sw; also varies for different offload devs) session = None - match = re.match(r'/.*/(session\d+)', info["sysfs_path"]) + match = re.match(r'/.*/(session\d+)', device_get_sysfs_path(info)) if match: session = match.groups()[0] else: @@ -694,7 +647,7 @@ def device_get_iscsi_nic(info): def device_get_iscsi_initiator(info): initiator = None if device_is_partoff_iscsi(info): - host = re.match(r'.*/(host\d+)', info["sysfs_path"]).groups()[0] + host = re.match(r'.*/(host\d+)', device_get_sysfs_path(info)).groups()[0] if host: initiator_file = "/sys/class/iscsi_host/%s/initiatorname" % host if os.access(initiator_file, os.R_OK): @@ -738,10 +691,10 @@ def device_get_iscsi_initiator(info):
def _detect_broadcom_fcoe(info): re_pci_host=re.compile(r'/(.*)/(host\d+)') - match = re_pci_host.match(info["sysfs_path"]) + match = re_pci_host.match(device_get_sysfs_path(info)) if match: sysfs_pci, host = match.groups() - if os.access('/sys/%s/%s/fc_host' %(sysfs_pci, host), os.X_OK) and \ + if os.access('%s/%s/fc_host' %(sysfs_pci, host), os.X_OK) and \ 'net' in sysfs_pci: return (sysfs_pci, host) return (None, None) @@ -757,7 +710,7 @@ def device_is_fcoe(info): path_components[2] == "fc": return True
- if path.startswith("fc-") and "fcoe" in info["sysfs_path"]: + if path.startswith("fc-") and "fcoe" in device_get_sysfs_path(info): return True
if _detect_broadcom_fcoe(info) != (None, None): @@ -768,20 +721,21 @@ def device_is_fcoe(info): def device_get_fcoe_nic(info): path = info.get("ID_PATH", "") path_components = path.split("-") + sysfs_path = device_get_sysfs_path(info)
if path.startswith("pci-eth") and len(path_components) >= 4 and \ path_components[2] == "fc": return path_components[1]
- if path.startswith("fc-") and "fcoe" in info["sysfs_path"]: - return info["sysfs_path"].split("/")[4].split(".")[0] + if path.startswith("fc-") and "fcoe" in sysfs_path: + return sysfs_path.split("/")[4].split(".")[0]
(sysfs_pci, host) = _detect_broadcom_fcoe(info) if (sysfs_pci, host) != (None, None): - net, iface = info['sysfs_path'].split("/")[5:7] + net, iface = sysfs_path.split("/")[5:7] if net != "net": - log.warning("unexpected sysfs_path of bnx2fc device: %s", info['sysfs_path']) - match = re.compile(r'.*/net/([^/]*)').match(info['sysfs_path']) + log.warning("unexpected sysfs_path of bnx2fc device: %s", sysfs_path) + match = re.compile(r'.*/net/([^/]*)').match(sysfs_path) if match: return match.groups()[0].split(".")[0] else: @@ -795,7 +749,7 @@ def device_get_fcoe_identifier(info): path_components[2] == "fc": return path_components[3]
- if path.startswith("fc-") and "fcoe" in info["sysfs_path"]: + if path.startswith("fc-") and "fcoe" in device_get_sysfs_path(info): return path_components[1]
if device_is_fcoe(info) and len(path_components) >= 4 and \ diff --git a/blivet/util.py b/blivet/util.py index a15bb4a..28dc125 100644 --- a/blivet/util.py +++ b/blivet/util.py @@ -178,7 +178,7 @@ def get_sysfs_attr(path, attr): log.debug("get_sysfs_attr() called with attr=None") return None
- attribute = "/sys%s/%s" % (path, attr) + attribute = "%s/%s" % (path, attr) attribute = os.path.realpath(attribute)
if not os.path.isfile(attribute) and not os.path.islink(attribute): diff --git a/python-blivet.spec b/python-blivet.spec index abbbb0f..b668d94 100644 --- a/python-blivet.spec +++ b/python-blivet.spec @@ -28,6 +28,7 @@ Requires: python Requires: python-six Requires: pykickstart >= %{pykickstartver} Requires: util-linux >= %{utillinuxver} +Requires: python-pyudev Requires: parted >= %{partedver} Requires: pyparted >= %{pypartedver} Requires: device-mapper >= %{dmver} diff --git a/tests/udev_test.py b/tests/udev_test.py index a7b09ab..86cc434 100644 --- a/tests/udev_test.py +++ b/tests/udev_test.py @@ -11,94 +11,11 @@ class UdevTest(unittest.TestCase): blivet.udev.os = mock.Mock() blivet.udev.log = mock.Mock()
- def test_udev_enumerate_devices(self): + def test_udev_get_device(self): import blivet.udev - ENUMERATE_LIST = [ - '/sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda', - '/sys/devices/virtual/block/loop0', - '/sys/devices/virtual/block/loop1', - '/sys/devices/virtual/block/ram0', - '/sys/devices/virtual/block/ram1', - '/sys/devices/virtual/block/dm-0', - ] - blivet.udev.global_udev.enumerate_devices = mock.Mock(return_value=ENUMERATE_LIST) - ret = blivet.udev.enumerate_devices() - self.assertEqual(set(ret), - set(['/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda', - '/devices/virtual/block/loop0', '/devices/virtual/block/loop1', - '/devices/virtual/block/ram0', '/devices/virtual/block/ram1', - '/devices/virtual/block/dm-0']) - ) - - def test_udev_get_device_1(self): - import blivet.udev - - class Device(object): - def __init__(self): - self.sysname = 'loop1' - self.dict = {'symlinks': ['/dev/block/7:1'], - 'SUBSYSTEM': 'block', - 'MAJOR': '7', - 'DEVPATH': '/devices/virtual/block/loop1', - 'UDISKS_PRESENTATION_NOPOLICY': '1', - 'UDEV_LOG': '3', - 'DEVNAME': '/dev/loop1', - 'DEVTYPE': 'disk', - 'DEVLINKS': '/dev/block/7:1', - 'MINOR': '1' - } - - def __getitem__(self, key): - return self.dict[key] - - def __setitem__(self, key, value): - self.dict[key] = value - - blivet.udev.os.path.exists.return_value = True - DEV_PATH = '/devices/virtual/block/loop1' - dev = Device() - blivet.udev.global_udev = mock.Mock() - blivet.udev.global_udev.create_device.return_value = dev - - saved = blivet.udev.parse_uevent_file - blivet.udev.parse_uevent_file = mock.Mock(return_value=dev) - - ret = blivet.udev.get_device(DEV_PATH) - self.assertTrue(isinstance(ret, Device)) - self.assertEqual(ret['name'], ret.sysname) - self.assertEqual(ret['sysfs_path'], DEV_PATH) - self.assertTrue(blivet.udev.parse_uevent_file.called) - - blivet.udev.parse_uevent_file = saved - - def test_udev_get_device_2(self): - import blivet.udev - blivet.udev.os.path.exists.return_value = False - ret = blivet.udev.get_device('') - self.assertEqual(ret, None) - - def test_udev_get_device_3(self): - import blivet.udev - blivet.udev.os.path.exists.return_value = True - blivet.udev.global_udev = mock.Mock() - blivet.udev.global_udev.create_device.return_value = None - ret = blivet.udev.get_device('') - self.assertEqual(ret, None) - - def test_udev_get_devices(self): - import blivet.udev - saved = blivet.udev.settle - blivet.udev.settle = mock.Mock() - DEVS = \ - ['/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda', - '/devices/virtual/block/loop0', '/devices/virtual/block/loop1', - '/devices/virtual/block/ram0', '/devices/virtual/block/ram1', - '/devices/virtual/block/dm-0'] - blivet.udev.enumerate_devices = mock.Mock(return_value=DEVS) - blivet.udev.get_device = lambda x: x - ret = blivet.udev.get_devices() - self.assertEqual(ret, DEVS) - blivet.udev.settle = saved + devices = blivet.udev.global_udev.list_devices(subsystem="block") + for device in devices: + self.assertNotEqual(blivet.udev.get_device(device.sys_path), None)
def test_udev_parse_uevent_file_1(self): import blivet.udev
On Tue, 2014-07-29 at 13:10 -0500, David Lehman wrote:
def backupConfigs(self, restore=False):@@ -2154,18 +2154,19 @@ class DeviceTree(object): # blocks or since previous iterations. while True: devices = []
new_devices = udev.get_block_devices()
new_devices = udev.get_devices() for new_device in new_devices:
if new_device['name'] not in old_devices:old_devices[new_device['name']] = new_device
new_name = udev.device_get_name(new_device)if not old_devices.has_key(new_name):
This is not valid in Python3, please use 'new_name in old_devices' instead of has_key()
diff --git a/blivet/udev.py b/blivet/udev.py index b66e30b..b87295b 100644 --- a/blivet/udev.py +++ b/blivet/udev.py @@ -27,45 +27,28 @@ import re from . import util from .size import Size
-from . import pyudev -global_udev = pyudev.Udev() +import pyudev +global_udev = pyudev.Context()
import logging log = logging.getLogger("blivet")
-def enumerate_devices(deviceClass="block"):
- devices = global_udev.enumerate_devices(subsystem=deviceClass)
- return [path[4:] for path in devices]
def get_device(sysfs_path):
- if not os.path.exists("/sys%s" % sysfs_path):
log.debug("%s does not exist", sysfs_path)return None- # XXX we remove the /sys part when enumerating devices,
- # so we have to prepend it when creating the device
- dev = global_udev.create_device("/sys" + sysfs_path)
- if dev:
dev["name"] = dev.sysnamedev["sysfs_path"] = sysfs_path# now add in the contents of the uevent file since they're handydev = parse_uevent_file(dev)
try:
dev = pyudev.Device.from_sys_path(global_udev, sysfs_path)except pyudev.DeviceNotFoundError as e:
log.error(e)dev = Nonereturn dev
-def get_devices(deviceClass="block"): +def get_devices(subsystem="block"): settle()
- entries = []
- for path in enumerate_devices(deviceClass):
entry = get_device(path)if entry:entries.append(entry)- return entries
- return [d for d in global_udev.list_devices(subsystem=subsystem)
if not __is_blacklisted_blockdev(d.sys_name)]def parse_uevent_file(dev):
- path = os.path.normpath("/sys/%s/uevent" % dev['sysfs_path'])
- path = os.path.normpath("%s/uevent" % device_get_sysfs_path(dev)) if not os.access(path, os.R_OK): return dev
@@ -103,7 +86,7 @@ def resolve_devspec(devspec): from . import devices
ret = None
- for dev in get_block_devices():
- for dev in get_devices(): if devspec.startswith("LABEL="): if device_get_label(dev) == devspec[6:]: ret = dev
@@ -120,7 +103,7 @@ def resolve_devspec(devspec): if not spec.startswith("/dev/"): spec = os.path.normpath("/dev/" + spec)
for link in dev["symlinks"]:
for link in device_get_symlinks(dev): if spec == link: ret = dev break@@ -135,37 +118,18 @@ def resolve_glob(glob): if not glob: return ret
- for dev in get_block_devices():
for dev in get_devices(): name = device_get_name(dev)
if fnmatch.fnmatch(name, glob): ret.append(name) else:
for link in dev["symlinks"]:
for link in device_get_symlinks(dev): if fnmatch.fnmatch(link, glob): ret.append(name)return ret
-def get_block_devices():
- settle()
- entries = []
- for path in enumerate_block_devices():
entry = get_block_device(path)if entry:if entry["name"].startswith("md"):# mdraid is really braindead, when a device is stopped# it is no longer usefull in anyway (and we should not# probe it) yet it still sticks around, see bug rh523387state = Nonestate_file = "/sys/%s/md/array_state" % entry["sysfs_path"]if os.access(state_file, os.R_OK):state = open(state_file).read().strip()if state == "clear":continueentries.append(entry)- return entries
def __is_blacklisted_blockdev(dev_name): """Is this a blockdev we never want for an install?""" if dev_name.startswith("ram") or dev_name.startswith("fd"): @@ -180,17 +144,6 @@ def __is_blacklisted_blockdev(dev_name):
return False-def enumerate_block_devices():
- return [d for d in enumerate_devices(deviceClass="block") if not __is_blacklisted_blockdev(os.path.basename(d))]
-def get_block_device(sysfs_path):
- dev = get_device(sysfs_path)
- if not dev or 'name' not in dev:
return None- else:
return dev# These are functions for retrieving specific pieces of information from # udev database entries. def device_get_name(udev_info): @@ -198,7 +151,7 @@ def device_get_name(udev_info): if "DM_NAME" in udev_info: name = udev_info["DM_NAME"] else:
name = udev_info["name"]
name = udev_info.sys_namereturn name
@@ -236,7 +189,7 @@ def device_is_md(info):
# The udev information keeps shifting around. Only md arrays have a # /sys/class/block/<name>/md/ subdirectory.
- md_dir = "/sys" + device_get_sysfs_path(info) + "/md"
- md_dir = device_get_sysfs_path(info) + "/md" return os.path.exists(md_dir)
def device_is_cciss(info): @@ -256,7 +209,7 @@ def device_is_zfcp(info): if info.get("DEVTYPE") != "disk": return False
- subsystem = "/sys" + info.get("sysfs_path")
subsystem = device_get_sysfs_path(info)
while True: topdir = os.path.realpath(os.path.dirname(subsystem))
@@ -284,7 +237,7 @@ def device_get_zfcp_attribute(info, attr=None): log.debug("device_get_zfcp_attribute() called with attr=None") return None
- attribute = "/sys%s/device/%s" % (info.get("sysfs_path"), attr,)
- attribute = "%s/device/%s" % (device_get_sysfs_path(info), attr,)
You can remove the trailing comma here while changing this line.
On Tue, Jul 29, 2014 at 01:10:58PM -0500, David Lehman wrote:
- if path.startswith("fc-") and "fcoe" in info["sysfs_path"]:
return info["sysfs_path"].split("/")[4].split(".")[0]
if path.startswith("fc-") and "fcoe" in sysfs_path:
return sysfs_path.split("/")[4].split(".")[0](sysfs_pci, host) = _detect_broadcom_fcoe(info) if (sysfs_pci, host) != (None, None):
net, iface = info['sysfs_path'].split("/")[5:7]
net, iface = sysfs_path.split("/")[5:7]
Shouldn't these indexes be bumped up by 1 now that /sys/ is in the path?
On 07/31/2014 06:37 PM, Brian C. Lane wrote:
On Tue, Jul 29, 2014 at 01:10:58PM -0500, David Lehman wrote:
- if path.startswith("fc-") and "fcoe" in info["sysfs_path"]:
return info["sysfs_path"].split("/")[4].split(".")[0]
if path.startswith("fc-") and "fcoe" in sysfs_path:
return sysfs_path.split("/")[4].split(".")[0] (sysfs_pci, host) = _detect_broadcom_fcoe(info) if (sysfs_pci, host) != (None, None):
net, iface = info['sysfs_path'].split("/")[5:7]
net, iface = sysfs_path.split("/")[5:7]Shouldn't these indexes be bumped up by 1 now that /sys/ is in the path?
Yes -- good catch.
Since we cannot inject data into pyudev's Device entries we have to store the metadata for use from various places unless we want to call mdexamine multiple times per device. --- blivet/devicetree.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 80ddbb1..670203a 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -134,7 +134,7 @@ class DeviceTree(object): self.unusedRaidMembers = []
# initialize attributes that may later hold cached lvm info - self.dropLVMCache() + self.dropCache()
self.__passphrases = [] if passphrase: @@ -188,6 +188,20 @@ class DeviceTree(object): self._pvInfo = None # pylint: disable=attribute-defined-outside-init self._lvInfo = None # pylint: disable=attribute-defined-outside-init
+ @property + def mdInfo(self): + if self._mdInfo is None: + self._mdInfo = {} + + return self._mdInfo + + def dropMDCache(self): + self._mdInfo = None + + def dropCache(self): + self.dropLVMCache() + self.dropMDCache() + def pruneActions(self): """ Remove redundant/obsolete actions from the action list. """ for action in reversed(self._actions[:]): @@ -1816,11 +1830,12 @@ class DeviceTree(object): # luks/dmcrypt kwargs["name"] = "luks-%s" % uuid elif format_type in formats.mdraid.MDRaidMember._udevTypes: - info.update(mdraid.mdexamine(device.path)) + md_info = mdraid.mdexamine(device.path) + self.mdInfo[device.path] = md_info
# mdraid try: - kwargs["mdUuid"] = udev.device_get_md_uuid(info) + kwargs["mdUuid"] = udev.device_get_md_uuid(md_info) except KeyError: log.warning("mdraid member %s has no md uuid", name)
@@ -2116,7 +2131,7 @@ class DeviceTree(object): # this has proven useful when populating after opening a LUKS device udev.settle()
- self.dropLVMCache() + self.dropCache()
if flags.installer_mode and not flags.image_install: mpath.set_friendly_names(enabled=flags.multipath_friendly_names)
The functionality this block originally provided should be covered by the UUID lookup we do in addUdevMDDevice.
This block no longer works because it relies on md-specific data being in the udev database itself, which is no longer the case. It hasn't been since we removed anaconda's udev rules. --- blivet/devicetree.py | 29 ----------------------------- 1 file changed, 29 deletions(-)
diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 670203a..e2d0db1 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -1599,35 +1599,6 @@ class DeviceTree(object): md_metadata = None md_name = None
- # check the list of devices udev knows about to see if the array - # this device belongs to is already active - for dev in udev.get_devices(): - if not udev.device_is_md(dev): - continue - - try: - dev_uuid = udev.device_get_md_uuid(dev) - dev_level = udev.device_get_md_level(dev) - except KeyError: - continue - - if dev_uuid is None or dev_level is None: - continue - - if dev_uuid == md_uuid and dev_level == md_level: - md_name = udev.device_get_md_name(dev) - md_metadata = udev.device_get_md_metadata(dev) - if not md_name: - # containers don't typically have names and they also - # don't have a symlink in /dev/md - md_name = udev.device_get_name(dev) - if md_level != "container" and \ - re.match(r'md\d+$', md_name): - # md0 -> 0 - md_name = md_name[2:] - - break - # mdexamine yields MD_METADATA only for metadata version > 0.90 # if MD_METADATA is missing, assume metadata version is 0.90 md_metadata = md_metadata or udev.device_get_md_metadata(info) or "0.90"
----- Original Message -----
From: "David Lehman" dlehman@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Tuesday, July 29, 2014 8:11:00 PM Subject: [PATCH 3/5] Remove an obsolete block related to unpredictable md device names.
The functionality this block originally provided should be covered by the UUID lookup we do in addUdevMDDevice.
This block no longer works because it relies on md-specific data being in the udev database itself, which is no longer the case. It hasn't been since we removed anaconda's udev rules.
blivet/devicetree.py | 29 ----------------------------- 1 file changed, 29 deletions(-)
diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 670203a..e2d0db1 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -1599,35 +1599,6 @@ class DeviceTree(object): md_metadata = None md_name = None
# check the list of devices udev knows about to see if the array# this device belongs to is already activefor dev in udev.get_devices():if not udev.device_is_md(dev):continuetry:dev_uuid = udev.device_get_md_uuid(dev)dev_level = udev.device_get_md_level(dev)except KeyError:continueif dev_uuid is None or dev_level is None:continueif dev_uuid == md_uuid and dev_level == md_level:md_name = udev.device_get_md_name(dev)md_metadata = udev.device_get_md_metadata(dev)if not md_name:# containers don't typically have names and theyalso
# don't have a symlink in /dev/mdmd_name = udev.device_get_name(dev)if md_level != "container" and \re.match(r'md\d+$', md_name):# md0 -> 0md_name = md_name[2:]break# mdexamine yields MD_METADATA only for metadata version > 0.90 # if MD_METADATA is missing, assume metadata version is 0.90 md_metadata = md_metadata or udev.device_get_md_metadata(info) or "0.90"-- 1.9.3
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
I'm not nearly done with this patch set as a whole, but for this patch...
It's great to see this chunk of code disappear. You can disappear a few more lines by getting rid of the assignments to md_metadata and md_name just above. You can then simplify md_metadata = md_metadata or udev.device_get_md_metadata(info) or "0.90 to md_metadata = udev.device_get_md_metadata(info) or "0.90
There's an if block below with condition "not md_name". But md_name is unset at this point, so the code in the block should be executed unconditionally. (I don't fully understand what this code does, actually. Perhaps it needs some revision as well, I can't tell.)
Also, it's wierd but some of this md info _is_ obtained by udev/pyudev, i.e., not mdraid specific tools. I was so surprised to find that out that I annotated the md_* methods in udev.py with information about the provenance of the various key/value pairs.
- mulhern
On 07/30/2014 07:11 AM, Anne Mulhern wrote:
----- Original Message -----
From: "David Lehman" dlehman@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Tuesday, July 29, 2014 8:11:00 PM Subject: [PATCH 3/5] Remove an obsolete block related to unpredictable md device names.
The functionality this block originally provided should be covered by the UUID lookup we do in addUdevMDDevice.
This block no longer works because it relies on md-specific data being in the udev database itself, which is no longer the case. It hasn't been since we removed anaconda's udev rules.
blivet/devicetree.py | 29 ----------------------------- 1 file changed, 29 deletions(-)
diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 670203a..e2d0db1 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -1599,35 +1599,6 @@ class DeviceTree(object): md_metadata = None md_name = None
# check the list of devices udev knows about to see if the array# this device belongs to is already activefor dev in udev.get_devices():if not udev.device_is_md(dev):continuetry:dev_uuid = udev.device_get_md_uuid(dev)dev_level = udev.device_get_md_level(dev)except KeyError:continueif dev_uuid is None or dev_level is None:continueif dev_uuid == md_uuid and dev_level == md_level:md_name = udev.device_get_md_name(dev)md_metadata = udev.device_get_md_metadata(dev)if not md_name:# containers don't typically have names and theyalso
# don't have a symlink in /dev/mdmd_name = udev.device_get_name(dev)if md_level != "container" and \re.match(r'md\d+$', md_name):# md0 -> 0md_name = md_name[2:]break# mdexamine yields MD_METADATA only for metadata version > 0.90 # if MD_METADATA is missing, assume metadata version is 0.90 md_metadata = md_metadata or udev.device_get_md_metadata(info) or "0.90"-- 1.9.3
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
I'm not nearly done with this patch set as a whole, but for this patch...
It's great to see this chunk of code disappear. You can disappear a few more lines by getting rid of the assignments to md_metadata and md_name just above. You can then simplify md_metadata = md_metadata or udev.device_get_md_metadata(info) or "0.90 to md_metadata = udev.device_get_md_metadata(info) or "0.90
There's an if block below with condition "not md_name". But md_name is unset at this point, so the code in the block should be executed unconditionally. (I don't fully understand what this code does, actually. Perhaps it needs some revision as well, I can't tell.)
Also, it's wierd but some of this md info _is_ obtained by udev/pyudev, i.e., not mdraid specific tools. I was so surprised to find that out that I annotated the md_* methods in udev.py with information about the provenance of the various key/value pairs.
I forgot that the md udev rules include the examine output for arrays. That would be enough to make the removed block functional, but I would still prefer that we keep the more concise approach from addUdevMDDevice instead. Unfortunately, there's a little bug in that method that prevents it from being very useful for this. I am withdrawing this patch from the set since it needs work.
David
- mulhern
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
We can't inject data into pyudev.Device instances like we could when the udev db entries were plain dicts. --- blivet/devicetree.py | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/blivet/devicetree.py b/blivet/devicetree.py index e2d0db1..d67c114 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -1534,10 +1534,11 @@ class DeviceTree(object):
def handleUdevLVMPVFormat(self, info, device): log_method_call(self, name=device.name, type=device.format.type) + pv_info = self.pvInfo.get(device.path, {}) # lookup/create the VG and LVs try: - vg_name = udev.device_get_vg_name(info) - vg_uuid = udev.device_get_vg_uuid(info) + vg_name = udev.device_get_vg_name(pv_info) + vg_uuid = udev.device_get_vg_uuid(pv_info) except KeyError: # no vg name means no vg -- we're done with this pv return @@ -1551,12 +1552,12 @@ class DeviceTree(object): vg_device.parents.append(device) else: try: - vg_size = udev.device_get_vg_size(info) - vg_free = udev.device_get_vg_free(info) - pe_size = udev.device_get_vg_extent_size(info) - pe_count = udev.device_get_vg_extent_count(info) - pe_free = udev.device_get_vg_free_extents(info) - pv_count = udev.device_get_vg_pv_count(info) + vg_size = udev.device_get_vg_size(pv_info) + vg_free = udev.device_get_vg_free(pv_info) + pe_size = udev.device_get_vg_extent_size(pv_info) + pe_count = udev.device_get_vg_extent_count(pv_info) + pe_free = udev.device_get_vg_free_extents(pv_info) + pv_count = udev.device_get_vg_pv_count(pv_info) except (KeyError, ValueError) as e: log.warning("invalid data for %s: %s", device.name, e) return @@ -1577,7 +1578,7 @@ class DeviceTree(object):
def handleUdevMDMemberFormat(self, info, device): log_method_call(self, name=device.name, type=device.format.type) - + md_info = self.mdInfo.get(device.path, {}) md_array = self.getDeviceByUuid(device.format.mdUuid, incomplete=True) if device.format.mdUuid and md_array: md_array.parents.append(device) @@ -1585,9 +1586,9 @@ class DeviceTree(object): # create the array with just this one member try: # level is reported as, eg: "raid1" - md_level = udev.device_get_md_level(info) - md_devices = udev.device_get_md_devices(info) - md_uuid = udev.device_get_md_uuid(info) + md_level = udev.device_get_md_level(md_info) + md_devices = udev.device_get_md_devices(md_info) + md_uuid = udev.device_get_md_uuid(md_info) except (KeyError, ValueError) as e: log.warning("invalid data for %s: %s", device.name, e) return @@ -1601,10 +1602,10 @@ class DeviceTree(object):
# mdexamine yields MD_METADATA only for metadata version > 0.90 # if MD_METADATA is missing, assume metadata version is 0.90 - md_metadata = md_metadata or udev.device_get_md_metadata(info) or "0.90" + md_metadata = md_metadata or udev.device_get_md_metadata(md_info) or "0.90"
if not md_name: - md_path = info.get("DEVICE", "") + md_path = md_info.get("DEVICE", "") if md_path: md_name = devicePathToName(md_path) if re.match(r'md\d+$', md_name): @@ -1817,18 +1818,18 @@ class DeviceTree(object): kwargs["biosraid"] = udev.device_is_biosraid_member(info) elif format_type == "LVM2_member": # lvm - info.update(self.pvInfo.get(device.path, {})) + pv_info = self.pvInfo.get(device.path, {})
try: - kwargs["vgName"] = udev.device_get_vg_name(info) + kwargs["vgName"] = udev.device_get_vg_name(pv_info) except KeyError: log.warning("PV %s has no vg_name", name) try: - kwargs["vgUuid"] = udev.device_get_vg_uuid(info) + kwargs["vgUuid"] = udev.device_get_vg_uuid(pv_info) except KeyError: log.warning("PV %s has no vg_uuid", name) try: - kwargs["peStart"] = udev.device_get_pv_pe_start(info) + kwargs["peStart"] = udev.device_get_pv_pe_start(pv_info) except KeyError: log.warning("PV %s has no pe_start", name) elif format_type == "vfat":
The main difference is that we can't write to pyudev.Device instances. --- blivet/devicetree.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/blivet/devicetree.py b/blivet/devicetree.py index d67c114..d4e8656 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -1089,9 +1089,6 @@ class DeviceTree(object): minor=udev.device_get_minor(info), sysfsPath=sysfs_path, **kwargs)
- if mpath.is_multipath_member(device.path): - info["ID_FS_TYPE"] = "multipath_member" - if diskType == DASDDevice: self.dasd.append(device)
@@ -1177,8 +1174,6 @@ class DeviceTree(object):
if device and device.isDisk and \ mpath.is_multipath_member(device.path): - # mark as multipath_member also when repopulating devicetree - info["ID_FS_TYPE"] = "multipath_member" # newly added device (eg iSCSI) could make this one a multipath member if device.format and device.format.type != "multipath_member": log.debug("%s newly detected as multipath member, dropping old format and removing kids", device.name) @@ -1767,11 +1762,15 @@ class DeviceTree(object): format_type = udev.device_get_format(info) serial = udev.device_get_serial(info)
+ is_multipath_member = mpath.is_multipath_member(device.path) + if is_multipath_member: + format_type = "multipath_member" + # Now, if the device is a disk, see if there is a usable disklabel. # If not, see if the user would like to create one. # XXX ignore disklabels on multipath or biosraid member disks if not udev.device_is_biosraid_member(info) and \ - not udev.device_is_multipath_member(info) and \ + not is_multipath_member and \ format_type != "iso9660": self.handleUdevDiskLabelFormat(info, device) if device.partitioned or self.isIgnored(info) or \
This should also include removal of device_is_multipath_member and device_get_multipath_name from blivet.udev since neither are used once this patch is in place.
David
----- Original Message -----
From: "David Lehman" dlehman@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Tuesday, July 29, 2014 8:37:47 PM Subject: Re: [PATCH 5/5] Adapt multipath detection code to external pyudev module.
This should also include removal of device_is_multipath_member and device_get_multipath_name from blivet.udev since neither are used once this patch is in place.
David _______________________________________________ anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
This looks fine to me.
- mulhern
On Tue, 2014-07-29 at 13:10 -0500, David Lehman wrote:
This is nice in that we can drop some custom code, but python-pyudev also provides classes that make it really easy to listen for uevents.
David Lehman (5): Replace our pyudev with the package python-pyudev. Cache MD metadata while populating the devicetree. Remove an obsolete block related to unpredictable md device names. Keep lvm and md metadata separate from udev info. Adapt multipath detection code to external pyudev module.
blivet/deviceaction.py | 4 +- blivet/devices.py | 95 +++++++-------------- blivet/devicetree.py | 147 +++++++++++++++----------------- blivet/pyudev.py | 226 ------------------------------------------------- blivet/udev.py | 122 +++++++++----------------- blivet/util.py | 2 +- python-blivet.spec | 1 + tests/udev_test.py | 91 +------------------- 8 files changed, 141 insertions(+), 547 deletions(-) delete mode 100644 blivet/pyudev.py
Other than that two comments these look good to me. Good riddance! And a giant leap towards listening to udev events, I guess?
anaconda-patches@lists.fedorahosted.org
-
Anne Mulhern -
Brian C. Lane -
David Lehman -
Vratislav Podzimek