[PATCH 2/3] Recognize and process cached logical volumes

Vratislav Podzimek vpodzime at redhat.com
Tue Jun 23 07:46:23 UTC 2015


On Mon, 2015-06-22 at 16:52 -0700, Brian C. Lane wrote:
> On Mon, Jun 22, 2015 at 08:27:08PM +0200, Vratislav Podzimek wrote:
> > When there's an existing cached LV we need to gather some basic information
> > about the cache and ammend it to base/data LV so that it can report such
> > information when queried.
> > 
> > Related: rhbz#1120421
> > Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> > ---
> >  blivet/devicelibs/lvm.py   |  5 ++++
> >  blivet/devices/__init__.py |  2 +-
> >  blivet/devices/lvm.py      | 57 +++++++++++++++++++++++++++++++++++++++++++++-
> >  blivet/devicetree.py       |  9 +++++++-
> >  4 files changed, 70 insertions(+), 3 deletions(-)
> > 
> > diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py
> > index 6568d14..c1fbe90 100644
> > --- a/blivet/devicelibs/lvm.py
> > +++ b/blivet/devicelibs/lvm.py
> > @@ -634,3 +634,8 @@ def thinlvpoolname(vg_name, lv_name):
> >          pool = ''
> >  
> >      return pool
> > +
> > +def cachepoolname(vg_name, lv_name):
> > +    # exactly the same as thin pool name
> > +    return thinlvpoolname(vg_name, lv_name)
> > +
> > diff --git a/blivet/devices/__init__.py b/blivet/devices/__init__.py
> > index 0b2eef2..85e9827 100644
> > --- a/blivet/devices/__init__.py
> > +++ b/blivet/devices/__init__.py
> > @@ -26,7 +26,7 @@ from .disk import DiskDevice, DiskFile, DMRaidArrayDevice, MultipathDevice, iScs
> >  from .partition import PartitionDevice
> >  from .dm import DMDevice, DMLinearDevice, DMCryptDevice
> >  from .luks import LUKSDevice
> > -from .lvm import LVMVolumeGroupDevice, LVMLogicalVolumeDevice, LVMSnapShotDevice, LVMThinPoolDevice, LVMThinLogicalVolumeDevice, LVMThinSnapShotDevice
> > +from .lvm import LVMVolumeGroupDevice, LVMLogicalVolumeDevice, LVMSnapShotDevice, LVMThinPoolDevice, LVMThinLogicalVolumeDevice, LVMThinSnapShotDevice, LVMCache
> >  from .md import MDRaidArrayDevice
> >  from .btrfs import BTRFSDevice, BTRFSVolumeDevice, BTRFSSubVolumeDevice, BTRFSSnapShotDevice
> >  from .file import FileDevice, DirectoryDevice, SparseFileDevice
> > diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py
> > index 61e8e26..356446c 100644
> > --- a/blivet/devices/lvm.py
> > +++ b/blivet/devices/lvm.py
> > @@ -515,6 +515,7 @@ class LVMLogicalVolumeDevice(DMDevice):
> >          self.metaDataSize = 0
> >          self.segType = segType or "linear"
> >          self.snapshots = []
> > +        self._cache = None
> >  
> >          self.req_grow = None
> >          self.req_max_size = Size(0)
> > @@ -582,8 +583,11 @@ class LVMLogicalVolumeDevice(DMDevice):
> >      @property
> >      def vgSpaceUsed(self):
> >          """ Space occupied by this LV, not including snapshots. """
> > +        cache_size = Size(0)
> > +        if self.cached:
> > +            cache_size = self.cache.size
> >          return (self.vg.align(self.size, roundup=True) * self.copies
> > -                + self.logSize + self.metaDataSize)
> > +                + self.logSize + self.metaDataSize + cache_size)
> >  
> >      @property
> >      def vg(self):
> > @@ -802,6 +806,18 @@ class LVMLogicalVolumeDevice(DMDevice):
> >  
> >          return True
> >  
> > +    @property
> > +    def cached(self):
> > +        return bool(self._cache)
> > +
> > +    @property
> > +    def cache(self):
> > +        return self._cache
> > +
> > +    @cache.setter
> > +    def cache(self, val):
> > +        self._cache = val
> 
> I've never seen the point in wrapping things like this when the property
> isn't adding any functionality (checking syntax/object type/etc.) Why
> not just use self.cache?
Same here, but these are expected to become more complex in future when
we support more from the LVM cache area -- e.g. checks when assigning an
unexisting cache to an existing LV or creating the LVMCache object
dynamically for an existing LV based on its internal LVs (see the PR
with these changes targeting the master branch [1]) etc. So I'd like to
have this ready for such amendments.

