[PATCH 3/5] Improve handling of device removals/additions from the devicetree.
David Lehman
dlehman at redhat.com
Tue Sep 16 21:01:51 UTC 2014
Move device-type-specific code for managing relationships between devices
and the devices that contain them into the appropriate classes. This will
ensure, among other things, that canceling a destroy action for an lv
correctly adds the lv back into the LVMVolumeGroupDevice's lvs list.
(cherry picked from commit bd1cb17ae95682c82b318da3ce8077a23d1f7dca)
Fedora:
Resolves: rhbz#1121383
RHEL:
Resolves: rhbz#1085201
Resolves: rhbz#1077906
Resolves: rhbz#1129595
Resolves: rhbz#1075671
---
blivet/__init__.py | 2 +-
blivet/devices.py | 108 +++++++++++++++++++++++++++++++++++++++++++++++++
blivet/devicetree.py | 43 +++++---------------
blivet/partitioning.py | 4 +-
4 files changed, 122 insertions(+), 35 deletions(-)
diff --git a/blivet/__init__.py b/blivet/__init__.py
index de76353..c809f55 100644
--- a/blivet/__init__.py
+++ b/blivet/__init__.py
@@ -878,7 +878,7 @@ class Blivet(object):
# We can't schedule the magic partition for removal
# because parted will not allow us to remove it from the
# disk. Still, we need it out of the devicetree.
- self.devicetree._removeDevice(part, moddisk=False)
+ self.devicetree._removeDevice(part, modparent=False)
if len(disk.format.partitions) > expected:
raise ValueError("cannot initialize a disk that has partitions")
diff --git a/blivet/devices.py b/blivet/devices.py
index a0f8f56..ba8de63 100644
--- a/blivet/devices.py
+++ b/blivet/devices.py
@@ -1123,6 +1123,33 @@ class StorageDevice(Device):
return -1
return 0
+ # pylint: disable=unused-argument
+ def removeHook(self, modparent=True):
+ """ Perform actions related to removing a device from the devicetree.
+
+ :keyword bool modparent: whether to account for removal in parents
+
+ Parent child counts are adjusted regardless of modparent's value.
+ The intended use of modparent is to prevent doing things like
+ removing a parted.Partition from the disk that contains it as part
+ of msdos extended partition management. In general, you should not
+ override the default value of modparent in new code.
+ """
+ for parent in self.parents:
+ parent.removeChild()
+
+ def addHook(self, new=True):
+ """ Perform actions related to adding a device to the devicetree.
+
+ :keyword bool new: whether this device is new to the devicetree
+
+ The only intended use case for new=False is when unhiding a device
+ from the devicetree. Additional operations are performed when new is
+ False that are normally performed as part of the device constructor.
+ """
+ if not new:
+ map(lambda p: p.addChild(), self.parents)
+
def populateKSData(self, data):
# the common pieces are basically the formatting
self.format.populateKSData(data)
@@ -1657,6 +1684,43 @@ class PartitionDevice(StorageDevice):
magic = self.disk.format.magicPartitionNumber
return (number == magic)
+ def removeHook(self, modparent=True):
+ if modparent:
+ # if this partition hasn't been allocated it could not have
+ # a disk attribute
+ if not self.disk:
+ return
+
+ if self.partedPartition.type == parted.PARTITION_EXTENDED and \
+ len(self.disk.format.logicalPartitions) > 0:
+ raise ValueError("Cannot remove extended partition %s. "
+ "Logical partitions present." % self.name)
+
+ self.disk.format.removePartition(self.partedPartition)
+
+ super(PartitionDevice, self).removeHook(modparent=modparent)
+
+ def addHook(self, new=True):
+ super(PartitionDevice, self).addHook(new=new)
+ if new:
+ return
+
+ if not self.disk or not self.partedPartition or \
+ self.partedPartition in self.disk.format.partitions:
+ return
+
+ self.disk.format.addPartition(self.partedPartition)
+
+ # Look up the path by start sector to deal with automatic renumbering of
+ # logical partitions on msdos disklabels.
+ if self.isExtended:
+ partition = self.disk.format.extendedPartition
+ else:
+ start = self.partedPartition.geometry.start
+ partition = self.disk.format.partedDisk.getPartitionBySector(start)
+
+ self.partedPartition = partition
+
def probe(self):
""" Probe for any missing information about this device.
@@ -3087,6 +3151,20 @@ class LVMLogicalVolumeDevice(DMDevice):
return -1
return 0
+ def removeHook(self, modparent=True):
+ if modparent:
+ self.vg._removeLogVol(self)
+
+ super(LVMLogicalVolumeDevice, self).removeHook(modparent=modparent)
+
+ def addHook(self, new=True):
+ super(LVMLogicalVolumeDevice, self).addHook(new=new)
+ if new:
+ return
+
+ if self not in self.vg.lvs:
+ self.vg._addLogVol(self)
+
def populateKSData(self, data):
super(LVMLogicalVolumeDevice, self).populateKSData(data)
data.vgname = self.vg.name
@@ -3472,6 +3550,22 @@ class LVMThinLogicalVolumeDevice(LVMLogicalVolumeDevice):
lvm.thinlvcreate(self.vg.name, self.pool.lvname, self.lvname,
self.size)
+ def removeHook(self, modparent=True):
+ if modparent:
+ self.pool._removeLogVol(self)
+
+ # pylint: disable=bad-super-call
+ super(LVMLogicalVolumeDevice, self).removeHook(modparent=modparent)
+
+ def addHook(self, new=True):
+ # pylint: disable=bad-super-call
+ super(LVMLogicalVolumeDevice, self).addHook(new=new)
+ if new:
+ return
+
+ if self not in self.pool.lvs:
+ self.pool._addLogVol(self)
+
def populateKSData(self, data):
super(LVMThinLogicalVolumeDevice, self).populateKSData(data)
data.thin_volume = True
@@ -5358,6 +5452,20 @@ class BTRFSSubVolumeDevice(BTRFSDevice):
btrfs.delete_subvolume(mountpoint, self.name)
self.volume._undo_temp_mount()
+ def removeHook(self, modparent=True):
+ if modparent:
+ self.volume._removeSubVolume(self.name)
+
+ super(BTRFSSubVolumeDevice, self).removeHook(modparent=modparent)
+
+ def addHook(self, new=True):
+ super(BTRFSSubVolumeDevice, self).addHook(new=new)
+ if new:
+ return
+
+ if self not in self.volume.subvolumes:
+ self.volume._addSubVolume(self)
+
def populateKSData(self, data):
super(BTRFSSubVolumeDevice, self).populateKSData(data)
data.subvol = True
diff --git a/blivet/devicetree.py b/blivet/devicetree.py
index 7c92288..ea45b2a 100644
--- a/blivet/devicetree.py
+++ b/blivet/devicetree.py
@@ -374,7 +374,7 @@ class DeviceTree(object):
self._postProcessActions()
- def _addDevice(self, newdev):
+ def _addDevice(self, newdev, new=True):
""" Add a device to the tree.
:param newdev: the device to add
@@ -392,6 +392,7 @@ class DeviceTree(object):
if parent not in self._devices:
raise DeviceTreeError("parent device not in tree")
+ newdev.addHook(new=new)
self._devices.append(newdev)
# don't include "req%d" partition names
@@ -404,15 +405,15 @@ class DeviceTree(object):
newdev.name,
newdev.id)
- def _removeDevice(self, dev, force=None, moddisk=True):
+ def _removeDevice(self, dev, force=None, modparent=True):
""" Remove a device from the tree.
:param dev: the device to remove
:type dev: a subclass of :class:`~.devices.StorageDevice`
:keyword force: whether to force removal of a non-leaf device
:type force: bool
- :keyword moddisk: update parent disk's format (partitions only)
- :type moddisk: bool
+ :keyword modparent: update parent device to account for removal
+ :type modparent: bool
.. note::
@@ -425,18 +426,10 @@ class DeviceTree(object):
log.debug("%s has %d kids", dev.name, dev.kids)
raise ValueError("Cannot remove non-leaf device '%s'" % dev.name)
- if moddisk:
+ dev.removeHook(modparent=modparent)
+ if modparent:
# if this is a partition we need to remove it from the parted.Disk
if isinstance(dev, PartitionDevice) and dev.disk is not None:
- # if this partition hasn't been allocated it could not have
- # a disk attribute
- if dev.partedPartition.type == parted.PARTITION_EXTENDED and \
- len(dev.disk.format.logicalPartitions) > 0:
- raise ValueError("Cannot remove extended partition %s. "
- "Logical partitions present." % dev.name)
-
- dev.disk.format.removePartition(dev.partedPartition)
-
# adjust all other PartitionDevice instances belonging to the
# same disk so the device name matches the potentially altered
# name of the parted.Partition
@@ -444,12 +437,6 @@ class DeviceTree(object):
if isinstance(device, PartitionDevice) and \
device.disk == dev.disk:
device.updateName()
- elif hasattr(dev, "pool"):
- dev.pool._removeLogVol(dev)
- elif hasattr(dev, "vg"):
- dev.vg._removeLogVol(dev)
- elif hasattr(dev, "volume"):
- dev.volume._removeSubVolume(dev.name)
self._devices.remove(dev)
if dev.name in self.names and getattr(dev, "complete", True):
@@ -458,20 +445,15 @@ class DeviceTree(object):
dev.name,
dev.id)
- for parent in dev.parents:
- # Will this cause issues with garbage collection?
- # Do we care about garbage collection? At all?
- parent.removeChild()
-
def _removeChildrenFromTree(self, device):
devs_to_remove = self.getDependentDevices(device)
while devs_to_remove:
leaves = [d for d in devs_to_remove if d.isleaf]
for leaf in leaves:
- self._removeDevice(leaf, moddisk=False)
+ self._removeDevice(leaf, modparent=False)
devs_to_remove.remove(leaf)
if len(devs_to_remove) == 1 and devs_to_remove[0].isExtended:
- self._removeDevice(devs_to_remove[0], force=True, moddisk=False)
+ self._removeDevice(devs_to_remove[0], force=True, modparent=False)
break
def registerAction(self, action):
@@ -521,7 +503,7 @@ class DeviceTree(object):
self._removeDevice(action.device)
elif action.isDestroy and action.isDevice:
# add the device back into the tree
- self._addDevice(action.device)
+ self._addDevice(action.device, new=False)
action.cancel()
self._actions.remove(action)
@@ -1969,7 +1951,7 @@ class DeviceTree(object):
if not device.exists:
return
- self._removeDevice(device, force=True, moddisk=False)
+ self._removeDevice(device, force=True, modparent=False)
self._hidden.append(device)
lvm.lvm_cc_addFilterRejectRegexp(device.name)
@@ -2002,9 +1984,6 @@ class DeviceTree(object):
self._hidden.remove(hidden)
self._devices.append(hidden)
lvm.lvm_cc_removeFilterRejectRegexp(hidden.name)
- for parent in hidden.parents:
- parent.addChild()
-
if isinstance(device, DASDDevice):
self.dasd.append(device)
diff --git a/blivet/partitioning.py b/blivet/partitioning.py
index 957ce04..82a3fcb 100644
--- a/blivet/partitioning.py
+++ b/blivet/partitioning.py
@@ -805,7 +805,7 @@ def updateExtendedPartitions(storage, disks):
if part.exists:
storage.destroyDevice(part)
else:
- storage.devicetree._removeDevice(part, moddisk=False)
+ storage.devicetree._removeDevice(part, modparent=False)
continue
extendedName = devicePathToName(extended.getDeviceNodeName())
@@ -822,7 +822,7 @@ def updateExtendedPartitions(storage, disks):
if part.exists:
storage.destroyDevice(part)
else:
- storage.devicetree._removeDevice(part, moddisk=False)
+ storage.devicetree._removeDevice(part, modparent=False)
if device:
continue
--
1.9.3
More information about the anaconda-patches
mailing list