[lxc-devel] [lxd/master] lxd/storage: Fix bad UsedBy check

stgraber on Github lxc-bot at linuxcontainers.org
Wed Aug 14 02:51:57 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 354 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190813/d76232f3/attachment.bin>
-------------- next part --------------
From 436b6219af6a785d6891b4c0ef10467f5d9070fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 13 Aug 2019 22:45:35 -0400
Subject: [PATCH] lxd/storage: Fix bad UsedBy check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: St├ęphane Graber <stgraber at ubuntu.com>
---
 lxd/storage.go                  |  9 +--------
 lxd/storage_btrfs.go            |  2 +-
 lxd/storage_ceph.go             |  2 +-
 lxd/storage_cephfs.go           |  2 +-
 lxd/storage_dir.go              |  2 +-
 lxd/storage_lvm.go              |  2 +-
 lxd/storage_volumes.go          |  8 ++++----
 lxd/storage_volumes_snapshot.go |  2 +-
 lxd/storage_volumes_utils.go    | 17 ++++++-----------
 lxd/storage_zfs.go              |  2 +-
 10 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/lxd/storage.go b/lxd/storage.go
index a880b1aae4..56ef5c11ea 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -469,18 +469,11 @@ func storagePoolVolumeAttachInit(s *state.State, poolName string, volumeName str
 	// get mountpoint of storage volume
 	remapPath := getStoragePoolVolumeMountPoint(poolName, volumeName)
 
-	// Convert the volume type name to our internal integer representation.
-	volumeTypeName, err := storagePoolVolumeTypeToName(volumeType)
-	if err != nil {
-		return nil, err
-	}
-
 	if !nextIdmap.Equals(lastIdmap) {
 		logger.Debugf("Shifting storage volume")
 
 		if !shared.IsTrue(poolVolumePut.Config["security.shifted"]) {
-			volumeUsedBy, err := storagePoolVolumeUsedByContainersGet(s,
-				"default", volumeName, volumeTypeName)
+			volumeUsedBy, err := storagePoolVolumeUsedByContainersGet(s, "default", poolName, volumeName)
 			if err != nil {
 				return nil, err
 			}
diff --git a/lxd/storage_btrfs.go b/lxd/storage_btrfs.go
index e4b0328ed5..043b2f7aad 100644
--- a/lxd/storage_btrfs.go
+++ b/lxd/storage_btrfs.go
@@ -743,7 +743,7 @@ func (s *storageBtrfs) StoragePoolVolumeRename(newName string) error {
 		return err
 	}
 
-	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.volume.Name, storagePoolVolumeTypeNameCustom)
+	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.pool.Name, s.volume.Name)
 	if err != nil {
 		return err
 	}
diff --git a/lxd/storage_ceph.go b/lxd/storage_ceph.go
index ec4bd72736..e603190047 100644
--- a/lxd/storage_ceph.go
+++ b/lxd/storage_ceph.go
@@ -663,7 +663,7 @@ func (s *storageCeph) StoragePoolVolumeRename(newName string) error {
 		return err
 	}
 
-	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.volume.Name, storagePoolVolumeTypeNameCustom)
+	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.pool.Name, s.volume.Name)
 	if err != nil {
 		return err
 	}
diff --git a/lxd/storage_cephfs.go b/lxd/storage_cephfs.go
index f95933c3f1..7f4d5f86ec 100644
--- a/lxd/storage_cephfs.go
+++ b/lxd/storage_cephfs.go
@@ -584,7 +584,7 @@ func (s *storageCephFs) StoragePoolVolumeRename(newName string) error {
 	logger.Infof(`Renaming CEPHFS storage volume on storage pool "%s" from "%s" to "%s`, s.pool.Name, s.volume.Name, newName)
 
 	// Sanity check
-	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.volume.Name, storagePoolVolumeTypeNameCustom)
+	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.pool.Name, s.volume.Name)
 	if err != nil {
 		return err
 	}
diff --git a/lxd/storage_dir.go b/lxd/storage_dir.go
index 87e5261ee4..967a4656f1 100644
--- a/lxd/storage_dir.go
+++ b/lxd/storage_dir.go
@@ -464,7 +464,7 @@ func (s *storageDir) StoragePoolVolumeRename(newName string) error {
 		return err
 	}
 
-	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.volume.Name, storagePoolVolumeTypeNameCustom)
+	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.pool.Name, s.volume.Name)
 	if err != nil {
 		return err
 	}
diff --git a/lxd/storage_lvm.go b/lxd/storage_lvm.go
index eac2d7780c..d805035435 100644
--- a/lxd/storage_lvm.go
+++ b/lxd/storage_lvm.go
@@ -877,7 +877,7 @@ func (s *storageLvm) StoragePoolVolumeRename(newName string) error {
 		return err
 	}
 
-	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.volume.Name, storagePoolVolumeTypeNameCustom)
+	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.pool.Name, s.volume.Name)
 	if err != nil {
 		return err
 	}
diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 771b8ac670..38250235d0 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -134,7 +134,7 @@ func storagePoolVolumesGet(d *Daemon, r *http.Request) Response {
 						version.APIVersion, poolName, apiEndpoint, volume.Name))
 			}
 		} else {
-			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), project, volume.Name, volume.Type)
+			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), project, poolName, volume.Name, volume.Type)
 			if err != nil {
 				return InternalError(err)
 			}
@@ -209,7 +209,7 @@ func storagePoolVolumesTypeGet(d *Daemon, r *http.Request) Response {
 				continue
 			}
 
-			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), project, vol.Name, vol.Type)
+			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), project, poolName, vol.Name, vol.Type)
 			if err != nil {
 				return SmartError(err)
 			}
@@ -703,7 +703,7 @@ func storagePoolVolumeTypeGet(d *Daemon, r *http.Request, volumeTypeName string)
 		return SmartError(err)
 	}
 
-	volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), project, volume.Name, volume.Type)
+	volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), project, poolName, volume.Name, volume.Type)
 	if err != nil {
 		return SmartError(err)
 	}
@@ -995,7 +995,7 @@ func storagePoolVolumeTypeDelete(d *Daemon, r *http.Request, volumeTypeName stri
 		return BadRequest(fmt.Errorf("storage volumes of type \"%s\" cannot be deleted with the storage api", volumeTypeName))
 	}
 
-	volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), project, volumeName, volumeTypeName)
+	volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), project, poolName, volumeName, volumeTypeName)
 	if err != nil {
 		return SmartError(err)
 	}
diff --git a/lxd/storage_volumes_snapshot.go b/lxd/storage_volumes_snapshot.go
index 17acfc403d..3b2b2cfa10 100644
--- a/lxd/storage_volumes_snapshot.go
+++ b/lxd/storage_volumes_snapshot.go
@@ -202,7 +202,7 @@ func storagePoolVolumeSnapshotsTypeGet(d *Daemon, r *http.Request) Response {
 				continue
 			}
 
-			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), "default", vol.Name, vol.Type)
+			volumeUsedBy, err := storagePoolVolumeUsedByGet(d.State(), "default", poolName, vol.Name, vol.Type)
 			if err != nil {
 				return SmartError(err)
 			}
diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index ca695152c7..db0bb12221 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -241,26 +241,22 @@ func storagePoolVolumeSnapshotUpdate(state *state.State, poolName string, volume
 	return nil
 }
 
-func storagePoolVolumeUsedByContainersGet(s *state.State, project, volumeName string,
-	volumeTypeName string) ([]string, error) {
+func storagePoolVolumeUsedByContainersGet(s *state.State, project, poolName string, volumeName string) ([]string, error) {
 	cts, err := containerLoadByProject(s, project)
 	if err != nil {
 		return []string{}, err
 	}
 
 	ctsUsingVolume := []string{}
-	volumeNameWithType := fmt.Sprintf("%s/%s", volumeTypeName, volumeName)
 	for _, c := range cts {
-		for _, dev := range c.LocalDevices() {
+		for _, dev := range c.ExpandedDevices() {
 			if dev["type"] != "disk" {
 				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 == volumeName || cleanSource == volumeNameWithType {
+			if dev["pool"] == poolName && dev["source"] == volumeName {
 				ctsUsingVolume = append(ctsUsingVolume, c.Name())
+				break
 			}
 		}
 	}
@@ -438,7 +434,7 @@ func storagePoolVolumeUsedByRunningContainersWithProfilesGet(s *state.State,
 }
 
 // volumeUsedBy = append(volumeUsedBy, fmt.Sprintf("/%s/containers/%s", version.APIVersion, ct))
-func storagePoolVolumeUsedByGet(s *state.State, project, volumeName string, volumeTypeName string) ([]string, error) {
+func storagePoolVolumeUsedByGet(s *state.State, project, poolName string, volumeName string, volumeTypeName string) ([]string, error) {
 	// Handle container volumes
 	if volumeTypeName == "container" {
 		cName, sName, snap := containerGetParentAndSnapshotName(volumeName)
@@ -456,8 +452,7 @@ func storagePoolVolumeUsedByGet(s *state.State, project, volumeName string, volu
 	}
 
 	// Look for containers using this volume
-	ctsUsingVolume, err := storagePoolVolumeUsedByContainersGet(s,
-		project, volumeName, volumeTypeName)
+	ctsUsingVolume, err := storagePoolVolumeUsedByContainersGet(s, project, poolName, volumeName)
 	if err != nil {
 		return []string{}, err
 	}
diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 1d1a892b91..5d9500b9b8 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -724,7 +724,7 @@ func (s *storageZfs) StoragePoolVolumeRename(newName string) error {
 	logger.Infof(`Renaming ZFS storage volume on storage pool "%s" from "%s" to "%s`,
 		s.pool.Name, s.volume.Name, newName)
 
-	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.volume.Name, storagePoolVolumeTypeNameCustom)
+	usedBy, err := storagePoolVolumeUsedByContainersGet(s.s, "default", s.pool.Name, s.volume.Name)
 	if err != nil {
 		return err
 	}


More information about the lxc-devel mailing list