[lxc-devel] [lxd/master] Storage: BTRFS CreateVolumeFromCopy only set volume size when source is image

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 397 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200429/8d067002/attachment.bin>
-------------- next part --------------
From c47463ee9fdd271412812212a3383ae25f83e5ad Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 29 Apr 2020 15:11:16 +0100
Subject: [PATCH] lxd/storage/drivers/driver/btrfs/volumes:
 CreateVolumeFromCopy only set volume size when source is image

Also properly revert on failure.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_btrfs_volumes.go | 64 ++++++++++++---------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go
index 3825145f93..19875d7698 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -234,16 +234,23 @@ func (d *btrfs) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData i
 
 // CreateVolumeFromCopy provides same-pool volume copying functionality.
 func (d *btrfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool, op *operations.Operation) error {
+	revert := revert.New()
+	defer revert.Fail()
+
 	// Recursively copy the main volume.
 	err := d.snapshotSubvolume(srcVol.MountPath(), vol.MountPath(), false, true)
 	if err != nil {
 		return err
 	}
 
-	// Set quota for volume.
-	err = d.SetVolumeQuota(vol, vol.ExpandedConfig("size"), op)
-	if err != nil {
-		return err
+	revert.Add(func() { d.deleteSubvolume(vol.MountPath(), true) })
+
+	// Set quota for volume if source volume is an image, otherwise leave quota the same as source.
+	if srcVol.volType == VolumeTypeImage {
+		err = d.SetVolumeQuota(vol, vol.ExpandedConfig("size"), op)
+		if err != nil {
+			return err
+		}
 	}
 
 	// Fixup permissions after snapshot created.
@@ -252,39 +259,40 @@ func (d *btrfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bo
 		return err
 	}
 
-	// If we're not copying any snapshots, we're done here.
-	if !copySnapshots || srcVol.IsSnapshot() {
-		return nil
-	}
+	var snapshots []string
 
-	// Get the list of snapshots.
-	snapshots, err := d.VolumeSnapshots(srcVol, op)
-	if err != nil {
-		return err
+	// Get snapshot list if copying snapshots.
+	if copySnapshots && !srcVol.IsSnapshot() {
+		// Get the list of snapshots.
+		snapshots, err = d.VolumeSnapshots(srcVol, op)
+		if err != nil {
+			return err
+		}
 	}
 
-	// If no snapshots, we're done here.
-	if len(snapshots) == 0 {
-		return nil
-	}
+	// Copy any snapshots needed.
+	if len(snapshots) > 0 {
+		// Create the parent directory.
+		err = createParentSnapshotDirIfMissing(d.name, vol.volType, vol.name)
+		if err != nil {
+			return err
+		}
 
-	// Create the parent directory.
-	err = createParentSnapshotDirIfMissing(d.name, vol.volType, vol.name)
-	if err != nil {
-		return err
-	}
+		// Copy the snapshots.
+		for _, snapName := range snapshots {
+			srcSnapshot := GetVolumeMountPath(d.name, srcVol.volType, GetSnapshotVolumeName(srcVol.name, snapName))
+			dstSnapshot := GetVolumeMountPath(d.name, vol.volType, GetSnapshotVolumeName(vol.name, snapName))
 
-	// Copy the snapshots.
-	for _, snapName := range snapshots {
-		srcSnapshot := GetVolumeMountPath(d.name, srcVol.volType, GetSnapshotVolumeName(srcVol.name, snapName))
-		dstSnapshot := GetVolumeMountPath(d.name, vol.volType, GetSnapshotVolumeName(vol.name, snapName))
+			err = d.snapshotSubvolume(srcSnapshot, dstSnapshot, true, false)
+			if err != nil {
+				return err
+			}
 
-		err = d.snapshotSubvolume(srcSnapshot, dstSnapshot, true, false)
-		if err != nil {
-			return err
+			revert.Add(func() { d.deleteSubvolume(dstSnapshot, true) })
 		}
 	}
 
+	revert.Success()
 	return nil
 }