From d0dbd9b6da97e5d51c50ad33e268a10ae8721284 Mon Sep 17 00:00:00 2001 From: David Lehman Date: Wed, 5 Nov 2014 16:42:37 -0600 Subject: [PATCH 3/5] Don't use parted.Device to obtain size info. In order to see updated size info when a device is resize from outside of blivet, it is necessary to delete the parted.Device and make another one. This is happening from a change uevent handler. Deleting the parted.Device causes parted to close its rw fd for the device, which triggers another change uevent. We would have to ignore some of these change events to avoid an infinite loop. Something similar happens when resetting a Blivet instance. All devices are deleted, including their parted.Device instances. This triggers change uevents, which causes mdadm's udev rules to activate all arrays on those devices, which is obviously not something we want to be triggering on a reset. This one seems impossible to avoid as long as we're using the parted.Device. Related: rhbz#1069597 --- blivet/devices/device.py | 2 +- blivet/devices/disk.py | 13 +----- blivet/devices/dm.py | 3 +- blivet/devices/file.py | 10 +++++ blivet/devices/lib.py | 3 ++ blivet/devices/luks.py | 6 +-- blivet/devices/md.py | 6 +-- blivet/devices/partition.py | 30 +++++++------- blivet/devices/storage.py | 96 ++++++++++++++++++++------------------------- blivet/devicetree.py | 7 +++- blivet/platform.py | 10 +++-- tests/devices_test.py | 2 +- tests/storagetestcase.py | 7 +--- 13 files changed, 93 insertions(+), 102 deletions(-) diff --git a/blivet/devices/device.py b/blivet/devices/device.py index 3931eda..58dad62 100644 --- a/blivet/devices/device.py +++ b/blivet/devices/device.py @@ -97,7 +97,7 @@ class Device(util.ObjectID): """ return util.variable_copy(self, memo, omit=('_raidSet', 'node'), - shallow=('_partedDevice', '_partedPartition')) + shallow=('_partedPartition')) def __repr__(self): s = ("%(type)s instance (%(id)s) --\n" diff --git a/blivet/devices/disk.py b/blivet/devices/disk.py index f14638d..48bb899 100644 --- a/blivet/devices/disk.py +++ b/blivet/devices/disk.py @@ -89,8 +89,7 @@ class DiskDevice(StorageDevice): def __repr__(self): s = StorageDevice.__repr__(self) - s += (" removable = %(removable)s partedDevice = %(partedDevice)r" % - {"removable": self.removable, "partedDevice": self.partedDevice}) + s += (" removable = %(removable)s" % {"removable": self.removable}) return s @property @@ -98,23 +97,15 @@ class DiskDevice(StorageDevice): if flags.testing: return True - if not self.partedDevice: - return False - # Some drivers (cpqarray ) make block device nodes for # controllers with no disks attached and then report a 0 size, # treat this as no media present - return Size(self.partedDevice.getLength(unit="B")) != Size(0) + return self.exists and self.currentSize > Size(0) @property def description(self): return self.model - @property - def size(self): - """ The disk's size """ - return super(DiskDevice, self).size - def _preDestroy(self): """ Destroy the device. """ log_method_call(self, self.name, status=self.status) diff --git a/blivet/devices/dm.py b/blivet/devices/dm.py index 9234c6e..978425d 100644 --- a/blivet/devices/dm.py +++ b/blivet/devices/dm.py @@ -34,6 +34,7 @@ import logging log = logging.getLogger("blivet") from .storage import StorageDevice +from .lib import LINUX_SECTOR_SIZE class DMDevice(StorageDevice): """ A device-mapper device """ @@ -182,7 +183,7 @@ class DMLinearDevice(DMDevice): """ Open, or set up, a device. """ log_method_call(self, self.name, orig=orig, status=self.status, controllable=self.controllable) - slave_length = self.slave.partedDevice.length + slave_length = self.slave.currentSize / LINUX_SECTOR_SIZE dm.dm_create_linear(self.name, self.slave.path, slave_length, self.dmUuid) diff --git a/blivet/devices/file.py b/blivet/devices/file.py index 0997035..2db7908 100644 --- a/blivet/devices/file.py +++ b/blivet/devices/file.py @@ -21,9 +21,11 @@ # import os +import stat from .. import util from ..storage_log import log_method_call +from ..size import Size import logging log = logging.getLogger("blivet") @@ -79,6 +81,14 @@ class FileDevice(StorageDevice): return os.path.normpath("%s%s" % (root, self.name)) + def _getSize(self): + size = self._size + if self.exists: + st = os.stat(self.path) + size = Size(st[stat.ST_SIZE]) + + return size + def _preSetup(self, orig=False): if self.format and self.format.exists and not self.format.status: self.format.device = self.path diff --git a/blivet/devices/lib.py b/blivet/devices/lib.py index 369b64d..14e9501 100644 --- a/blivet/devices/lib.py +++ b/blivet/devices/lib.py @@ -22,6 +22,9 @@ import os from .. import errors from .. import udev +from ..size import Size + +LINUX_SECTOR_SIZE = Size(512) def get_device_majors(): majors = {} diff --git a/blivet/devices/luks.py b/blivet/devices/luks.py index f02cd28..197926f 100644 --- a/blivet/devices/luks.py +++ b/blivet/devices/luks.py @@ -22,8 +22,6 @@ # device backend modules from ..devicelibs import crypto -from ..size import Size - import logging log = logging.getLogger("blivet") @@ -63,10 +61,10 @@ class LUKSDevice(DMCryptDevice): @property def size(self): - if not self.exists or not self.partedDevice: + if not self.exists: size = self.slave.size - crypto.LUKS_METADATA_SIZE else: - size = Size(self.partedDevice.getLength(unit="B")) + size = self.currentSize return size def _postCreate(self): diff --git a/blivet/devices/md.py b/blivet/devices/md.py index 105ee1e..d7f5167 100644 --- a/blivet/devices/md.py +++ b/blivet/devices/md.py @@ -200,7 +200,7 @@ class MDRaidArrayDevice(ContainerDevice, RaidDevice): """Returns the actual or estimated size depending on whether or not the array exists. """ - if not self.exists or not self.partedDevice: + if not self.exists or not self.mediaPresent: try: size = self.level.get_size([d.size for d in self.devices], self.memberDevices, @@ -211,7 +211,7 @@ class MDRaidArrayDevice(ContainerDevice, RaidDevice): size = Size(0) log.debug("non-existent RAID %s size == %s", self.level, size) else: - size = Size(self.partedDevice.getLength(unit="B")) + size = self.currentSize log.debug("existing RAID %s size == %s", self.level, size) return size @@ -531,7 +531,7 @@ class MDRaidArrayDevice(ContainerDevice, RaidDevice): if flags.testing: return True else: - return self.partedDevice is not None + return super(MDRaidArrayDevice, self).mediaPresent @property def model(self): diff --git a/blivet/devices/partition.py b/blivet/devices/partition.py index a532fc5..46e2f13 100644 --- a/blivet/devices/partition.py +++ b/blivet/devices/partition.py @@ -134,7 +134,6 @@ class PartitionDevice(StorageDevice): self._partType = None self._partedPartition = None self._origPath = None - self._currentSize = Size(0) # FIXME: Validate size, but only if this is a new partition. # For existing partitions we will get the size from @@ -515,7 +514,6 @@ class PartitionDevice(StorageDevice): return self._size = Size(self.partedPartition.getLength(unit="B")) - self._currentSize = self._size self.targetSize = self._size self._partType = self.partedPartition.type @@ -577,7 +575,6 @@ class PartitionDevice(StorageDevice): DeviceFormat(device=self.path, exists=True).destroy() StorageDevice._postCreate(self) - self._currentSize = Size(self.partedPartition.getLength(unit="B")) def create(self): """ Create the device. """ @@ -646,7 +643,6 @@ class PartitionDevice(StorageDevice): end=geometry.end) self.disk.format.commit() - self._currentSize = Size(partition.getLength(unit="B")) def _preDestroy(self): StorageDevice._preDestroy(self) @@ -699,25 +695,36 @@ class PartitionDevice(StorageDevice): return size def _setSize(self, newsize): - """ Set the device's size (for resize, not creation). + """ Set the device's size. - Arguments: + + Most devices have two scenarios for setting a size: - newsize -- the new size + 1) set actual/current size + 2) set target for resize + Partitions have a third scenario: + + 3) update size of an allocated-but-non-existent partition """ log_method_call(self, self.name, status=self.status, size=self._size, newsize=newsize) if not isinstance(newsize, Size): raise ValueError("new size must of type Size") - if not self.exists: + if not self.exists and not self.partedPartition: # device does not exist (a partition request), just set basic value self._size = newsize self.req_size = newsize self.req_base_size = newsize return + if self.exists: + super(PartitionDevice, self)._setSize(newsize) + return + + # the rest is for changing the size of an allocated-but-not-existing + # partition, which I'm not sure is advisable if newsize > self.disk.size: raise ValueError("partition size would exceed disk size") @@ -802,13 +809,6 @@ class PartitionDevice(StorageDevice): return self.alignTargetSize(unalignedMax) @property - def currentSize(self): - if self.exists: - return self._currentSize - else: - return Size(0) - - @property def resizable(self): return super(PartitionDevice, self).resizable and \ self.disk.type != 'dasd' diff --git a/blivet/devices/storage.py b/blivet/devices/storage.py index 311df3e..6c66a2f 100644 --- a/blivet/devices/storage.py +++ b/blivet/devices/storage.py @@ -22,8 +22,6 @@ import os import copy -import parted -import _ped import pyudev from .. import errors @@ -39,6 +37,7 @@ log = logging.getLogger("blivet") from .device import Device from .network import NetworkStorageDevice +from .lib import LINUX_SECTOR_SIZE class StorageDevice(Device): """ A generic storage device. @@ -106,7 +105,11 @@ class StorageDevice(Device): super(StorageDevice, self).__init__(name, parents=parents) self._format = None + + # XXX Should we trust passed in size or always consult /sys? + # LVM snapshots probably need us to trust passed-in value. self._size = Size(util.numeric_type(size)) + self._currentSize = self._size if self.exists else Size(0) self.major = util.numeric_type(major) self.minor = util.numeric_type(minor) self._serial = serial @@ -125,17 +128,6 @@ class StorageDevice(Device): self.deviceLinks = [] - if self.exists and flags.testing and not self._size: - def read_int_from_sys(path): - return int(open(path).readline().strip()) - - device_root = "/sys/class/block/%s" % self.name - if os.path.exists("%s/queue" % device_root): - sector_size = read_int_from_sys("%s/queue/logical_block_size" - % device_root) - size = read_int_from_sys("%s/size" % device_root) - self._size = Size(size * sector_size) - def __str__(self): exist = "existing" if not self.exists: @@ -199,26 +191,6 @@ class StorageDevice(Device): """ True if this device, or any it requires, is encrypted. """ return self._encrypted or any(p.encrypted for p in self.parents) - def _getPartedDevicePath(self): - return self.path - - @property - def partedDevice(self): - devicePath = self._getPartedDevicePath() - if self.exists and self.status and not self._partedDevice: - log.debug("looking up parted Device: %s", devicePath) - - # We aren't guaranteed to be able to get a device. In - # particular, built-in USB flash readers show up as devices but - # do not always have any media present, so parted won't be able - # to find a device. - try: - self._partedDevice = parted.Device(path=devicePath) - except (_ped.IOException, _ped.DeviceException): - pass - - return self._partedDevice - @property def raw_device(self): """ The device itself, or when encrypted, the backing device. """ @@ -280,12 +252,12 @@ class StorageDevice(Device): " format = %(format)s\n" " major = %(major)s minor = %(minor)s exists = %(exists)s" " protected = %(protected)s\n" - " sysfs path = %(sysfs)s partedDevice = %(partedDevice)s\n" + " sysfs path = %(sysfs)s\n" " target size = %(targetSize)s path = %(path)s\n" " format args = %(formatArgs)s originalFormat = %(origFmt)s" % {"uuid": self.uuid, "format": self.format, "size": self.size, "major": self.major, "minor": self.minor, "exists": self.exists, - "sysfs": self.sysfsPath, "partedDevice": self.partedDevice, + "sysfs": self.sysfsPath, "targetSize": self.targetSize, "path": self.path, "protected": self.protected, "formatArgs": self.formatArgs, "origFmt": self.originalFormat.type}) @@ -509,7 +481,6 @@ class StorageDevice(Device): # make sure that targetSize is updated to reflect the actual size if self.resizable: - self._partedDevice = None self._targetSize = self.currentSize self._updateNetDevMountOption() @@ -558,32 +529,46 @@ class StorageDevice(Device): def _getSize(self): """ Get the device's size, accounting for pending changes. """ - if self.exists and not self.mediaPresent: - return Size(0) - - if self.exists and self.partedDevice: - self._size = self.currentSize - size = self._size - if self.exists and self.resizable: + if self.exists and self.resizable and self.targetSize: size = self.targetSize return size def _setSize(self, newsize): - """ Set the device's size to a new value. """ + """ Set the device's size to a new value. + + This is not adequate to set up a resize as it does not set a new + target size for the device. + """ if not isinstance(newsize, Size): raise ValueError("new size must of type Size") - if self.maxSize and newsize > self.maxSize: + # only calculate these once + max_size = self.maxSize + min_size = self.minSize + if max_size and newsize > max_size: raise errors.DeviceError("device cannot be larger than %s" % - (self.maxSize,), self.name) + max_size, self.name) + elif min_size and newsize < min_size: + raise errors.DeviceError("device cannot be smaller than %s" % + min_size, self.name) + self._size = newsize size = property(lambda x: x._getSize(), lambda x, y: x._setSize(y), doc="The device's size, accounting for pending changes") + def readCurrentSize(self): + size = Size(0) + if self.exists and os.path.exists(self.path) and \ + os.path.isdir(self.sysfsPath): + blocks = int(util.get_sysfs_attr(self.sysfsPath, "size")) + size = Size(blocks * LINUX_SECTOR_SIZE) + + return size + @property def currentSize(self): """ The device's actual size, generally the size discovered by using @@ -592,12 +577,17 @@ class StorageDevice(Device): If the device does not exist, then the actual size is 0. """ - size = Size(0) - if self.exists and self.partedDevice: - size = Size(self.partedDevice.getLength(unit="B")) - elif self.exists: - size = self._size - return size + if self._currentSize == Size(0): + self._currentSize = self.readCurrentSize() + return self._currentSize + + def updateSize(self): + """ Update size, currentSize, and targetSize to actual size. """ + self._currentSize = Size(0) + new_size = self.currentSize + self._size = new_size + self._targetSize = new_size # bypass setter checks + log.debug("updated %s size to %s (%s)", self.name, self.size, new_size) @property def minSize(self): @@ -699,8 +689,6 @@ class StorageDevice(Device): @property def model(self): - if not self._model: - self._model = getattr(self.partedDevice, "model", "") return self._model @property diff --git a/blivet/devicetree.py b/blivet/devicetree.py index df6762b..d36894b 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -1073,6 +1073,7 @@ class DeviceTree(object): device = diskType(name, major=udev.device_get_major(info), minor=udev.device_get_minor(info), + model=udev.device_get_model(info), sysfsPath=sysfs_path, **kwargs) if diskType == DASDDevice: @@ -1192,8 +1193,7 @@ class DeviceTree(object): # if device: # we successfully looked up the device. skip to format handling. - # first, grab the parted.Device while it's active - _unused = device.partedDevice + pass elif udev.device_is_loop(info): log.info("%s is a loop device", name) device = self.addUdevLoopDevice(info) @@ -1233,6 +1233,9 @@ class DeviceTree(object): log.debug("no device obtained for %s", name) return + # first, update the size since the device is active now + device.updateSize() + # If this device is read-only, mark it as such now. if self.udevDeviceIsDisk(info) and \ util.get_sysfs_attr(udev.device_get_sysfs_path(info), 'ro') == '1': diff --git a/blivet/platform.py b/blivet/platform.py index 491aa42..6d38382 100644 --- a/blivet/platform.py +++ b/blivet/platform.py @@ -125,10 +125,12 @@ class Platform(object): if flags.testing: return self.defaultDiskLabelType + parted_device = parted.Device(path=device.path) + # if there's a required type for this device type, use that - labelType = self.requiredDiskLabelType(device.partedDevice.type) + labelType = self.requiredDiskLabelType(parted_device.type) log.debug("required disklabel type for %s (%s) is %s", - device.name, device.partedDevice.type, labelType) + device.name, parted_device.type, labelType) if not labelType: # otherwise, use the first supported type for this platform # that is large enough to address the whole device @@ -136,8 +138,8 @@ class Platform(object): log.debug("default disklabel type for %s is %s", device.name, labelType) for lt in self.diskLabelTypes: - l = parted.freshDisk(device=device.partedDevice, ty=lt) - if l.maxPartitionStartSector > device.partedDevice.length: + l = parted.freshDisk(device=parted_device, ty=lt) + if l.maxPartitionStartSector > parted_device.length: labelType = lt log.debug("selecting %s disklabel for %s based on size", labelType, device.name) diff --git a/tests/devices_test.py b/tests/devices_test.py index c526c74..84cbe6c 100644 --- a/tests/devices_test.py +++ b/tests/devices_test.py @@ -967,7 +967,7 @@ class PartitionDeviceTestCase(unittest.TestCase): free = disk.format.partedDisk.getFreeSpaceRegions()[-1] raw_start = int(Size("9 MiB") / sector_size) start = disk.format.alignment.alignUp(free, raw_start) + 3 - disk.format.addPartition(start, disk.partedDevice.length - 1) + disk.format.addPartition(start, disk.format.partedDevice.length - 1) # Verify the end of the free region immediately following the first # partition is unaligned. diff --git a/tests/storagetestcase.py b/tests/storagetestcase.py index 0b3f26d..ad1216f 100644 --- a/tests/storagetestcase.py +++ b/tests/storagetestcase.py @@ -12,8 +12,6 @@ from blivet.formats import getFormat from blivet.devices import StorageDevice from blivet.devices import PartitionDevice -from blivet.size import B - class StorageTestCase(unittest.TestCase): """ StorageTestCase @@ -94,10 +92,7 @@ class StorageTestCase(unittest.TestCase): device = device_class(*args, **kwargs) if exists: - # set up mock parted.Device w/ correct size - device._partedDevice = Mock() - device._partedDevice.getLength = Mock(return_value=int(device._size.convertTo(B))) - device._partedDevice.sectorSize = 512 + device._currentSize = kwargs.get("size") if isinstance(device, blivet.devices.PartitionDevice): #if exists: -- 1.9.3