[PATCH 09/17] Add Device classes for snapshots.
David Lehman
dlehman at redhat.com
Mon Jun 9 16:51:04 UTC 2014
On 06/06/2014 02:02 AM, Vratislav Podzimek wrote:
> 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.
I get really irritated with this issue because it breeds inconsistency.
My personal preference is to use actual words from the english language
as the standalone words, which yields SnapShot, MountPoint, &c. It is
more predictable IMO and is what I've used throughout _most_ of blivet
thus far.
>
>> + """ 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.
A docstring can be added. I don't really remember what vorigin snapshots
are for. What I do know is that they are snapshots that use free space
from the VG instead of having a real origin/source. I'm guessing they
are sort of a hackish precursor to thinp.
>
>> + 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.
>
The snapshot base class is intended to be sort of an add-in or mix-in
since thinp has already complicated the lvm class hierarchy
considerably. I am somewhat tempted to make snapshot code part of
LVMLogicalVolumeDevice instead of having distinct classes for snapshots.
More information about the anaconda-patches
mailing list