Skip to content

Commit

Permalink
Don't use parted.Device to obtain size info.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dwlehman committed Apr 7, 2015
1 parent b3c0693 commit 73d9c99
Show file tree
Hide file tree
Showing 18 changed files with 119 additions and 122 deletions.
9 changes: 0 additions & 9 deletions blivet/deviceaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions blivet/devices/btrfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions blivet/devices/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,6 @@ def remove(self, member):

if member in self.parents:
self.parents.remove(member)

def updateSize(self):
pass
4 changes: 2 additions & 2 deletions blivet/devices/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 2 additions & 11 deletions blivet/devices/disk.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,32 +90,23 @@ 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
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)
Expand Down
3 changes: 2 additions & 1 deletion blivet/devices/dm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 """
Expand Down Expand Up @@ -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)

Expand Down
10 changes: 10 additions & 0 deletions blivet/devices/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions blivet/devices/lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@

from .. import errors
from .. import udev
from ..size import Size

LINUX_SECTOR_SIZE = Size(512)

def get_device_majors():
majors = {}
Expand Down
6 changes: 2 additions & 4 deletions blivet/devices/luks.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
# device backend modules
from ..devicelibs import crypto

from ..size import Size

import logging
log = logging.getLogger("blivet")

Expand Down Expand Up @@ -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):
Expand Down
9 changes: 6 additions & 3 deletions blivet/devices/md.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down
3 changes: 3 additions & 0 deletions blivet/devices/nfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 /
Expand Down
2 changes: 2 additions & 0 deletions blivet/devices/nodev.py
Original file line number Diff line number Diff line change
Expand Up @@ -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. """
Expand Down
42 changes: 21 additions & 21 deletions blivet/devices/partition.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -806,13 +813,6 @@ def maxSize(self):
unalignedMax = min(maxFormatSize, maxPartSize) if maxFormatSize else maxPartSize
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 \
Expand Down

0 comments on commit 73d9c99

Please sign in to comment.