From d3f8aa457939916b195846b79d0d33e64584e7f3 Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Tue, 2 Jun 2015 12:57:26 +0200 Subject: [PATCH 1/9] Don't crash when processing cached LVs Just ignore the unsupported "sub-LVs" and consider cached LVs normal LVs. --- blivet/populator.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/blivet/populator.py b/blivet/populator.py index 157178ae0..9514afcd1 100644 --- a/blivet/populator.py +++ b/blivet/populator.py @@ -937,6 +937,9 @@ def addLV(lv): if lv_name.endswith("_pmspare]"): # spare metadata area for any thin pool that needs repair return + elif lv_name.endswith("_cmeta]"): + # cache metadata volume (skip, we ignore cache volumes) + return # raid metadata volume lv_name = re.sub(r'_[tr]meta.*', '', lv_name[1:-1]) @@ -974,7 +977,7 @@ def addLV(lv): elif lv_name.endswith(']'): # Internal LVM2 device return - elif lv_attr[0] not in '-mMrRoO': + elif lv_attr[0] not in '-mMrRoOC': # Ignore anything else except for the following: # - normal lv # m mirrored @@ -983,6 +986,7 @@ def addLV(lv): # R raid without initial sync # o origin # O origin with merging snapshot + # C cached LV return lv_dev = self.getDeviceByUuid(lv_uuid) From 9a405555712a538303ce8e0c1700ca1120156d63 Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Tue, 2 Jun 2015 19:21:04 +0200 Subject: [PATCH 2/9] Rearrange and group some of the StorageDevice's methods/properties Those that are related to each other should be next to each other and titled. --- blivet/devices/storage.py | 124 ++++++++++++++++++++++---------------- 1 file changed, 71 insertions(+), 53 deletions(-) diff --git a/blivet/devices/storage.py b/blivet/devices/storage.py index 88f8b8e5e..ed398d78f 100644 --- a/blivet/devices/storage.py +++ b/blivet/devices/storage.py @@ -486,6 +486,9 @@ def _postDestroy(self): """ Perform post-destruction operations. """ self.exists = False + # + # parents' modifications/notifications + # def setupParents(self, orig=False): """ Run setup method of all parent devices. """ log_method_call(self, name=self.name, orig=orig, kids=self.kids) @@ -500,6 +503,37 @@ def setupParents(self, orig=False): if _format.type and _format.exists: _format.setup() + # 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: + for p in self.parents: + p.addChild() + + # + # size manipulations + # def _getSize(self): """ Get the device's size, accounting for pending changes. """ size = self._size @@ -574,6 +608,29 @@ def maxSize(self): """ The maximum size this device can be. """ return self.alignTargetSize(self.format.maxSize) if self.resizable else self.currentSize + @property + def growable(self): + """ True if this device or its component devices are growable. """ + return getattr(self, "req_grow", False) or any(p.growable for p in self.parents) + + def checkSize(self): + """ Check to make sure the size of the device is allowed by the + format used. + + Returns: + 0 - ok + 1 - Too large + -1 - Too small + """ + if self.format.maxSize and self.size > self.format.maxSize: + return 1 + elif self.format.minSize and self.size < self.format.minSize: + return -1 + return 0 + + # + # status + # @property def mediaPresent(self): """ True if this device contains usable media. """ @@ -591,6 +648,9 @@ def status(self): return False return os.access(self.path, os.W_OK) + # + # format manipulations + # def _setFormat(self, fmt): """ Set the Device's format. """ if not fmt: @@ -633,6 +693,14 @@ def preCommitFixup(self, *args, **kwargs): """ Do any necessary pre-commit fixups.""" pass + @property + def formatImmutable(self): + """ Is it possible to execute format actions on this device? """ + return self._formatImmutable or self.protected + + # + # misc properties + # @property def removable(self): devpath = os.path.normpath(self.sysfsPath) @@ -641,11 +709,6 @@ def removable(self): os.access(remfile, os.R_OK) and open(remfile).readline().strip() == "1") - @property - def formatImmutable(self): - """ Is it possible to execute format actions on this device? """ - return self._formatImmutable or self.protected - @property def direct(self): """ Is this device directly accessible? """ @@ -675,54 +738,6 @@ def model(self): def vendor(self): return self._vendor - @property - def growable(self): - """ True if this device or its component devices are growable. """ - return getattr(self, "req_grow", False) or any(p.growable for p in self.parents) - - def checkSize(self): - """ Check to make sure the size of the device is allowed by the - format used. - - Returns: - 0 - ok - 1 - Too large - -1 - Too small - """ - if self.format.maxSize and self.size > self.format.maxSize: - return 1 - elif self.format.minSize and self.size < self.format.minSize: - 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: - for p in self.parents: - p.addChild() - def populateKSData(self, data): # the common pieces are basically the formatting self.format.populateKSData(data) @@ -751,6 +766,9 @@ def isNameValid(cls, name): badchars = any(c in ('\x00', '/') for c in name) return not(badchars or name == '.' or name == '..') + # + # dependencies + # @classmethod def typeExternalDependencies(cls): """ A list of external dependencies of this device type. From 30c61caca2798eaf2b39f1cfd862a691e8f4d700 Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Thu, 4 Jun 2015 14:53:56 +0200 Subject: [PATCH 3/9] Move parents checking and update into a seprarate methods Internal LVs are not going to be referenced by the DeviceTree instance and are going to have no parents. Their "parent" LVs are going to be referenced as 'self.parent_lv' instead of 'self.parents[0]' because all the other devices use the opposite logic for parent-children relations -- children are consisting of or built on top of parents (whereas "parent" LVs are consisting of their internal LVs). This way internal LVs can easily override these methods. --- blivet/devices/lvm.py | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 3b94a22dd..394aa4051 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -489,16 +489,6 @@ def __init__(self, name, parents=None, size=None, uuid=None, :type percent: int """ - if isinstance(parents, list): - if len(parents) != 1: - raise ValueError("constructor requires a single %s instance" % self._containerClass.__name__) - - container = parents[0] - else: - container = parents - - if not isinstance(container, self._containerClass): - raise ValueError("constructor requires a %s instance" % self._containerClass.__name__) DMDevice.__init__(self, name, size=size, fmt=fmt, sysfsPath=sysfsPath, parents=parents, @@ -523,8 +513,29 @@ def __init__(self, name, parents=None, size=None, uuid=None, self.req_size = self._size self.req_percent = util.numeric_type(percent) - # here we go with the circular references - self.parents[0]._addLogVol(self) + # check that we got parents as expected and add this device to them + self._check_parents() + self._add_to_parents() + + def _check_parents(self): + """Check that this device has parents as expected""" + + if isinstance(self.parents, list): + if len(self.parents) != 1: + raise ValueError("constructor requires a single %s instance" % self._containerClass.__name__) + + container = self.parents[0] + else: + container = self.parents + + if not isinstance(container, self._containerClass): + raise ValueError("constructor requires a %s instance" % self._containerClass.__name__) + + def _add_to_parents(self): + """Add this device to its parents""" + + # a normal LV has only exactly parent -- the VG it belongs to + self._parents[0]._addLogVol(self) def __repr__(self): s = DMDevice.__repr__(self) From e46eca828a05bf183d26f0b9fb8c3ef03cc22fd5 Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Thu, 4 Jun 2015 14:57:23 +0200 Subject: [PATCH 4/9] Add classes for the internal LVs of various types Also make the LVMLogicalVolumeDevice class ready for managing internal LVs. --- blivet/devices/lvm.py | 235 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 232 insertions(+), 3 deletions(-) diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 394aa4051..5c99a78f4 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -48,6 +48,8 @@ from .dm import DMDevice from .md import MDRaidArrayDevice +INTERNAL_LV_CLASSES = [] + class LVMVolumeGroupDevice(ContainerDevice): """ An LVM Volume Group """ _type = "lvmvg" @@ -495,9 +497,6 @@ def __init__(self, name, parents=None, size=None, uuid=None, exists=exists) self.uuid = uuid - self.copies = copies - self.logSize = logSize or Size(0) - self.metaDataSize = Size(0) self.segType = segType or "linear" self.snapshots = [] @@ -517,6 +516,8 @@ def __init__(self, name, parents=None, size=None, uuid=None, self._check_parents() self._add_to_parents() + self._internal_lvs = [] + def _check_parents(self): """Check that this device has parents as expected""" @@ -808,6 +809,234 @@ def isNameValid(cls, name): return True + def addInternalLV(self, int_lv): + if int_lv not in self._internal_lvs: + self._internal_lvs.append(int_lv) + + def removeInternalLV(self, int_lv): + if int_lv in self._internal_lvs: + self._internal_lvs.remove(int_lv) + else: + msg = "the specified internal LV '%s' doesn't belong to this LV ('%s')" % (int_lv.lv_name, + self.name) + raise ValueError(msg) + +@add_metaclass(abc.ABCMeta) +class LVMInternalLogicalVolumeDevice(LVMLogicalVolumeDevice): + """Abstract base class for internal LVs + + A common class for all classes representing internal Logical Volumes like + data and metadata parts of pools, RAID images, etc. + + Internal LVs are only referenced by their "parent" LVs (normal LVs they + back) as all queries and manipulations with them should be done via their + parent LVs. + + """ + + _type = "lvminternallv" + + # generally changes should be done on the parent LV (exceptions should + # override these) + _resizable = False + _readonly = True + + attr_letters = abc.abstractproperty(doc="letters representing the type of the internal LV in the attrs") + name_suffix = abc.abstractproperty(doc="pattern matching typical/default suffices for internal LVs of this type") + takes_extra_space = abc.abstractproperty(doc="whether LVs of this type take space in a VG or are part of their parent LVs") + + @classmethod + def isNameValid(cls, name): + # override checks for normal LVs, internal LVs typically have names that + # are forbidden for normal LVs + return True + + def __init__(self, name, vg, parent_lv=None, size=None, uuid=None, + exists=False, segType=None, sysfsPath=''): + """ + :param vg: the VG this internal LV belongs to + :type vg: :class:`LVMVolumeGroupDevice` + :param parent_lv: the parent LV of this internal LV + :type parent_lv: :class:`LVMLogicalVolumeDevice` + + See :method:`LVMLogicalVolumeDevice.__init__` for details about the + rest of the parameters. + """ + + # VG name has to be set for parent class' constructors + self._vg = vg + + # so does the parent LV + self._parent_lv = parent_lv + + # construct the internal LV just like a normal one just with no parents + # and some parameters set to values reflecting the fact that this is an + # internal LV + super(LVMInternalLogicalVolumeDevice, self).__init__(name, parents=None, + size=size, uuid=uuid, segType=segType, fmt=None, exists=exists, + sysfsPath=sysfsPath, grow=None, maxsize=None, percent=None) + + if parent_lv: + self._parent_lv.addInternalLV(self) + + def _check_parents(self): + # an internal LV should have no parents + if self._parents: + raise ValueError("an internal LV should have no parents") + + def _add_to_parents(self): + # nothing to do here, an internal LV has no parents (in the DeviceTree's + # meaning of 'parents') + pass + + @property + def vg(self): + return self._vg + + @vg.setter + def vg(self, vg): + # pylint: disable=arguments-differ + self._vg = vg + + @property + def parent_lv(self): + return self._parent_lv + + @parent_lv.setter + def parent_lv(self, parent_lv): + if self._parent_lv: + self._parent_lv.removeInternalLV(self) + self._parent_lv = parent_lv + if self._parent_lv: + self._parent_lv.addInternalLV(self) + + # internal LVs follow different rules limitting size + def _setSize(self, size): + if not isinstance(size, Size): + raise ValueError("new size must of type Size") + + if not self.takes_extra_space: + if size <= self.parent_lv.size: + self._size = size + else: + raise ValueError("Internal LV cannot be bigger than its parent LV") + else: + # same rules apply as for any other LV + super(LVMInternalLogicalVolumeDevice, self)._setSize(size) + + @property + def maxSize(self): + # no format, so maximum size is only limitted by either the parent LV or the VG + if not self.takes_extra_space: + return self._parent_lv.maxSize() + else: + return self.size + self.vg.freeSpace + + def __repr__(self): + s = "%s:\n" % self.__class__.__name__ + s += (" name = %s, status = %s exists = %s\n" % (self.lvname, self.status, self.exists)) + s += (" uuid = %s, size = %s\n" % (self.uuid, self.size)) + s += (" parent LV = %r\n" % self.parent_lv) + s += (" VG device = %(vgdev)r\n" + " segment type = %(type)s percent = %(percent)s\n" + " mirror copies = %(copies)d" + " VG space used = %(vgspace)s" % + {"vgdev": self.vg, "percent": self.req_percent, + "copies": self.copies, "type": self.segType, + "vgspace": self.vgSpaceUsed }) + return s + + # generally changes should be done on the parent LV (exceptions should + # override these) + def setup(self, orig=False): + raise errors.DeviceError("An internal LV cannot be set up separately") + + def teardown(self, recursive=None): + raise errors.DeviceError("An internal LV cannot be torn down separately") + + def destroy(self): + raise errors.DeviceError("An internal LV cannot be destroyed separately") + + def resize(self): + raise errors.DeviceError("An internal LV cannot be resized") + + @property + def growable(self): + return False + + @property + def display_lvname(self): + """Name of the internal LV as displayed by the lvm utilities""" + return "[%s]" % self.lvname + + # these two methods are not needed right now, because they are only called + # when devices are added/removed to/from the DeviceTree, but they may come + # handy in the future + def addHook(self, new=True): + # skip LVMLogicalVolumeDevice in the class hierarchy -- we don't want to + # add an internal LV to the VG (it's only referenced by the parent LV) + # pylint: disable=bad-super-call + super(LVMLogicalVolumeDevice, self).addHook(new=new) + self._parent_lv.addInternalLV(self) + + def removeHook(self, modparent=True): + if modparent: + self._parent_lv.removeInternalLV(self) + + # skip LVMLogicalVolumeDevice in the class hierarchy -- we cannot remove + # an internal LV from the VG (it's only referenced by the parent LV) + # pylint: disable=bad-super-call + super(LVMLogicalVolumeDevice, self).removeHook(modparent=modparent) + + @property + def direct(self): + # internal LVs are not directly accessible + return False + +class LVMDataLogicalVolumeDevice(LVMInternalLogicalVolumeDevice): + """Internal data LV (used by thin/cache pools)""" + + attr_letters = ["T", "C"] + name_suffix = r"_[tc]data" + takes_extra_space = False +_INTERNAL_LV_CLASSES.append(LVMDataLogicalVolumeDevice) + +class LVMMetadataLogicalVolumeDevice(LVMInternalLogicalVolumeDevice): + """Internal metadata LV (used by thin/cache pools, RAIDs, etc.)""" + + attr_letters = ["e"] + # RAIDs can have multiple (numbered) metadata LVs + name_suffix = r"_[trc]meta(_[0-9]+)?" + takes_extra_space = True + + # TODO: override and allow resize() +_INTERNAL_LV_CLASSES.append(LVMMetadataLogicalVolumeDevice) + +class LVMLogLogicalVolumeDevice(LVMInternalLogicalVolumeDevice): + """Internal log LV (used by mirrored LVs)""" + + attr_letters = ["l", "L"] + name_suffix = "_mlog" + takes_extra_space = True +_INTERNAL_LV_CLASSES.append(LVMLogLogicalVolumeDevice) + +class LVMImageLogicalVolumeDevice(LVMInternalLogicalVolumeDevice): + """Internal image LV (used by mirror/RAID LVs)""" + + attr_letters = ["i"] + # RAIDs have multiple (numbered) image LVs + name_suffix = r"_[rm]image(_[0-9]+)?" + takes_extra_space = False +_INTERNAL_LV_CLASSES.append(LVMImageLogicalVolumeDevice) + +class LVMOriginLogicalVolumeDevice(LVMInternalLogicalVolumeDevice): + """Internal origin LV (e.g. the raw/uncached part of a cached LV)""" + + attr_letters = ["o"] + name_suffix = r"_c?orig" + takes_extra_space = False +_INTERNAL_LV_CLASSES.append(LVMOriginLogicalVolumeDevice) + @add_metaclass(abc.ABCMeta) class LVMSnapShotBase(object): """ Abstract base class for lvm snapshots From 2a8499624a8bfe158bf71b54138cc14ca1f264f7 Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Thu, 4 Jun 2015 14:57:49 +0200 Subject: [PATCH 5/9] Create and use internal LVs instead of static values Instead of calculating static values for number of copies, log size and metadata size of a given LV, we should use internal LVs assigned to their "parent" (normal) LVs and calculate such values dynamically from those internal LVs. --- blivet/devicelibs/lvm.py | 33 ++++++++++ blivet/devices/lvm.py | 47 +++++++++++---- blivet/populator.py | 126 ++++++++++++++++++++++++--------------- 3 files changed, 147 insertions(+), 59 deletions(-) diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py index 0b7658c2f..57aa740d0 100644 --- a/blivet/devicelibs/lvm.py +++ b/blivet/devicelibs/lvm.py @@ -20,6 +20,8 @@ # Author(s): Dave Lehman # +import re + from collections import namedtuple from gi.repository import BlockDev as blockdev @@ -115,3 +117,34 @@ def lvm_cc_removeFilterRejectRegexp(regexp): def lvm_cc_resetFilter(): config_args_data["filterRejects"] = [] config_args_data["filterAccepts"] = [] + +def determine_parent_lv(vg_name, internal_lv, lvs): + """Try to determine which of the lvs is the parent of the internal_lv + + :param str vg_name: name of the VG the internal_lv and lvs belong to + :type internal_lv: :class:`~.devices.lvm.LMVInternalLogicalVolumeDevice` + :type lvs: :class:`~.devices.lvm.LMVLogicalVolumeDevice` + + """ + for lv in lvs: + if internal_lv.lvname == lv.lvname: + # skip the internal_lv itself + continue + + # check if the lv's name is the name of the internal LV without the suffix + # e.g. 'pool' and 'pool_tmeta' + if re.match(lv.lvname+internal_lv.name_suffix+"$", internal_lv.lvname): + return lv + + # cache pools are internal LVs of cached LVs + try: + pool_name = blockdev.lvm.cache_pool_name(vg_name, lv.lvname) + if pool_name == internal_lv.lvname: + return lv + except blockdev.LVMError: + # doesn't have a pool + pass + + # TODO: use 'data_lv' and 'metadata_lv' on appropriate internal LVs + + return None diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 5c99a78f4..bb77cb519 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -48,7 +48,15 @@ from .dm import DMDevice from .md import MDRaidArrayDevice -INTERNAL_LV_CLASSES = [] +_INTERNAL_LV_CLASSES = [] + +def get_internal_lv_class(lv_attr): + # XXX: need to do some heuristic on the LV name? + for cls in _INTERNAL_LV_CLASSES: + if lv_attr[0] in cls.attr_letters: + return cls + + return None class LVMVolumeGroupDevice(ContainerDevice): """ An LVM Volume Group """ @@ -452,10 +460,9 @@ class LVMLogicalVolumeDevice(DMDevice): _containerClass = LVMVolumeGroupDevice _external_dependencies = [availability.BLOCKDEV_LVM_PLUGIN] - def __init__(self, name, parents=None, size=None, uuid=None, - copies=1, logSize=None, segType=None, - fmt=None, exists=False, sysfsPath='', - grow=None, maxsize=None, percent=None): + def __init__(self, name, parents=None, size=None, uuid=None, segType=None, + fmt=None, exists=False, sysfsPath='', grow=None, maxsize=None, + percent=None): """ :param name: the device name (generally a device node's basename) :type name: str @@ -516,6 +523,7 @@ def __init__(self, name, parents=None, size=None, uuid=None, self._check_parents() self._add_to_parents() + self._metaDataSize = Size(0) self._internal_lvs = [] def _check_parents(self): @@ -538,6 +546,24 @@ def _add_to_parents(self): # a normal LV has only exactly parent -- the VG it belongs to self._parents[0]._addLogVol(self) + @property + def copies(self): + image_lvs = [int_lv for int_lv in self._internal_lvs if isinstance(int_lv, LVMImageLogicalVolumeDevice)] + return len(image_lvs) or 1 + + @property + def logSize(self): + log_lvs = (int_lv for int_lv in self._internal_lvs if isinstance(int_lv, LVMLogLogicalVolumeDevice)) + return sum(lv.size for lv in log_lvs) + + @property + def metaDataSize(self): + if self._metaDataSize: + return self._metaDataSize + + md_lvs = (int_lv for int_lv in self._internal_lvs if isinstance(int_lv, LVMMetadataLogicalVolumeDevice)) + return sum(lv.size for lv in md_lvs) + def __repr__(self): s = DMDevice.__repr__(self) s += (" VG device = %(vgdev)r\n" @@ -1135,11 +1161,9 @@ class LVMSnapShotDevice(LVMSnapShotBase, LVMLogicalVolumeDevice): _type = "lvmsnapshot" _formatImmutable = True - def __init__(self, name, parents=None, size=None, uuid=None, - copies=1, logSize=None, segType=None, - fmt=None, exists=False, sysfsPath='', - grow=None, maxsize=None, percent=None, - origin=None, vorigin=False): + def __init__(self, name, parents=None, size=None, uuid=None, segType=None, + fmt=None, exists=False, sysfsPath='', grow=None, maxsize=None, + percent=None, origin=None, vorigin=False): """ Create an LVMSnapShotDevice instance. This class is for the old-style (not thin) lvm snapshots. The origin @@ -1178,7 +1202,6 @@ def __init__(self, name, parents=None, size=None, uuid=None, LVMLogicalVolumeDevice.__init__(self, name, parents=parents, size=size, uuid=uuid, fmt=None, exists=exists, - copies=copies, logSize=logSize, segType=segType, sysfsPath=sysfsPath, grow=grow, maxsize=maxsize, percent=percent) @@ -1282,7 +1305,7 @@ def __init__(self, name, parents=None, size=None, uuid=None, percent=percent, segType=segType) - self.metaDataSize = metadatasize or Size(0) + self._metaDataSize = metadatasize or Size(0) self.chunkSize = chunksize or Size(0) self.profile = profile self._lvs = [] diff --git a/blivet/populator.py b/blivet/populator.py index 9514afcd1..d86fa3913 100644 --- a/blivet/populator.py +++ b/blivet/populator.py @@ -41,6 +41,7 @@ from .devices import MultipathDevice, OpticalDevice from .devices import PartitionDevice, ZFCPDiskDevice, iScsiDiskDevice from .devices import devicePathToName +from .devices.lvm import get_internal_lv_class from . import formats from .devicelibs import lvm from .devicelibs import raid @@ -863,6 +864,9 @@ def handleVgLvs(self, vg_device): log.debug("no LVs listed for VG %s", vg_name) return + all_lvs = [] + internal_lvs = [] + def addRequiredLV(name, msg): """ Add a prerequisite/parent LV. @@ -879,7 +883,9 @@ def addRequiredLV(name, msg): """ vol = self.getDeviceByName(name) if vol is None: - addLV(lv_info[name]) + new_lv = addLV(lv_info[name]) + if new_lv: + all_lvs.append(new_lv) vol = self.getDeviceByName(name) if vol is None: @@ -926,33 +932,11 @@ def addLV(lv): elif lv_attr[0] == 'v': # skip vorigins return - elif lv_attr[0] in 'Ii': - # mirror image - rname = re.sub(r'_[rm]image.+', '', lv_name[1:-1]) - name = "%s-%s" % (vg_name, rname) - addRequiredLV(name, "failed to look up raid lv") - raid_items[name]["copies"] += 1 - return - elif lv_attr[0] == 'e': - if lv_name.endswith("_pmspare]"): - # spare metadata area for any thin pool that needs repair - return - elif lv_name.endswith("_cmeta]"): - # cache metadata volume (skip, we ignore cache volumes) - return - - # raid metadata volume - lv_name = re.sub(r'_[tr]meta.*', '', lv_name[1:-1]) - name = "%s-%s" % (vg_name, lv_name) - addRequiredLV(name, "failed to look up raid lv") - raid_items[name]["meta"] += lv_size - return - elif lv_attr[0] == 'l': - # log volume - rname = re.sub(r'_mlog.*', '', lv_name[1:-1]) - name = "%s-%s" % (vg_name, rname) - addRequiredLV(name, "failed to look up log lv") - raid_items[name]["log"] = lv_size + elif lv_attr[0] in 'IielTCo' and lv_name.endswith(']'): + # an internal LV, add the an instance of the appropriate class + # to internal_lvs for later processing when non-internal LVs are + # processed + internal_lvs.append(lv_name) return elif lv_attr[0] == 't': # thin pool @@ -975,7 +959,7 @@ def addLV(lv): lv_parents = [self.getDeviceByName(pool_device_name)] elif lv_name.endswith(']'): - # Internal LVM2 device + # unrecognized Internal LVM2 device return elif lv_attr[0] not in '-mMrRoOC': # Ignore anything else except for the following: @@ -1004,30 +988,78 @@ def addLV(lv): 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 + return lv_device # do format handling now self.addUdevDevice(lv_info) - raid_items = dict((n.replace("[", "").replace("]", ""), - {"copies": 0, "log": Size(0), "meta": Size(0)}) - for n in lv_info.keys()) - for lv in lv_info.values(): - addLV(lv) + return lv_device - for name, data in raid_items.items(): - lv_dev = self.getDeviceByName(name) - if not lv_dev: - # hidden lv, eg: pool00_tdata - continue + return None + + def createInternalLV(lv): + lv_name = lv.lv_name + lv_uuid = lv.uuid + lv_attr = lv.attr + lv_size = Size(lv.size) + lv_type = lv.segtype + + matching_cls = get_internal_lv_class(lv_attr) + if matching_cls is None: + raise DeviceTreeError("No internal LV class supported for type '%s'" % lv_attr[0]) + + # strip the "[]"s marking the LV as internal + lv_name = lv_name.strip("[]") + + # don't know the parent LV yet, will be set later + new_lv = matching_cls(lv_name, vg_device, parent_lv=None, size=lv_size, uuid=lv_uuid, exists=True, segType=lv_type) + if new_lv.status: + new_lv.updateSysfsPath() + new_lv.updateSize() - lv_dev.copies = data["copies"] or 1 - lv_dev.metaDataSize = data["meta"] - lv_dev.logSize = data["log"] - log.debug("set %s copies to %d, metadata size to %s, log size " - "to %s, total size %s", - lv_dev.name, lv_dev.copies, lv_dev.metaDataSize, - lv_dev.logSize, lv_dev.vgSpaceUsed) + lv_info = udev.get_device(new_lv.sysfsPath) + if not lv_info: + log.error("failed to get udev data for lv %s", new_lv.name) + return new_lv + + return new_lv + + # add LVs + for lv in lv_info.values(): + # add the LV to the DeviceTree + new_lv = addLV(lv) + + if new_lv: + # save the reference for later use + all_lvs.append(new_lv) + + # Instead of doing a topological sort on internal LVs to make sure the + # parent LV is always created before its internal LVs (an internal LV + # can have internal LVs), we just create all the instances here and + # assign their parents later. Those who are not assinged a parent (which + # would hold a reference to them) will get eaten by the garbage + # collector. + + # create device instances for the internal LVs + orphan_lvs = dict() + for lv_name in internal_lvs: + full_name = "%s-%s" % (vg_name, lv_name) + try: + new_lv = createInternalLV(lv_info[full_name]) + except DeviceTreeError as e: + log.warning("Failed to process an internal LV '%s': %s", full_name, e) + else: + orphan_lvs[full_name] = new_lv + all_lvs.append(new_lv) + + # assign parents to internal LVs (and vice versa, see + # :class:`~.devices.lvm.LVMInternalLogicalVolumeDevice`) + for lv in orphan_lvs.values(): + parent_lv = lvm.determine_parent_lv(vg_name, lv, all_lvs) + if parent_lv: + lv.parent_lv = parent_lv + else: + log.warning("Failed to determine parent LV for an internal LV '%s'", lv.name) def handleUdevLVMPVFormat(self, info, device): # pylint: disable=unused-argument From f468d8b25975e2fb8e52d7be1b80350f0d37e0d7 Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Thu, 4 Jun 2015 19:51:01 +0200 Subject: [PATCH 6/9] Accept both list and ParentList when checking LVs parents These are both valid types for the self.parents attribute. --- blivet/devices/lvm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index bb77cb519..6c929c260 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -41,7 +41,7 @@ import logging log = logging.getLogger("blivet") -from .lib import LINUX_SECTOR_SIZE +from .lib import LINUX_SECTOR_SIZE, ParentList from .device import Device from .storage import StorageDevice from .container import ContainerDevice @@ -529,7 +529,7 @@ def __init__(self, name, parents=None, size=None, uuid=None, segType=None, def _check_parents(self): """Check that this device has parents as expected""" - if isinstance(self.parents, list): + if isinstance(self.parents, (list, ParentList)): if len(self.parents) != 1: raise ValueError("constructor requires a single %s instance" % self._containerClass.__name__) From 5903e71e1aca47815bbc848865289e2e0e7be4f9 Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Mon, 8 Jun 2015 10:23:28 +0200 Subject: [PATCH 7/9] Make Blivet.lvs return all LVs not just traditional/thick LVs Thin pools and thin LVs are also LVs so they should be included in the result. Also don't use the to-be-deprecated-soon getDevicesByType() method. --- blivet/blivet.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/blivet/blivet.py b/blivet/blivet.py index 6a322b19d..306a479dc 100644 --- a/blivet/blivet.py +++ b/blivet/blivet.py @@ -408,9 +408,8 @@ def lvs(self): does not necessarily reflect the actual on-disk state of the system's disks. """ - lvs = self.devicetree.getDevicesByType("lvmlv") - lvs.sort(key=lambda d: d.name) - return lvs + lvs = (d for d in self.devices if d.type in ("lvmlv", "lvmthinpool", "lvmthinlv")) + return sorted(lvs, key=lambda d: d.name) @property def thinlvs(self): From 3c80131af11629eb8bf1002ba58e2f1c1a19158a Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Mon, 8 Jun 2015 16:11:30 +0200 Subject: [PATCH 8/9] Use relations between LVs to determine parent LV Thin/cache pools have internal data/metadata LVs the name of which can be queried. Let's use this functionality to help determining the parent LV if name matching fails. --- blivet/devicelibs/lvm.py | 26 +++++++++++++++++++++++--- python-blivet.spec | 2 +- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py index 57aa740d0..ec4ca1e5e 100644 --- a/blivet/devicelibs/lvm.py +++ b/blivet/devicelibs/lvm.py @@ -126,6 +126,7 @@ def determine_parent_lv(vg_name, internal_lv, lvs): :type lvs: :class:`~.devices.lvm.LMVLogicalVolumeDevice` """ + # try name matching first (fast, cheap, often works) for lv in lvs: if internal_lv.lvname == lv.lvname: # skip the internal_lv itself @@ -136,15 +137,34 @@ def determine_parent_lv(vg_name, internal_lv, lvs): if re.match(lv.lvname+internal_lv.name_suffix+"$", internal_lv.lvname): return lv + # now try checking relations between LVs + for lv in lvs: # cache pools are internal LVs of cached LVs try: pool_name = blockdev.lvm.cache_pool_name(vg_name, lv.lvname) + except blockdev.LVMError: + # cannot determine, just go on + pass + else: if pool_name == internal_lv.lvname: return lv + + # pools have internal data and metadata LVs + try: + data_lv_name = blockdev.lvm.data_lv_name(vg_name, lv.lvname) except blockdev.LVMError: - # doesn't have a pool + # cannot determine, just go on pass - - # TODO: use 'data_lv' and 'metadata_lv' on appropriate internal LVs + else: + if data_lv_name == internal_lv.lvname: + return lv + try: + metadata_lv_name = blockdev.lvm.metadata_lv_name(vg_name, lv.lvname) + except blockdev.LVMError: + # cannot determine, just go on + pass + else: + if metadata_lv_name == internal_lv.lvname: + return lv return None diff --git a/python-blivet.spec b/python-blivet.spec index f40bc2117..ec39a146c 100644 --- a/python-blivet.spec +++ b/python-blivet.spec @@ -18,7 +18,7 @@ Source0: http://github.com/dwlehman/blivet/archive/%{realname}-%{version}.tar.gz %define pypartedver 3.10.4 %define e2fsver 1.41.0 %define utillinuxver 2.15.1 -%define libblockdevver 1.0 +%define libblockdevver 1.1 BuildArch: noarch BuildRequires: gettext From 86748b36c2c15597b3dda9307080b27793dba54e Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Mon, 8 Jun 2015 12:45:28 +0200 Subject: [PATCH 9/9] Implement the support for resizing internal metadata LVs of thin pools Internal metadata LVs of thin pools can be resized so our representation should also allow it. --- blivet/devices/lvm.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 6c929c260..e56873c08 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -1030,12 +1030,34 @@ class LVMDataLogicalVolumeDevice(LVMInternalLogicalVolumeDevice): class LVMMetadataLogicalVolumeDevice(LVMInternalLogicalVolumeDevice): """Internal metadata LV (used by thin/cache pools, RAIDs, etc.)""" + # thin pool metadata LVs can be resized directly + _resizable = True + attr_letters = ["e"] # RAIDs can have multiple (numbered) metadata LVs name_suffix = r"_[trc]meta(_[0-9]+)?" takes_extra_space = True - # TODO: override and allow resize() + # (only) thin pool metadata LVs can be resized directly + @property + def resizable(self): + if self._parent_lv: + return isinstance(self._parent_lv, LVMThinPoolDevice) + else: + # hard to say at this point, just use the name + return not re.search(r'_[rc]meta', self.lvname) + + # (only) thin pool metadata LVs can be resized directly + def resize(self): + if ((self._parent_lv and not isinstance(self._parent_lv, LVMThinPoolDevice)) or + re.search(r'_[rc]meta', self.lvname)): + raise errors.DeviceError("RAID and cache pool metadata LVs cannot be resized directly") + + # skip the generic LVMInternalLogicalVolumeDevice class and call the + # resize() method of the LVMLogicalVolumeDevice + # pylint: disable=bad-super-call + super(LVMInternalLogicalVolumeDevice, self).resize() + _INTERNAL_LV_CLASSES.append(LVMMetadataLogicalVolumeDevice) class LVMLogLogicalVolumeDevice(LVMInternalLogicalVolumeDevice):