[lxc-devel] [lxd/master] Storage support updating pool config during create

tomponline on Github lxc-bot at linuxcontainers.org
Fri Dec 13 11:43:22 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 558 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191213/408c4fde/attachment-0001.bin>
-------------- next part --------------
From 71bfa4b863926163047770c318e7f500356b9721 Mon Sep 17 00:00:00 2001
From: Thomas Hipp <thomas.hipp at canonical.com>
Date: Thu, 12 Dec 2019 11:12:07 +0100
Subject: [PATCH 1/6] lxd/storage/drivers: Always pass Volume argument

This changes the drivers interface to always take a Volume argument
instead of separate volType and volName.

Signed-off-by: Thomas Hipp <thomas.hipp at canonical.com>
---
 lxd/storage/drivers/interface.go | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/lxd/storage/drivers/interface.go b/lxd/storage/drivers/interface.go
index 31b42d925c..6b4260f8f4 100644
--- a/lxd/storage/drivers/interface.go
+++ b/lxd/storage/drivers/interface.go
@@ -23,7 +23,7 @@ type Driver interface {
 	// Internal.
 	Config() map[string]string
 	Info() Info
-	HasVolume(volType VolumeType, volName string) bool
+	HasVolume(vol Volume) bool
 
 	// Pool.
 	Create() error
@@ -39,33 +39,33 @@ type Driver interface {
 	CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Operation) error
 	CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool, op *operations.Operation) error
 	RefreshVolume(vol Volume, srcVol Volume, srcSnapshots []Volume, op *operations.Operation) error
-	DeleteVolume(volType VolumeType, volName string, op *operations.Operation) error
-	RenameVolume(volType VolumeType, volName string, newName string, op *operations.Operation) error
+	DeleteVolume(vol Volume, op *operations.Operation) error
+	RenameVolume(vol Volume, newName string, op *operations.Operation) error
 	UpdateVolume(vol Volume, changedConfig map[string]string) error
-	GetVolumeUsage(volType VolumeType, volName string) (int64, error)
-	SetVolumeQuota(volType VolumeType, volName, size string, op *operations.Operation) error
-	GetVolumeDiskPath(volType VolumeType, volName string) (string, error)
+	GetVolumeUsage(vol Volume) (int64, error)
+	SetVolumeQuota(vol Volume, size string, op *operations.Operation) error
+	GetVolumeDiskPath(vol Volume) (string, error)
 
 	// MountVolume mounts a storage volume, returns true if we caused a new mount, false if
 	// already mounted.
-	MountVolume(volType VolumeType, volName string, op *operations.Operation) (bool, error)
+	MountVolume(vol Volume, op *operations.Operation) (bool, error)
 
 	// MountVolumeSnapshot mounts a storage volume snapshot as readonly, returns true if we
 	// caused a new mount, false if already mounted.
-	MountVolumeSnapshot(volType VolumeType, volName, snapshotName string, op *operations.Operation) (bool, error)
+	MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error)
 
 	// UnmountVolume unmounts a storage volume, returns true if unmounted, false if was not
 	// mounted.
-	UnmountVolume(volType VolumeType, volName string, op *operations.Operation) (bool, error)
+	UnmountVolume(vol Volume, op *operations.Operation) (bool, error)
 
 	// UnmountVolume unmounts a storage volume snapshot, returns true if unmounted, false if was
 	// not mounted.
-	UnmountVolumeSnapshot(VolumeType VolumeType, volName, snapshotName string, op *operations.Operation) (bool, error)
+	UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error)
 
-	CreateVolumeSnapshot(volType VolumeType, volName string, newSnapshotName string, op *operations.Operation) error
-	DeleteVolumeSnapshot(volType VolumeType, volName string, snapshotName string, op *operations.Operation) error
-	RenameVolumeSnapshot(volType VolumeType, volName string, snapshotName string, newSnapshotName string, op *operations.Operation) error
-	VolumeSnapshots(volType VolumeType, volName string, op *operations.Operation) ([]string, error)
+	CreateVolumeSnapshot(snapVol Volume, op *operations.Operation) error
+	DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) error
+	RenameVolumeSnapshot(snapVol Volume, newSnapshotName string, op *operations.Operation) error
+	VolumeSnapshots(vol Volume, op *operations.Operation) ([]string, error)
 	RestoreVolume(vol Volume, snapshotName string, op *operations.Operation) error
 
 	// Migration.

From 4b5994a5a59d2515a01c91e7c50debd7a2303fed Mon Sep 17 00:00:00 2001
From: Thomas Hipp <thomas.hipp at canonical.com>
Date: Thu, 12 Dec 2019 11:12:55 +0100
Subject: [PATCH 2/6] lxd/storage/drivers: Use new driver interface

Signed-off-by: Thomas Hipp <thomas.hipp at canonical.com>
---
 lxd/storage/drivers/driver_cephfs.go | 130 +++++++++++++++------------
 lxd/storage/drivers/driver_dir.go    | 101 +++++++++++----------
 lxd/storage/drivers/volume.go        |  12 +--
 3 files changed, 133 insertions(+), 110 deletions(-)

diff --git a/lxd/storage/drivers/driver_cephfs.go b/lxd/storage/drivers/driver_cephfs.go
index c2ede30ed0..c13bfbd1be 100644
--- a/lxd/storage/drivers/driver_cephfs.go
+++ b/lxd/storage/drivers/driver_cephfs.go
@@ -67,8 +67,8 @@ func (d *cephfs) Info() Info {
 	}
 }
 
