[PATCH 06/17] Add backend functions for creating and managing snapshots.

David Lehman dlehman at redhat.com
Mon Jun 9 16:58:34 UTC 2014


On 06/05/2014 02:42 PM, Anne Mulhern wrote:
>
>
>
>
> ----- Original Message -----
>> From: "David Lehman" <dlehman at redhat.com>
>> To: anaconda-patches at lists.fedorahosted.org
>> Sent: Thursday, June 5, 2014 12:47:26 PM
>> Subject: [PATCH 06/17] Add backend functions for creating and managing	snapshots.
>>
>> ---
>>   blivet/devicelibs/btrfs.py | 20 +++++++++++++++-----
>>   blivet/devicelibs/lvm.py   | 35 +++++++++++++++++++++++++++++++++++
>>   2 files changed, 50 insertions(+), 5 deletions(-)
>>
>> diff --git a/blivet/devicelibs/btrfs.py b/blivet/devicelibs/btrfs.py
>> index 61697ba..e1d08d0 100644
>> --- a/blivet/devicelibs/btrfs.py
>> +++ b/blivet/devicelibs/btrfs.py
>> @@ -107,20 +107,23 @@ def delete_subvolume(mountpoint, name):
>>       args = ["subvol", "delete", path]
>>       return btrfs(args)
>>
>> -_SUBVOL_REGEX_STR = r'ID (\d+) gen \d+ parent (\d+) top level \d+ path
>> (.+)\n'
>> +_SUBVOL_REGEX_STR = r'ID (?P<id>\d+) gen \d+ (cgen \d+ )?parent
>> (?P<parent>\d+) top level \d+ (otime \d{4}-\d{2}-\d{2} \d\d:\d\d:\d\d )?path
>> (?P<path>.+)\n'
>>   _SUBVOL_REGEX = re.compile(_SUBVOL_REGEX_STR)
>>
>>   # get a list of subvolumes from a mounted btrfs filesystem
>> -def list_subvolumes(mountpoint):
>> +def list_subvolumes(mountpoint, snapshots_only=False):
>>       if not os.path.ismount(mountpoint):
>>           raise ValueError("volume not mounted")
>>
>>       args = ["subvol", "list", "-p", mountpoint]
>> +    if snapshots_only:
>> +        args.insert(2, "-s")
>> +
>>       buf = btrfs(args, capture=True)
>>       vols = []
>> -    for group in _SUBVOL_REGEX.findall(buf):
>> -        vols.append({"id": int(group[0]), "parent": int(group[1]),
>> -                     "path": group[2]})
>> +    for m in _SUBVOL_REGEX.finditer(buf):
>> +        vols.append({"id": int(m.group('id')), "parent":
>> int(m.group('parent')),
>> +                     "path": m.group('path')})
>>
>>       return vols
>>
>> @@ -138,6 +141,13 @@ def get_default_subvolume(mountpoint):
>>
>>       return default
>>
>> +def create_snapshot(source, dest):
>> +    if not os.path.ismount(source):
>> +        raise ValueError("source is not a mounted subvolume")
>> +
>> +    args = ["subvol", "snapshot", source, dest]
>> +    return btrfs(args)
>> +
>>   _DEVICE_REGEX_STR = r'devid[ \t]+(\d+)[ \t]+size[ \t]+(\S+)[ \t]+used[
>>   \t]+(\S+)[ \t]+path[ \t]+(\S+)\n'
>>   _DEVICE_REGEX = re.compile(_DEVICE_REGEX_STR)
>>
>> diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py
>> index f2b85ac..a66f4d6 100644
>> --- a/blivet/devicelibs/lvm.py
>> +++ b/blivet/devicelibs/lvm.py
>> @@ -520,6 +520,32 @@ def lvdeactivate(vg_name, lv_name):
>>       except LVMError as msg:
>>           raise LVMError("lvdeactivate failed for %s: %s" % (lv_name, msg))
>>
>> +def lvsnapshotmerge(vg_name, lv_name):
>> +    """ Merge(/rollback/revert) a snapshot volume into its origin.
>> +
>> +        .. note::
>> +
>> +            This is an asynchronous procedure. See lvconvert(8) for details
>> of
>> +            how merge is handled by lvm.
>> +
>> +    """
>> +    args = ["lvconvert", "--merge", "%s/%s" % (vg_name, lv_name)]
>> +    args += _getConfigArgs() + [lv_name]
>> +
>> +    try:
>> +        lvm(args)
>> +    except LVMError as msg:
>> +        raise LVMError("lvsnapshotmerge failed for %s: %s" % (lv_name, msg))
>> +
>> +def lvsnapshotcreate(vg_name, lv_name, size, origin):
>> +    args = ["lvcreate", "-s", "-L", "%dm" % size.convertTo(spec="MiB"),
>> +            "-n", lv_name] + _getConfigArgs() + ["%s/%s" % (vg_name,
>> origin)]
>> +
>> +    try:
>> +        lvm(args)
>> +    except LVMError as msg:
>> +        raise LVMError("lvsnapshotcreate failed for %s/%s: %s" % (vg_name,
>> lv_name, msg))
>> +
>>   def thinpoolcreate(vg_name, lv_name, size, metadatasize=None,
>>   chunksize=None):
>>       args = ["lvcreate", "--thinpool", "%s/%s" % (vg_name, lv_name),
>>               "--size", "%dm" % size.convertTo(spec="mib")]
>> @@ -550,6 +576,15 @@ def thinlvcreate(vg_name, pool_name, lv_name, size):
>>       except LVMError as msg:
>>           raise LVMError("lvcreate failed for %s/%s: %s" % (vg_name, lv_name,
>>           msg))
>>
>> +def thinsnapshotcreate(vg_name, lv_name, origin_name, pool_name=None):
>> +    args = ["lvcreate", "-s", "-n", lv_name, "%s/%s" % (vg_name,
>> origin_name)]
>> +    if pool_name:
>> +        args.extend(["--thinpool", pool_name])
>> +    try:
>> +        lvm(args)
>> +    except LVMError as msg:
>> +        raise LVMError("lvcreate (snapshot) failed for %s/%s: %s" %
>> (vg_name, lv_name, msg))
>> +
>>   def thinlvpoolname(vg_name, lv_name):
>>       args = ["lvs", "--noheadings", "-o", "pool_lv"] + \
>>               _getConfigArgs(read_only_locking=True) + \
>> --
>> 1.9.0
>>
>> _______________________________________________
>> anaconda-patches mailing list
>> anaconda-patches at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>>
>
> For the two *snapshotcreate methods, the name for the third parameter
> should probably be the same.

Yes, they should.

>
> It also might be helpful to have a docstring that documents origin_name.

Will do.

>
> And, I may be wrong, but would 'snap_name' be a better choice than 'lv_name'
> for the second parameter of these two?

It certainly doesn't seem to be a worse choice. I'll change it.

>
> - mulhern
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>



More information about the anaconda-patches mailing list