From: David Lehman <dlehman(a)redhat.com>
Deleting a parted.Device causes parted to close its rw fd for the device,
which triggers a change uevent on that device, which could in turn trigger
any number of actions via udev rules. One example is when we reset a Blivet
instance: All devices are deleted, including their parted.Device instances,
which triggers a change uevent for every device. In response to these
events, mdadm's udev rules activate all arrays on those devices. Activating
devices -- even indirectly -- without cause, is not acceptable behavior for
a storage library.
This also adds a trailing comma to 1-tuples in variable_copy arguments.
In my testing, ('foo') is the string 'foo' -- not a tuple with lone
element
'foo'.
Related: rhbz#1069597
Followup for removal of parted.Device.
---
blivet/deviceaction.py | 9 ---
blivet/devices/btrfs.py | 3 +
blivet/devices/container.py | 3 +
blivet/devices/device.py | 4 +-
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 | 9 ++-
blivet/devices/nfs.py | 3 +
blivet/devices/nodev.py | 2 +
blivet/devices/partition.py | 42 +++++++-------
blivet/devices/storage.py | 107 ++++++++++++++++-------------------
blivet/platform.py | 10 ++--
blivet/populator.py | 5 +-
tests/devices_test/partition_test.py | 2 +-
tests/storagetestcase.py | 7 +--
18 files changed, 119 insertions(+), 122 deletions(-)
diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py
index ce7e90e..4d10765 100644
--- a/blivet/deviceaction.py
+++ b/blivet/deviceaction.py
@@ -28,7 +28,6 @@
from .devices import StorageDevice
from .devices import PartitionDevice
from .formats import getFormat, luks
-from .storage_log import log_exception_info
from parted import partitionFlag, PARTITION_LBA
from .i18n import _, N_
from .callbacks import CreateFormatPreData, CreateFormatPostData
@@ -345,14 +344,6 @@ def execute(self, callbacks=None):
super(ActionDestroyDevice, self).execute(callbacks=None)
self.device.destroy()
- # Make sure libparted does not keep cached info for this device
- # and returns it when we create a new device with the same name
- if self.device.partedDevice:
- try:
- self.device.partedDevice.removeFromCache()
- except Exception: # pylint: disable=broad-except
- log_exception_info(fmt_str="failed to remove info for device %s from
libparted cache", fmt_args=[self.device])
-
def requires(self, action):
""" Return True if self requires action.
diff --git a/blivet/devices/btrfs.py b/blivet/devices/btrfs.py
index 65904bb..8c59443 100644
--- a/blivet/devices/btrfs.py
+++ b/blivet/devices/btrfs.py
@@ -65,6 +65,9 @@ def updateSysfsPath(self):
self.sysfsPath = self.parents[0].sysfsPath
log.debug("%s sysfsPath set to %s", self.name, self.sysfsPath)
+ def updateSize(self):
+ pass
+
def _postCreate(self):
super(BTRFSDevice, self)._postCreate()
self.format.exists = True
diff --git a/blivet/devices/container.py b/blivet/devices/container.py
index 612b9fc..0de6ca7 100644
--- a/blivet/devices/container.py
+++ b/blivet/devices/container.py
@@ -187,3 +187,6 @@ def remove(self, member):
if member in self.parents:
self.parents.remove(member)
+
+ def updateSize(self):
+ pass
diff --git a/blivet/devices/device.py b/blivet/devices/device.py
index 90a2718..41c8008 100644
--- a/blivet/devices/device.py
+++ b/blivet/devices/device.py
@@ -95,8 +95,8 @@ def __deepcopy__(self, memo):
For these parted objects, we just do a shallow copy.
"""
return util.variable_copy(self, memo,
- omit=('node'),
- shallow=('_partedDevice', '_partedPartition'))
+ omit=('node',),
+ 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 0d129d2..5f35ce6 100644
--- a/blivet/devices/disk.py
+++ b/blivet/devices/disk.py
@@ -90,8 +90,7 @@ def __init__(self, name, fmt=None,
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
@@ -99,23 +98,15 @@ def mediaPresent(self):
if flags.testing:
return True
- if not self.partedDevice:
- return False
-
# Some drivers (cpqarray <blegh>) 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 e178cec..e834abf 100644
--- a/blivet/devices/dm.py
+++ b/blivet/devices/dm.py
@@ -33,6 +33,7 @@
log = logging.getLogger("blivet")
from .storage import StorageDevice
+from .lib import LINUX_SECTOR_SIZE
class DMDevice(StorageDevice):
""" A device-mapper device """
@@ -181,7 +182,7 @@ def _setup(self, orig=False):
""" 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
blockdev.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 cd8eb79..640d51d 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 @@ def path(self):
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 @@
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 @@ def raw_device(self):
@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 0e8813e..fe19252 100644
--- a/blivet/devices/md.py
+++ b/blivet/devices/md.py
@@ -204,7 +204,7 @@ def size(self):
"""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,
@@ -215,11 +215,14 @@ def size(self):
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
+ def updateSize(self):
+ super(ContainerDevice, self).updateSize()
+
@property
def description(self):
levelstr = self.level.nick if self.level.nick else self.level.name
@@ -531,7 +534,7 @@ def mediaPresent(self):
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/nfs.py b/blivet/devices/nfs.py
index 513a11c..da00114 100644
--- a/blivet/devices/nfs.py
+++ b/blivet/devices/nfs.py
@@ -69,6 +69,9 @@ def destroy(self):
""" Destroy the device. """
log_method_call(self, self.name, status=self.status)
+ def updateSize(self):
+ pass
+
@classmethod
def isNameValid(cls, name):
# Override StorageDevice.isNameValid to allow /
diff --git a/blivet/devices/nodev.py b/blivet/devices/nodev.py
index 5d197b8..6d7570a 100644
--- a/blivet/devices/nodev.py
+++ b/blivet/devices/nodev.py
@@ -70,6 +70,8 @@ def destroy(self):
log_method_call(self, self.name, status=self.status)
self._preDestroy()
+ def udpateSize(self):
+ pass
class TmpFSDevice(NoDevice):
""" A nodev device for a tmpfs filesystem. """
diff --git a/blivet/devices/partition.py b/blivet/devices/partition.py
index 0498592..edd37d0 100644
--- a/blivet/devices/partition.py
+++ b/blivet/devices/partition.py
@@ -125,6 +125,12 @@ def __init__(self, name, fmt=None,
self._bootable = False
+ # FIXME: Validate partType, but only if this is a new partition
+ # Otherwise, overwrite it with the partition's type.
+ self._partType = None
+ self._partedPartition = None
+ self._origPath = None
+
StorageDevice.__init__(self, name, fmt=fmt, size=size,
major=major, minor=minor, exists=exists,
sysfsPath=sysfsPath, parents=parents)
@@ -134,13 +140,6 @@ def __init__(self, name, fmt=None,
self.req_disks = list(self.parents)
self.parents = []
- # FIXME: Validate partType, but only if this is a new partition
- # Otherwise, overwrite it with the partition's type.
- 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
# parted.
@@ -520,7 +519,6 @@ def probe(self):
return
self._size = Size(self.partedPartition.getLength(unit="B"))
- self._currentSize = self._size
self.targetSize = self._size
self._partType = self.partedPartition.type
@@ -595,7 +593,6 @@ def _postCreate(self):
DeviceFormat(device=self.path, exists=True).destroy()
StorageDevice._postCreate(self)
- self._currentSize = Size(self.partedPartition.getLength(unit="B"))
def _computeResize(self, partition, newsize=None):
""" Return a new constraint and end-aligned geometry for new
size.
@@ -651,7 +648,7 @@ def resize(self):
end=geometry.end)
self.disk.format.commit()
- self._currentSize = Size(partition.getLength(unit="B"))
+ self.updateSize()
def _preDestroy(self):
StorageDevice._preDestroy(self)
@@ -704,25 +701,35 @@ def _getSize(self):
return size
def _setSize(self, newsize):
- """ Set the device's size (for resize, not creation).
+ """ Set the device's size.
+
+ Most devices have two scenarios for setting a size:
- Arguments:
+ 1) set actual/current size
+ 2) set target for resize
- newsize -- the new size
+ 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")
@@ -807,13 +814,6 @@ def maxSize(self):
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 be848ac..fc4cf90 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 @@
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 @@ def __init__(self, name, fmt=None, uuid=None,
super(StorageDevice, self).__init__(name, parents=parents)
self._format = None
+
+ # The size will be overridden by a call to updateSize at the end of this
+ # method for existing and active devices.
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,16 +128,8 @@ def __init__(self, name, fmt=None, uuid=None,
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)
+ if self.exists and self.status:
+ self.updateSize()
def __str__(self):
exist = "existing"
@@ -169,26 +164,6 @@ def encrypted(self):
""" 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.
"""
@@ -253,12 +228,12 @@ def __repr__(self):
" 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})
@@ -404,9 +379,10 @@ def setup(self, orig=False):
def _postSetup(self):
""" Perform post-setup operations. """
udev.settle()
- # we always probe since the device may not be set up when we want
- # information about it
- self._size = self.currentSize
+ self.updateSysfsPath()
+ # the device may not be set up when we want information about it
+ if self._size == Size(0):
+ self.updateSize()
#
# teardown
@@ -477,9 +453,7 @@ def _postCreate(self):
udev.settle()
# make sure that targetSize is updated to reflect the actual size
- if self.resizable:
- self._partedDevice = None
- self._targetSize = self.currentSize
+ self.updateSize()
self._updateNetDevMountOption()
@@ -527,12 +501,6 @@ def setupParents(self, orig=False):
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:
size = self.targetSize
@@ -540,19 +508,41 @@ def _getSize(self):
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):
+ log_method_call(self, exists=self.exists, path=self.path,
+ sysfsPath=self.sysfsPath)
+ 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
@@ -561,12 +551,17 @@ def currentSize(self):
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):
@@ -668,8 +663,6 @@ def serial(self):
@property
def model(self):
- if not self._model:
- self._model = getattr(self.partedDevice, "model", "")
return self._model
@property
diff --git a/blivet/platform.py b/blivet/platform.py
index 119cc95..4f9f15c 100644
--- a/blivet/platform.py
+++ b/blivet/platform.py
@@ -125,10 +125,12 @@ def bestDiskLabelType(self, device):
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 @@ def bestDiskLabelType(self, device):
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/blivet/populator.py b/blivet/populator.py
index 2acfc1d..ac53a06 100644
--- a/blivet/populator.py
+++ b/blivet/populator.py
@@ -545,6 +545,7 @@ def addUdevDiskDevice(self, info):
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:
@@ -668,9 +669,6 @@ def addUdevDevice(self, info):
#
device_added = True
if device:
- # we successfully looked up the device. skip to format handling.
- # first, grab the parted.Device while it's active
- _unused = device.partedDevice
device_added = False
elif udev.device_is_loop(info):
log.info("%s is a loop device", name)
@@ -985,6 +983,7 @@ def addLV(lv):
if lv_device.status:
lv_device.updateSysfsPath()
+ lv_device.updateSize()
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)
diff --git a/tests/devices_test/partition_test.py b/tests/devices_test/partition_test.py
index c203d58..b4fef6c 100644
--- a/tests/devices_test/partition_test.py
+++ b/tests/devices_test/partition_test.py
@@ -141,7 +141,7 @@ def testMinMaxSizeAlignment(self):
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.devices import StorageDevice
from blivet.devices import PartitionDevice
-from blivet.size import B
-
class StorageTestCase(unittest.TestCase):
""" StorageTestCase
@@ -94,10 +92,7 @@ def newDevice(self, *args, **kwargs):
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:
--
To view this commit on github, visit
https://github.com/rhinstaller/blivet/commit/73d9c995c8d46f1aa248d1ade3bb...