-func (d *cephfs) HasVolume(volType VolumeType, volName string) bool {
-	if shared.PathExists(GetVolumeMountPath(d.name, volType, volName)) {
+func (d *cephfs) HasVolume(vol Volume) bool {
+	if shared.PathExists(vol.MountPath()) {
 		return true
 	}
 
@@ -289,7 +289,7 @@ func (d *cephfs) ValidateVolume(vol Volume, removeUnknownKeys bool) error {
 }
 
 // GetVolumeDiskPath returns the location of a root disk block device.
-func (d *cephfs) GetVolumeDiskPath(volType VolumeType, volName string) (string, error) {
+func (d *cephfs) GetVolumeDiskPath(vol Volume) (string, error) {
 	return "", ErrNotImplemented
 }
 
@@ -355,7 +355,10 @@ func (d *cephfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots b
 
 		// Remove any paths created if we are reverting.
 		for _, snapName := range revertSnaps {
-			d.DeleteVolumeSnapshot(vol.volType, vol.name, snapName, op)
+			fullSnapName := GetSnapshotVolumeName(vol.name, snapName)
+
+			snapVol := NewVolume(d, d.name, vol.volType, vol.contentType, fullSnapName, vol.config)
+			d.DeleteVolumeSnapshot(snapVol, op)
 		}
 
 		os.RemoveAll(volPath)
@@ -382,7 +385,7 @@ func (d *cephfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots b
 				}, op)
 
 				// Create the snapshot itself.
-				err = d.CreateVolumeSnapshot(vol.volType, vol.name, snapName, op)
+				err = d.CreateVolumeSnapshot(srcSnapshot, op)
 				if err != nil {
 					return err
 				}
@@ -393,7 +396,7 @@ func (d *cephfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots b
 		}
 
 		// Apply the volume quota if specified.
-		err = d.SetVolumeQuota(vol.volType, vol.name, vol.config["size"], op)
+		err = d.SetVolumeQuota(vol, vol.config["size"], op)
 		if err != nil {
 			return err
 		}
@@ -416,12 +419,12 @@ func (d *cephfs) RefreshVolume(vol Volume, srcVol Volume, srcSnapshots []Volume,
 	return ErrNotImplemented
 }
 
-func (d *cephfs) DeleteVolume(volType VolumeType, volName string, op *operations.Operation) error {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) DeleteVolume(vol Volume, op *operations.Operation) error {
+	if vol.volType != VolumeTypeCustom {
 		return fmt.Errorf("Volume type not supported")
 	}
 
-	snapshots, err := d.VolumeSnapshots(volType, volName, op)
+	snapshots, err := d.VolumeSnapshots(vol, op)
 	if err != nil {
 		return err
 	}
@@ -430,7 +433,7 @@ func (d *cephfs) DeleteVolume(volType VolumeType, volName string, op *operations
 		return fmt.Errorf("Cannot remove a volume that has snapshots")
 	}
 
-	volPath := GetVolumeMountPath(d.name, volType, volName)
+	volPath := GetVolumeMountPath(d.name, vol.volType, vol.name)
 
 	// If the volume doesn't exist, then nothing more to do.
 	if !shared.PathExists(volPath) {
@@ -445,7 +448,7 @@ func (d *cephfs) DeleteVolume(volType VolumeType, volName string, op *operations
 
 	// Although the volume snapshot directory should already be removed, lets remove it here
 	// to just in case the top-level directory is left.
-	snapshotDir := GetVolumeSnapshotDir(d.name, volType, volName)
+	snapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, vol.name)
 
 	err = os.RemoveAll(snapshotDir)
 	if err != nil {
@@ -455,15 +458,13 @@ func (d *cephfs) DeleteVolume(volType VolumeType, volName string, op *operations
 	return nil
 }
 
-func (d *cephfs) RenameVolume(volType VolumeType, volName string, newName string, op *operations.Operation) error {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) RenameVolume(vol Volume, newName string, op *operations.Operation) error {
+	if vol.volType != VolumeTypeCustom {
 		return fmt.Errorf("Volume type not supported")
 	}
 
-	vol := NewVolume(d, d.name, volType, ContentTypeFS, volName, nil)
-
 	// Create new snapshots directory.
-	snapshotDir := GetVolumeSnapshotDir(d.name, volType, newName)
+	snapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, newName)
 
 	err := os.MkdirAll(snapshotDir, 0711)
 	if err != nil {
@@ -495,10 +496,10 @@ func (d *cephfs) RenameVolume(volType VolumeType, volName string, newName string
 	}()
 
 	// Rename the snapshot directory first.
-	srcSnapshotDir := GetVolumeSnapshotDir(d.name, volType, volName)
+	srcSnapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, vol.name)
 
 	if shared.PathExists(srcSnapshotDir) {
-		targetSnapshotDir := GetVolumeSnapshotDir(d.name, volType, newName)
+		targetSnapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, newName)
 
 		err = os.Rename(srcSnapshotDir, targetSnapshotDir)
 		if err != nil {
@@ -517,16 +518,16 @@ func (d *cephfs) RenameVolume(volType VolumeType, volName string, newName string
 		return err
 	}
 
-	sourcePath := GetVolumeMountPath(d.name, volType, newName)
-	targetPath := GetVolumeMountPath(d.name, volType, newName)
+	sourcePath := GetVolumeMountPath(d.name, vol.volType, newName)
+	targetPath := GetVolumeMountPath(d.name, vol.volType, newName)
 
 	for _, snapshot := range snapshots {
 		// Figure out the snapshot paths.
 		_, snapName, _ := shared.InstanceGetParentAndSnapshotName(snapshot.name)
 		oldCephSnapPath := filepath.Join(sourcePath, ".snap", snapName)
 		newCephSnapPath := filepath.Join(targetPath, ".snap", snapName)
-		oldPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, snapName))
-		newPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(newName, snapName))
+		oldPath := GetVolumeMountPath(d.name, vol.volType, GetSnapshotVolumeName(vol.name, snapName))
+		newPath := GetVolumeMountPath(d.name, vol.volType, GetSnapshotVolumeName(newName, snapName))
 
 		// Update the symlink.
 		err = os.Symlink(newCephSnapPath, newPath)
@@ -541,8 +542,8 @@ func (d *cephfs) RenameVolume(volType VolumeType, volName string, newName string
 		})
 	}
 
-	oldPath := GetVolumeMountPath(d.name, volType, volName)
-	newPath := GetVolumeMountPath(d.name, volType, newName)
+	oldPath := GetVolumeMountPath(d.name, vol.volType, vol.name)
+	newPath := GetVolumeMountPath(d.name, vol.volType, newName)
 	err = os.Rename(oldPath, newPath)
 	if err != nil {
 		return err
@@ -563,11 +564,11 @@ func (d *cephfs) UpdateVolume(vol Volume, changedConfig map[string]string) error
 		return nil
 	}
 
-	return d.SetVolumeQuota(vol.volType, vol.name, value, nil)
+	return d.SetVolumeQuota(vol, value, nil)
 }
 
-func (d *cephfs) GetVolumeUsage(volType VolumeType, volName string) (int64, error) {
-	out, err := shared.RunCommand("getfattr", "-n", "ceph.quota.max_bytes", "--only-values", GetVolumeMountPath(d.name, volType, volName))
+func (d *cephfs) GetVolumeUsage(vol Volume) (int64, error) {
+	out, err := shared.RunCommand("getfattr", "-n", "ceph.quota.max_bytes", "--only-values", GetVolumeMountPath(d.name, vol.volType, vol.name))
 	if err != nil {
 		return -1, err
 	}
@@ -580,7 +581,7 @@ func (d *cephfs) GetVolumeUsage(volType VolumeType, volName string) (int64, erro
 	return size, nil
 }
 
-func (d *cephfs) SetVolumeQuota(volType VolumeType, volName, size string, op *operations.Operation) error {
+func (d *cephfs) SetVolumeQuota(vol Volume, size string, op *operations.Operation) error {
 	if size == "" || size == "0" {
 		size = d.config["volume.size"]
 	}
@@ -590,57 +591,59 @@ func (d *cephfs) SetVolumeQuota(volType VolumeType, volName, size string, op *op
 		return err
 	}
 
-	_, err = shared.RunCommand("setfattr", "-n", "ceph.quota.max_bytes", "-v", fmt.Sprintf("%d", sizeBytes), GetVolumeMountPath(d.name, volType, volName))
+	_, err = shared.RunCommand("setfattr", "-n", "ceph.quota.max_bytes", "-v", fmt.Sprintf("%d", sizeBytes), GetVolumeMountPath(d.name, vol.volType, vol.name))
 	return err
 }
 
-func (d *cephfs) MountVolume(volType VolumeType, volName string, op *operations.Operation) (bool, error) {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
+	if vol.volType != VolumeTypeCustom {
 		return false, fmt.Errorf("Volume type not supported")
 	}
 
 	return false, nil
 }
 
-func (d *cephfs) MountVolumeSnapshot(volType VolumeType, VolName, snapshotName string, op *operations.Operation) (bool, error) {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	if snapVol.volType != VolumeTypeCustom {
 		return false, fmt.Errorf("Volume type not supported")
 	}
 
 	return false, nil
 }
 
-func (d *cephfs) UnmountVolume(volType VolumeType, volName string, op *operations.Operation) (bool, error) {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) UnmountVolume(vol Volume, op *operations.Operation) (bool, error) {
+	if vol.volType != VolumeTypeCustom {
 		return false, fmt.Errorf("Volume type not supported")
 	}
 
 	return false, nil
 }
 
-func (d *cephfs) UnmountVolumeSnapshot(volType VolumeType, volName, snapshotName string, op *operations.Operation) (bool, error) {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	if snapVol.volType != VolumeTypeCustom {
 		return false, fmt.Errorf("Volume type not supported")
 	}
 
 	return false, nil
 }
 
-func (d *cephfs) CreateVolumeSnapshot(volType VolumeType, volName string, newSnapshotName string, op *operations.Operation) error {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) CreateVolumeSnapshot(snapVol Volume, op *operations.Operation) error {
+	if snapVol.volType != VolumeTypeCustom {
 		return fmt.Errorf("Volume type not supported")
 	}
 
+	parentName, snapName, _ := shared.InstanceGetParentAndSnapshotName(snapVol.name)
+
 	// Create the snapshot.
-	sourcePath := GetVolumeMountPath(d.name, volType, volName)
-	cephSnapPath := filepath.Join(sourcePath, ".snap", newSnapshotName)
+	sourcePath := GetVolumeMountPath(d.name, snapVol.volType, parentName)
+	cephSnapPath := filepath.Join(sourcePath, ".snap", snapName)
 
 	err := os.Mkdir(cephSnapPath, 0711)
 	if err != nil {
 		return err
 	}
 
-	targetPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, newSnapshotName))
+	targetPath := snapVol.MountPath()
 
 	err = os.MkdirAll(filepath.Dir(targetPath), 0711)
 	if err != nil {
@@ -655,14 +658,16 @@ func (d *cephfs) CreateVolumeSnapshot(volType VolumeType, volName string, newSna
 	return nil
 }
 
-func (d *cephfs) DeleteVolumeSnapshot(volType VolumeType, volName string, snapshotName string, op *operations.Operation) error {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) error {
+	if snapVol.volType != VolumeTypeCustom {
 		return fmt.Errorf("Volume type not supported")
 	}
 
+	parentName, snapName, _ := shared.InstanceGetParentAndSnapshotName(snapVol.name)
+
 	// Delete the snapshot itself.
-	sourcePath := GetVolumeMountPath(d.name, volType, volName)
-	cephSnapPath := filepath.Join(sourcePath, ".snap", snapshotName)
+	sourcePath := GetVolumeMountPath(d.name, snapVol.volType, parentName)
+	cephSnapPath := filepath.Join(sourcePath, ".snap", snapName)
 
 	err := os.Remove(cephSnapPath)
 	if err != nil {
@@ -670,7 +675,7 @@ func (d *cephfs) DeleteVolumeSnapshot(volType VolumeType, volName string, snapsh
 	}
 
 	// Remove the symlink.
-	snapPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, snapshotName))
+	snapPath := snapVol.MountPath()
 	err = os.Remove(snapPath)
 	if err != nil {
 		return err
@@ -679,13 +684,14 @@ func (d *cephfs) DeleteVolumeSnapshot(volType VolumeType, volName string, snapsh
 	return nil
 }
 
-func (d *cephfs) RenameVolumeSnapshot(volType VolumeType, volName string, snapshotName string, newSnapshotName string, op *operations.Operation) error {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) RenameVolumeSnapshot(snapVol Volume, newSnapshotName string, op *operations.Operation) error {
+	if snapVol.volType != VolumeTypeCustom {
 		return fmt.Errorf("Volume type not supported")
 	}
 
-	sourcePath := GetVolumeMountPath(d.name, volType, volName)
-	oldCephSnapPath := filepath.Join(sourcePath, ".snap", snapshotName)
+	parentName, snapName, _ := shared.InstanceGetParentAndSnapshotName(snapVol.name)
+	sourcePath := GetVolumeMountPath(d.name, snapVol.volType, parentName)
+	oldCephSnapPath := filepath.Join(sourcePath, ".snap", snapName)
 	newCephSnapPath := filepath.Join(sourcePath, ".snap", newSnapshotName)
 
 	err := os.Rename(oldCephSnapPath, newCephSnapPath)
@@ -694,13 +700,13 @@ func (d *cephfs) RenameVolumeSnapshot(volType VolumeType, volName string, snapsh
 	}
 
 	// Re-generate the snapshot symlink.
-	oldPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, snapshotName))
+	oldPath := snapVol.MountPath()
 	err = os.Remove(oldPath)
 	if err != nil {
 		return err
 	}
 
-	newPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, newSnapshotName))
+	newPath := GetVolumeMountPath(d.name, snapVol.volType, GetSnapshotVolumeName(parentName, newSnapshotName))
 	err = os.Symlink(newCephSnapPath, newPath)
 	if err != nil {
 		return err
@@ -709,12 +715,12 @@ func (d *cephfs) RenameVolumeSnapshot(volType VolumeType, volName string, snapsh
 	return nil
 }
 
-func (d *cephfs) VolumeSnapshots(volType VolumeType, volName string, op *operations.Operation) ([]string, error) {
-	if volType != VolumeTypeCustom {
+func (d *cephfs) VolumeSnapshots(vol Volume, op *operations.Operation) ([]string, error) {
+	if vol.volType != VolumeTypeCustom {
 		return nil, fmt.Errorf("Volume type not supported")
 	}
 
-	snapshotDir := GetVolumeSnapshotDir(d.name, volType, volName)
+	snapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, vol.name)
 	snapshots := []string{}
 
 	ents, err := ioutil.ReadDir(snapshotDir)
@@ -834,7 +840,10 @@ func (d *cephfs) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser,
 
 		// Remove any paths created if we are reverting.
 		for _, snapName := range revertSnaps {
-			d.DeleteVolumeSnapshot(vol.volType, vol.name, snapName, op)
+			fullSnapName := GetSnapshotVolumeName(vol.name, snapName)
+			snapVol := NewVolume(d, d.name, vol.volType, vol.contentType, fullSnapName, vol.config)
+
+			d.DeleteVolumeSnapshot(snapVol, op)
 		}
 
 		os.RemoveAll(volPath)
@@ -857,8 +866,11 @@ func (d *cephfs) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser,
 				return err
 			}
 
+			fullSnapName := GetSnapshotVolumeName(vol.name, snapName)
+			snapVol := NewVolume(d, d.name, vol.volType, vol.contentType, fullSnapName, vol.config)
+
 			// Create the snapshot itself.
-			err = d.CreateVolumeSnapshot(vol.volType, vol.name, snapName, op)
+			err = d.CreateVolumeSnapshot(snapVol, op)
 			if err != nil {
 				return err
 			}
@@ -868,7 +880,7 @@ func (d *cephfs) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser,
 		}
 
 		// Apply the volume quota if specified.
-		err = d.SetVolumeQuota(vol.volType, vol.name, vol.config["size"], op)
+		err = d.SetVolumeQuota(vol, vol.config["size"], op)
 		if err != nil {
 			return err
 		}
diff --git a/lxd/storage/drivers/driver_dir.go b/lxd/storage/drivers/driver_dir.go
index 4a07ea51ce..a879d16f7c 100644
--- a/lxd/storage/drivers/driver_dir.go
+++ b/lxd/storage/drivers/driver_dir.go
@@ -138,15 +138,15 @@ func (d *dir) GetResources() (*api.ResourcesStoragePool, error) {
 }
 
 // GetVolumeUsage returns the disk space used by the volume.
-func (d *dir) GetVolumeUsage(volType VolumeType, volName string) (int64, error) {
-	volPath := GetVolumeMountPath(d.name, volType, volName)
+func (d *dir) GetVolumeUsage(vol Volume) (int64, error) {
+	volPath := GetVolumeMountPath(d.name, vol.volType, vol.name)
 	ok, err := quota.Supported(volPath)
 	if err != nil || !ok {
 		return 0, nil
 	}
 
 	// Get the volume ID for the volume to access quota.
-	volID, err := d.getVolID(volType, volName)
+	volID, err := d.getVolID(vol.volType, vol.name)
 	if err != nil {
 		return -1, err
 	}
@@ -168,8 +168,8 @@ func (d *dir) ValidateVolume(vol Volume, removeUnknownKeys bool) error {
 }
 
 // HasVolume indicates whether a specific volume exists on the storage pool.
-func (d *dir) HasVolume(volType VolumeType, volName string) bool {
-	if shared.PathExists(GetVolumeMountPath(d.name, volType, volName)) {
+func (d *dir) HasVolume(vol Volume) bool {
+	if shared.PathExists(GetVolumeMountPath(d.name, vol.volType, vol.name)) {
 		return true
 	}
 
@@ -177,8 +177,8 @@ func (d *dir) HasVolume(volType VolumeType, volName string) bool {
 }
 
 // GetVolumeDiskPath returns the location of a disk volume.
-func (d *dir) GetVolumeDiskPath(volType VolumeType, volName string) (string, error) {
-	return filepath.Join(GetVolumeMountPath(d.name, volType, volName), "root.img"), nil
+func (d *dir) GetVolumeDiskPath(vol Volume) (string, error) {
+	return filepath.Join(GetVolumeMountPath(d.name, vol.volType, vol.name), "root.img"), nil
 }
 
 // setupInitialQuota enables quota on a new volume and sets with an initial quota from config.
@@ -246,7 +246,7 @@ func (d *dir) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
 	rootBlockPath := ""
 	if vol.contentType == ContentTypeBlock {
 		// We expect the filler to copy the VM image into this path.
-		rootBlockPath, err = d.GetVolumeDiskPath(vol.volType, vol.name)
+		rootBlockPath, err = d.GetVolumeDiskPath(vol)
 		if err != nil {
 			return err
 		}
@@ -389,7 +389,9 @@ func (d *dir) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser, vol
 
 		// Remove any paths created if we are reverting.
 		for _, snapName := range revertSnaps {
-			d.DeleteVolumeSnapshot(vol.volType, vol.name, snapName, op)
+			fullSnapName := GetSnapshotVolumeName(vol.name, snapName)
+			snapVol := NewVolume(d, d.name, vol.volType, vol.contentType, fullSnapName, vol.config)
+			d.DeleteVolumeSnapshot(snapVol, op)
 		}
 
 		os.RemoveAll(volPath)
@@ -424,7 +426,10 @@ func (d *dir) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser, vol
 			}
 
 			// Create the snapshot itself.
-			err = d.CreateVolumeSnapshot(vol.volType, vol.name, snapName, op)
+			fullSnapshotName := GetSnapshotVolumeName(vol.name, snapName)
+			snapshotVol := NewVolume(d, d.name, vol.volType, vol.contentType, fullSnapshotName, vol.config)
+
+			err = d.CreateVolumeSnapshot(snapshotVol, op)
 			if err != nil {
 				return err
 			}
@@ -531,7 +536,9 @@ func (d *dir) copyVolume(vol Volume, srcVol Volume, srcSnapshots []Volume, op *o
 
 		// Remove any paths created if we are reverting.
 		for _, snapName := range revertSnaps {
-			d.DeleteVolumeSnapshot(vol.volType, vol.name, snapName, op)
+			fullSnapName := GetSnapshotVolumeName(vol.name, snapName)
+			snapVol := NewVolume(d, d.name, vol.volType, vol.contentType, fullSnapName, vol.config)
+			d.DeleteVolumeSnapshot(snapVol, op)
 		}
 
 		os.RemoveAll(volPath)
@@ -551,8 +558,11 @@ func (d *dir) copyVolume(vol Volume, srcVol Volume, srcSnapshots []Volume, op *o
 					return err
 				}, op)
 
+				fullSnapName := GetSnapshotVolumeName(vol.name, snapName)
+				snapVol := NewVolume(d, d.name, vol.volType, vol.contentType, fullSnapName, vol.config)
+
 				// Create the snapshot itself.
-				err = d.CreateVolumeSnapshot(vol.volType, vol.name, snapName, op)
+				err = d.CreateVolumeSnapshot(snapVol, op)
 				if err != nil {
 					return err
 				}
@@ -589,8 +599,8 @@ func (d *dir) copyVolume(vol Volume, srcVol Volume, srcSnapshots []Volume, op *o
 }
 
 // VolumeSnapshots returns a list of snapshots for the volume.
-func (d *dir) VolumeSnapshots(volType VolumeType, volName string, op *operations.Operation) ([]string, error) {
-	snapshotDir := GetVolumeSnapshotDir(d.name, volType, volName)
+func (d *dir) VolumeSnapshots(vol Volume, op *operations.Operation) ([]string, error) {
+	snapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, vol.name)
 	snapshots := []string{}
 
 	ents, err := ioutil.ReadDir(snapshotDir)
@@ -642,11 +652,9 @@ func (d *dir) UpdateVolume(vol Volume, changedConfig map[string]string) error {
 }
 
 // RenameVolume renames a volume and its snapshots.
-func (d *dir) RenameVolume(volType VolumeType, volName string, newVolName string, op *operations.Operation) error {
-	vol := NewVolume(d, d.name, volType, ContentTypeFS, volName, nil)
-
+func (d *dir) RenameVolume(vol Volume, newVolName string, op *operations.Operation) error {
 	// Create new snapshots directory.
-	snapshotDir := GetVolumeSnapshotDir(d.name, volType, newVolName)
+	snapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, newVolName)
 
 	err := os.MkdirAll(snapshotDir, 0711)
 	if err != nil {
@@ -694,8 +702,8 @@ func (d *dir) RenameVolume(volType VolumeType, volName string, newVolName string
 		})
 	}
 
-	oldPath := GetVolumeMountPath(d.name, volType, volName)
-	newPath := GetVolumeMountPath(d.name, volType, newVolName)
+	oldPath := GetVolumeMountPath(d.name, vol.volType, vol.name)
+	newPath := GetVolumeMountPath(d.name, vol.volType, newVolName)
 	err = os.Rename(oldPath, newPath)
 	if err != nil {
 		return err
@@ -707,7 +715,7 @@ func (d *dir) RenameVolume(volType VolumeType, volName string, newVolName string
 	})
 
 	// Remove old snapshots directory.
-	oldSnapshotDir := GetVolumeSnapshotDir(d.name, volType, volName)
+	oldSnapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, vol.name)
 
 	err = os.RemoveAll(oldSnapshotDir)
 	if err != nil {
@@ -739,8 +747,8 @@ func (d *dir) RestoreVolume(vol Volume, snapshotName string, op *operations.Oper
 
 // DeleteVolume deletes a volume of the storage device. If any snapshots of the volume remain then
 // this function will return an error.
-func (d *dir) DeleteVolume(volType VolumeType, volName string, op *operations.Operation) error {
-	snapshots, err := d.VolumeSnapshots(volType, volName, op)
+func (d *dir) DeleteVolume(vol Volume, op *operations.Operation) error {
+	snapshots, err := d.VolumeSnapshots(vol, op)
 	if err != nil {
 		return err
 	}
@@ -749,7 +757,7 @@ func (d *dir) DeleteVolume(volType VolumeType, volName string, op *operations.Op
 		return fmt.Errorf("Cannot remove a volume that has snapshots")
 	}
 
-	volPath := GetVolumeMountPath(d.name, volType, volName)
+	volPath := GetVolumeMountPath(d.name, vol.volType, vol.name)
 
 	// If the volume doesn't exist, then nothing more to do.
 	if !shared.PathExists(volPath) {
@@ -757,7 +765,7 @@ func (d *dir) DeleteVolume(volType VolumeType, volName string, op *operations.Op
 	}
 
 	// Get the volume ID for the volume, which is used to remove project quota.
-	volID, err := d.getVolID(volType, volName)
+	volID, err := d.getVolID(vol.volType, vol.name)
 	if err != nil {
 		return err
 	}
@@ -776,7 +784,7 @@ func (d *dir) DeleteVolume(volType VolumeType, volName string, op *operations.Op
 
 	// Although the volume snapshot directory should already be removed, lets remove it here
 	// to just in case the top-level directory is left.
-	err = deleteParentSnapshotDirIfEmpty(d.name, volType, volName)
+	err = deleteParentSnapshotDirIfEmpty(d.name, vol.volType, vol.name)
 	if err != nil {
 		return err
 	}
@@ -786,32 +794,32 @@ func (d *dir) DeleteVolume(volType VolumeType, volName string, op *operations.Op
 
 // MountVolume simulates mounting a volume. As dir driver doesn't have volumes to mount it returns
 // false indicating that there is no need to issue an unmount.
-func (d *dir) MountVolume(volType VolumeType, volName string, op *operations.Operation) (bool, error) {
+func (d *dir) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
 	return false, nil
 }
 
 // MountVolumeSnapshot sets up a read-only mount on top of the snapshot to avoid accidental modifications.
-func (d *dir) MountVolumeSnapshot(volType VolumeType, volName, snapshotName string, op *operations.Operation) (bool, error) {
-	snapPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, snapshotName))
+func (d *dir) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	snapPath := snapVol.MountPath()
 	return mountReadOnly(snapPath, snapPath)
 }
 
 // UnmountVolume simulates unmounting a volume. As dir driver doesn't have volumes to unmount it
 // returns false indicating the volume was already unmounted.
-func (d *dir) UnmountVolume(volType VolumeType, volName string, op *operations.Operation) (bool, error) {
+func (d *dir) UnmountVolume(vol Volume, op *operations.Operation) (bool, error) {
 	return false, nil
 }
 
 // UnmountVolumeSnapshot removes the read-only mount placed on top of a snapshot.
-func (d *dir) UnmountVolumeSnapshot(volType VolumeType, volName, snapshotName string, op *operations.Operation) (bool, error) {
-	snapPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, snapshotName))
+func (d *dir) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	snapPath := snapVol.MountPath()
 	return forceUnmount(snapPath)
 }
 
 // SetVolumeQuota sets the quota on the volume.
-func (d *dir) SetVolumeQuota(volType VolumeType, volName, size string, op *operations.Operation) error {
-	volPath := GetVolumeMountPath(d.name, volType, volName)
-	volID, err := d.getVolID(volType, volName)
+func (d *dir) SetVolumeQuota(vol Volume, size string, op *operations.Operation) error {
+	volPath := GetVolumeMountPath(d.name, vol.volType, vol.name)
+	volID, err := d.getVolID(vol.volType, vol.name)
 	if err != nil {
 		return err
 	}
@@ -903,10 +911,9 @@ func (d *dir) deleteQuota(path string, volID int64) error {
 }
 
 // CreateVolumeSnapshot creates a snapshot of a volume.
-func (d *dir) CreateVolumeSnapshot(volType VolumeType, volName string, newSnapshotName string, op *operations.Operation) error {
-	srcPath := GetVolumeMountPath(d.name, volType, volName)
-	fullSnapName := GetSnapshotVolumeName(volName, newSnapshotName)
-	snapVol := NewVolume(d, d.name, volType, ContentTypeFS, fullSnapName, nil)
+func (d *dir) CreateVolumeSnapshot(snapVol Volume, op *operations.Operation) error {
+	parentName, _, _ := shared.InstanceGetParentAndSnapshotName(snapVol.name)
+	srcPath := GetVolumeMountPath(d.name, snapVol.volType, parentName)
 	snapPath := snapVol.MountPath()
 
 	// Create snapshot directory.
@@ -936,8 +943,8 @@ func (d *dir) CreateVolumeSnapshot(volType VolumeType, volName string, newSnapsh
 
 // DeleteVolumeSnapshot removes a snapshot from the storage device. The volName and snapshotName
 // must be bare names and should not be in the format "volume/snapshot".
-func (d *dir) DeleteVolumeSnapshot(volType VolumeType, volName string, snapshotName string, op *operations.Operation) error {
-	snapPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, snapshotName))
+func (d *dir) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) error {
+	snapPath := snapVol.MountPath()
 
 	// Remove the snapshot from the storage device.
 	err := os.RemoveAll(snapPath)
@@ -945,8 +952,10 @@ func (d *dir) DeleteVolumeSnapshot(volType VolumeType, volName string, snapshotN
 		return err
 	}
 
+	parentName, _, _ := shared.InstanceGetParentAndSnapshotName(snapVol.name)
+
 	// Remove the parent snapshot directory if this is the last snapshot being removed.
-	err = deleteParentSnapshotDirIfEmpty(d.name, volType, volName)
+	err = deleteParentSnapshotDirIfEmpty(d.name, snapVol.volType, parentName)
 	if err != nil {
 		return err
 	}
@@ -955,9 +964,11 @@ func (d *dir) DeleteVolumeSnapshot(volType VolumeType, volName string, snapshotN
 }
 
 // RenameVolumeSnapshot renames a volume snapshot.
-func (d *dir) RenameVolumeSnapshot(volType VolumeType, volName string, snapshotName string, newSnapshotName string, op *operations.Operation) error {
-	oldPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, snapshotName))
-	newPath := GetVolumeMountPath(d.name, volType, GetSnapshotVolumeName(volName, newSnapshotName))
+func (d *dir) RenameVolumeSnapshot(snapVol Volume, newSnapshotName string, op *operations.Operation) error {
+	parentName, _, _ := shared.InstanceGetParentAndSnapshotName(snapVol.name)
+	oldPath := snapVol.MountPath()
+	newPath := GetVolumeMountPath(d.name, snapVol.volType, GetSnapshotVolumeName(parentName, newSnapshotName))
+
 	err := os.Rename(oldPath, newPath)
 	if err != nil {
 		return err
diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go
index 1f0df96278..8cddd9fe8c 100644
--- a/lxd/storage/drivers/volume.go
+++ b/lxd/storage/drivers/volume.go
@@ -106,14 +106,14 @@ func (v Volume) CreateMountPath() error {
 // MountTask runs the supplied task after mounting the volume if needed. If the volume was mounted
 // for this then it is unmounted when the task finishes.
 func (v Volume) MountTask(task func(mountPath string, op *operations.Operation) error, op *operations.Operation) error {
-	parentName, snapName, isSnap := shared.InstanceGetParentAndSnapshotName(v.name)
+	isSnap := v.IsSnapshot()
 
 	// If the volume is a snapshot then call the snapshot specific mount/unmount functions as
 	// these will mount the snapshot read only.
 	if isSnap {
 		unlock := locking.Lock(v.pool, string(v.volType), v.name)
 
-		ourMount, err := v.driver.MountVolumeSnapshot(v.volType, parentName, snapName, op)
+		ourMount, err := v.driver.MountVolumeSnapshot(v, op)
 		if err != nil {
 			unlock()
 			return err
@@ -124,14 +124,14 @@ func (v Volume) MountTask(task func(mountPath string, op *operations.Operation)
 		if ourMount {
 			defer func() {
 				unlock := locking.Lock(v.pool, string(v.volType), v.name)
-				v.driver.UnmountVolumeSnapshot(v.volType, parentName, snapName, op)
+				v.driver.UnmountVolumeSnapshot(v, op)
 				unlock()
 			}()
 		}
 	} else {
 		unlock := locking.Lock(v.pool, string(v.volType), v.name)
 
-		ourMount, err := v.driver.MountVolume(v.volType, v.name, op)
+		ourMount, err := v.driver.MountVolume(v, op)
 		if err != nil {
 			unlock()
 			return err
@@ -142,7 +142,7 @@ func (v Volume) MountTask(task func(mountPath string, op *operations.Operation)
 		if ourMount {
 			defer func() {
 				unlock := locking.Lock(v.pool, string(v.volType), v.name)
-				v.driver.UnmountVolume(v.volType, v.name, op)
+				v.driver.UnmountVolume(v, op)
 				unlock()
 			}()
 		}
@@ -157,7 +157,7 @@ func (v Volume) Snapshots(op *operations.Operation) ([]Volume, error) {
 		return nil, fmt.Errorf("Volume is a snapshot")
 	}
 
-	snapshots, err := v.driver.VolumeSnapshots(v.volType, v.name, op)
+	snapshots, err := v.driver.VolumeSnapshots(v, op)
 	if err != nil {
 		return nil, err
 	}

From 5074301020cf2f8a8c5874b78c64837482af8de9 Mon Sep 17 00:00:00 2001
From: Thomas Hipp <thomas.hipp at canonical.com>
Date: Thu, 12 Dec 2019 12:09:21 +0100
Subject: [PATCH 3/6] lxd/storage: Always pass Volume to drivers

This updates the calls to the driver functions in that it now always
passes a Volume.

Signed-off-by: Thomas Hipp <thomas.hipp at canonical.com>
---
 lxd/storage/backend_lxd.go | 230 ++++++++++++++++++++++++++++---------
 1 file changed, 177 insertions(+), 53 deletions(-)

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index f4cd4e8650..a44ddb0f13 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -503,22 +503,19 @@ func (b *lxdBackend) CreateInstanceFromCopy(inst instance.Instance, src instance
 
 	contentType := InstanceContentType(inst)
 
-	if b.driver.HasVolume(volType, project.Prefix(inst.Project(), inst.Name())) {
-		return fmt.Errorf("Cannot create volume, already exists on target")
-	}
-
 	// Get the root disk device config.
 	_, rootDiskConf, err := shared.GetRootDiskDevice(inst.ExpandedDevices().CloneNative())
 	if err != nil {
 		return err
 	}
 
-	// Get the volume name on storage.
 	volStorageName := project.Prefix(inst.Project(), inst.Name())
-
-	// Initialise a new volume containing the root disk config supplied in the new instance.
 	vol := b.newVolume(volType, contentType, volStorageName, rootDiskConf)
 
+	if b.driver.HasVolume(vol) {
+		return fmt.Errorf("Cannot create volume, already exists on target")
+	}
+
 	// Get the src volume name on storage.
 	srcVolStorageName := project.Prefix(src.Project(), src.Name())
 
@@ -879,13 +876,6 @@ func (b *lxdBackend) CreateInstanceFromMigration(inst instance.Instance, conn io
 
 	contentType := InstanceContentType(inst)
 
-	volExists := b.driver.HasVolume(volType, project.Prefix(inst.Project(), inst.Name()))
-	if args.Refresh && !volExists {
-		return fmt.Errorf("Cannot refresh volume, doesn't exist on target")
-	} else if !args.Refresh && volExists {
-		return fmt.Errorf("Cannot create volume, already exists on target")
-	}
-
 	// Find the root device config for instance.
 	_, rootDiskConf, err := shared.GetRootDiskDevice(inst.ExpandedDevices().CloneNative())
 	if err != nil {
@@ -901,6 +891,13 @@ func (b *lxdBackend) CreateInstanceFromMigration(inst instance.Instance, conn io
 
 	vol := b.newVolume(volType, contentType, volStorageName, args.Config)
 
+	volExists := b.driver.HasVolume(vol)
+	if args.Refresh && !volExists {
+		return fmt.Errorf("Cannot refresh volume, doesn't exist on target")
+	} else if !args.Refresh && volExists {
+		return fmt.Errorf("Cannot create volume, already exists on target")
+	}
+
 	var preFiller drivers.VolumeFiller
 
 	revert := true
@@ -1041,7 +1038,12 @@ func (b *lxdBackend) RenameInstance(inst instance.Instance, newName string, op *
 	// Rename the volume and its snapshots on the storage device.
 	volStorageName := project.Prefix(inst.Project(), inst.Name())
 	newVolStorageName := project.Prefix(inst.Project(), newName)
-	err = b.driver.RenameVolume(volType, volStorageName, newVolStorageName, op)
+	contentType := InstanceContentType(inst)
+
+	// There's no need to pass config as it's not needed when renaming a volume.
+	vol := b.newVolume(volType, contentType, volStorageName, nil)
+
+	err = b.driver.RenameVolume(vol, newVolStorageName, op)
 	if err != nil {
 		return err
 	}
@@ -1108,11 +1110,15 @@ func (b *lxdBackend) DeleteInstance(inst instance.Instance, op *operations.Opera
 
 	// Get the volume name on storage.
 	volStorageName := project.Prefix(inst.Project(), inst.Name())
+	contentType := InstanceContentType(inst)
+
+	// There's no need to pass config as it's not needed when deleting a volume.
+	vol := b.newVolume(volType, contentType, volStorageName, nil)
 
 	// Delete the volume from the storage device. Must come after snapshots are removed.
 	// Must come before DB StoragePoolVolumeDelete so that the volume ID is still available.
 	logger.Debug("Deleting instance volume", log.Ctx{"volName": volStorageName})
-	err = b.driver.DeleteVolume(volType, volStorageName, op)
+	err = b.driver.DeleteVolume(vol, op)
 	if err != nil {
 		return err
 	}
@@ -1213,7 +1219,13 @@ func (b *lxdBackend) GetInstanceUsage(inst instance.Instance) (int64, error) {
 	defer logger.Debug("GetInstanceUsage finished")
 
 	if inst.Type() == instancetype.Container {
-		return b.driver.GetVolumeUsage(drivers.VolumeTypeContainer, inst.Name())
+		contentType := InstanceContentType(inst)
+
+		// There's no need to pass config as it's not needed when retrieving
+		// the volume usage.
+		vol := b.newVolume(drivers.VolumeTypeContainer, contentType, inst.Name(), nil)
+
+		return b.driver.GetVolumeUsage(vol)
 	}
 
 	return -1, ErrNotImplemented
@@ -1237,10 +1249,14 @@ func (b *lxdBackend) SetInstanceQuota(inst instance.Instance, size string, op *o
 		return err
 	}
 
-	// Get the volume name on storage.
+	contentVolume := InstanceContentType(inst)
 	volStorageName := project.Prefix(inst.Project(), inst.Name())
 
-	return b.driver.SetVolumeQuota(volType, volStorageName, size, op)
+	// Get the volume.
+	// There's no need to pass config as it's not needed when setting quotas.
+	vol := b.newVolume(volType, contentVolume, volStorageName, nil)
+
+	return b.driver.SetVolumeQuota(vol, size, op)
 }
 
 // MountInstance mounts the instance's root volume.
@@ -1255,10 +1271,19 @@ func (b *lxdBackend) MountInstance(inst instance.Instance, op *operations.Operat
 		return false, err
 	}
 
-	// Get the volume name on storage.
+	// Get the root disk device config.
+	_, rootDiskConf, err := shared.GetRootDiskDevice(inst.ExpandedDevices().CloneNative())
+	if err != nil {
+		return false, err
+	}
+
+	contentType := InstanceContentType(inst)
 	volStorageName := project.Prefix(inst.Project(), inst.Name())
 
-	return b.driver.MountVolume(volType, volStorageName, op)
+	// Get the volume.
+	vol := b.newVolume(volType, contentType, volStorageName, rootDiskConf)
+
+	return b.driver.MountVolume(vol, op)
 }
 
 // UnmountInstance unmounts the instance's root volume.
@@ -1273,10 +1298,19 @@ func (b *lxdBackend) UnmountInstance(inst instance.Instance, op *operations.Oper
 		return false, err
 	}
 
-	// Get the volume name on storage.
+	// Get the root disk device config.
+	_, rootDiskConf, err := shared.GetRootDiskDevice(inst.ExpandedDevices().CloneNative())
+	if err != nil {
+		return false, err
+	}
+
+	contentType := InstanceContentType(inst)
 	volStorageName := project.Prefix(inst.Project(), inst.Name())
 
-	return b.driver.UnmountVolume(volType, volStorageName, op)
+	// Get the volume.
+	vol := b.newVolume(volType, contentType, volStorageName, rootDiskConf)
+
+	return b.driver.UnmountVolume(vol, op)
 }
 
 // GetInstanceDisk returns the location of the disk.
@@ -1291,11 +1325,16 @@ func (b *lxdBackend) GetInstanceDisk(inst instance.Instance) (string, error) {
 		return "", err
 	}
 
-	// Get the volume name on storage.
+	contentType := InstanceContentType(inst)
 	volStorageName := project.Prefix(inst.Project(), inst.Name())
 
+	// Get the volume.
+	// There's no need to pass config as it's not needed when getting the
+	// location of the disk block device.
+	vol := b.newVolume(volType, contentType, volStorageName, nil)
+
 	// Get the location of the disk block device.
-	diskPath, err := b.driver.GetVolumeDiskPath(volType, volStorageName)
+	diskPath, err := b.driver.GetVolumeDiskPath(vol)
 	if err != nil {
 		return "", err
 	}
@@ -1336,15 +1375,21 @@ func (b *lxdBackend) CreateInstanceSnapshot(inst instance.Instance, src instance
 		defer src.Unfreeze()
 	}
 
-	parentName, snapName, isSnap := shared.InstanceGetParentAndSnapshotName(inst.Name())
+	isSnap := inst.IsSnapshot()
+
 	if !isSnap {
 		return fmt.Errorf("Volume name must be a snapshot")
 	}
 
-	// Get the volume name on storage.
-	volStorageName := project.Prefix(inst.Project(), parentName)
+	contentType := InstanceContentType(inst)
+	volStorageName := project.Prefix(inst.Project(), inst.Name())
+
+	// Get the volume.
+	// There's no need to pass config as it's not needed when creating volume
+	// snapshots.
+	vol := b.newVolume(volType, contentType, volStorageName, nil)
 
-	err = b.driver.CreateVolumeSnapshot(volType, volStorageName, snapName, op)
+	err = b.driver.CreateVolumeSnapshot(vol, op)
 	if err != nil {
 		return err
 	}
@@ -1387,9 +1432,15 @@ func (b *lxdBackend) RenameInstanceSnapshot(inst instance.Instance, newName stri
 		return fmt.Errorf("Volume name must be a snapshot")
 	}
 
+	contentType := InstanceContentType(inst)
+	volStorageName := project.Prefix(inst.Project(), inst.Name())
+
 	// Rename storage volume snapshot.
-	volStorageName := project.Prefix(inst.Project(), parentName)
-	err = b.driver.RenameVolumeSnapshot(volType, volStorageName, oldSnapshotName, newName, op)
+	// There's no need to pass config as it's not needed when renaming a volume
+	// snapshot.
+	vol := b.newVolume(volType, contentType, volStorageName, nil)
+
+	err = b.driver.RenameVolumeSnapshot(vol, newName, op)
 	if err != nil {
 		return err
 	}
@@ -1398,7 +1449,11 @@ func (b *lxdBackend) RenameInstanceSnapshot(inst instance.Instance, newName stri
 	err = b.state.Cluster.StoragePoolVolumeRename(inst.Project(), inst.Name(), newVolName, volDBType, b.ID())
 	if err != nil {
 		// Revert rename.
-		b.driver.RenameVolumeSnapshot(drivers.VolumeTypeCustom, parentName, newName, oldSnapshotName, op)
+		// There's no need to pass config as it's not needed when renaming a
+		// volume snapshot.
+		vol := b.newVolume(volType, contentType, newName, nil)
+
+		b.driver.RenameVolumeSnapshot(vol, oldSnapshotName, op)
 		return err
 	}
 
@@ -1427,13 +1482,22 @@ func (b *lxdBackend) DeleteInstanceSnapshot(inst instance.Instance, op *operatio
 		return err
 	}
 
+	contentType := InstanceContentType(inst)
+
 	// Get the parent volume name on storage.
 	parentStorageName := project.Prefix(inst.Project(), parentName)
 
 	// Delete the snapshot from the storage device.
 	// Must come before DB StoragePoolVolumeDelete so that the volume ID is still available.
 	logger.Debug("Deleting instance snapshot volume", log.Ctx{"volName": parentStorageName, "snapshotName": snapName})
-	err = b.driver.DeleteVolumeSnapshot(volType, parentStorageName, snapName, op)
+
+	snapVolName := drivers.GetSnapshotVolumeName(parentStorageName, snapName)
+
+	// There's no need to pass config as it's not needed when deleting a volume
+	// snapshot.
+	vol := b.newVolume(volType, contentType, snapVolName, nil)
+
+	err = b.driver.DeleteVolumeSnapshot(vol, op)
 	if err != nil {
 		return err
 	}
@@ -1525,13 +1589,21 @@ func (b *lxdBackend) MountInstanceSnapshot(inst instance.Instance, op *operation
 		return false, err
 	}
 
+	contentType := InstanceContentType(inst)
+
+	// Get the root disk device config.
+	_, rootDiskConf, err := shared.GetRootDiskDevice(inst.ExpandedDevices().CloneNative())
+	if err != nil {
+		return false, err
+	}
+
 	// Get the parent and snapshot name.
-	parentName, snapName, _ := shared.InstanceGetParentAndSnapshotName(inst.Name())
+	volStorageName := project.Prefix(inst.Project(), inst.Name())
 
-	// Get the volume name on storage.
-	volStorageName := project.Prefix(inst.Project(), parentName)
+	// Get the volume.
+	vol := b.newVolume(volType, contentType, volStorageName, rootDiskConf)
 
-	return b.driver.MountVolumeSnapshot(volType, volStorageName, snapName, op)
+	return b.driver.MountVolumeSnapshot(vol, op)
 }
 
 // UnmountInstanceSnapshot unmounts an instance snapshot.
@@ -1550,13 +1622,21 @@ func (b *lxdBackend) UnmountInstanceSnapshot(inst instance.Instance, op *operati
 		return false, err
 	}
 
+	// Get the root disk device config.
+	_, rootDiskConf, err := shared.GetRootDiskDevice(inst.ExpandedDevices().CloneNative())
+	if err != nil {
+		return false, err
+	}
+
+	contentType := InstanceContentType(inst)
+
 	// Get the parent and snapshot name.
-	parentName, snapName, _ := shared.InstanceGetParentAndSnapshotName(inst.Name())
+	volStorageName := project.Prefix(inst.Project(), inst.Name())
 
-	// Get the volume name on storage.
-	volStorageName := project.Prefix(inst.Project(), parentName)
+	// Get the volume.
+	vol := b.newVolume(volType, contentType, volStorageName, rootDiskConf)
 
-	return b.driver.UnmountVolumeSnapshot(volType, volStorageName, snapName, op)
+	return b.driver.UnmountVolumeSnapshot(vol, op)
 }
 
 // EnsureImage creates an optimized volume of the image if supported by the storage pool driver and
@@ -1575,8 +1655,12 @@ func (b *lxdBackend) EnsureImage(fingerprint string, op *operations.Operation) e
 	unlock := locking.Lock(b.name, string(drivers.VolumeTypeImage), fingerprint)
 	defer unlock()
 
+	// There's no need to pass the content type or config. Both are not needed
+	// when checking the existence of volumes.
+	vol := b.newVolume(drivers.VolumeTypeImage, "", fingerprint, nil)
+
 	// Check if we already have a suitable volume.
-	if b.driver.HasVolume(drivers.VolumeTypeImage, fingerprint) {
+	if b.driver.HasVolume(vol) {
 		return nil
 	}
 
@@ -1629,7 +1713,11 @@ func (b *lxdBackend) DeleteImage(fingerprint string, op *operations.Operation) e
 		return fmt.Errorf("Invalid fingerprint")
 	}
 
-	err = b.driver.DeleteVolume(drivers.VolumeTypeImage, fingerprint, op)
+	// There's no need to pass the content type or config. Both are not needed
+	// when removing an image.
+	vol := b.newVolume(drivers.VolumeTypeImage, "", fingerprint, nil)
+
+	err = b.driver.DeleteVolume(vol, op)
 	if err != nil {
 		return err
 	}
@@ -1983,7 +2071,11 @@ func (b *lxdBackend) RenameCustomVolume(volName string, newVolName string, op *o
 		oldName: volName,
 	})
 
-	err = b.driver.RenameVolume(drivers.VolumeTypeCustom, volName, newVolName, op)
+	// There's no need to pass the config as it's not needed when renaming a
+	// volume.
+	vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, volName, nil)
+
+	err = b.driver.RenameVolume(vol, newVolName, op)
 	if err != nil {
 		return err
 	}
@@ -2112,8 +2204,11 @@ func (b *lxdBackend) DeleteCustomVolume(volName string, op *operations.Operation
 		}
 	}
 
+	// There's no need to pass config as it's not needed when deleting a volume.
+	vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, volName, nil)
+
 	// Delete the volume from the storage device. Must come after snapshots are removed.
-	err = b.driver.DeleteVolume(drivers.VolumeTypeCustom, volName, op)
+	err = b.driver.DeleteVolume(vol, op)
 	if err != nil {
 		return err
 	}
@@ -2129,7 +2224,11 @@ func (b *lxdBackend) DeleteCustomVolume(volName string, op *operations.Operation
 
 // GetCustomVolumeUsage returns the disk space used by the custom volume.
 func (b *lxdBackend) GetCustomVolumeUsage(volName string) (int64, error) {
-	return b.driver.GetVolumeUsage(drivers.VolumeTypeCustom, volName)
+	// There's no need to pass config as it's not needed when getting the volume
+	// usage.
+	vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, volName, nil)
+
+	return b.driver.GetVolumeUsage(vol)
 }
 
 // MountCustomVolume mounts a custom volume.
@@ -2138,7 +2237,14 @@ func (b *lxdBackend) MountCustomVolume(volName string, op *operations.Operation)
 	logger.Debug("MountCustomVolume started")
 	defer logger.Debug("MountCustomVolume finished")
 
-	return b.driver.MountVolume(drivers.VolumeTypeCustom, volName, op)
+	_, volume, err := b.state.Cluster.StoragePoolNodeVolumeGetType(volName, db.StoragePoolVolumeTypeCustom, b.id)
+	if err != nil {
+		return false, err
+	}
+
+	vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, volName, volume.Config)
+
+	return b.driver.MountVolume(vol, op)
 }
 
 // UnmountCustomVolume unmounts a custom volume.
@@ -2147,7 +2253,14 @@ func (b *lxdBackend) UnmountCustomVolume(volName string, op *operations.Operatio
 	logger.Debug("UnmountCustomVolume started")
 	defer logger.Debug("UnmountCustomVolume finished")
 
-	return b.driver.UnmountVolume(drivers.VolumeTypeCustom, volName, op)
+	_, volume, err := b.state.Cluster.StoragePoolNodeVolumeGetType(volName, db.StoragePoolVolumeTypeCustom, b.id)
+	if err != nil {
+		return false, err
+	}
+
+	vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, volName, volume.Config)
+
+	return b.driver.UnmountVolume(vol, op)
 }
 
 // CreateCustomVolumeSnapshot creates a snapshot of a custom volume.
@@ -2199,8 +2312,10 @@ func (b *lxdBackend) CreateCustomVolumeSnapshot(volName string, newSnapshotName
 		}
 	}()
 
+	vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, fullSnapshotName, parentVol.Config)
+
 	// Create the snapshot on the storage device.
-	err = b.driver.CreateVolumeSnapshot(drivers.VolumeTypeCustom, volName, newSnapshotName, op)
+	err = b.driver.CreateVolumeSnapshot(vol, op)
 	if err != nil {
 		return err
 	}
@@ -2224,7 +2339,10 @@ func (b *lxdBackend) RenameCustomVolumeSnapshot(volName string, newSnapshotName
 		return fmt.Errorf("Invalid new snapshot name")
 	}
 
-	err := b.driver.RenameVolumeSnapshot(drivers.VolumeTypeCustom, parentName, oldSnapshotName, newSnapshotName, op)
+	// There's no need to pass config as it's not needed when renaming a volume.
+	vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, volName, nil)
+
+	err := b.driver.RenameVolumeSnapshot(vol, newSnapshotName, op)
 	if err != nil {
 		return err
 	}
@@ -2233,7 +2351,8 @@ func (b *lxdBackend) RenameCustomVolumeSnapshot(volName string, newSnapshotName
 	err = b.state.Cluster.StoragePoolVolumeRename("default", volName, newVolName, db.StoragePoolVolumeTypeCustom, b.ID())
 	if err != nil {
 		// Revert rename.
-		b.driver.RenameVolumeSnapshot(drivers.VolumeTypeCustom, parentName, newSnapshotName, oldSnapshotName, op)
+		newVol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, newVolName, nil)
+		b.driver.RenameVolumeSnapshot(newVol, oldSnapshotName, op)
 		return err
 	}
 
@@ -2246,14 +2365,19 @@ func (b *lxdBackend) DeleteCustomVolumeSnapshot(volName string, op *operations.O
 	logger.Debug("DeleteCustomVolumeSnapshot started")
 	defer logger.Debug("DeleteCustomVolumeSnapshot finished")
 
-	parentName, snapName, isSnap := shared.InstanceGetParentAndSnapshotName(volName)
+	isSnap := shared.IsSnapshot(volName)
+
 	if !isSnap {
 		return fmt.Errorf("Volume name must be a snapshot")
 	}
 
+	// There's no need to pass config as it's not needed when deleting a volume
+	// snapshot.
+	vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, volName, nil)
+
 	// Delete the snapshot from the storage device.
 	// Must come before DB StoragePoolVolumeDelete so that the volume ID is still available.
-	err := b.driver.DeleteVolumeSnapshot(drivers.VolumeTypeCustom, parentName, snapName, op)
+	err := b.driver.DeleteVolumeSnapshot(vol, op)
 	if err != nil {
 		return err
 	}

From d2126ed0ddd04c66ad65d79694c03727cc1ae9e1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 13 Dec 2019 11:38:35 +0000
Subject: [PATCH 4/6] lxd/storage/backend/lxd: Adds support for detecting
 changed config in pool create()

Updates any changed config in the DB after pool driver has created pool on storage device.
Also removes dbPool argument as can already be accessed by b.db.

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index a44ddb0f13..ee2d919378 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -59,8 +59,8 @@ func (b *lxdBackend) MigrationTypes(contentType drivers.ContentType, refresh boo
 
 // create creates the storage pool layout on the storage device.
 // localOnly is used for clustering where only a single node should do remote storage setup.
-func (b *lxdBackend) create(dbPool *api.StoragePoolsPost, localOnly bool, op *operations.Operation) error {
-	logger := logging.AddContext(b.logger, log.Ctx{"args": dbPool})
+func (b *lxdBackend) create(localOnly bool, op *operations.Operation) error {
+	logger := logging.AddContext(b.logger, log.Ctx{"config": b.db.Config, "description": b.db.Description})
 	logger.Debug("create started")
 	defer logger.Debug("created finished")
 
@@ -87,6 +87,12 @@ func (b *lxdBackend) create(dbPool *api.StoragePoolsPost, localOnly bool, op *op
 		os.RemoveAll(path)
 	}()
 
+	// Create copy of original config before creating pool so we can detect any changes later.
+	originalConfig := make(map[string]string, len(b.db.Config))
+	for k, v := range b.db.Config {
+		originalConfig[k] = v
+	}
+
 	// Create the storage pool on the storage device.
 	err = b.driver.Create()
 	if err != nil {
@@ -111,6 +117,20 @@ func (b *lxdBackend) create(dbPool *api.StoragePoolsPost, localOnly bool, op *op
 		return err
 	}
 
+	// In case the storage pool config was changed during the pool creation, we need to update
+	// the database to reflect this change. This can e.g. happen, when we create a loop file
+	// image. This means we append ".img" to the path the user gave us and update the config in
+	// the storage callback. So diff the config here to see if something like this has happened.
+	configDiff, _ := ConfigDiff(originalConfig, b.db.Config)
+	if len(configDiff) > 0 {
+		logger.Debug("Updating changed config", log.Ctx{"changedFields": configDiff})
+		// Update the database entry for the storage pool.
+		err = b.state.Cluster.StoragePoolUpdate(b.db.Name, b.db.Description, b.db.Config)
+		if err != nil {
+			return err
+		}
+	}
+
 	revertPath = false
 	return nil
 }

From ef89b52f6ed6ff8e857ac5584032e88853c3e00f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 13 Dec 2019 11:39:41 +0000
Subject: [PATCH 5/6] lxd/storage/load: Updates usage of create() function

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

diff --git a/lxd/storage/load.go b/lxd/storage/load.go
index f89304c649..d8f206b01a 100644
--- a/lxd/storage/load.go
+++ b/lxd/storage/load.go
@@ -99,12 +99,13 @@ func CreatePool(state *state.State, poolID int64, dbPool *api.StoragePoolsPost,
 		Name:           dbPool.Name,
 		Driver:         dbPool.Driver,
 	}
+
 	pool.name = dbPool.Name
 	pool.state = state
 	pool.logger = logger
 
 	// Create the pool itself on the storage device..
-	err = pool.create(dbPool, localOnly, op)
+	err = pool.create(localOnly, op)
 	if err != nil {
 		return nil, err
 	}

From 8ae6993c93613a758ce980735130b16853a7e7fc Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 13 Dec 2019 11:40:08 +0000
Subject: [PATCH 6/6] lxd/storage/drivers/interface: Comments on pool
 mount/unmount definitions

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

diff --git a/lxd/storage/drivers/interface.go b/lxd/storage/drivers/interface.go
index 6b4260f8f4..3dd635a633 100644
--- a/lxd/storage/drivers/interface.go
+++ b/lxd/storage/drivers/interface.go
@@ -28,7 +28,12 @@ type Driver interface {
 	// Pool.
 	Create() error
 	Delete(op *operations.Operation) error
+	// Mount mounts a storage pool if needed, returns true if we caused a new mount, false if
+	// already mounted.
 	Mount() (bool, error)
+
+	// Unmount unmounts a storage pool if needed, returns true if unmounted, false if was not
+	// mounted.
 	Unmount() (bool, error)
 	GetResources() (*api.ResourcesStoragePool, error)
 	Validate(config map[string]string) error


More information about the lxc-devel mailing list