[PATCH 09/17] Add Device classes for snapshots.

Vratislav Podzimek vpodzime at redhat.com
Fri Jun 6 07:02:53 UTC 2014


On Thu, 2014-06-05 at 11:47 -0500, David Lehman wrote:
> ---
>  blivet/devices.py | 321 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 317 insertions(+), 4 deletions(-)
> 
> diff --git a/blivet/devices.py b/blivet/devices.py
> index 2e911f8..53a1234 100644
> --- a/blivet/devices.py
> +++ b/blivet/devices.py
> @@ -2537,6 +2537,11 @@ class LVMVolumeGroupDevice(ContainerDevice):
>          log.debug("Adding %s/%s to %s", lv.name, lv.size, self.name)
>          self._lvs.append(lv)
>  
> +        # snapshot accounting
> +        origin = getattr(lv, "origin", None)
> +        if origin:
> +            origin.snapshots.append(lv)
> +
>      def _removeLogVol(self, lv):
>          """ Remove an LV from this VG. """
>          if lv not in self.lvs:
> @@ -2547,6 +2552,11 @@ class LVMVolumeGroupDevice(ContainerDevice):
>          if self.poolMetaData and not self.thinpools:
>              self.poolMetaData = 0
>  
> +        # snapshot accounting
> +        origin = getattr(lv, "origin", None)
> +        if origin:
> +            origin.snapshots.remove(lv)
> +
>      def _addParent(self, member):
>          super(LVMVolumeGroupDevice, self)._addParent(member)
>  
> @@ -2800,6 +2810,7 @@ class LVMLogicalVolumeDevice(DMDevice):
>          self.metaDataSize = 0
>          self.singlePV = singlePV
>          self.segType = segType or "linear"
> +        self.snapshots = []
>  
>          self.req_grow = None
>          self.req_max_size = Size(0)
> @@ -3013,6 +3024,15 @@ class LVMLogicalVolumeDevice(DMDevice):
>          udev.udev_settle()
>          lvm.lvresize(self.vg.name, self._name, self.size)
>  
> +    @property
> +    def isleaf(self):
> +        # Thin snapshots do not need to be removed prior to removal of the
> +        # origin, but the old snapshots do.
> +        non_thin_snapshots = any(s for s in self.snapshots
> +                                    if not isinstance(s, LVMThinSnapShotDevice))
> +        return (super(LVMLogicalVolumeDevice, self).isleaf and
> +                not non_thin_snapshots)
> +
>      def dracutSetupArgs(self):
>          # Note no mapName usage here, this is a lvm cmdline name, which
>          # is different (ofcourse)
> @@ -3080,6 +3100,165 @@ class LVMLogicalVolumeDevice(DMDevice):
>  
>          return True
>  
> +class LVMSnapShotBase(object):
Do we consider "shot" a standalone word/part of "snapshot"? I think
LVMSnapshotBase makes more sense than LVMSnapShotBase.

> +    """ Abstract base class for lvm snapshots
> +
> +        This class is intended to be used with multiple inheritance in addition
> +        to some subclass of :class:`~.StorageDevice`.
> +
> +        Snapshots do not have their origin/source volume as parent. They are
> +        like other LVs except that they have an origin attribute and are in that
> +        instance's snapshots list.
> +
> +        Normal/old snapshots must be removed with their origin, while thin
> +        snapshots can remain after their origin is removed.
> +
> +        It is also impossible to set the format for a snapshot explicitly as it
> +        always has the same format as its origin.
> +    """
> +    _type = "lvmsnapshotbase"
> +    __metaclass__ = abc.ABCMeta
> +
> +    def __init__(self, origin=None, vorigin=False, exists=False):
I believe I know quite something about LVM snapshots, but I have no idea
what 'vorigin' here stands for. Maybe a better name for it? Or at least
a docstring would be welcome.

> +        self._originSpecifiedCheck(origin, vorigin, exists)
> +        self._originTypeCheck(origin)
> +        self._originExistenceCheck(origin)
> +        self._voriginExistenceCheck(vorigin, exists)
> +
> +        self.origin = origin
> +        """ the snapshot's source volume """
> +
> +        self.vorigin = vorigin
> +        """ a boolean flag indicating a vorigin snapshot """
> +
> +    def _originSpecifiedCheck(self, origin, vorigin, exists):
> +        # pylint: disable=unused-argument
> +        if not origin and not vorigin:
> +            raise ValueError("lvm snapshot devices require an origin lv")
> +
> +    def _originTypeCheck(self, origin):
> +        if origin and not isinstance(origin, LVMLogicalVolumeDevice):
> +            raise ValueError("lvm snapshot origin must be a logical volume")
> +
> +    def _originExistenceCheck(self, origin):
> +        if origin and not origin.exists:
> +            raise ValueError("lvm snapshot origin volume must already exist")
> +
> +    def _voriginExistenceCheck(self, vorigin, exists):
> +        if vorigin and not exists:
> +            raise ValueError("only existing vorigin snapshots are supported")
> +
> +    def _setFormat(self, fmt):
> +        pass
> +
> +    def _getFormat(self):
> +        if self.origin is None:
> +            fmt = getFormat(None)
> +        else:
> +            fmt = self.origin.format
> +        return fmt
> +
> +    @abc.abstractmethod
> +    def _create(self):
> +        """ Create the device. """
> +        raise NotImplementedError()
> +
> +    def merge(self):
> +        """ Merge the snapshot back into its origin volume. """
> +        log_method_call(self, self.name, status=self.status) # pylint: disable=no-member
> +        self.vg.setup()    # pylint: disable=no-member
Seeing this pylint inhibitors makes me think about the class hierarchy
we have. Shouldn't the LVMSnapshotBase inherit from the
LVMLogicalVolumeDevice class somehow? All its children classes have to
be LVs. It's probably not possible to directly inherit from
LVMLogicalVolume, but maybe we should have some LVMLogicalVolumeBase
class and inherit it here so that things like 'self.vg' really make
sense.

-- 
Vratislav Podzimek

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




More information about the anaconda-patches mailing list