[1]
https://github.com/vpodzime/blivet/commit/113058f9f04d5a14f76e57308533653778f37809

> 
> 
> > +
> >  class LVMSnapShotBase(object):
> >      """ Abstract base class for lvm snapshots
> >  
> > @@ -1243,3 +1259,42 @@ class LVMThinSnapShotDevice(LVMSnapShotBase, LVMThinLogicalVolumeDevice):
> >          # once a thin snapshot exists it no longer depends on its origin
> >          return ((self.origin == dep and not self.exists) or
> >                  super(LVMThinSnapShotDevice, self).dependsOn(dep))
> > +
> > +class LVMCache(object):
> > +    """Class providing the cache-related functionality of a cached LV"""
> > +
> > +    def __init__(self, cached_lv, size=None, exists=False, fast_pv_names=None, mode=None):
> > +        """
> > +        :param cached_lv: the LV the cache functionality of which to provide
> > +        :type cached_lv: :class:`LVMLogicalVolumeDevice`
> > +        :param size: size of the cache (useful mainly for non-existing caches
> > +                     that cannot determine their size dynamically)
> > +        :type size: :class:`~.size.Size`
> > +        :param bool exists: whether the cache exists or not
> > +        :param fast_pv_names: PVs to allocate the cache on/from (ignored for existing)
> > +        :type fast_pv_names: list of str
> > +        :param str mode: desired mode for non-existing cache (ignored for existing)
> > +
> > +        """
> > +        self._cached_lv = cached_lv
> > +        self._size = size
> > +        self._exists = exists
> > +        self._fast_pvs = fast_pv_names
> > +        self._mode = mode
> > +
> > +    @property
> > +    def size(self):
> > +        return self._size
> > +
> > +    @property
> > +    def exists(self):
> > +        return self._exists
> > +
> > +    @property
> > +    def fast_pv_names(self):
> > +        return self._fast_pvs
> > +
> > +    @property
> > +    def mode(self):
> > +        return self._mode
> > +
> > diff --git a/blivet/devicetree.py b/blivet/devicetree.py
> > index e691a2c..23f0721 100644
> > --- a/blivet/devicetree.py
> > +++ b/blivet/devicetree.py
> > @@ -33,7 +33,7 @@ from .devices import DASDDevice, DMDevice, DMLinearDevice, DMRaidArrayDevice, Di
> >  from .devices import FcoeDiskDevice, FileDevice, LoopDevice, LUKSDevice
> >  from .devices import LVMLogicalVolumeDevice, LVMVolumeGroupDevice
> >  from .devices import LVMThinPoolDevice, LVMThinLogicalVolumeDevice
> > -from .devices import LVMSnapShotDevice, LVMThinSnapShotDevice
> > +from .devices import LVMSnapShotDevice, LVMThinSnapShotDevice, LVMCache
> >  from .devices import MDRaidArrayDevice, MultipathDevice, NoDevice, OpticalDevice
> >  from .devices import PartitionDevice, ZFCPDiskDevice, iScsiDiskDevice
> >  from .devices import devicePathToName
> > @@ -1504,6 +1504,13 @@ class DeviceTree(object):
> >                  lv_device = lv_class(lv_name, parents=lv_parents,
> >                                       uuid=lv_uuid, size=lv_size,segType=lv_type,
> >                                       exists=True, **lv_kwargs)
> > +                if lv_attr[0] == "C":
> > +                    cache_pool_name = lvm.cachepoolname(vg_name, lv_name)
> > +                    if cache_pool_name:
> > +                        cache_pool_info = self.lvInfo["%s-%s" % (vg_name, cache_pool_name)]
> > +                        cache_size = udev.device_get_lv_size(cache_pool_info)
> > +                        cache = LVMCache(lv_dev, cache_size, exists=True)
> > +                        lv_device.cache = cache
> 
> May as well do the assignment in one line instead of using an
> intermediate variable.
Good point, fixing locally.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list