[lxc-devel] [lxd/master] Storage: Further bug fixes in relation to volume used by detection

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 346 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201109/377e96cc/attachment-0001.bin>
-------------- next part --------------
From 1cb4bb18b2257dd3bc7e731def934feb148d87c2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 30 Oct 2020 13:50:50 +0000
Subject: [PATCH 01/44] lxd/db/storage/volumes: Adds workaround for old remote
 volume schema in GetStorageVolumeNodeAddresses

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/db/storage_volumes.go | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lxd/db/storage_volumes.go b/lxd/db/storage_volumes.go
index 738c51849c..a028910c37 100644
--- a/lxd/db/storage_volumes.go
+++ b/lxd/db/storage_volumes.go
@@ -677,8 +677,23 @@ func (c *ClusterTx) GetStorageVolumeNodeAddresses(poolID int64, projectName stri
 
 	sort.Strings(addresses)
 
-	if len(addresses) == 0 {
+	addressCount := len(addresses)
+	if addressCount == 0 {
 		return nil, ErrNoSuchObject
+	} else if addressCount > 1 {
+		driver, err := c.GetStoragePoolDriver(poolID)
+		if err != nil {
+			return nil, err
+		}
+
+		// Earlier schema versions created a volume DB record for each cluster member for remote storage
+		// pools, so if the storage driver is one of those remote pools and the addressCount is >1 then we
+		// take this to mean that the volume doesn't have an explicit cluster member and is therefore
+		// equivalent to db.ErrNoClusterMember that is used in newer schemas where a single remote volume
+		// DB record is created that is not associated to any single member.
+		if driver == "ceph" || driver == "cephfs" {
+			return nil, ErrNoClusterMember
+		}
 	}
 
 	return addresses, nil

From f151a8be2f78d3d4e82d5849111fe0a49e0c9e96 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 11:12:19 +0000
Subject: [PATCH 02/44] lxd/db/storage/volumes: Renames
 GetStorageVolumeNodeAddresses to GetStorageVolumeNodes

And updates to return a list of db.NodeInfo structs rather than just addresses. This is more useful and removes the percularities around returning empty node address for local nodes.

This makes it dependent on the caller to identify a local node.

Also unifies the function to handle both current and legacy volume records for remote drivers.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/db/storage_volumes.go | 58 +++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/lxd/db/storage_volumes.go b/lxd/db/storage_volumes.go
index a028910c37..261784ce1f 100644
--- a/lxd/db/storage_volumes.go
+++ b/lxd/db/storage_volumes.go
@@ -5,7 +5,6 @@ package db
 import (
 	"database/sql"
 	"fmt"
-	"sort"
 	"strings"
 	"time"
 
@@ -622,27 +621,20 @@ type StorageVolumeArgs struct {
 	ContentType string
 }
 
-// GetStorageVolumeNodeAddresses returns the addresses of all nodes on which the
-// volume with the given name if defined.
-//
+// GetStorageVolumeNodes returns the node info of all nodes on which the volume with the given name is defined.
 // The volume name can be either a regular name or a volume snapshot name.
-//
-// The empty string is used in place of the address of the current node.
-func (c *ClusterTx) GetStorageVolumeNodeAddresses(poolID int64, projectName string, volumeName string, volumeType int) ([]string, error) {
-	nodes := []struct {
-		id      int64
-		address string
-	}{}
-	dest := func(i int) []interface{} {
-		nodes = append(nodes, struct {
-			id      int64
-			address string
-		}{})
-		return []interface{}{&nodes[i].id, &nodes[i].address}
+// If the volume is defined, but without a specific node, then the ErrNoClusterMember error is returned.
+// If the volume is not found then the ErrNoSuchObject error is returned.
+func (c *ClusterTx) GetStorageVolumeNodes(poolID int64, projectName string, volumeName string, volumeType int) ([]NodeInfo, error) {
+	nodes := []NodeInfo{}
 
+	dest := func(i int) []interface{} {
+		nodes = append(nodes, NodeInfo{})
+		return []interface{}{&nodes[i].ID, &nodes[i].Address, &nodes[i].Name}
 	}
+
 	sql := `
-	SELECT coalesce(nodes.id,0) AS nodeID, coalesce(nodes.address,"") AS nodeAddress
+	SELECT coalesce(nodes.id,0) AS nodeID, coalesce(nodes.address,"") AS nodeAddress, coalesce(nodes.name,"") AS nodeName
 	FROM storage_volumes_all
 	JOIN projects ON projects.id = storage_volumes_all.project_id
 	LEFT JOIN nodes ON storage_volumes_all.node_id=nodes.id
@@ -661,26 +653,21 @@ func (c *ClusterTx) GetStorageVolumeNodeAddresses(poolID int64, projectName stri
 		return nil, err
 	}
 
-	addresses := []string{}
+	if len(nodes) == 0 {
+		return nil, ErrNoSuchObject
+	}
+
 	for _, node := range nodes {
 		// Volume is defined without a cluster member.
-		if node.id == 0 && node.address == "" {
+		if node.ID == 0 {
 			return nil, ErrNoClusterMember
 		}
-
-		address := node.address
-		if node.id == c.nodeID {
-			address = ""
-		}
-		addresses = append(addresses, address)
 	}
 
-	sort.Strings(addresses)
-
-	addressCount := len(addresses)
-	if addressCount == 0 {
+	nodeCount := len(nodes)
+	if nodeCount == 0 {
 		return nil, ErrNoSuchObject
-	} else if addressCount > 1 {
+	} else if nodeCount > 1 {
 		driver, err := c.GetStoragePoolDriver(poolID)
 		if err != nil {
 			return nil, err
@@ -691,12 +678,17 @@ func (c *ClusterTx) GetStorageVolumeNodeAddresses(poolID int64, projectName stri
 		// take this to mean that the volume doesn't have an explicit cluster member and is therefore
 		// equivalent to db.ErrNoClusterMember that is used in newer schemas where a single remote volume
 		// DB record is created that is not associated to any single member.
-		if driver == "ceph" || driver == "cephfs" {
+		if StorageRemoteDriverNames == nil {
+			return nil, fmt.Errorf("No remote storage drivers function defined")
+		}
+
+		remoteDrivers := StorageRemoteDriverNames()
+		if shared.StringInSlice(driver, remoteDrivers) {
 			return nil, ErrNoClusterMember
 		}
 	}
 
-	return addresses, nil
+	return nodes, nil
 }
 
 // Return the name of the node a storage volume is on.

From 29b3a0efacfa8fc6a3004963dfc58466dc9889f2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 11:05:45 +0000
Subject: [PATCH 03/44] lxd/cluster/connect: Updates ConnectIfVolumeIsRemote to
 use tx.GetStorageVolumeNodes

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/cluster/connect.go | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/lxd/cluster/connect.go b/lxd/cluster/connect.go
index 335341b022..825d3aee23 100644
--- a/lxd/cluster/connect.go
+++ b/lxd/cluster/connect.go
@@ -125,20 +125,22 @@ func ConnectIfVolumeIsRemote(s *state.State, poolName string, projectName string
 		return nil, err
 	}
 
-	var addresses []string
+	localNodeID := s.Cluster.GetNodeID()
+	var nodes []db.NodeInfo
 	err = s.Cluster.Transaction(func(tx *db.ClusterTx) error {
 		poolID, err := tx.GetStoragePoolID(poolName)
 		if err != nil {
 			return err
 		}
 
-		addresses, err = tx.GetStorageVolumeNodeAddresses(poolID, projectName, volumeName, volumeType)
+		nodes, err = tx.GetStorageVolumeNodes(poolID, projectName, volumeName, volumeType)
 		if err != nil {
 			return err
 		}
 
 		return nil
 	})
+
 	if err != nil && err != db.ErrNoClusterMember {
 		return nil, err
 	}
@@ -162,29 +164,30 @@ func ConnectIfVolumeIsRemote(s *state.State, poolName string, projectName string
 				return nil, errors.Wrapf(err, "Failed getting cluster member info for %q", remoteInstance.Node)
 			}
 
-			// Replace address list with instance's cluster member.
-			addresses = []string{instNode.Address}
+			// Replace node list with instance's cluster member node (which might be local member).
+			nodes = []db.NodeInfo{instNode}
 		} else {
-			// Volume isn't exclusively attached to an instance and has no fixed node.
-			addresses = []string{""} // Use local cluster member.
+			// Volume isn't exclusively attached to an instance. Use local cluster member.
+			return nil, nil
 		}
 	}
 
-	addressCount := len(addresses)
-	if addressCount > 1 {
-		return nil, fmt.Errorf("More than one cluster member has a volume named %q", volumeName)
-	} else if addressCount < 1 {
+	nodeCount := len(nodes)
+	if nodeCount > 1 {
+		return nil, fmt.Errorf("More than one cluster member has a volume named %q. Use --target flag to specify member", volumeName)
+	} else if nodeCount < 1 {
+		// Should never get here.
 		return nil, fmt.Errorf("Volume %q has empty cluster member list", volumeName)
 	}
 
-	address := addresses[0]
-	// Use local cluster member.
-	if address == "" {
+	node := nodes[0]
+	if node.ID == localNodeID {
+		// Use local cluster member if volume belongs to this local node.
 		return nil, nil
 	}
 
 	// Connect to remote cluster member.
-	return Connect(address, cert, false)
+	return Connect(node.Address, cert, false)
 }
 
 // SetupTrust is a convenience around InstanceServer.CreateCertificate that

From eae87f41435c75945346ee9a15c60ffe46cf7dc1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 11:31:07 +0000
Subject: [PATCH 04/44] lxd/db/storage/volumes/test: Updates test for
 TestGetStorageVolumeNodes

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/cluster/connect.go         |  1 -
 lxd/db/storage_volumes_test.go | 21 ++++++++++++++++++---
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/lxd/cluster/connect.go b/lxd/cluster/connect.go
index 825d3aee23..f746cf7f52 100644
--- a/lxd/cluster/connect.go
+++ b/lxd/cluster/connect.go
@@ -140,7 +140,6 @@ func ConnectIfVolumeIsRemote(s *state.State, poolName string, projectName string
 
 		return nil
 	})
-
 	if err != nil && err != db.ErrNoClusterMember {
 		return nil, err
 	}
diff --git a/lxd/db/storage_volumes_test.go b/lxd/db/storage_volumes_test.go
index 3e13fc5f68..2dd65f699b 100644
--- a/lxd/db/storage_volumes_test.go
+++ b/lxd/db/storage_volumes_test.go
@@ -11,7 +11,11 @@ import (
 )
 
 // Addresses of all nodes with matching volume name are returned.
-func TestStorageVolumeNodeAddresses(t *testing.T) {
+func TestGetStorageVolumeNodes(t *testing.T) {
+	db.StorageRemoteDriverNames = func() []string {
+		return []string{"ceph", "cephfs"}
+	}
+
 	tx, cleanup := db.NewTestClusterTx(t)
 	defer cleanup()
 
@@ -29,10 +33,21 @@ func TestStorageVolumeNodeAddresses(t *testing.T) {
 	addVolume(t, tx, poolID, nodeID3, "volume2")
 	addVolume(t, tx, poolID, nodeID2, "volume2")
 
-	addresses, err := tx.GetStorageVolumeNodeAddresses(poolID, "default", "volume1", 1)
+	nodes, err := tx.GetStorageVolumeNodes(poolID, "default", "volume1", 1)
 	require.NoError(t, err)
 
-	assert.Equal(t, []string{"", "1.2.3.4:666"}, addresses)
+	assert.Equal(t, []db.NodeInfo{
+		{
+			ID:      nodeID1,
+			Name:    "none",
+			Address: "0.0.0.0",
+		},
+		{
+			ID:      nodeID2,
+			Name:    "node2",
+			Address: "1.2.3.4:666",
+		},
+	}, nodes)
 }
 
 func addPool(t *testing.T, tx *db.ClusterTx, name string) int64 {

From 506ed58663c79d30b4db625bb4dede35d2219a7c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:22:23 +0000
Subject: [PATCH 05/44] lxd/storage/utils: Updates VolumeUsedByInstances to
 accept an api.StorageVolume arg

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index f802933416..2c5d645450 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -696,14 +696,14 @@ func InstanceContentType(inst instance.Instance) drivers.ContentType {
 // VolumeUsedByInstances finds instances using a volume (either directly or via their expanded profiles if
 // expandDevices is true) and passes them to instanceFunc for evaluation. If instanceFunc returns an error then it
 // is returned immediately. The instanceFunc is executed during a DB transaction, so DB queries are not permitted.
-func VolumeUsedByInstances(s *state.State, poolName string, projectName string, volumeName string, volumeTypeName string, expandDevices bool, instanceFunc func(inst db.Instance, project api.Project, profiles []api.Profile) error) error {
+func VolumeUsedByInstances(s *state.State, poolName string, projectName string, vol *api.StorageVolume, expandDevices bool, instanceFunc func(inst db.Instance, project api.Project, profiles []api.Profile) error) error {
 	// Convert the volume type name to our internal integer representation.
-	volumeType, err := VolumeTypeNameToDBType(volumeTypeName)
+	volumeType, err := VolumeTypeNameToDBType(vol.Type)
 	if err != nil {
 		return err
 	}
 
-	volumeNameWithType := fmt.Sprintf("%s/%s", volumeTypeName, volumeName)
+	volumeNameWithType := fmt.Sprintf("%s/%s", vol.Type, vol.Name)
 
 	return s.Cluster.InstanceList(func(inst db.Instance, p api.Project, profiles []api.Profile) error {
 		instStorageProject := project.StorageVolumeProjectFromRecord(&p, volumeType)
@@ -740,7 +740,7 @@ func VolumeUsedByInstances(s *state.State, poolName string, projectName string,
 			// Make sure that we don't compare against stuff like
 			// "container////bla" but only against "container/bla".
 			cleanSource := filepath.Clean(dev["source"])
-			if cleanSource == volumeName || cleanSource == volumeNameWithType {
+			if cleanSource == vol.Name || cleanSource == volumeNameWithType {
 				err = instanceFunc(inst, p, profiles)
 				if err != nil {
 					return err

From cebec009e5de30872149d878f5acff07aff71aa7 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:22:43 +0000
Subject: [PATCH 06/44] lxd/storage/utils: Updates
 VolumeUsedByExclusiveRemoteInstancesWithProfiles to use an api.StorageVolume
 arg

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 2c5d645450..b48c65c1dc 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -754,7 +754,7 @@ func VolumeUsedByInstances(s *state.State, poolName string, projectName string,
 
 // VolumeUsedByExclusiveRemoteInstancesWithProfiles checks if custom volume is exclusively attached to a remote
 // instance. Returns the remote instance that has the volume exclusively attached. Returns nil if volume available.
-func VolumeUsedByExclusiveRemoteInstancesWithProfiles(s *state.State, poolName string, projectName string, volumeName string, volumeTypeName string) (*db.Instance, error) {
+func VolumeUsedByExclusiveRemoteInstancesWithProfiles(s *state.State, poolName string, projectName string, vol *api.StorageVolume) (*db.Instance, error) {
 	pool, err := GetPoolByName(s, poolName)
 	if err != nil {
 		return nil, errors.Wrapf(err, "Failed loading storage pool %q", poolName)
@@ -780,7 +780,7 @@ func VolumeUsedByExclusiveRemoteInstancesWithProfiles(s *state.State, poolName s
 	// Find if volume is attached to a remote instance.
 	var errAttached = fmt.Errorf("Volume is remotely attached")
 	var remoteInstance *db.Instance
-	VolumeUsedByInstances(s, poolName, projectName, volumeName, volumeTypeName, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+	VolumeUsedByInstances(s, poolName, projectName, vol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
 		if dbInst.Node != localNode {
 			remoteInstance = &dbInst
 			return errAttached // Stop the search, this volume is attached to a remote instance.

From 7dc60c523a33f2588f65af769524d4cb97bb58a2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:26:40 +0000
Subject: [PATCH 07/44] lxd/storage/volumes/utils: Updates
 storagePoolVolumeUsedByGet to accept an api.StorageVolume arg

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

diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index 7ec0d50213..931bbaf93b 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -205,10 +205,10 @@ func storagePoolVolumeUpdateUsers(d *Daemon, projectName string, oldPoolName str
 }
 
 // volumeUsedBy = append(volumeUsedBy, fmt.Sprintf("/%s/containers/%s", version.APIVersion, ct))
-func storagePoolVolumeUsedByGet(s *state.State, projectName string, poolName string, volumeName string, volumeTypeName string) ([]string, error) {
+func storagePoolVolumeUsedByGet(s *state.State, projectName string, poolName string, vol *api.StorageVolume) ([]string, error) {
 	// Handle instance volumes.
-	if volumeTypeName == "container" || volumeTypeName == "virtual-machine" {
-		cName, sName, snap := shared.InstanceGetParentAndSnapshotName(volumeName)
+	if vol.Type == db.StoragePoolVolumeTypeNameContainer || vol.Type == db.StoragePoolVolumeTypeNameVM {
+		cName, sName, snap := shared.InstanceGetParentAndSnapshotName(vol.Name)
 		if snap {
 			if projectName == project.Default {
 				return []string{fmt.Sprintf("/%s/instances/%s/snapshots/%s", version.APIVersion, cName, sName)}, nil
@@ -225,16 +225,16 @@ func storagePoolVolumeUsedByGet(s *state.State, projectName string, poolName str
 	}
 
 	// Handle image volumes.
-	if volumeTypeName == "image" {
+	if vol.Type == db.StoragePoolVolumeTypeNameImage {
 		if projectName == project.Default {
-			return []string{fmt.Sprintf("/%s/images/%s", version.APIVersion, volumeName)}, nil
+			return []string{fmt.Sprintf("/%s/images/%s", version.APIVersion, vol.Name)}, nil
 		} else {
-			return []string{fmt.Sprintf("/%s/images/%s?project=%s", version.APIVersion, volumeName, projectName)}, nil
+			return []string{fmt.Sprintf("/%s/images/%s?project=%s", version.APIVersion, vol.Name, projectName)}, nil
 		}
 	}
 
 	// Check if the daemon itself is using it.
-	used, err := storagePools.VolumeUsedByDaemon(s, poolName, volumeName)
+	used, err := storagePools.VolumeUsedByDaemon(s, poolName, vol.Name)
 	if err != nil {
 		return []string{}, err
 	}
@@ -250,7 +250,7 @@ func storagePoolVolumeUsedByGet(s *state.State, projectName string, poolName str
 
 	// Pass false to expandDevices, as we only want to see instances directly using a volume, rather than their
 	// profiles using a volume.
-	err = storagePools.VolumeUsedByInstances(s, poolName, projectName, volumeName, volumeTypeName, false, func(inst db.Instance, project api.Project, profiles []api.Profile) error {
+	err = storagePools.VolumeUsedByInstances(s, poolName, projectName, vol, false, func(inst db.Instance, project api.Project, profiles []api.Profile) error {
 		instancesUsingVolume = append(instancesUsingVolume, &inst)
 		return nil
 	})
@@ -267,7 +267,7 @@ func storagePoolVolumeUsedByGet(s *state.State, projectName string, poolName str
 	}
 
 	// Look for profiles using this volume.
-	profiles, err := profilesUsingPoolVolumeGetNames(s.Cluster, volumeName, volumeTypeName)
+	profiles, err := profilesUsingPoolVolumeGetNames(s.Cluster, vol.Name, vol.Type)
 	if err != nil {
 		return []string{}, err
 	}

From 7e617573334c5048f2e7577d5e50480287db7767 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:19:27 +0000
Subject: [PATCH 08/44] lxd/cluster/connect: Updates ConnectIfVolumeIsRemote to
 use VolumeUsedByExclusiveRemoteInstancesWithProfiles with vol arg

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/cluster/connect.go | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/lxd/cluster/connect.go b/lxd/cluster/connect.go
index f746cf7f52..f7485f955c 100644
--- a/lxd/cluster/connect.go
+++ b/lxd/cluster/connect.go
@@ -120,15 +120,12 @@ func ConnectIfInstanceIsRemote(cluster *db.Cluster, projectName string, name str
 // defined. If it's not the local cluster member it will connect to it and return the connected client, otherwise
 // it just returns nil. If there is more than one cluster member with a matching volume name, an error is returned.
 func ConnectIfVolumeIsRemote(s *state.State, poolName string, projectName string, volumeName string, volumeType int, cert *shared.CertInfo) (lxd.InstanceServer, error) {
-	volumeTypeName, err := storagePools.VolumeDBTypeToTypeName(volumeType)
-	if err != nil {
-		return nil, err
-	}
-
 	localNodeID := s.Cluster.GetNodeID()
+	var err error
 	var nodes []db.NodeInfo
+	var poolID int64
 	err = s.Cluster.Transaction(func(tx *db.ClusterTx) error {
-		poolID, err := tx.GetStoragePoolID(poolName)
+		poolID, err = tx.GetStoragePoolID(poolName)
 		if err != nil {
 			return err
 		}
@@ -148,7 +145,13 @@ func ConnectIfVolumeIsRemote(s *state.State, poolName string, projectName string
 	// whether it is exclusively attached to remote instance, and if so then we need to forward the request to
 	// the node whereit is currently used. This avoids conflicting with another member when using it locally.
 	if err == db.ErrNoClusterMember {
-		remoteInstance, err := storagePools.VolumeUsedByExclusiveRemoteInstancesWithProfiles(s, poolName, projectName, volumeName, volumeTypeName)
+		// GetLocalStoragePoolVolume returns a volume with an empty Location field for remote drivers.
+		_, vol, err := s.Cluster.GetLocalStoragePoolVolume(projectName, volumeName, volumeType, poolID)
+		if err != nil {
+			return nil, err
+		}
+
+		remoteInstance, err := storagePools.VolumeUsedByExclusiveRemoteInstancesWithProfiles(s, poolName, projectName, vol)
 		if err != nil {
 			return nil, errors.Wrapf(err, "Failed checking if volume %q is available", volumeName)
 		}

From b836b85b55f334dd4498120e802e2a34897a8158 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:20:09 +0000
Subject: [PATCH 09/44] lxd/device/disk: Updates validateConfig to use
 storagePools.VolumeUsedByExclusiveRemoteInstancesWithProfiles with vol arg

And merges custom volume validation for block type to avoid duplicated DB lookups.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/disk.go | 50 ++++++++++++++++++++--------------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 4271892549..705ab57bcc 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -166,50 +166,44 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 			return fmt.Errorf("Storage volumes cannot be specified as absolute paths")
 		}
 
-		poolID, err := d.state.Cluster.GetStoragePoolID(d.config["pool"])
-		if err != nil {
-			return fmt.Errorf("The %q storage pool doesn't exist", d.config["pool"])
-		}
+		// Only perform expensive instance custom volume checks when not validating a profile and after
+		// device expansion has occurred (to avoid doing it twice).
+		if instConf.Type() != instancetype.Any && len(instConf.ExpandedDevices()) > 0 && d.config["source"] != "" && d.config["path"] != "/" {
+			poolID, err := d.state.Cluster.GetStoragePoolID(d.config["pool"])
+			if err != nil {
+				return fmt.Errorf("The %q storage pool doesn't exist", d.config["pool"])
+			}
 
-		// If instance is supplied, use it's project to derive the effective storage project name.
-		var storageProjectName string
-		if d.inst != nil {
-			storageProjectName, err = project.StorageVolumeProject(d.state.Cluster, d.inst.Project(), db.StoragePoolVolumeTypeCustom)
+			// Derive the effective storage project name from the instance config's project.
+			storageProjectName, err := project.StorageVolumeProject(d.state.Cluster, instConf.Project(), db.StoragePoolVolumeTypeCustom)
 			if err != nil {
 				return err
 			}
-		}
 
-		// Only check storate volume is available if we are validating an instance device and not a profile
-		// device (check for instancetype.Any), and we have least one expanded device (this is so we only
-		// do this expensive check after devices have been expanded).
-		if instConf.Type() != instancetype.Any && len(instConf.ExpandedDevices()) > 0 && d.config["source"] != "" && d.config["path"] != "/" {
-			remoteInstance, err := storagePools.VolumeUsedByExclusiveRemoteInstancesWithProfiles(d.state, d.config["pool"], storageProjectName, d.config["source"], db.StoragePoolVolumeTypeNameCustom)
+			// GetLocalStoragePoolVolume returns a volume with an empty Location field for remote drivers.
+			_, vol, err := d.state.Cluster.GetLocalStoragePoolVolume(storageProjectName, d.config["source"], db.StoragePoolVolumeTypeCustom, poolID)
 			if err != nil {
-				return errors.Wrapf(err, "Failed checking if volume is exclusively attached")
+				return errors.Wrapf(err, "Failed loading custom volume")
 			}
 
-			if remoteInstance != nil {
-				return fmt.Errorf("Storage volume %q is already attached to an instance on a different node", d.config["source"])
+			// Check storage volume is available to mount on this cluster member.
+			remoteInstance, err := storagePools.VolumeUsedByExclusiveRemoteInstancesWithProfiles(d.state, d.config["pool"], storageProjectName, vol)
+			if err != nil {
+				return errors.Wrapf(err, "Failed checking if custom volume is exclusively attached to another instance")
 			}
 
-			return nil
-		}
-
-		// Block volumes may only be attached to VMs.
-		if d.inst != nil && d.config["path"] != "/" {
-			_, volume, err := d.state.Cluster.GetLocalStoragePoolVolume(storageProjectName, d.config["source"], db.StoragePoolVolumeTypeCustom, poolID)
-			if err != nil {
-				return err
+			if remoteInstance != nil {
+				return fmt.Errorf("Custom volume is already attached to an instance on a different node")
 			}
 
-			contentType, err := storagePools.VolumeContentTypeNameToContentType(volume.ContentType)
+			// Check only block type volumes are attached to VM instances.
+			contentType, err := storagePools.VolumeContentTypeNameToContentType(vol.ContentType)
 			if err != nil {
 				return err
 			}
 
-			if contentType == db.StoragePoolVolumeContentTypeBlock && d.inst.Type() != instancetype.VM {
-				return fmt.Errorf("Block volumes cannot be used on containers")
+			if contentType == db.StoragePoolVolumeContentTypeBlock && instConf.Type() != instancetype.VM {
+				return fmt.Errorf("Custom block volumes cannot be used on containers")
 			}
 		}
 	}

From a06c535ec50283c1d4773aea09aeac4bedef0254 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:20:36 +0000
Subject: [PATCH 10/44] lxd/device/disk: Updates storagePoolVolumeAttachShift
 to use storagePools.VolumeUsedByInstances with vol arg

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/disk.go | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 705ab57bcc..af9ba22f3a 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -1067,13 +1067,8 @@ func (d *disk) storagePoolVolumeAttachShift(projectName, poolName, volumeName st
 		logger.Debugf("Shifting storage volume")
 
 		if !shared.IsTrue(poolVolumePut.Config["security.shifted"]) {
-			volumeTypeName, err := storagePools.VolumeDBTypeToTypeName(volumeType)
-			if err != nil {
-				return err
-			}
-
 			volumeUsedBy := []instance.Instance{}
-			err = storagePools.VolumeUsedByInstances(d.state, poolName, projectName, volumeName, volumeTypeName, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+			err = storagePools.VolumeUsedByInstances(d.state, poolName, projectName, volume, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
 				inst, err := instance.Load(d.state, db.InstanceToArgs(&dbInst), profiles)
 				if err != nil {
 					return err

From 8ef94947a217aea6b616e3e56bd873505842890d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:21:18 +0000
Subject: [PATCH 11/44] lxd/storage/backend/lxd: Updates UpdateCustomVolume to
 use VolumeUsedByInstances with vol arg

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 92a5c38d7e..127eff51d5 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -2842,7 +2842,7 @@ func (b *lxdBackend) UpdateCustomVolume(projectName string, volName string, newD
 
 		// Check for config changing that is not allowed when running instances are using it.
 		if (changedConfig["size"] != "" && !runningQuotaResize) || newConfig["security.shifted"] != curVol.Config["security.shifted"] {
-			err = VolumeUsedByInstances(b.state, b.name, projectName, volName, db.StoragePoolVolumeTypeNameCustom, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+			err = VolumeUsedByInstances(b.state, b.name, projectName, curVol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
 				inst, err := instance.Load(b.state, db.InstanceToArgs(&dbInst), profiles)
 				if err != nil {
 					return err

From f5b59296cc343b29aac33fb74d07afb73f8d000a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:21:54 +0000
Subject: [PATCH 12/44] lxd/storage/backend/lxd: Updates RestoreCustomVolume to
 use VolumeUsedByInstances with vol arg

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 127eff51d5..355588393a 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -3261,8 +3261,18 @@ func (b *lxdBackend) RestoreCustomVolume(projectName, volName string, snapshotNa
 		return fmt.Errorf("Invalid snapshot name")
 	}
 
+	// Get current volume.
+	_, curVol, err := b.state.Cluster.GetLocalStoragePoolVolume(projectName, volName, db.StoragePoolVolumeTypeCustom, b.ID())
+	if err != nil {
+		if err == db.ErrNoSuchObject {
+			return fmt.Errorf("Volume doesn't exist")
+		}
+
+		return err
+	}
+
 	// Check that the volume isn't in use by running instances.
-	err := VolumeUsedByInstances(b.state, b.Name(), projectName, volName, db.StoragePoolVolumeTypeNameCustom, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+	err = VolumeUsedByInstances(b.state, b.Name(), projectName, curVol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
 		inst, err := instance.Load(b.state, db.InstanceToArgs(&dbInst), profiles)
 		if err != nil {
 			return err

From 06aa5dd55e83b00c129dc9f9d555234fa3492e7c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:23:25 +0000
Subject: [PATCH 13/44] lxd/storage/volumes: storagePoolVolumeUsedByGet usage

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

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 7df58c7048..8064ed7e2f 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -182,7 +182,7 @@ func storagePoolVolumesGet(d *Daemon, r *http.Request) response.Response {
 				}
 			}
 		} else {
-			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, volume.Name, volume.Type)
+			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, volume)
 			if err != nil {
 				return response.InternalError(err)
 			}
@@ -260,7 +260,7 @@ func storagePoolVolumesTypeGet(d *Daemon, r *http.Request) response.Response {
 				continue
 			}
 
-			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, vol.Name, vol.Type)
+			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, vol)
 			if err != nil {
 				return response.SmartError(err)
 			}

From a5b0ed1acb7c15702f2f9dbd857fd0de5d459fee Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:23:42 +0000
Subject: [PATCH 14/44] lxd/storage/volumes: Updates storagePoolVolumeTypePost
 to use storagePools.VolumeUsedByInstances with a vol arg

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage_volumes.go | 51 +++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 8064ed7e2f..9ad4862c22 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -572,7 +572,7 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
 	}
 
 	// Get the name of the storage pool the volume is supposed to be attached to.
-	poolName := mux.Vars(r)["pool"]
+	srcPoolName := mux.Vars(r)["pool"]
 
 	req := api.StorageVolumePost{}
 
@@ -603,17 +603,6 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
 		return response.SmartError(err)
 	}
 
-	// Retrieve ID of the storage pool (and check if the storage pool exists).
-	var poolID int64
-	if req.Pool != "" {
-		poolID, err = d.cluster.GetStoragePoolID(req.Pool)
-	} else {
-		poolID, err = d.cluster.GetStoragePoolID(poolName)
-	}
-	if err != nil {
-		return response.SmartError(err)
-	}
-
 	// We need to restore the body of the request since it has already been read, and if we
 	// forwarded it now no body would be written out.
 	buf := bytes.Buffer{}
@@ -634,18 +623,29 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
 		return response.BadRequest(err)
 	}
 
-	resp = forwardedResponseIfVolumeIsRemote(d, r, poolName, projectName, volumeName, volumeType)
+	resp = forwardedResponseIfVolumeIsRemote(d, r, srcPoolName, projectName, volumeName, volumeType)
 	if resp != nil {
 		return resp
 	}
 
 	// This is a migration request so send back requested secrets.
 	if req.Migration {
-		return storagePoolVolumeTypePostMigration(d.State(), projectName, poolName, volumeName, req)
+		return storagePoolVolumeTypePostMigration(d.State(), projectName, srcPoolName, volumeName, req)
+	}
+
+	// Retrieve ID of the storage pool (and check if the storage pool exists).
+	var targetPoolID int64
+	if req.Pool != "" {
+		targetPoolID, err = d.cluster.GetStoragePoolID(req.Pool)
+	} else {
+		targetPoolID, err = d.cluster.GetStoragePoolID(srcPoolName)
+	}
+	if err != nil {
+		return response.SmartError(err)
 	}
 
 	// Check that the name isn't already in use.
-	_, err = d.cluster.GetStoragePoolNodeVolumeID(projectName, req.Name, volumeType, poolID)
+	_, err = d.cluster.GetStoragePoolNodeVolumeID(projectName, req.Name, volumeType, targetPoolID)
 	if err != db.ErrNoSuchObject {
 		if err != nil {
 			return response.InternalError(err)
@@ -655,7 +655,7 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
 	}
 
 	// Check if the daemon itself is using it.
-	used, err := storagePools.VolumeUsedByDaemon(d.State(), poolName, volumeName)
+	used, err := storagePools.VolumeUsedByDaemon(d.State(), srcPoolName, volumeName)
 	if err != nil {
 		return response.SmartError(err)
 	}
@@ -664,8 +664,19 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
 		return response.SmartError(fmt.Errorf("Volume is used by LXD itself and cannot be renamed"))
 	}
 
+	// Load source volume.
+	srcPoolID, err := d.cluster.GetStoragePoolID(srcPoolName)
+	if err != nil {
+		return response.SmartError(err)
+	}
+
+	_, vol, err := d.cluster.GetLocalStoragePoolVolume(projectName, volumeName, volumeType, srcPoolID)
+	if err != nil {
+		return response.SmartError(err)
+	}
+
 	// Check if a running instance is using it.
-	err = storagePools.VolumeUsedByInstances(d.State(), poolName, projectName, volumeName, volumeTypeName, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+	err = storagePools.VolumeUsedByInstances(d.State(), srcPoolName, projectName, vol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
 		inst, err := instance.Load(d.State(), db.InstanceToArgs(&dbInst), profiles)
 		if err != nil {
 			return err
@@ -682,12 +693,12 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
 	}
 
 	// Detect a rename request.
-	if req.Pool == "" || req.Pool == poolName {
-		return storagePoolVolumeTypePostRename(d, projectName, poolName, volumeName, volumeType, req)
+	if req.Pool == "" || req.Pool == srcPoolName {
+		return storagePoolVolumeTypePostRename(d, projectName, srcPoolName, volumeName, volumeType, req)
 	}
 
 	// Otherwise this is a move request.
-	return storagePoolVolumeTypePostMove(d, projectName, poolName, volumeName, volumeType, req)
+	return storagePoolVolumeTypePostMove(d, projectName, srcPoolName, volumeName, volumeType, req)
 }
 
 // storagePoolVolumeTypePostMigration handles volume migration type POST requests.

From bfcbd6623e0c532d086d8ebf694f8b4648a7e1d2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:24:51 +0000
Subject: [PATCH 15/44] lxd/storage/volumes: Use db.StoragePoolVolumeTypeName
 constants

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

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 9ad4862c22..37e10c2587 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -804,19 +804,19 @@ func storagePoolVolumeTypePostMove(d *Daemon, projectName, poolName, volumeName
 }
 
 func storagePoolVolumeTypeContainerPost(d *Daemon, r *http.Request) response.Response {
-	return storagePoolVolumeTypePost(d, r, "container")
+	return storagePoolVolumeTypePost(d, r, db.StoragePoolVolumeTypeNameContainer)
 }
 
 func storagePoolVolumeTypeVMPost(d *Daemon, r *http.Request) response.Response {
-	return storagePoolVolumeTypePost(d, r, "virtual-machine")
+	return storagePoolVolumeTypePost(d, r, db.StoragePoolVolumeTypeNameVM)
 }
 
 func storagePoolVolumeTypeCustomPost(d *Daemon, r *http.Request) response.Response {
-	return storagePoolVolumeTypePost(d, r, "custom")
+	return storagePoolVolumeTypePost(d, r, db.StoragePoolVolumeTypeNameCustom)
 }
 
 func storagePoolVolumeTypeImagePost(d *Daemon, r *http.Request) response.Response {
-	return storagePoolVolumeTypePost(d, r, "image")
+	return storagePoolVolumeTypePost(d, r, db.StoragePoolVolumeTypeNameImage)
 }
 
 // storageGetVolumeNameFromURL retrieves the volume name from the URL name segment.

From 71324673f77d8112971308cc9e785258301b77bc Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:25:20 +0000
Subject: [PATCH 16/44] lxd/storage/volumes: Updates storagePoolVolumeTypeGet
 to use storagePoolVolumeUsedByGet with a vol arg

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

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 37e10c2587..d3d7e6c93f 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -864,12 +864,6 @@ func storagePoolVolumeTypeGet(d *Daemon, r *http.Request, volumeTypeName string)
 		return response.SmartError(err)
 	}
 
-	// Get the ID of the storage pool the storage volume is supposed to be attached to.
-	poolID, err := d.cluster.GetStoragePoolID(poolName)
-	if err != nil {
-		return response.SmartError(err)
-	}
-
 	resp := forwardedResponseIfTargetIsRemote(d, r)
 	if resp != nil {
 		return resp
@@ -880,13 +874,19 @@ func storagePoolVolumeTypeGet(d *Daemon, r *http.Request, volumeTypeName string)
 		return resp
 	}
 
+	// Get the ID of the storage pool the storage volume is supposed to be attached to.
+	poolID, err := d.cluster.GetStoragePoolID(poolName)
+	if err != nil {
+		return response.SmartError(err)
+	}
+
 	// Get the storage volume.
 	_, volume, err := d.cluster.GetLocalStoragePoolVolume(projectName, volumeName, volumeType, poolID)
 	if err != nil {
 		return response.SmartError(err)
 	}
 
-	volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, volume.Name, volume.Type)
+	volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, volume)
 	if err != nil {
 		return response.SmartError(err)
 	}

From 00de6df32c759a768800567059d576db6b538328 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:25:53 +0000
Subject: [PATCH 17/44] lxd/storage/volumes: Updates
 storagePoolVolumeTypeDelete to use storagePoolVolumeUsedByGet with a vol arg

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage_volumes.go | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index d3d7e6c93f..5e99a4779d 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -1198,22 +1198,29 @@ func storagePoolVolumeTypeDelete(d *Daemon, r *http.Request, volumeTypeName stri
 		return response.BadRequest(fmt.Errorf("Storage volumes of type %q cannot be deleted with the storage API", volumeTypeName))
 	}
 
-	volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, volumeName, volumeTypeName)
+	// Get the storage pool the storage volume is supposed to be attached to.
+	pool, err := storagePools.GetPoolByName(d.State(), poolName)
 	if err != nil {
 		return response.SmartError(err)
 	}
 
-	if len(volumeUsedBy) > 0 {
-		if len(volumeUsedBy) != 1 || volumeType != db.StoragePoolVolumeTypeImage || volumeUsedBy[0] != fmt.Sprintf("/%s/images/%s", version.APIVersion, volumeName) {
-			return response.BadRequest(fmt.Errorf("The storage volume is still in use"))
-		}
+	// Get the storage volume.
+	_, volume, err := d.cluster.GetLocalStoragePoolVolume(projectName, volumeName, volumeType, pool.ID())
+	if err != nil {
+		return response.SmartError(err)
 	}
 
-	pool, err := storagePools.GetPoolByName(d.State(), poolName)
+	volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, volume)
 	if err != nil {
 		return response.SmartError(err)
 	}
 
+	if len(volumeUsedBy) > 0 {
+		if len(volumeUsedBy) != 1 || volumeType != db.StoragePoolVolumeTypeImage || volumeUsedBy[0] != fmt.Sprintf("/%s/images/%s", version.APIVersion, volumeName) {
+			return response.BadRequest(fmt.Errorf("The storage volume is still in use"))
+		}
+	}
+
 	switch volumeType {
 	case db.StoragePoolVolumeTypeCustom:
 		err = pool.DeleteCustomVolume(projectName, volumeName, nil)

From ac7cdde39f4f67fa1bfcc836a9c92322ccce1bf9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 13:26:13 +0000
Subject: [PATCH 18/44] lxd/storage/volumes/snapshots:
 storagePoolVolumeUsedByGet usage

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

diff --git a/lxd/storage_volumes_snapshot.go b/lxd/storage_volumes_snapshot.go
index 53218ba9e0..6fac4fbb20 100644
--- a/lxd/storage_volumes_snapshot.go
+++ b/lxd/storage_volumes_snapshot.go
@@ -223,7 +223,7 @@ func storagePoolVolumeSnapshotsTypeGet(d *Daemon, r *http.Request) response.Resp
 				continue
 			}
 
-			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, vol.Name, vol.Type)
+			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), projectName, poolName, vol)
 			if err != nil {
 				return response.SmartError(err)
 			}

From e62a41145f85d8f9168136f81e164251e3277df6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 16:36:00 +0000
Subject: [PATCH 19/44] lxd/storage/volumes/utils: Removes storagePoolVolumeAPI
 constants and converter functions

These are duplicated by constants and functions in DB package, and their existance is confusing.

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

diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index 931bbaf93b..67dc347909 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -16,49 +16,9 @@ import (
 	"github.com/lxc/lxd/shared/version"
 )
 
-// Leave the string type in here! This guarantees that go treats this is as a
-// typed string constant. Removing it causes go to treat these as untyped string
-// constants which is not what we want.
-const (
-	storagePoolVolumeAPIEndpointContainers string = "containers"
-	storagePoolVolumeAPIEndpointVMs        string = "virtual-machines"
-	storagePoolVolumeAPIEndpointImages     string = "images"
-	storagePoolVolumeAPIEndpointCustom     string = "custom"
-)
-
 var supportedVolumeTypes = []int{db.StoragePoolVolumeTypeContainer, db.StoragePoolVolumeTypeVM, db.StoragePoolVolumeTypeCustom, db.StoragePoolVolumeTypeImage}
 var supportedVolumeTypesInstances = []int{db.StoragePoolVolumeTypeContainer, db.StoragePoolVolumeTypeVM}
 
-func storagePoolVolumeTypeNameToAPIEndpoint(volumeTypeName string) (string, error) {
-	switch volumeTypeName {
-	case db.StoragePoolVolumeTypeNameContainer:
-		return storagePoolVolumeAPIEndpointContainers, nil
-	case db.StoragePoolVolumeTypeNameVM:
-		return storagePoolVolumeAPIEndpointVMs, nil
-	case db.StoragePoolVolumeTypeNameImage:
-		return storagePoolVolumeAPIEndpointImages, nil
-	case db.StoragePoolVolumeTypeNameCustom:
-		return storagePoolVolumeAPIEndpointCustom, nil
-	}
-
-	return "", fmt.Errorf("Invalid storage volume type name")
-}
-
-func storagePoolVolumeTypeToAPIEndpoint(volumeType int) (string, error) {
-	switch volumeType {
-	case db.StoragePoolVolumeTypeContainer:
-		return storagePoolVolumeAPIEndpointContainers, nil
-	case db.StoragePoolVolumeTypeVM:
-		return storagePoolVolumeAPIEndpointVMs, nil
-	case db.StoragePoolVolumeTypeImage:
-		return storagePoolVolumeAPIEndpointImages, nil
-	case db.StoragePoolVolumeTypeCustom:
-		return storagePoolVolumeAPIEndpointCustom, nil
-	}
-
-	return "", fmt.Errorf("Invalid storage volume type")
-}
-
 func storagePoolVolumeUpdateUsers(d *Daemon, projectName string, oldPoolName string, oldVolumeName string, newPoolName string, newVolumeName string) error {
 	s := d.State()
 	// update all instances

From 60c0131d64e430ff234d6a1e12ba28c755d8c833 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 16:36:56 +0000
Subject: [PATCH 20/44] lxd/patches: Recreates patchStoragePoolVolumeAPI
 constants and function for historical patches

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/patches.go | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/lxd/patches.go b/lxd/patches.go
index b213597234..d3ca662f9b 100644
--- a/lxd/patches.go
+++ b/lxd/patches.go
@@ -42,6 +42,16 @@ const (
 	patchPostDaemonStorage
 )
 
+// Leave the string type in here! This guarantees that go treats this is as a
+// typed string constant. Removing it causes go to treat these as untyped string
+// constants which is not what we want.
+const (
+	patchStoragePoolVolumeAPIEndpointContainers string = "containers"
+	patchStoragePoolVolumeAPIEndpointVMs        string = "virtual-machines"
+	patchStoragePoolVolumeAPIEndpointImages     string = "images"
+	patchStoragePoolVolumeAPIEndpointCustom     string = "custom"
+)
+
 /* Patches are one-time actions that are sometimes needed to update
    existing container configuration or move things around on the
    filesystem.
@@ -1453,8 +1463,8 @@ func upgradeFromStorageTypeLvm(name string, d *Daemon, defaultPoolName string, d
 		// permissions and ownership.
 		newContainerMntPoint := driver.GetContainerMountPoint("default", defaultPoolName, ct)
 		ctLvName := lvmNameToLVName(ct)
-		newContainerLvName := fmt.Sprintf("%s_%s", storagePoolVolumeAPIEndpointContainers, ctLvName)
-		containerLvDevPath := lvmDevPath("default", defaultPoolName, storagePoolVolumeAPIEndpointContainers, ctLvName)
+		newContainerLvName := fmt.Sprintf("%s_%s", patchStoragePoolVolumeAPIEndpointContainers, ctLvName)
+		containerLvDevPath := lvmDevPath("default", defaultPoolName, patchStoragePoolVolumeAPIEndpointContainers, ctLvName)
 		if !shared.PathExists(containerLvDevPath) {
 			oldLvDevPath := fmt.Sprintf("/dev/%s/%s", defaultPoolName, ctLvName)
 			// If the old LVM device path for the logical volume
@@ -1615,8 +1625,8 @@ func upgradeFromStorageTypeLvm(name string, d *Daemon, defaultPoolName string, d
 
 			// Make sure we use a valid lv name.
 			csLvName := lvmNameToLVName(cs)
-			newSnapshotLvName := fmt.Sprintf("%s_%s", storagePoolVolumeAPIEndpointContainers, csLvName)
-			snapshotLvDevPath := lvmDevPath("default", defaultPoolName, storagePoolVolumeAPIEndpointContainers, csLvName)
+			newSnapshotLvName := fmt.Sprintf("%s_%s", patchStoragePoolVolumeAPIEndpointContainers, csLvName)
+			snapshotLvDevPath := lvmDevPath("default", defaultPoolName, patchStoragePoolVolumeAPIEndpointContainers, csLvName)
 			if !shared.PathExists(snapshotLvDevPath) {
 				oldLvDevPath := fmt.Sprintf("/dev/%s/%s", defaultPoolName, csLvName)
 				if shared.PathExists(oldLvDevPath) {
@@ -1807,8 +1817,8 @@ func upgradeFromStorageTypeLvm(name string, d *Daemon, defaultPoolName string, d
 		}
 
 		// Rename the logical volume device.
-		newImageLvName := fmt.Sprintf("%s_%s", storagePoolVolumeAPIEndpointImages, img)
-		imageLvDevPath := lvmDevPath("default", defaultPoolName, storagePoolVolumeAPIEndpointImages, img)
+		newImageLvName := fmt.Sprintf("%s_%s", patchStoragePoolVolumeAPIEndpointImages, img)
+		imageLvDevPath := lvmDevPath("default", defaultPoolName, patchStoragePoolVolumeAPIEndpointImages, img)
 		oldLvDevPath := fmt.Sprintf("/dev/%s/%s", defaultPoolName, img)
 		// Only create logical volumes for images that have a logical
 		// volume on the pre-storage-api LXD instance. If not, we don't
@@ -2739,6 +2749,21 @@ func patchStorageApiDetectLVSize(name string, d *Daemon) error {
 			return fmt.Errorf("The \"lvm.vg_name\" key should not be empty")
 		}
 
+		storagePoolVolumeTypeNameToAPIEndpoint := func(volumeTypeName string) (string, error) {
+			switch volumeTypeName {
+			case db.StoragePoolVolumeTypeNameContainer:
+				return patchStoragePoolVolumeAPIEndpointContainers, nil
+			case db.StoragePoolVolumeTypeNameVM:
+				return patchStoragePoolVolumeAPIEndpointVMs, nil
+			case db.StoragePoolVolumeTypeNameImage:
+				return patchStoragePoolVolumeAPIEndpointImages, nil
+			case db.StoragePoolVolumeTypeNameCustom:
+				return patchStoragePoolVolumeAPIEndpointCustom, nil
+			}
+
+			return "", fmt.Errorf("Invalid storage volume type name")
+		}
+
 		for _, volume := range volumes {
 			// Make sure that config is not empty.
 			if volume.Config == nil {

From f811714e50f706f5a14f0361bba964d647d11964 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 16:37:52 +0000
Subject: [PATCH 21/44] lxd/storage/volumes: Simplifies volume type in URL in
 storagePoolVolumes routes

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage_volumes.go | 38 ++++++--------------------------------
 1 file changed, 6 insertions(+), 32 deletions(-)

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 5e99a4779d..dda161c976 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -145,40 +145,27 @@ func storagePoolVolumesGet(d *Daemon, r *http.Request) response.Response {
 
 	resultString := []string{}
 	for _, volume := range volumes {
-		apiEndpoint, err := storagePoolVolumeTypeNameToAPIEndpoint(volume.Type)
-		if err != nil {
-			return response.InternalError(err)
-		}
-
-		if apiEndpoint == storagePoolVolumeAPIEndpointContainers {
-			apiEndpoint = "container"
-		} else if apiEndpoint == storagePoolVolumeAPIEndpointVMs {
-			apiEndpoint = "virtual-machine"
-		} else if apiEndpoint == storagePoolVolumeAPIEndpointImages {
-			apiEndpoint = "image"
-		}
-
 		if !recursion {
 			volName, snapName, ok := shared.InstanceGetParentAndSnapshotName(volume.Name)
 			if ok {
 				if projectName == project.Default {
 					resultString = append(resultString,
 						fmt.Sprintf("/%s/storage-pools/%s/volumes/%s/%s/snapshots/%s",
-							version.APIVersion, poolName, apiEndpoint, volName, snapName))
+							version.APIVersion, poolName, volume.Type, volName, snapName))
 				} else {
 					resultString = append(resultString,
 						fmt.Sprintf("/%s/storage-pools/%s/volumes/%s/%s/snapshots/%s?project=%s",
-							version.APIVersion, poolName, apiEndpoint, volName, snapName, projectName))
+							version.APIVersion, poolName, volume.Type, volName, snapName, projectName))
 				}
 			} else {
 				if projectName == project.Default {
 					resultString = append(resultString,
 						fmt.Sprintf("/%s/storage-pools/%s/volumes/%s/%s",
-							version.APIVersion, poolName, apiEndpoint, volume.Name))
+							version.APIVersion, poolName, volume.Type, volume.Name))
 				} else {
 					resultString = append(resultString,
 						fmt.Sprintf("/%s/storage-pools/%s/volumes/%s/%s?project=%s",
-							version.APIVersion, poolName, apiEndpoint, volume.Name, projectName))
+							version.APIVersion, poolName, volume.Type, volume.Name, projectName))
 				}
 			}
 		} else {
@@ -240,20 +227,7 @@ func storagePoolVolumesTypeGet(d *Daemon, r *http.Request) response.Response {
 	resultMap := []*api.StorageVolume{}
 	for _, volume := range volumes {
 		if !recursion {
-			apiEndpoint, err := storagePoolVolumeTypeToAPIEndpoint(volumeType)
-			if err != nil {
-				return response.InternalError(err)
-			}
-
-			if apiEndpoint == storagePoolVolumeAPIEndpointContainers {
-				apiEndpoint = "container"
-			} else if apiEndpoint == storagePoolVolumeAPIEndpointVMs {
-				apiEndpoint = "virtual-machine"
-			} else if apiEndpoint == storagePoolVolumeAPIEndpointImages {
-				apiEndpoint = "image"
-			}
-
-			resultString = append(resultString, fmt.Sprintf("/%s/storage-pools/%s/volumes/%s/%s", version.APIVersion, poolName, apiEndpoint, volume))
+			resultString = append(resultString, fmt.Sprintf("/%s/storage-pools/%s/volumes/%s/%s", version.APIVersion, poolName, volumeTypeName, volume))
 		} else {
 			_, vol, err := d.cluster.GetLocalStoragePoolVolume(projectName, volume, volumeType, poolID)
 			if err != nil {
@@ -759,7 +733,7 @@ func storagePoolVolumeTypePostRename(d *Daemon, projectName, poolName, volumeNam
 		return response.SmartError(err)
 	}
 
-	return response.SyncResponseLocation(true, nil, fmt.Sprintf("/%s/storage-pools/%s/volumes/%s", version.APIVersion, poolName, storagePoolVolumeAPIEndpointCustom))
+	return response.SyncResponseLocation(true, nil, fmt.Sprintf("/%s/storage-pools/%s/volumes/%s", version.APIVersion, poolName, db.StoragePoolVolumeTypeNameCustom))
 }
 
 // storagePoolVolumeTypePostMove handles volume move type POST requests.

From 1f561cb8f2cfae8fdf6fa2cf5c510ccbe12f99e6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 16:39:07 +0000
Subject: [PATCH 22/44] lxd/storage/volumes/snapshot: Simplifies volume type in
 URL generation

Allows access to all volume type config for GET only.

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

diff --git a/lxd/storage_volumes_snapshot.go b/lxd/storage_volumes_snapshot.go
index 6fac4fbb20..7d56b5cefd 100644
--- a/lxd/storage_volumes_snapshot.go
+++ b/lxd/storage_volumes_snapshot.go
@@ -212,11 +212,7 @@ func storagePoolVolumeSnapshotsTypeGet(d *Daemon, r *http.Request) response.Resp
 		_, snapshotName, _ := shared.InstanceGetParentAndSnapshotName(volume.Name)
 
 		if !recursion {
-			apiEndpoint, err := storagePoolVolumeTypeToAPIEndpoint(volumeType)
-			if err != nil {
-				return response.InternalError(err)
-			}
-			resultString = append(resultString, fmt.Sprintf("/%s/storage-pools/%s/volumes/%s/%s/snapshots/%s", version.APIVersion, poolName, apiEndpoint, volumeName, snapshotName))
+			resultString = append(resultString, fmt.Sprintf("/%s/storage-pools/%s/volumes/%s/%s/snapshots/%s", version.APIVersion, poolName, volumeTypeName, volumeName, snapshotName))
 		} else {
 			_, vol, err := d.cluster.GetLocalStoragePoolVolume(projectName, volume.Name, volumeType, poolID)
 			if err != nil {
@@ -342,11 +338,6 @@ func storagePoolVolumeSnapshotTypeGet(d *Daemon, r *http.Request) response.Respo
 		return response.BadRequest(err)
 	}
 
-	// Check that the storage volume type is valid.
-	if volumeType != db.StoragePoolVolumeTypeCustom {
-		return response.BadRequest(fmt.Errorf("Invalid storage volume type %q", volumeTypeName))
-	}
-
 	projectName, err := project.StorageVolumeProject(d.State().Cluster, projectParam(r), volumeType)
 	if err != nil {
 		return response.SmartError(err)

From 39049ca30cea18f6f7b35e9c3cf17cac4517a1b2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 17:29:21 +0000
Subject: [PATCH 23/44] lxd/storage/volumes: Removes unnecessary var init in
 storagePoolVolumeTypePostMove

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

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index dda161c976..f05eec2f5a 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -738,8 +738,6 @@ func storagePoolVolumeTypePostRename(d *Daemon, projectName, poolName, volumeNam
 
 // storagePoolVolumeTypePostMove handles volume move type POST requests.
 func storagePoolVolumeTypePostMove(d *Daemon, projectName, poolName, volumeName string, volumeType int, req api.StorageVolumePost) response.Response {
-	var run func(op *operations.Operation) error
-
 	srcPool, err := storagePools.GetPoolByName(d.State(), poolName)
 	if err != nil {
 		return response.SmartError(err)
@@ -750,7 +748,7 @@ func storagePoolVolumeTypePostMove(d *Daemon, projectName, poolName, volumeName
 		return response.SmartError(err)
 	}
 
-	run = func(op *operations.Operation) error {
+	run := func(op *operations.Operation) error {
 		// Notify users of the volume that it's name is changing.
 		err := storagePoolVolumeUpdateUsers(d, projectName, poolName, volumeName, req.Pool, req.Name)
 		if err != nil {

From 818025227e5144ac91f63b55b64e6caf8fd9abba Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 21:27:20 +0000
Subject: [PATCH 24/44] lxd/storage/drivers/driver/ceph/volumes: Fix rbd device
 leak in RenameVolume

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

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index d09e3e48cf..846a393489 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -1001,42 +1001,26 @@ func (d *ceph) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Opera
 
 // RenameVolume renames a volume and its snapshots.
 func (d *ceph) RenameVolume(vol Volume, newName string, op *operations.Operation) error {
-	revert := revert.New()
-	defer revert.Fail()
-
-	_, err := d.UnmountVolume(vol, false, op)
-	if err != nil {
-		return err
-	}
-
-	err = d.rbdUnmapVolume(vol, true)
-	if err != nil {
-		return nil
-	}
-
-	revert.Add(func() { d.rbdMapVolume(vol) })
-
-	err = d.rbdRenameVolume(vol, newName)
-	if err != nil {
-		return err
-	}
+	return vol.UnmountTask(func(op *operations.Operation) error {
+		revert := revert.New()
+		defer revert.Fail()
 
-	newVol := NewVolume(d, d.name, vol.volType, vol.contentType, newName, nil, nil)
+		err := d.rbdRenameVolume(vol, newName)
+		if err != nil {
+			return err
+		}
 
-	revert.Add(func() { d.rbdRenameVolume(newVol, vol.name) })
+		newVol := NewVolume(d, d.name, vol.volType, vol.contentType, newName, nil, nil)
+		revert.Add(func() { d.rbdRenameVolume(newVol, vol.name) })
 
-	_, err = d.rbdMapVolume(newVol)
-	if err != nil {
-		return err
-	}
+		err = genericVFSRenameVolume(d, vol, newName, op)
+		if err != nil {
+			return err
+		}
 
-	err = genericVFSRenameVolume(d, vol, newName, op)
-	if err != nil {
+		revert.Success()
 		return nil
-	}
-
-	revert.Success()
-	return nil
+	}, false, op)
 }
 
 // MigrateVolume sends a volume for migration.

From 167ac007cd3d57f7bc7ef367327399e5c1b2f3e4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 21:27:49 +0000
Subject: [PATCH 25/44] lxd/storage/drivers/generic/vfs: Use revert package in
 genericVFSRenameVolume

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

diff --git a/lxd/storage/drivers/generic_vfs.go b/lxd/storage/drivers/generic_vfs.go
index 22be27d851..755c0cddb8 100644
--- a/lxd/storage/drivers/generic_vfs.go
+++ b/lxd/storage/drivers/generic_vfs.go
@@ -57,24 +57,19 @@ func genericVFSRenameVolume(d Driver, vol Volume, newVolName string, op *operati
 		return fmt.Errorf("Volume must not be a snapshot")
 	}
 
+	revert := revert.New()
+	defer revert.Fail()
+
 	// Rename the volume itself.
 	srcVolumePath := GetVolumeMountPath(d.Name(), vol.volType, vol.name)
 	dstVolumePath := GetVolumeMountPath(d.Name(), vol.volType, newVolName)
 
-	revertRename := true
 	if shared.PathExists(srcVolumePath) {
 		err := os.Rename(srcVolumePath, dstVolumePath)
 		if err != nil {
-			return errors.Wrapf(err, "Failed to rename '%s' to '%s'", srcVolumePath, dstVolumePath)
+			return errors.Wrapf(err, "Failed to rename %q to %q", srcVolumePath, dstVolumePath)
 		}
-
-		defer func() {
-			if !revertRename {
-				return
-			}
-
-			os.Rename(dstVolumePath, srcVolumePath)
-		}()
+		revert.Add(func() { os.Rename(dstVolumePath, srcVolumePath) })
 	}
 
 	// And if present, the snapshots too.
@@ -84,11 +79,12 @@ func genericVFSRenameVolume(d Driver, vol Volume, newVolName string, op *operati
 	if shared.PathExists(srcSnapshotDir) {
 		err := os.Rename(srcSnapshotDir, dstSnapshotDir)
 		if err != nil {
-			return errors.Wrapf(err, "Failed to rename '%s' to '%s'", srcSnapshotDir, dstSnapshotDir)
+			return errors.Wrapf(err, "Failed to rename %q to %q", srcSnapshotDir, dstSnapshotDir)
 		}
+		revert.Add(func() { os.Rename(dstSnapshotDir, srcSnapshotDir) })
 	}
 
-	revertRename = false
+	revert.Success()
 	return nil
 }
 

From e4868260d481a2c4dd1b12f3c83bed74fc44f4ae Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 22:24:36 +0000
Subject: [PATCH 26/44] lxd/storage/utils: Adds matching of instances on same
 node as local volume in VolumeUsedByInstances

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index b48c65c1dc..701be0a545 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -706,6 +706,12 @@ func VolumeUsedByInstances(s *state.State, poolName string, projectName string,
 	volumeNameWithType := fmt.Sprintf("%s/%s", vol.Type, vol.Name)
 
 	return s.Cluster.InstanceList(func(inst db.Instance, p api.Project, profiles []api.Profile) error {
+		//If the volume has a specific cluster member which is different than the instance then skip as
+		// instance cannot be using this volume.
+		if vol.Location != "" && inst.Node != vol.Location {
+			return nil
+		}
+
 		instStorageProject := project.StorageVolumeProjectFromRecord(&p, volumeType)
 		if err != nil {
 			return err

From dadf80ca52d6c81a377d7a04fbecb842dd4ebe7d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 23:05:40 +0000
Subject: [PATCH 27/44] lxd/storage/volume: Removes need for loading storage
 volume when doing lxc storage volume attach

Avoids the need for a --target argument when a volume exists on multiple members, as now request is routed to the member that has the instance.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxc/storage_volume.go | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/lxc/storage_volume.go b/lxc/storage_volume.go
index 27f077d4e6..4b60ebe273 100644
--- a/lxc/storage_volume.go
+++ b/lxc/storage_volume.go
@@ -194,22 +194,12 @@ func (c *cmdStorageVolumeAttach) Run(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf(i18n.G("Only \"custom\" volumes can be attached to instances"))
 	}
 
-	// Check if the requested storage volume actually exists
-	vol, _, err := resource.server.GetStoragePoolVolume(resource.name, volType, volName)
-	if err != nil {
-		return err
-	}
-
 	// Prepare the instance's device entry
 	device := map[string]string{
 		"type":   "disk",
 		"pool":   resource.name,
-		"source": vol.Name,
-	}
-
-	// Ignore path for block volumes
-	if vol.ContentType != "block" {
-		device["path"] = devPath
+		"source": volName,
+		"path":   devPath,
 	}
 
 	// Add the device to the instance

From 2083ba09b905d15bd77cd25d5947fd1684b92aa4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 23:06:38 +0000
Subject: [PATCH 28/44] lxd/device/disk: Reject path property for block disk
 devices

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

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index af9ba22f3a..58e9fa6681 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -202,8 +202,14 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 				return err
 			}
 
-			if contentType == db.StoragePoolVolumeContentTypeBlock && instConf.Type() != instancetype.VM {
-				return fmt.Errorf("Custom block volumes cannot be used on containers")
+			if contentType == db.StoragePoolVolumeContentTypeBlock {
+				if instConf.Type() != instancetype.VM {
+					return fmt.Errorf("Custom block volumes cannot be used on containers")
+				}
+
+				if d.config["path"] != "" {
+					return fmt.Errorf("Custom block volumes cannot have a path defined")
+				}
 			}
 		}
 	}

From bc40d43c4496caeff81246611f2c1af8cb92a0ff Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 11:11:34 +0000
Subject: [PATCH 29/44] lxd/device/disk:
 storagePools.VolumeUsedByInstanceDevices usage

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

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 58e9fa6681..badf7b80a1 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -1074,7 +1074,7 @@ func (d *disk) storagePoolVolumeAttachShift(projectName, poolName, volumeName st
 
 		if !shared.IsTrue(poolVolumePut.Config["security.shifted"]) {
 			volumeUsedBy := []instance.Instance{}
-			err = storagePools.VolumeUsedByInstances(d.state, poolName, projectName, volume, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+			err = storagePools.VolumeUsedByInstanceDevices(d.state, poolName, projectName, volume, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile, usedByDevices []string) error {
 				inst, err := instance.Load(d.state, db.InstanceToArgs(&dbInst), profiles)
 				if err != nil {
 					return err

From b49402966962b07e144fcc448ffdad0ed2b2e336 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 11:11:54 +0000
Subject: [PATCH 30/44] lxd/storage/backend/lxd: VolumeUsedByInstanceDevices
 usage

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 355588393a..5c0a0019f1 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -2842,7 +2842,7 @@ func (b *lxdBackend) UpdateCustomVolume(projectName string, volName string, newD
 
 		// Check for config changing that is not allowed when running instances are using it.
 		if (changedConfig["size"] != "" && !runningQuotaResize) || newConfig["security.shifted"] != curVol.Config["security.shifted"] {
-			err = VolumeUsedByInstances(b.state, b.name, projectName, curVol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+			err = VolumeUsedByInstanceDevices(b.state, b.name, projectName, curVol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile, usedByDevices []string) error {
 				inst, err := instance.Load(b.state, db.InstanceToArgs(&dbInst), profiles)
 				if err != nil {
 					return err
@@ -3272,7 +3272,7 @@ func (b *lxdBackend) RestoreCustomVolume(projectName, volName string, snapshotNa
 	}
 
 	// Check that the volume isn't in use by running instances.
-	err = VolumeUsedByInstances(b.state, b.Name(), projectName, curVol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+	err = VolumeUsedByInstanceDevices(b.state, b.Name(), projectName, curVol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile, usedByDevices []string) error {
 		inst, err := instance.Load(b.state, db.InstanceToArgs(&dbInst), profiles)
 		if err != nil {
 			return err

From aaf25e20d7fe8480a2a89c253b63aec4b24647c1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 11:12:19 +0000
Subject: [PATCH 31/44] lxd/storage/utils: Renames VolumeUsedByInstanceDevices
 and passes usedByDevices into callback function

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 701be0a545..74bd98747b 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -693,10 +693,12 @@ func InstanceContentType(inst instance.Instance) drivers.ContentType {
 	return contentType
 }
 
-// VolumeUsedByInstances finds instances using a volume (either directly or via their expanded profiles if
+// VolumeUsedByInstanceDevices finds instances using a volume (either directly or via their expanded profiles if
 // expandDevices is true) and passes them to instanceFunc for evaluation. If instanceFunc returns an error then it
 // is returned immediately. The instanceFunc is executed during a DB transaction, so DB queries are not permitted.
-func VolumeUsedByInstances(s *state.State, poolName string, projectName string, vol *api.StorageVolume, expandDevices bool, instanceFunc func(inst db.Instance, project api.Project, profiles []api.Profile) error) error {
+// The instanceFunc is provided with a instance config, project config, instance's profiles and a list of device
+// names that are using the volume.
+func VolumeUsedByInstanceDevices(s *state.State, poolName string, projectName string, vol *api.StorageVolume, expandDevices bool, instanceFunc func(inst db.Instance, project api.Project, profiles []api.Profile, usedByDevices []string) error) error {
 	// Convert the volume type name to our internal integer representation.
 	volumeType, err := VolumeTypeNameToDBType(vol.Type)
 	if err != nil {
@@ -732,9 +734,11 @@ func VolumeUsedByInstances(s *state.State, poolName string, projectName string,
 			devices = db.ExpandInstanceDevices(deviceConfig.NewDevices(inst.Devices), profiles).CloneNative()
 		}
 
+		var usedByDevices []string
+
 		// Iterate through each of the instance's devices, looking for disks in the same pool as volume.
 		// Then try and match the volume name against the instance device's "source" property.
-		for _, dev := range devices {
+		for devName, dev := range devices {
 			if dev["type"] != "disk" {
 				continue
 			}
@@ -747,10 +751,14 @@ func VolumeUsedByInstances(s *state.State, poolName string, projectName string,
 			// "container////bla" but only against "container/bla".
 			cleanSource := filepath.Clean(dev["source"])
 			if cleanSource == vol.Name || cleanSource == volumeNameWithType {
-				err = instanceFunc(inst, p, profiles)
-				if err != nil {
-					return err
-				}
+				usedByDevices = append(usedByDevices, devName)
+			}
+		}
+
+		if len(usedByDevices) > 0 {
+			err = instanceFunc(inst, p, profiles, usedByDevices)
+			if err != nil {
+				return err
 			}
 		}
 

From 9abb30fde95108721ab59137cc133b5545d64de8 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 11:12:37 +0000
Subject: [PATCH 32/44] lxd/storage/utils: VolumeUsedByInstanceDevices usage

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 74bd98747b..72bec54e2c 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -794,7 +794,7 @@ func VolumeUsedByExclusiveRemoteInstancesWithProfiles(s *state.State, poolName s
 	// Find if volume is attached to a remote instance.
 	var errAttached = fmt.Errorf("Volume is remotely attached")
 	var remoteInstance *db.Instance
-	VolumeUsedByInstances(s, poolName, projectName, vol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+	err = VolumeUsedByInstanceDevices(s, poolName, projectName, vol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile, usedByDevices []string) error {
 		if dbInst.Node != localNode {
 			remoteInstance = &dbInst
 			return errAttached // Stop the search, this volume is attached to a remote instance.

From fc8a0a6f61c06653d5b8a0c1e3aeefe74a11c31f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 11:13:13 +0000
Subject: [PATCH 33/44] lxd/storage/volumes/utils:
 storagePools.VolumeUsedByInstanceDevices usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage_volumes_utils.go | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index 67dc347909..01aa081a84 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -206,24 +206,19 @@ func storagePoolVolumeUsedByGet(s *state.State, projectName string, poolName str
 	// Look for instances using this volume.
 	volumeUsedBy := []string{}
 
-	instancesUsingVolume := []*db.Instance{}
-
 	// Pass false to expandDevices, as we only want to see instances directly using a volume, rather than their
 	// profiles using a volume.
-	err = storagePools.VolumeUsedByInstances(s, poolName, projectName, vol, false, func(inst db.Instance, project api.Project, profiles []api.Profile) error {
-		instancesUsingVolume = append(instancesUsingVolume, &inst)
-		return nil
-	})
-	if err != nil {
-		return []string{}, err
-	}
-
-	for _, inst := range instancesUsingVolume {
+	err = storagePools.VolumeUsedByInstanceDevices(s, poolName, projectName, vol, false, func(inst db.Instance, p api.Project, profiles []api.Profile, usedByDevices []string) error {
 		if inst.Project == project.Default {
 			volumeUsedBy = append(volumeUsedBy, fmt.Sprintf("/%s/instances/%s", version.APIVersion, inst.Name))
 		} else {
 			volumeUsedBy = append(volumeUsedBy, fmt.Sprintf("/%s/instances/%s?project=%s", version.APIVersion, inst.Name, inst.Project))
 		}
+
+		return nil
+	})
+	if err != nil {
+		return []string{}, err
 	}
 
 	// Look for profiles using this volume.

From 73dea878a10f225101ad0fc4924028d08e3870d1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 11:13:55 +0000
Subject: [PATCH 34/44] lxd/storage/volumes:
 storagePools.VolumeUsedByInstanceDevices usage

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

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index f05eec2f5a..47f0a6f5b9 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -650,7 +650,7 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
 	}
 
 	// Check if a running instance is using it.
-	err = storagePools.VolumeUsedByInstances(d.State(), srcPoolName, projectName, vol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile) error {
+	err = storagePools.VolumeUsedByInstanceDevices(d.State(), srcPoolName, projectName, vol, true, func(dbInst db.Instance, project api.Project, profiles []api.Profile, usedByDevices []string) error {
 		inst, err := instance.Load(d.State(), db.InstanceToArgs(&dbInst), profiles)
 		if err != nil {
 			return err

From 2732712d30f2ee4ff9d7232a62f6d072ca8c0d45 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 11:15:07 +0000
Subject: [PATCH 35/44] lxd/storage/volumes: Updates storagePoolVolumeTypePost
 to use updated storagePoolVolumeTypePostRename and
 storagePoolVolumeTypePostMove

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

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 47f0a6f5b9..e51d8de5ea 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -668,11 +668,11 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request, volumeTypeName string
 
 	// Detect a rename request.
 	if req.Pool == "" || req.Pool == srcPoolName {
-		return storagePoolVolumeTypePostRename(d, projectName, srcPoolName, volumeName, volumeType, req)
+		return storagePoolVolumeTypePostRename(d, srcPoolName, projectName, vol, req)
 	}
 
 	// Otherwise this is a move request.
-	return storagePoolVolumeTypePostMove(d, projectName, srcPoolName, volumeName, volumeType, req)
+	return storagePoolVolumeTypePostMove(d, srcPoolName, projectName, vol, req)
 }
 
 // storagePoolVolumeTypePostMigration handles volume migration type POST requests.

From f9fc909b0beafd3e2ca246df157c1441dca944ea Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 6 Nov 2020 17:29:03 +0000
Subject: [PATCH 36/44] lxd/storage/volumes: Updates
 storagePoolVolumeTypePostRename to use updated storagePoolVolumeUpdateUsers

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

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index e51d8de5ea..5b2eca1998 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -714,26 +714,31 @@ func storagePoolVolumeTypePostMigration(state *state.State, projectName, poolNam
 }
 
 // storagePoolVolumeTypePostRename handles volume rename type POST requests.
-func storagePoolVolumeTypePostRename(d *Daemon, projectName, poolName, volumeName string, volumeType int, req api.StorageVolumePost) response.Response {
-	// Notify users of the volume that it's name is changing.
-	err := storagePoolVolumeUpdateUsers(d, projectName, poolName, volumeName, req.Pool, req.Name)
+func storagePoolVolumeTypePostRename(d *Daemon, poolName string, projectName string, vol *api.StorageVolume, req api.StorageVolumePost) response.Response {
+	newVol := *vol
+	newVol.Name = req.Name
+
+	pool, err := storagePools.GetPoolByName(d.State(), poolName)
 	if err != nil {
 		return response.SmartError(err)
 	}
 
-	pool, err := storagePools.GetPoolByName(d.State(), poolName)
+	revert := revert.New()
+	defer revert.Fail()
+
+	// Update devices using the volume in instances and profiles.
+	err = storagePoolVolumeUpdateUsers(d, projectName, pool.Name(), vol, pool.Name(), &newVol)
 	if err != nil {
 		return response.SmartError(err)
 	}
 
-	err = pool.RenameCustomVolume(projectName, volumeName, req.Name, nil)
+	err = pool.RenameCustomVolume(projectName, vol.Name, req.Name, nil)
 	if err != nil {
-		// Notify users of the volume that it's name is changing back.
-		storagePoolVolumeUpdateUsers(d, projectName, req.Pool, req.Name, poolName, volumeName)
 		return response.SmartError(err)
 	}
 
-	return response.SyncResponseLocation(true, nil, fmt.Sprintf("/%s/storage-pools/%s/volumes/%s", version.APIVersion, poolName, db.StoragePoolVolumeTypeNameCustom))
+	revert.Success()
+	return response.SyncResponseLocation(true, nil, fmt.Sprintf("/%s/storage-pools/%s/volumes/%s", version.APIVersion, pool.Name(), db.StoragePoolVolumeTypeNameCustom))
 }
 
 // storagePoolVolumeTypePostMove handles volume move type POST requests.

From 59688526be777886cbe1dd7266ef066445f9c03a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 11:16:55 +0000
Subject: [PATCH 37/44] lxd/storage/volumes: Updates
 storagePoolVolumeTypePostMove to use updated storagePoolVolumeUpdateUsers

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

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 5b2eca1998..2c95560f3e 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -742,34 +742,45 @@ func storagePoolVolumeTypePostRename(d *Daemon, poolName string, projectName str
 }
 
 // storagePoolVolumeTypePostMove handles volume move type POST requests.
-func storagePoolVolumeTypePostMove(d *Daemon, projectName, poolName, volumeName string, volumeType int, req api.StorageVolumePost) response.Response {
-	srcPool, err := storagePools.GetPoolByName(d.State(), poolName)
+func storagePoolVolumeTypePostMove(d *Daemon, poolName string, projectName string, vol *api.StorageVolume, req api.StorageVolumePost) response.Response {
+	newVol := *vol
+	newVol.Name = req.Name
+
+	pool, err := storagePools.GetPoolByName(d.State(), poolName)
 	if err != nil {
 		return response.SmartError(err)
 	}
 
-	pool, err := storagePools.GetPoolByName(d.State(), req.Pool)
+	newPool, err := storagePools.GetPoolByName(d.State(), req.Pool)
 	if err != nil {
 		return response.SmartError(err)
 	}
 
 	run := func(op *operations.Operation) error {
-		// Notify users of the volume that it's name is changing.
-		err := storagePoolVolumeUpdateUsers(d, projectName, poolName, volumeName, req.Pool, req.Name)
+		revert := revert.New()
+		defer revert.Fail()
+
+		// Update devices using the volume in instances and profiles.
+		err = storagePoolVolumeUpdateUsers(d, projectName, pool.Name(), vol, newPool.Name(), &newVol)
 		if err != nil {
 			return err
 		}
+		revert.Add(func() { storagePoolVolumeUpdateUsers(d, projectName, newPool.Name(), &newVol, pool.Name(), vol) })
 
-		// Provide empty description and nil config to instruct
-		// CreateCustomVolumeFromCopy to copy it from source volume.
-		err = pool.CreateCustomVolumeFromCopy(projectName, req.Name, "", nil, poolName, volumeName, false, op)
+		// Provide empty description and nil config to instruct CreateCustomVolumeFromCopy to copy it
+		// from source volume.
+		err = newPool.CreateCustomVolumeFromCopy(projectName, newVol.Name, "", nil, pool.Name(), vol.Name, false, op)
 		if err != nil {
-			// Notify users of the volume that it's name is changing back.
-			storagePoolVolumeUpdateUsers(d, projectName, req.Pool, req.Name, poolName, volumeName)
 			return err
 		}
 
-		return srcPool.DeleteCustomVolume(projectName, volumeName, op)
+		err = pool.DeleteCustomVolume(projectName, vol.Name, op)
+		if err != nil {
+			return err
+		}
+
+		revert.Success()
+		return nil
 	}
 
 	op, err := operations.OperationCreate(d.State(), "", operations.OperationClassTask, db.OperationVolumeMove, nil, nil, run, nil, nil)

From d20618016800282f2bb0c63312b7616653a56a99 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 13:49:10 +0000
Subject: [PATCH 38/44] lxd/instance/drivers/driver/lxc: Removes common
 function LocalDevices implemented in LXC driver

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

diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go
index 31d1961697..faf251a23e 100644
--- a/lxd/instance/drivers/driver_lxc.go
+++ b/lxd/instance/drivers/driver_lxc.go
@@ -6726,11 +6726,6 @@ func (c *lxc) LocalConfig() map[string]string {
 	return c.localConfig
 }
 
-// LocalDevices returns local device config.
-func (c *lxc) LocalDevices() deviceConfig.Devices {
-	return c.localDevices
-}
-
 // CurrentIdmap returns current IDMAP.
 func (c *lxc) CurrentIdmap() (*idmap.IdmapSet, error) {
 	jsonIdmap, ok := c.LocalConfig()["volatile.idmap.current"]

From 2b467ec7251d901e15215bca1f15f54902759a84 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 13:57:18 +0000
Subject: [PATCH 39/44] lxd/db/instances: Better errors in InstanceList

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/db/instances.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lxd/db/instances.go b/lxd/db/instances.go
index 8b876736e1..f4f46107cf 100644
--- a/lxd/db/instances.go
+++ b/lxd/db/instances.go
@@ -334,12 +334,12 @@ func (c *Cluster) InstanceList(instanceFunc func(inst Instance, project api.Proj
 		var err error
 		instances, err = tx.GetInstances(InstanceFilter{Type: instancetype.Any})
 		if err != nil {
-			return errors.Wrap(err, "Load instances")
+			return errors.Wrap(err, "Failed loading instances")
 		}
 
 		projects, err := tx.GetProjects(ProjectFilter{})
 		if err != nil {
-			return errors.Wrap(err, "Load projects")
+			return errors.Wrap(err, "Failed loading projects")
 		}
 
 		// Index of all projects by name and record which projects have the profiles feature.
@@ -350,7 +350,7 @@ func (c *Cluster) InstanceList(instanceFunc func(inst Instance, project api.Proj
 
 		profiles, err := tx.GetProfiles(ProfileFilter{})
 		if err != nil {
-			return errors.Wrap(err, "Load profiles")
+			return errors.Wrap(err, "Failed loading profiles")
 		}
 
 		// Index of all profiles by project and name.

From 143e061f1113b87d59342bb5f6c578b258ccca4f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 13:57:41 +0000
Subject: [PATCH 40/44] lxd/storage/utils: Adds VolumeUsedByProfileDevices
 function

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 72bec54e2c..c6abed7e6b 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -693,6 +693,89 @@ func InstanceContentType(inst instance.Instance) drivers.ContentType {
 	return contentType
 }
 
+// VolumeUsedByProfileDevices finds profiles using a volume and passes them to profileFunc for evaluation.
+// The profileFunc is provided with a profile config, project config and a list of device names that are using
+// the volume.
+func VolumeUsedByProfileDevices(s *state.State, poolName string, projectName string, vol *api.StorageVolume, profileFunc func(profile db.Profile, project api.Project, usedByDevices []string) error) error {
+	// Convert the volume type name to our internal integer representation.
+	volumeType, err := VolumeTypeNameToDBType(vol.Type)
+	if err != nil {
+		return err
+	}
+
+	projectMap := map[string]api.Project{}
+	var profiles []db.Profile
+
+	// Retrieve required info from the database in single transaction for performance.
+	err = s.Cluster.Transaction(func(tx *db.ClusterTx) error {
+		var err error
+
+		projects, err := tx.GetProjects(db.ProjectFilter{})
+		if err != nil {
+			return errors.Wrap(err, "Failed loading projects")
+		}
+
+		// Index of all projects by name.
+		for i, project := range projects {
+			projectMap[project.Name] = projects[i]
+		}
+
+		profiles, err = tx.GetProfiles(db.ProfileFilter{})
+		if err != nil {
+			return errors.Wrap(err, "Failed loading profiles")
+		}
+
+		return nil
+	})
+	if err != nil {
+		return err
+	}
+
+	// Iterate all profiles, consider only those which belong to a project that has the same effective
+	// storage project as volume.
+	for _, profile := range profiles {
+		p := projectMap[profile.Project]
+		profileStorageProject := project.StorageVolumeProjectFromRecord(&p, volumeType)
+		if err != nil {
+			return err
+		}
+
+		// Check profile's storage project is the same as the volume's project.
+		// If not then the volume names mentioned in the profile's config cannot be referring to volumes
+		// in the volume's project we are trying to match, and this profile cannot possibly be using it.
+		if projectName != profileStorageProject {
+			continue
+		}
+
+		var usedByDevices []string
+
+		// Iterate through each of the profiles's devices, looking for disks in the same pool as volume.
+		// Then try and match the volume name against the profile device's "source" property.
+		for devName, dev := range profile.Devices {
+			if dev["type"] != "disk" {
+				continue
+			}
+
+			if dev["pool"] != poolName {
+				continue
+			}
+
+			if dev["source"] == vol.Name {
+				usedByDevices = append(usedByDevices, devName)
+			}
+		}
+
+		if len(usedByDevices) > 0 {
+			err = profileFunc(profile, p, usedByDevices)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 // VolumeUsedByInstanceDevices finds instances using a volume (either directly or via their expanded profiles if
 // expandDevices is true) and passes them to instanceFunc for evaluation. If instanceFunc returns an error then it
 // is returned immediately. The instanceFunc is executed during a DB transaction, so DB queries are not permitted.

From 570544485a441c403db1ef2051b1b942d8a208dc Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 13:57:53 +0000
Subject: [PATCH 41/44] lxd/storage/utils: Removes unused volume name matching
 logic in VolumeUsedByInstanceDevices

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index c6abed7e6b..f2e48c0ae5 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -788,8 +788,6 @@ func VolumeUsedByInstanceDevices(s *state.State, poolName string, projectName st
 		return err
 	}
 
-	volumeNameWithType := fmt.Sprintf("%s/%s", vol.Type, vol.Name)
-
 	return s.Cluster.InstanceList(func(inst db.Instance, p api.Project, profiles []api.Profile) error {
 		//If the volume has a specific cluster member which is different than the instance then skip as
 		// instance cannot be using this volume.
@@ -830,10 +828,7 @@ func VolumeUsedByInstanceDevices(s *state.State, poolName string, projectName st
 				continue
 			}
 
-			// Make sure that we don't compare against stuff like
-			// "container////bla" but only against "container/bla".
-			cleanSource := filepath.Clean(dev["source"])
-			if cleanSource == vol.Name || cleanSource == volumeNameWithType {
+			if dev["source"] == vol.Name {
 				usedByDevices = append(usedByDevices, devName)
 			}
 		}

From b1b0630210725742dce09f34ec00f76af63f4bb2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 13:59:20 +0000
Subject: [PATCH 42/44] lxd/storage/volumes/utils: Updates
 storagePoolVolumeUpdateUsers to use storagePools.VolumeUsedByProfileDevices
 and storagePools.VolumeUsedByInstanceDevices

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage_volumes_utils.go | 133 ++++++++---------------------------
 1 file changed, 28 insertions(+), 105 deletions(-)

diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index 01aa081a84..aaf36e5fe8 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"fmt"
-	"path/filepath"
 	"strings"
 
 	"github.com/lxc/lxd/lxd/backup"
@@ -19,67 +18,29 @@ import (
 var supportedVolumeTypes = []int{db.StoragePoolVolumeTypeContainer, db.StoragePoolVolumeTypeVM, db.StoragePoolVolumeTypeCustom, db.StoragePoolVolumeTypeImage}
 var supportedVolumeTypesInstances = []int{db.StoragePoolVolumeTypeContainer, db.StoragePoolVolumeTypeVM}
 
-func storagePoolVolumeUpdateUsers(d *Daemon, projectName string, oldPoolName string, oldVolumeName string, newPoolName string, newVolumeName string) error {
+func storagePoolVolumeUpdateUsers(d *Daemon, projectName string, oldPoolName string, oldVol *api.StorageVolume, newPoolName string, newVol *api.StorageVolume) error {
 	s := d.State()
-	// update all instances
-	insts, err := instance.LoadByProject(s, projectName)
-	if err != nil {
-		return err
-	}
-
-	for _, inst := range insts {
-		devices := inst.LocalDevices()
-		found := false
-		for k := range devices {
-			if devices[k]["type"] != "disk" {
-				continue
-			}
-
-			// Can't be a storage volume.
-			if filepath.IsAbs(devices[k]["source"]) {
-				continue
-			}
-
-			if filepath.Clean(devices[k]["pool"]) != oldPoolName {
-				continue
-			}
-
-			dir, file := filepath.Split(devices[k]["source"])
-			dir = filepath.Clean(dir)
-			if dir != db.StoragePoolVolumeTypeNameCustom {
-				continue
-			}
-
-			file = filepath.Clean(file)
-			if file != oldVolumeName {
-				continue
-			}
 
-			// found entry
-			found = true
-
-			if oldPoolName != newPoolName {
-				devices[k]["pool"] = newPoolName
-			}
-
-			if oldVolumeName != newVolumeName {
-				newSource := newVolumeName
-				if dir != "" {
-					newSource = fmt.Sprintf("%s/%s", db.StoragePoolVolumeTypeNameCustom, newVolumeName)
-				}
-				devices[k]["source"] = newSource
-			}
+	// Update all instances that are using the volume with a local (non-expanded) device.
+	err := storagePools.VolumeUsedByInstanceDevices(s, oldPoolName, projectName, oldVol, false, func(dbInst db.Instance, project api.Project, profiles []api.Profile, usedByDevices []string) error {
+		inst, err := instance.Load(s, db.InstanceToArgs(&dbInst), profiles)
+		if err != nil {
+			return err
 		}
 
-		if !found {
-			continue
+		localDevices := inst.LocalDevices()
+		for _, devName := range usedByDevices {
+			if _, exists := localDevices[devName]; exists {
+				localDevices[devName]["pool"] = newPoolName
+				localDevices[devName]["source"] = newVol.Name
+			}
 		}
 
 		args := db.InstanceArgs{
 			Architecture: inst.Architecture(),
 			Description:  inst.Description(),
 			Config:       inst.LocalConfig(),
-			Devices:      devices,
+			Devices:      localDevices,
 			Ephemeral:    inst.IsEphemeral(),
 			Profiles:     inst.Profiles(),
 			Project:      inst.Project(),
@@ -91,74 +52,36 @@ func storagePoolVolumeUpdateUsers(d *Daemon, projectName string, oldPoolName str
 		if err != nil {
 			return err
 		}
-	}
 
-	// update all profiles
-	profiles, err := s.Cluster.GetProfileNames(project.Default)
+		return nil
+	})
 	if err != nil {
 		return err
 	}
 
-	for _, pName := range profiles {
-		id, profile, err := s.Cluster.GetProfile(project.Default, pName)
-		if err != nil {
-			return err
-		}
-
-		found := false
-		for k := range profile.Devices {
-			if profile.Devices[k]["type"] != "disk" {
-				continue
-			}
-
-			// Can't be a storage volume.
-			if filepath.IsAbs(profile.Devices[k]["source"]) {
-				continue
-			}
-
-			if filepath.Clean(profile.Devices[k]["pool"]) != oldPoolName {
-				continue
-			}
-
-			dir, file := filepath.Split(profile.Devices[k]["source"])
-			dir = filepath.Clean(dir)
-			if dir != db.StoragePoolVolumeTypeNameCustom {
-				continue
-			}
-
-			file = filepath.Clean(file)
-			if file != oldVolumeName {
-				continue
-			}
-
-			// found entry
-			found = true
-
-			if oldPoolName != newPoolName {
-				profile.Devices[k]["pool"] = newPoolName
-			}
-
-			if oldVolumeName != newVolumeName {
-				newSource := newVolumeName
-				if dir != "" {
-					newSource = fmt.Sprintf("%s/%s", db.StoragePoolVolumeTypeNameCustom, newVolumeName)
-				}
-				profile.Devices[k]["source"] = newSource
+	// Update all profiles that are using the volume with a device.
+	err = storagePools.VolumeUsedByProfileDevices(s, oldPoolName, projectName, oldVol, func(profile db.Profile, p api.Project, usedByDevices []string) error {
+		for _, devName := range usedByDevices {
+			if _, exists := profile.Devices[devName]; exists {
+				profile.Devices[devName]["pool"] = newPoolName
+				profile.Devices[devName]["source"] = newVol.Name
 			}
 		}
 
-		if !found {
-			continue
-		}
-
 		pUpdate := api.ProfilePut{}
 		pUpdate.Config = profile.Config
 		pUpdate.Description = profile.Description
 		pUpdate.Devices = profile.Devices
-		err = doProfileUpdate(d, project.Default, pName, id, profile, pUpdate)
+		apiProfile := db.ProfileToAPI(&profile)
+		err = doProfileUpdate(d, profile.Project, profile.Name, int64(profile.ID), apiProfile, pUpdate)
 		if err != nil {
 			return err
 		}
+
+		return nil
+	})
+	if err != nil {
+		return err
 	}
 
 	return nil

From 96f4751d9309a377eee58b20856a85be4e9ca9c6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 14:00:52 +0000
Subject: [PATCH 43/44] lxd/storage/volumes/utils: Updates
 storagePoolVolumeUsedByGet to use storagePools.VolumeUsedByProfileDevices

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

diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index aaf36e5fe8..d58b7eb227 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -144,59 +144,20 @@ func storagePoolVolumeUsedByGet(s *state.State, projectName string, poolName str
 		return []string{}, err
 	}
 
-	// Look for profiles using this volume.
-	profiles, err := profilesUsingPoolVolumeGetNames(s.Cluster, vol.Name, vol.Type)
-	if err != nil {
-		return []string{}, err
-	}
-
-	for _, pName := range profiles {
-		if projectName == project.Default {
-			volumeUsedBy = append(volumeUsedBy, fmt.Sprintf("/%s/profiles/%s", version.APIVersion, pName))
+	err = storagePools.VolumeUsedByProfileDevices(s, poolName, projectName, vol, func(profile db.Profile, p api.Project, usedByDevices []string) error {
+		if profile.Project == project.Default {
+			volumeUsedBy = append(volumeUsedBy, fmt.Sprintf("/%s/profiles/%s", version.APIVersion, profile.Name))
 		} else {
-			volumeUsedBy = append(volumeUsedBy, fmt.Sprintf("/%s/profiles/%s?project=%s", version.APIVersion, pName, projectName))
+			volumeUsedBy = append(volumeUsedBy, fmt.Sprintf("/%s/profiles/%s?project=%s", version.APIVersion, profile.Name, profile.Project))
 		}
-	}
 
-	return volumeUsedBy, nil
-}
-
-func profilesUsingPoolVolumeGetNames(db *db.Cluster, volumeName string, volumeType string) ([]string, error) {
-	usedBy := []string{}
-
-	profiles, err := db.GetProfileNames(project.Default)
+		return nil
+	})
 	if err != nil {
-		return usedBy, err
-	}
-
-	for _, pName := range profiles {
-		_, profile, err := db.GetProfile(project.Default, pName)
-		if err != nil {
-			return usedBy, err
-		}
-
-		volumeNameWithType := fmt.Sprintf("%s/%s", volumeType, volumeName)
-		for _, v := range profile.Devices {
-			if v["type"] != "disk" {
-				continue
-			}
-
-			// Can't be a storage volume.
-			if filepath.IsAbs(v["source"]) {
-				continue
-			}
-
-			// Make sure that we don't compare against stuff
-			// like "container////bla" but only against
-			// "container/bla".
-			cleanSource := filepath.Clean(v["source"])
-			if cleanSource == volumeName || cleanSource == volumeNameWithType {
-				usedBy = append(usedBy, pName)
-			}
-		}
+		return []string{}, err
 	}
 
-	return usedBy, nil
+	return volumeUsedBy, nil
 }
 
 func storagePoolVolumeBackupLoadByName(s *state.State, projectName, poolName, backupName string) (*backup.VolumeBackup, error) {

From f38b9b2379a64c2da59b3c8bd07392da7011bff1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 14:01:06 +0000
Subject: [PATCH 44/44] lxd/storage/volumes/utils: Golint suggestions in
 storagePoolVolumeUsedByGet

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

diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index d58b7eb227..a66d57f454 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -95,25 +95,25 @@ func storagePoolVolumeUsedByGet(s *state.State, projectName string, poolName str
 		if snap {
 			if projectName == project.Default {
 				return []string{fmt.Sprintf("/%s/instances/%s/snapshots/%s", version.APIVersion, cName, sName)}, nil
-			} else {
-				return []string{fmt.Sprintf("/%s/instances/%s/snapshots/%s?project=%s", version.APIVersion, cName, sName, projectName)}, nil
 			}
+
+			return []string{fmt.Sprintf("/%s/instances/%s/snapshots/%s?project=%s", version.APIVersion, cName, sName, projectName)}, nil
 		}
 
 		if projectName == project.Default {
 			return []string{fmt.Sprintf("/%s/instances/%s", version.APIVersion, cName)}, nil
-		} else {
-			return []string{fmt.Sprintf("/%s/instances/%s?project=%s", version.APIVersion, cName, projectName)}, nil
 		}
+
+		return []string{fmt.Sprintf("/%s/instances/%s?project=%s", version.APIVersion, cName, projectName)}, nil
 	}
 
 	// Handle image volumes.
 	if vol.Type == db.StoragePoolVolumeTypeNameImage {
 		if projectName == project.Default {
 			return []string{fmt.Sprintf("/%s/images/%s", version.APIVersion, vol.Name)}, nil
-		} else {
-			return []string{fmt.Sprintf("/%s/images/%s?project=%s", version.APIVersion, vol.Name, projectName)}, nil
 		}
+
+		return []string{fmt.Sprintf("/%s/images/%s?project=%s", version.APIVersion, vol.Name, projectName)}, nil
 	}
 
 	// Check if the daemon itself is using it.