[lxc-devel] [lxd/master] Storage: Support ceph cached image regeneration

tomponline on Github lxc-bot at linuxcontainers.org
Tue Apr 28 14:18:12 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 825 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200428/cdf60636/attachment-0001.bin>
-------------- next part --------------
From d5ab08fb1679168bcf0768f4552af95afda482a4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 24 Apr 2020 11:35:30 +0100
Subject: [PATCH 01/24] lxd/storage/drivers/driver/ceph/volumes: Only check
 content type is block not volume type

No need to check volume type when checking for deleted volumes.

Also removes line wrapping.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_volumes.go | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index 623a12b182..d37908a4dc 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -40,11 +40,10 @@ func (d *ceph) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Ope
 	}
 
 	// Figure out the potential zombie volume.
-	zombieImageVol := NewVolume(d, d.name, VolumeType("zombie_image"), vol.contentType,
-		fmt.Sprintf("%s_%s", vol.name, d.getRBDFilesystem(vol)), nil, nil)
-	if (vol.volType == VolumeTypeVM || vol.volType == VolumeTypeImage) && vol.contentType == ContentTypeBlock {
-		zombieImageVol = NewVolume(d, d.name, VolumeType("zombie_image"), vol.contentType,
-			fmt.Sprintf("%s_%s.block", vol.name, d.getRBDFilesystem(vol)), nil, nil)
+	zombieImageVol := NewVolume(d, d.name, VolumeType("zombie_image"), vol.contentType, fmt.Sprintf("%s_%s", vol.name, d.getRBDFilesystem(vol)), nil, nil)
+
+	if vol.contentType == ContentTypeBlock {
+		zombieImageVol = NewVolume(d, d.name, VolumeType("zombie_image"), vol.contentType, fmt.Sprintf("%s_%s.block", vol.name, d.getRBDFilesystem(vol)), nil, nil)
 	}
 
 	// Check if we have a zombie image. If so, restore it otherwise

From 3ee38d3a370cf7ebf9a0ec55fcbfd8ad82a7145a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 24 Apr 2020 11:36:12 +0100
Subject: [PATCH 02/24] lxd/storage/drivers/driver/ceph/volumes: Removes line
 wrapping

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_volumes.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index d37908a4dc..1bdb10d7fc 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -46,8 +46,7 @@ func (d *ceph) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Ope
 		zombieImageVol = NewVolume(d, d.name, VolumeType("zombie_image"), vol.contentType, fmt.Sprintf("%s_%s.block", vol.name, d.getRBDFilesystem(vol)), nil, nil)
 	}
 
-	// Check if we have a zombie image. If so, restore it otherwise
-	// create a new image volume.
+	// Check if we have a zombie image. If so, restore it otherwise create a new image volume.
 	if vol.volType == VolumeTypeImage && d.HasVolume(zombieImageVol) {
 		// Figure out the names.
 		oldName := d.getRBDVolumeName(zombieImageVol, "", false, true)

From 131ec36441b6b57af205b718aabfc485f93bd575 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 24 Apr 2020 11:57:39 +0100
Subject: [PATCH 03/24] lxd/storage/drivers/driver/ceph/utils: Uses
 defaultBlockSize rather than hardcoded 10GB

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index 1643142f58..ac3d3b4086 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -528,7 +528,7 @@ func (d *ceph) getRBDSize(vol Volume) (string, error) {
 
 	// Safety net: Set to default value.
 	if sz == 0 {
-		sz, _ = units.ParseByteSizeString("10GB")
+		sz, _ = units.ParseByteSizeString(defaultBlockSize)
 	}
 
 	return fmt.Sprintf("%dB", sz), nil

From db30e750a06b249631438920495a1745b91ebd30 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 24 Apr 2020 14:15:40 +0100
Subject: [PATCH 04/24] lxd/storage/drivers/driver/ceph/volumes: Adds
 getVolumeSize function

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

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index 1bdb10d7fc..fbdf1cc762 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -207,6 +207,33 @@ func (d *ceph) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Ope
 	return nil
 }
 
+// getVolumeSize returns the volume's size in bytes.
+func (d *ceph) getVolumeSize(volumeName string) (int64, error) {
+	volInfo := struct {
+		Size int64 `json:"size"`
+	}{}
+
+	jsonInfo, err := shared.TryRunCommand(
+		"rbd",
+		"info",
+		"--format", "json",
+		"--id", d.config["ceph.user.name"],
+		"--cluster", d.config["ceph.cluster_name"],
+		"--pool", d.config["ceph.osd.pool_name"],
+		volumeName,
+	)
+	if err != nil {
+		return -1, err
+	}
+
+	err = json.Unmarshal([]byte(jsonInfo), &volInfo)
+	if err != nil {
+		return -1, err
+	}
+
+	return volInfo.Size, nil
+}
+
 // CreateVolumeFromBackup re-creates a volume from its exported state.
 func (d *ceph) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData io.ReadSeeker, optimizedStorage bool, op *operations.Operation) (func(vol Volume) error, func(), error) {
 	return genericVFSBackupUnpack(d, vol, snapshots, srcData, op)

From 55366e926830e7283e650c647a0547d342d4a0ef Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 24 Apr 2020 14:23:40 +0100
Subject: [PATCH 05/24] lxd/storage/drivers/driver/ceph/volumes: Restructures
 CreateVolume to reduce repetition

Will make it cleaner to support regeneration of cached image volumes.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_volumes.go | 54 ++++++++++++----------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index fbdf1cc762..d245e330d4 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -25,6 +25,30 @@ import (
 // CreateVolume creates an empty volume and can optionally fill it by executing the supplied
 // filler function.
 func (d *ceph) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Operation) error {
+	// Function to rename an RBD volume.
+	renameVolume := func(oldName string, newName string) error {
+		_, err := shared.RunCommand(
+			"rbd",
+			"--id", d.config["ceph.user.name"],
+			"--cluster", d.config["ceph.cluster_name"],
+			"mv",
+			oldName,
+			newName,
+		)
+		return err
+	}
+
+	// Function to generate the deleted zombie equivalent of a volume.
+	deletedVolume := func(v Volume) Volume {
+		deletedVol := NewVolume(d, d.name, VolumeType("zombie_image"), v.contentType, fmt.Sprintf("%s_%s", v.name, d.getRBDFilesystem(vol)), vol.config, vol.poolConfig)
+
+		if vol.contentType == ContentTypeBlock {
+			deletedVol = NewVolume(d, d.name, VolumeType("zombie_image"), v.contentType, fmt.Sprintf("%s_%s.block", v.name, d.getRBDFilesystem(vol)), vol.config, vol.poolConfig)
+		}
+
+		return deletedVol
+	}
+
 	// Revert handling.
 	revert := revert.New()
 	defer revert.Fail()
@@ -39,41 +63,23 @@ func (d *ceph) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Ope
 		revert.Add(func() { os.Remove(vol.MountPath()) })
 	}
 
-	// Figure out the potential zombie volume.
-	zombieImageVol := NewVolume(d, d.name, VolumeType("zombie_image"), vol.contentType, fmt.Sprintf("%s_%s", vol.name, d.getRBDFilesystem(vol)), nil, nil)
+	deletedVol := deletedVolume(vol)
 
-	if vol.contentType == ContentTypeBlock {
-		zombieImageVol = NewVolume(d, d.name, VolumeType("zombie_image"), vol.contentType, fmt.Sprintf("%s_%s.block", vol.name, d.getRBDFilesystem(vol)), nil, nil)
-	}
-
-	// Check if we have a zombie image. If so, restore it otherwise create a new image volume.
-	if vol.volType == VolumeTypeImage && d.HasVolume(zombieImageVol) {
-		// Figure out the names.
-		oldName := d.getRBDVolumeName(zombieImageVol, "", false, true)
-		newName := d.getRBDVolumeName(vol, "", false, true)
-
-		// Rename back to active.
-		_, err := shared.RunCommand(
-			"rbd",
-			"--id", d.config["ceph.user.name"],
-			"--cluster", d.config["ceph.cluster_name"],
-			"mv",
-			oldName,
-			newName)
+	// Check if we have a deleted zombie image. If so, restore it otherwise create a new image volume.
+	if vol.volType == VolumeTypeImage && d.HasVolume(deletedVol) {
+		err := renameVolume(d.getRBDVolumeName(deletedVol, "", false, true), d.getRBDVolumeName(vol, "", false, true))
 		if err != nil {
 			return err
 		}
 
-		// For VMs, also create the filesystem volume.
 		if vol.IsVMBlock() {
+			fsDeletedVol := deletedVol.NewVMBlockFilesystemVolume()
 			fsVol := vol.NewVMBlockFilesystemVolume()
 
-			err := d.CreateVolume(fsVol, nil, op)
+			err = renameVolume(d.getRBDVolumeName(fsDeletedVol, "", false, true), d.getRBDVolumeName(fsVol, "", false, true))
 			if err != nil {
 				return err
 			}
-
-			revert.Add(func() { d.DeleteVolume(fsVol, op) })
 		}
 
 		revert.Success()

From 47c1f01e1ea6b027e20859bdc55a5b3e11515d8c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 24 Apr 2020 15:13:33 +0100
Subject: [PATCH 06/24] lxd/storage/drivers/driver/ceph/volumes: Removes
 unnecessary mount/unmount

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_volumes.go | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index d245e330d4..74e09a916a 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -436,15 +436,6 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo
 		return err
 	}
 
-	ourMount, err := d.MountVolume(vol, op)
-	if err != nil {
-		return err
-	}
-
-	if ourMount {
-		defer d.UnmountVolume(vol, op)
-	}
-
 	revert.Success()
 
 	return nil

From 022d9e6ea97207f48c9f1e5d66d78fe843ee7330 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 24 Apr 2020 15:20:19 +0100
Subject: [PATCH 07/24] lxd/storage/drivers/driver/zfs/volumes: Clarify clone
 comments

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_zfs_volumes.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index 972c63211e..73bc4fac87 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -491,7 +491,7 @@ func (d *zfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool
 		}
 	}
 
-	// Handle zfs.clone_copy
+	// If zfs.clone_copy is disabled or source volume has snapshots, then use full copy mode.
 	if (d.config["zfs.clone_copy"] != "" && !shared.IsTrue(d.config["zfs.clone_copy"])) || len(snapshots) > 0 {
 		snapName := strings.SplitN(srcSnapshot, "@", 2)[1]
 
@@ -557,6 +557,7 @@ func (d *zfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool
 			}
 		}
 	} else {
+		// Perform volume clone.
 		args := []string{
 			"clone",
 			srcSnapshot,

From 62f0ecfe410d41548dab30ec91d36df4dddd4ae6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 24 Apr 2020 16:09:12 +0100
Subject: [PATCH 08/24] lxd/storage/drivers/driver/ceph/volumes: Dont wrap
 lines

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_volumes.go | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index 74e09a916a..5918e36232 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -248,11 +248,11 @@ func (d *ceph) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData io
 // CreateVolumeFromCopy provides same-pool volume copying functionality.
 func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool, op *operations.Operation) error {
 	var err error
-	snapshots := []string{}
-
 	revert := revert.New()
 	defer revert.Fail()
 
+	// Retrieve snapshots on the source.
+	snapshots := []string{}
 	if !srcVol.IsSnapshot() && copySnapshots {
 		snapshots, err = d.VolumeSnapshots(srcVol, op)
 		if err != nil {
@@ -262,16 +262,16 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo
 
 	// Copy without snapshots.
 	if !copySnapshots || len(snapshots) == 0 {
-		if d.config["ceph.rbd.clone_copy"] != "" &&
-			!shared.IsTrue(d.config["ceph.rbd.clone_copy"]) &&
-			srcVol.volType != VolumeTypeImage {
+		// If lightweight clone mode isn't enabled, perform a full copy of the volume.
+		if d.config["ceph.rbd.clone_copy"] != "" && !shared.IsTrue(d.config["ceph.rbd.clone_copy"]) && srcVol.volType != VolumeTypeImage {
 			_, err = shared.RunCommand(
 				"rbd",
 				"--id", d.config["ceph.user.name"],
 				"--cluster", d.config["ceph.cluster_name"],
 				"cp",
 				d.getRBDVolumeName(srcVol, "", false, true),
-				d.getRBDVolumeName(vol, "", false, true))
+				d.getRBDVolumeName(vol, "", false, true),
+			)
 			if err != nil {
 				return err
 			}
@@ -292,10 +292,8 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo
 				snapshotName = fmt.Sprintf("zombie_snapshot_%s", uuid.NewRandom().String())
 
 				if srcVol.IsSnapshot() {
-					srcParentName, srcSnapOnlyName, _ :=
-						shared.InstanceGetParentAndSnapshotName(srcVol.name)
+					srcParentName, srcSnapOnlyName, _ := shared.InstanceGetParentAndSnapshotName(srcVol.name)
 					snapshotName = fmt.Sprintf("snapshot_%s", srcSnapOnlyName)
-
 					parentVol = NewVolume(d, d.name, srcVol.volType, srcVol.contentType, srcParentName, nil, nil)
 				} else {
 					// Create snapshot.

From 5ed4129bf42c77b0d6cdc0c579d64c78f2e48c43 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 24 Apr 2020 17:24:53 +0100
Subject: [PATCH 09/24] lxd/storage/drivers/driver/ceph/volumes: Allow
 regeneration of image volumes in CreateVolume

When volume.size is smaller than cached image size.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_volumes.go | 82 +++++++++++++++-------
 1 file changed, 56 insertions(+), 26 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index 5918e36232..a023287a80 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -38,17 +38,6 @@ func (d *ceph) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Ope
 		return err
 	}
 
-	// Function to generate the deleted zombie equivalent of a volume.
-	deletedVolume := func(v Volume) Volume {
-		deletedVol := NewVolume(d, d.name, VolumeType("zombie_image"), v.contentType, fmt.Sprintf("%s_%s", v.name, d.getRBDFilesystem(vol)), vol.config, vol.poolConfig)
-
-		if vol.contentType == ContentTypeBlock {
-			deletedVol = NewVolume(d, d.name, VolumeType("zombie_image"), v.contentType, fmt.Sprintf("%s_%s.block", v.name, d.getRBDFilesystem(vol)), vol.config, vol.poolConfig)
-		}
-
-		return deletedVol
-	}
-
 	// Revert handling.
 	revert := revert.New()
 	defer revert.Fail()
@@ -63,37 +52,77 @@ func (d *ceph) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Ope
 		revert.Add(func() { os.Remove(vol.MountPath()) })
 	}
 
-	deletedVol := deletedVolume(vol)
+	// Create a "zombie" deleted volume representation of the specified volume to look for its existance.
+	deletedVol := NewVolume(d, d.name, cephVolumeTypeZombieImage, vol.contentType, vol.name, vol.config, vol.poolConfig)
 
 	// Check if we have a deleted zombie image. If so, restore it otherwise create a new image volume.
 	if vol.volType == VolumeTypeImage && d.HasVolume(deletedVol) {
-		err := renameVolume(d.getRBDVolumeName(deletedVol, "", false, true), d.getRBDVolumeName(vol, "", false, true))
+		canRestore := true
+
+		// Check if the cached image volume is larger than the current pool volume.size setting
+		// (if so we won't be able to resize the snapshot to that the smaller size later).
+		volSizeBytes, err := d.getVolumeSize(d.getRBDVolumeName(deletedVol, "", false, true))
 		if err != nil {
 			return err
 		}
 
-		if vol.IsVMBlock() {
-			fsDeletedVol := deletedVol.NewVMBlockFilesystemVolume()
-			fsVol := vol.NewVMBlockFilesystemVolume()
+		poolVolSize := defaultBlockSize
+		if vol.poolConfig["volume.size"] != "" {
+			poolVolSize = vol.poolConfig["volume.size"]
+		}
 
-			err = renameVolume(d.getRBDVolumeName(fsDeletedVol, "", false, true), d.getRBDVolumeName(fsVol, "", false, true))
+		poolVolSizeBytes, err := units.ParseByteSizeString(poolVolSize)
+		if err != nil {
+			return err
+		}
+
+		// If the cached volume is larger than the pool volume size, then we can't use the
+		// deleted cached image volume and instead we will rename it to a random UUID so it can't
+		// be restored in the future and a new cached image volume will be created instead.
+		if volSizeBytes > poolVolSizeBytes {
+			randomVol := NewVolume(d, d.name, deletedVol.volType, deletedVol.contentType, strings.Replace(uuid.NewRandom().String(), "-", "", -1), deletedVol.config, deletedVol.poolConfig)
+			err = renameVolume(d.getRBDVolumeName(deletedVol, "", false, true), d.getRBDVolumeName(randomVol, "", false, true))
 			if err != nil {
 				return err
 			}
+
+			if vol.IsVMBlock() {
+				fsDeletedVol := deletedVol.NewVMBlockFilesystemVolume()
+				fsRandomVol := randomVol.NewVMBlockFilesystemVolume()
+
+				err = renameVolume(d.getRBDVolumeName(fsDeletedVol, "", false, true), d.getRBDVolumeName(fsRandomVol, "", false, true))
+				if err != nil {
+					return err
+				}
+			}
+
+			canRestore = false
 		}
 
-		revert.Success()
-		return nil
-	}
+		// Restore the image.
+		if canRestore {
+			err = renameVolume(d.getRBDVolumeName(deletedVol, "", false, true), d.getRBDVolumeName(vol, "", false, true))
+			if err != nil {
+				return err
+			}
 
-	// Get size.
-	RBDSize, err := d.getRBDSize(vol)
-	if err != nil {
-		return err
+			if vol.IsVMBlock() {
+				fsDeletedVol := deletedVol.NewVMBlockFilesystemVolume()
+				fsVol := vol.NewVMBlockFilesystemVolume()
+
+				err = renameVolume(d.getRBDVolumeName(fsDeletedVol, "", false, true), d.getRBDVolumeName(fsVol, "", false, true))
+				if err != nil {
+					return err
+				}
+			}
+
+			revert.Success()
+			return nil
+		}
 	}
 
 	// Create volume.
-	err = d.rbdCreateVolume(vol, RBDSize)
+	err := d.rbdCreateVolume(vol, d.volumeSize(vol))
 	if err != nil {
 		return err
 	}
@@ -182,7 +211,8 @@ func (d *ceph) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Ope
 		}
 
 		if vol.contentType == ContentTypeBlock {
-			// Re-create the FS config volume's readonly snapshot now that the filler function has run and unpacked into both config and block volumes.
+			// Re-create the FS config volume's readonly snapshot now that the filler function has run
+			// and unpacked into both config and block volumes.
 			fsVol := NewVolume(d, d.name, vol.volType, ContentTypeFS, vol.name, vol.config, vol.poolConfig)
 
 			err := d.rbdUnprotectVolumeSnapshot(fsVol, "readonly")

From 9c24641cd9a07d4a5adecb09eb291c0a9bb7e76c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 27 Apr 2020 11:23:09 +0100
Subject: [PATCH 10/24] lxd/storage/drivers/driver/ceph/volumes: Dont use clone
 mode when creating volume from cached image when it is disabled

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_volumes.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index a023287a80..a602524ac5 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -293,7 +293,7 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo
 	// Copy without snapshots.
 	if !copySnapshots || len(snapshots) == 0 {
 		// If lightweight clone mode isn't enabled, perform a full copy of the volume.
-		if d.config["ceph.rbd.clone_copy"] != "" && !shared.IsTrue(d.config["ceph.rbd.clone_copy"]) && srcVol.volType != VolumeTypeImage {
+		if d.config["ceph.rbd.clone_copy"] != "" && !shared.IsTrue(d.config["ceph.rbd.clone_copy"]) {
 			_, err = shared.RunCommand(
 				"rbd",
 				"--id", d.config["ceph.user.name"],

From a79d560399a4a7973880557f3998aa1bc0400c16 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:05:10 +0100
Subject: [PATCH 11/24] lxd/storage/utils: VolumeDBCreate comment formatting

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/utils.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 7065b98708..e7f55e3fdd 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -133,8 +133,7 @@ func VolumeDBCreate(s *state.State, project, poolName, volumeName, volumeDescrip
 		return err
 	}
 
-	// Check that a storage volume of the same storage volume type does not
-	// already exist.
+	// Check that a storage volume of the same storage volume type does not already exist.
 	volumeID, _ := s.Cluster.StoragePoolNodeVolumeGetTypeIDByProject(project, volumeName, volumeType, poolID)
 	if volumeID > 0 {
 		return fmt.Errorf("A storage volume of type %s already exists", volumeTypeName)

From e20330d1c2777ecff8b9e3fcf8ed2ec8b942e914 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:05:32 +0100
Subject: [PATCH 12/24] lxd/storage/drivers/driver/zfs/volumes: Pass operation
 in CreateVolumeFromCopy

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_zfs_volumes.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index 73bc4fac87..abcade9d49 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -596,7 +596,7 @@ func (d *zfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool
 	}
 
 	// Resize the new volume and filesystem to the correct size.
-	err := d.SetVolumeQuota(vol, vol.ExpandedConfig("size"), nil)
+	err := d.SetVolumeQuota(vol, vol.ExpandedConfig("size"), op)
 	if err != nil {
 		return err
 	}

From 7c7a71eb0afbb4fb1b0b0f74ebf2bd1d5efa95ae Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:07:12 +0100
Subject: [PATCH 13/24] lxc/storage/drivers/driver/ceph/utils: Reworks
 parseParent to return a Volume struct

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 63 ++++++++++--------------
 1 file changed, 25 insertions(+), 38 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index ac3d3b4086..a90eee051d 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"regexp"
 	"strconv"
 	"strings"
 	"syscall"
@@ -853,54 +854,40 @@ func (d *ceph) deleteVolumeSnapshot(vol Volume, snapshotName string) (int, error
 // parseParent splits a string describing a RBD storage entity into its components.
 // This can be used on strings like
 // <osd-pool-name>/<lxd-specific-prefix>_<rbd-storage-volume>@<rbd-snapshot-name>
-// and will split it into
-// <osd-pool-name>, <rbd-storage-volume>, <lxd-specific-prefix>, <rbdd-snapshot-name>
-func (d *ceph) parseParent(parent string) (string, string, string, string, error) {
+// and will return a Volume and snapshot name.
+func (d *ceph) parseParent(parent string) (Volume, string, error) {
+	vol := Volume{}
+
 	idx := strings.Index(parent, "/")
 	if idx == -1 {
-		return "", "", "", "", fmt.Errorf("Unexpected parsing error")
+		return vol, "", fmt.Errorf("Pool delimiter not found")
 	}
 	slider := parent[(idx + 1):]
 	poolName := parent[:idx]
 
-	volumeType := slider
-	idx = strings.Index(slider, "zombie_")
-	if idx == 0 {
-		idx += len("zombie_")
-		volumeType = slider
-		slider = slider[idx:]
-	}
-
-	idxType := strings.Index(slider, "_")
-	if idxType == -1 {
-		return "", "", "", "", fmt.Errorf("Unexpected parsing error")
-	}
-
-	if idx == len("zombie_") {
-		idxType += idx
-	}
-	volumeType = volumeType[:idxType]
-
-	idx = strings.Index(slider, "_")
-	if idx == -1 {
-		return "", "", "", "", fmt.Errorf("Unexpected parsing error")
-	}
+	// Match image volumes and extract their various parts into a Volume struct.
+	// Looks for image volumes like:
+	// pool/zombie_image_9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb_ext4.block at readonly
+	// pool/image_9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb_xfs
+	reImage := regexp.MustCompile(`^((?:zombie_)?image)_([A-Za-z0-9]+)_([A-Za-z0-9]+)\.?(block)?@?([-\w]+)?$`)
+	if imageRes := reImage.FindStringSubmatch(slider); imageRes != nil {
+		vol.volType = VolumeType(imageRes[1])
+		vol.pool = poolName
+		vol.name = imageRes[2]
+		vol.config = map[string]string{
+			"block.filesystem": imageRes[3],
+		}
 
-	volumeName := slider
-	idx = strings.Index(volumeName, "_")
-	if idx == -1 {
-		return "", "", "", "", fmt.Errorf("Unexpected parsing error")
-	}
-	volumeName = volumeName[(idx + 1):]
+		if imageRes[4] == "block" {
+			vol.contentType = ContentTypeBlock
+		} else {
+			vol.contentType = ContentTypeFS
+		}
 
-	idx = strings.Index(volumeName, "@")
-	if idx == -1 {
-		return "", "", "", "", fmt.Errorf("Unexpected parsing error")
+		return vol, imageRes[5], nil
 	}
-	snapshotName := volumeName[(idx + 1):]
-	volumeName = volumeName[:idx]
 
-	return poolName, volumeType, volumeName, snapshotName, nil
+	return vol, "", fmt.Errorf("Unrecognised parent: %q", parent)
 }
 
 // parseClone splits a strings describing an RBD storage volume.

From 9c2396fc772fce1c52924bd5258e252f605ac78a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:07:36 +0100
Subject: [PATCH 14/24] lxd/storage/drivers/driver/ceph/utils: Adds
 cephVolumeTypeZombieImage constant

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

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index a90eee051d..5042d98824 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -22,6 +22,8 @@ import (
 	"github.com/lxc/lxd/shared/units"
 )
 
+const cephVolumeTypeZombieImage = VolumeType("zombie_image")
+
 // osdPoolExists checks whether a given OSD pool exists.
 func (d *ceph) osdPoolExists() bool {
 	_, err := shared.RunCommand(

From b8b091d740c8e7c1c15219f18be1f3fc63b40ac6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:08:00 +0100
Subject: [PATCH 15/24] lxd/storage/drivers/driver/ceph/utils: Updates
 rbdCreateVolume to accept string size

And parse it to bytes internally.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index 5042d98824..2fdd6bd1d0 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -70,6 +70,11 @@ func (d *ceph) osdDeletePool() error {
 // library and the kernel module are minimized. Otherwise random panics might
 // occur.
 func (d *ceph) rbdCreateVolume(vol Volume, size string) error {
+	sizeBytes, err := units.ParseByteSizeString(size)
+	if err != nil {
+		return err
+	}
+
 	cmd := []string{
 		"--id", d.config["ceph.user.name"],
 		"--image-feature", "layering,",
@@ -82,11 +87,11 @@ func (d *ceph) rbdCreateVolume(vol Volume, size string) error {
 	}
 
 	cmd = append(cmd,
-		"--size", size,
+		"--size", fmt.Sprintf("%dB", sizeBytes),
 		"create",
 		d.getRBDVolumeName(vol, "", false, false))
 
-	_, err := shared.RunCommand("rbd", cmd...)
+	_, err = shared.RunCommand("rbd", cmd...)
 	return err
 }
 

From fd9a7d9dfe3b7e75da09bee51d0f8a3431da9373 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:09:22 +0100
Subject: [PATCH 16/24] lxd/storage/drivers/driver/ceph/utils: Pass volume
 config in rbdMarkVolumeDeleted

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index 2fdd6bd1d0..33f795f50a 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -356,15 +356,19 @@ func (d *ceph) rbdListSnapshotClones(vol Volume, snapshotName string) ([]string,
 // creating a sparse copy of a container or when LXD updated an image and the
 // image still has dependent container clones.
 func (d *ceph) rbdMarkVolumeDeleted(vol Volume, newVolumeName string) error {
-	newVol := NewVolume(d, d.name, vol.volType, vol.contentType, newVolumeName, nil, nil)
+	// Ensure that new volume contains the config from the source volume to maintain filesystem suffix on
+	// new volume name generated in getRBDVolumeName.
+	newVol := NewVolume(d, d.name, vol.volType, vol.contentType, newVolumeName, vol.config, vol.poolConfig)
 	deletedName := d.getRBDVolumeName(newVol, "", true, true)
+
 	_, err := shared.RunCommand(
 		"rbd",
 		"--id", d.config["ceph.user.name"],
 		"--cluster", d.config["ceph.cluster_name"],
 		"mv",
 		d.getRBDVolumeName(vol, "", false, true),
-		deletedName)
+		deletedName,
+	)
 	if err != nil {
 		return err
 	}

From 625c8cfa86366f40e06c40ae78b1644d1fde99ce Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:09:42 +0100
Subject: [PATCH 17/24] lxd/storage/drivers/driver/ceph/utils: Pass volume
 config in rbdRenameVolume

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index 33f795f50a..1fc7b0819a 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -382,7 +382,9 @@ func (d *ceph) rbdMarkVolumeDeleted(vol Volume, newVolumeName string) error {
 // under its original name and the callers maps it under its new name the image
 // will be mapped twice. This will prevent it from being deleted.
 func (d *ceph) rbdRenameVolume(vol Volume, newVolumeName string) error {
-	newVol := NewVolume(d, d.name, vol.volType, vol.contentType, newVolumeName, nil, nil)
+	// Ensure that new volume contains the config from the source volume to maintain filesystem suffix on
+	// new volume name generated in getRBDVolumeName.
+	newVol := NewVolume(d, d.name, vol.volType, vol.contentType, newVolumeName, vol.config, vol.poolConfig)
 
 	_, err := shared.RunCommand(
 		"rbd",
@@ -390,7 +392,8 @@ func (d *ceph) rbdRenameVolume(vol Volume, newVolumeName string) error {
 		"--cluster", d.config["ceph.cluster_name"],
 		"mv",
 		d.getRBDVolumeName(vol, "", false, true),
-		d.getRBDVolumeName(newVol, "", false, true))
+		d.getRBDVolumeName(newVol, "", false, true),
+	)
 	if err != nil {
 		return err
 	}

From c73736ec551b31bd3f070ac667871dcc0c27ca62 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:10:07 +0100
Subject: [PATCH 18/24] lxd/storage/drivers/driver/ceph/utils: Replaces
 getRBDSize with volumeSize

Aligns with LVM behaviour.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index 1fc7b0819a..5326da6f97 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -529,24 +529,14 @@ func (d *ceph) rbdListVolumeSnapshots(vol Volume) ([]string, error) {
 	return snapshots, nil
 }
 
-// getRBDSize returns the size the RBD storage volume is supposed to be created with.
-func (d *ceph) getRBDSize(vol Volume) (string, error) {
-	size, ok := vol.config["size"]
-	if !ok {
-		size = vol.poolConfig["volume.size"]
+// volumeSize returns the size to use when creating new RBD volumes.
+func (d *ceph) volumeSize(vol Volume) string {
+	size := vol.ExpandedConfig("size")
+	if size == "" || size == "0" {
+		return defaultBlockSize
 	}
 
-	sz, err := units.ParseByteSizeString(size)
-	if err != nil {
-		return "", err
-	}
-
-	// Safety net: Set to default value.
-	if sz == 0 {
-		sz, _ = units.ParseByteSizeString(defaultBlockSize)
-	}
-
-	return fmt.Sprintf("%dB", sz), nil
+	return size
 }
 
 // getRBDFilesystem returns the filesystem the RBD storage volume is supposed to be created with.

From 3246555da3ae6f437b496ed628cbf3e1dc32b407 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:10:38 +0100
Subject: [PATCH 19/24] lxd/storage/drivers/driver/ceph/utils: Dont wrap lines

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index 5326da6f97..ce2e8209e2 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -658,8 +658,7 @@ func (d *ceph) deleteVolume(vol Volume) (int, error) {
 				return -1, err
 			}
 
-			if strings.HasPrefix(vol.name, "zombie_") ||
-				strings.HasPrefix(string(vol.volType), "zombie_") {
+			if strings.HasPrefix(vol.name, "zombie_") || strings.HasPrefix(string(vol.volType), "zombie_") {
 				return 1, nil
 			}
 

From ce3556c89698dac94bc96b773d549161f7763d80 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:10:59 +0100
Subject: [PATCH 20/24] lxd/storage/drivers/driver/ceph/utils: Updates usage of
 d.parseParent in deleteVolume

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index ce2e8209e2..cf1195e3d2 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -677,8 +677,7 @@ func (d *ceph) deleteVolume(vol Volume) (int, error) {
 
 		parent, err := d.rbdGetVolumeParent(vol)
 		if err == nil {
-			_, parentVolumeType, parentVolumeName,
-				parentSnapshotName, err := d.parseParent(parent)
+			parentVol, parentSnapshotName, err := d.parseParent(parent)
 			if err != nil {
 				return -1, err
 			}
@@ -695,13 +694,9 @@ func (d *ceph) deleteVolume(vol Volume) (int, error) {
 				return -1, err
 			}
 
-			parentVol := NewVolume(d, d.name, VolumeType(parentVolumeType), vol.contentType, parentVolumeName, nil, nil)
-
-			// Only delete the parent snapshot of the container if
-			// it is a zombie. If it is not we know that LXD is
-			// still using it.
-			if strings.HasPrefix(parentVolumeType, "zombie_") ||
-				strings.HasPrefix(parentSnapshotName, "zombie_") {
+			// Only delete the parent snapshot of the container if it is a zombie.
+			// If it is not we know that LXD is still using it.
+			if strings.HasPrefix(string(parentVol.volType), "zombie_") {
 				ret, err := d.deleteVolumeSnapshot(parentVol, parentSnapshotName)
 				if ret < 0 {
 					return -1, err

From 16b62ffd9ed5059922ed3724ec93babdce7219ef Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:11:26 +0100
Subject: [PATCH 21/24] lxd/storage/drivers/driver/ceph/utils: Updates RBD
 naming logic in getRBDVolumeName

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index cf1195e3d2..8c8e39d00a 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -1041,11 +1041,12 @@ func (d *ceph) getRBDVolumeName(vol Volume, snapName string, zombie bool, withPo
 	volumeType := string(vol.volType)
 	parentName, snapshotName, isSnapshot := shared.InstanceGetParentAndSnapshotName(vol.name)
 
-	if vol.volType == VolumeTypeImage {
+	// Only use filesystem suffix on filesystem type image volumes (for all content types).
+	if vol.volType == VolumeTypeImage || vol.volType == cephVolumeTypeZombieImage {
 		parentName = fmt.Sprintf("%s_%s", parentName, d.getRBDFilesystem(vol))
 	}
 
-	if (vol.volType == VolumeTypeVM || vol.volType == VolumeTypeImage) && vol.contentType == ContentTypeBlock {
+	if vol.contentType == ContentTypeBlock {
 		parentName = fmt.Sprintf("%s.block", parentName)
 	}
 

From 61770828dd7574c96a0363e18e2e31037e2ec125 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:11:52 +0100
Subject: [PATCH 22/24] lxd/storage/drivers/driver/ceph/utils: Adds tests for
 parseParent

Both for testing and documentation purposes.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_utils_test.go | 31 ++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/lxd/storage/drivers/driver_ceph_utils_test.go b/lxd/storage/drivers/driver_ceph_utils_test.go
index 7f558e3c73..59c7e5732b 100644
--- a/lxd/storage/drivers/driver_ceph_utils_test.go
+++ b/lxd/storage/drivers/driver_ceph_utils_test.go
@@ -1,6 +1,9 @@
 package drivers
 
-import "testing"
+import (
+	"fmt"
+	"testing"
+)
 
 func Test_ceph_getRBDVolumeName(t *testing.T) {
 	type args struct {
@@ -110,3 +113,29 @@ func Test_ceph_getRBDVolumeName(t *testing.T) {
 		})
 	}
 }
+func Example_ceph_parseParent() {
+	d := &ceph{}
+
+	parents := []string{
+		"pool/zombie_image_9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb_ext4.block at readonly",
+		"pool/zombie_image_9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb_ext4.block",
+		"pool/image_9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb_ext4.block at readonly",
+		"pool/zombie_image_9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb_ext4 at readonly",
+		"pool/zombie_image_9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb_ext4",
+		"pool/image_9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb_ext4 at readonly",
+		"pool/zombie_image_2cfc5a5567b8d74c0986f3d8a77a2a78e58fe22ea9abd2693112031f85afa1a1_xfs at zombie_snapshot_7f6d679b-ee25-419e-af49-bb805cb32088",
+	}
+
+	for _, parent := range parents {
+		vol, snapName, err := d.parseParent(parent)
+		fmt.Println(vol.pool, vol.volType, vol.name, vol.config["block.filesystem"], vol.contentType, snapName, err)
+	}
+
+	// Output: pool zombie_image 9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb ext4 block readonly <nil>
+	// pool zombie_image 9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb ext4 block  <nil>
+	// pool image 9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb ext4 block readonly <nil>
+	// pool zombie_image 9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb ext4 fs readonly <nil>
+	// pool zombie_image 9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb ext4 fs  <nil>
+	// pool image 9e90b7b9ccdd7a671a987fadcf07ab92363be57e7f056d18d42af452cdaf95bb ext4 fs readonly <nil>
+	// pool zombie_image 2cfc5a5567b8d74c0986f3d8a77a2a78e58fe22ea9abd2693112031f85afa1a1 xfs fs zombie_snapshot_7f6d679b-ee25-419e-af49-bb805cb32088 <nil>
+}

From eb9023127a94d949a32ddb5dfa5dc5c8f6b9c0f4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:13:55 +0100
Subject: [PATCH 23/24] lxd/storage/drivers/driver/ceph/volumes: Ensures
 CreateVolumeFromCopy always sizes new volume

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_volumes.go | 82 ++++++++++------------
 1 file changed, 39 insertions(+), 43 deletions(-)

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index a602524ac5..32a5d1eb1b 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -281,6 +281,37 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo
 	revert := revert.New()
 	defer revert.Fail()
 
+	// Function to run once the volume is created, which will regenerate the filesystem UUID (if needed),
+	// ensure permissions on mount path inside the volume are correct, and resize the volume to specified size.
+	postCreateTasks := func(v Volume) error {
+		// Map the RBD volume.
+		RBDDevPath, err := d.rbdMapVolume(v)
+		if err != nil {
+			return err
+		}
+		defer d.rbdUnmapVolume(v, true)
+
+		if vol.contentType == ContentTypeFS {
+			// Re-generate the UUID. Do this first as ensuring permissions and setting quota can
+			// rely on being able to mount the volume.
+			err = d.generateUUID(d.getRBDFilesystem(v), RBDDevPath)
+			if err != nil {
+				return err
+			}
+
+			// Mount the volume and ensure the permissions are set correctly inside the mounted volume.
+			err = v.MountTask(func(_ string, _ *operations.Operation) error {
+				return v.EnsureMountPath()
+			}, op)
+			if err != nil {
+				return err
+			}
+		}
+
+		// Resize the new volume and filesystem to the correct size.
+		return d.SetVolumeQuota(v, d.volumeSize(v), op)
+	}
+
 	// Retrieve snapshots on the source.
 	snapshots := []string{}
 	if !srcVol.IsSnapshot() && copySnapshots {
@@ -350,40 +381,21 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo
 			revert.Add(func() { d.DeleteVolume(vol, op) })
 		}
 
-		if vol.contentType == ContentTypeFS {
-			// Map the RBD volume.
-			RBDDevPath, err := d.rbdMapVolume(vol)
-			if err != nil {
-				return err
-			}
-			defer d.rbdUnmapVolume(vol, true)
-
-			// Re-generate the UUID.
-			err = d.generateUUID(d.getRBDFilesystem(vol), RBDDevPath)
-			if err != nil {
-				return err
-			}
-
-			// Mount the volume and ensure the permissions are set correctly inside the mounted volume.
-			err = vol.MountTask(func(_ string, _ *operations.Operation) error {
-				return vol.EnsureMountPath()
-			}, op)
-			if err != nil {
-				return err
-			}
-		}
-
 		// For VMs, also copy the filesystem volume.
 		if vol.IsVMBlock() {
 			srcFSVol := srcVol.NewVMBlockFilesystemVolume()
 			fsVol := vol.NewVMBlockFilesystemVolume()
-
 			err := d.CreateVolumeFromCopy(fsVol, srcFSVol, false, op)
 			if err != nil {
 				return err
 			}
 		}
 
+		err = postCreateTasks(vol)
+		if err != nil {
+			return err
+		}
+
 		revert.Success()
 		return nil
 	}
@@ -418,11 +430,7 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo
 
 		lastSnap = fmt.Sprintf("snapshot_%s", snap)
 		sourceVolumeName := d.getRBDVolumeName(srcVol, lastSnap, false, true)
-
-		err = d.copyWithSnapshots(
-			sourceVolumeName,
-			targetVolumeName,
-			prev)
+		err = d.copyWithSnapshots(sourceVolumeName, targetVolumeName, prev)
 		if err != nil {
 			return err
 		}
@@ -443,29 +451,17 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo
 	// Copy snapshot.
 	sourceVolumeName := d.getRBDVolumeName(srcVol, "", false, true)
 
-	err = d.copyWithSnapshots(
-		sourceVolumeName,
-		targetVolumeName,
-		lastSnap)
-	if err != nil {
-		return err
-	}
-
-	// Map the RBD volume.
-	RBDDevPath, err := d.rbdMapVolume(vol)
+	err = d.copyWithSnapshots(sourceVolumeName, targetVolumeName, lastSnap)
 	if err != nil {
 		return err
 	}
-	defer d.rbdUnmapVolume(vol, true)
 
-	// Re-generate the UUID.
-	err = d.generateUUID(d.getRBDFilesystem(vol), RBDDevPath)
+	err = postCreateTasks(vol)
 	if err != nil {
 		return err
 	}
 
 	revert.Success()
-
 	return nil
 }
 

From 6a1aa390e1dcb38e94c56cefb4dd5c1852bef21b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Apr 2020 15:14:16 +0100
Subject: [PATCH 24/24] lxd/storage/drivers/driver/ceph/volumes: If volume
 doesnt exist in DeleteVolume do nothing

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

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index 32a5d1eb1b..49d1b02557 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -580,6 +580,10 @@ func (d *ceph) RefreshVolume(vol Volume, srcVol Volume, srcSnapshots []Volume, o
 // DeleteVolume deletes a volume of the storage device. If any snapshots of the volume remain then
 // this function will return an error.
 func (d *ceph) DeleteVolume(vol Volume, op *operations.Operation) error {
+	if !d.HasVolume(vol) {
+		return nil
+	}
+
 	if vol.volType == VolumeTypeImage {
 		// Try to umount but don't fail.
 		d.UnmountVolume(vol, op)
@@ -630,10 +634,6 @@ func (d *ceph) DeleteVolume(vol Volume, op *operations.Operation) error {
 			return err
 		}
 	} else {
-		if !d.HasVolume(vol) {
-			return nil
-		}
-
 		_, err := d.UnmountVolume(vol, op)
 		if err != nil {
 			return err


More information about the lxc-devel mailing list