[lxc-devel] [lxd/master] Storage: Adds volume mount & unmount locking

tomponline on Github lxc-bot at linuxcontainers.org
Tue Nov 10 11:18:51 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201110/9df5ef72/attachment-0001.bin>
-------------- next part --------------
From 1f462d7515ecf46ab670d30c5604dd55f0ca6b66 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 17:23:29 +0000
Subject: [PATCH 1/7] lxd/locking/lock: Adds UnlockFunc type and updates Lock()
 signature

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

diff --git a/lxd/locking/lock.go b/lxd/locking/lock.go
index 02c0ec992d..054d60c174 100644
--- a/lxd/locking/lock.go
+++ b/lxd/locking/lock.go
@@ -13,10 +13,13 @@ var locks = map[string]chan struct{}{}
 // locksMutex is used to access locks safely.
 var locksMutex sync.Mutex
 
+// UnlockFunc unlocks the lock.
+type UnlockFunc func()
+
 // Lock creates a lock for a specific storage volume to allow activities that require exclusive access to occur.
 // Will block until the lock is established. On success, it returns an unlock function which needs to be called to
 // unlock the lock.
-func Lock(lockName string) func() {
+func Lock(lockName string) UnlockFunc {
 	for {
 		// Get exclusive access to the map and see if there is already an operation ongoing.
 		locksMutex.Lock()

From 84c6e7f0974e90827159acef204f4456803cb416 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 17:24:31 +0000
Subject: [PATCH 2/7] lxd/storage/drivers/utils: Extends OperationLockName to
 take into account content type.

Also re-arranges arguments to be hierarchical and to include specific types where possible to avoid confusion.

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

diff --git a/lxd/storage/drivers/utils.go b/lxd/storage/drivers/utils.go
index de985134bb..09e10212b7 100644
--- a/lxd/storage/drivers/utils.go
+++ b/lxd/storage/drivers/utils.go
@@ -812,6 +812,6 @@ func PathNameDecode(text string) string {
 }
 
 // OperationLockName returns the storage specific lock name to use with locking package.
-func OperationLockName(poolName string, volType string, volName string) string {
-	return fmt.Sprintf("%s/%s/%s", poolName, volType, volName)
+func OperationLockName(operationName string, poolName string, volType VolumeType, contentType ContentType, volName string) string {
+	return fmt.Sprintf("%s/%s/%s/%s/%s", operationName, poolName, volType, contentType, volName)
 }

From 865cc5611ee4d7194c684d4ad2dd7653f7ce99a9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 17:25:49 +0000
Subject: [PATCH 3/7] lxd/storage/drivers/volume: Adds MountLock function

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

diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go
index 16296a84bc..3ced59a0e0 100644
--- a/lxd/storage/drivers/volume.go
+++ b/lxd/storage/drivers/volume.go
@@ -134,6 +134,11 @@ func (v Volume) MountPath() string {
 	return GetVolumeMountPath(v.pool, v.volType, v.name)
 }
 
+// MountLock attempts to lock the mount lock for the volume and returns the UnlockFunc.
+func (v Volume) MountLock() locking.UnlockFunc {
+	return locking.Lock(OperationLockName("MountLock", v.pool, v.volType, v.contentType, v.name))
+}
+
 // EnsureMountPath creates the volume's mount path if missing, then sets the correct permission for the type.
 // If permission setting fails and the volume is a snapshot then the error is ignored as snapshots are read only.
 func (v Volume) EnsureMountPath() error {

From ca8f79852d7f4b6b96166ec839eb36bf690784ba Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 17:26:16 +0000
Subject: [PATCH 4/7] lxd/storage/drivers/driver/lvm/utils:
 drivers.OperationLockName usage

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

diff --git a/lxd/storage/drivers/driver_lvm_utils.go b/lxd/storage/drivers/driver_lvm_utils.go
index ee81d9e021..9d4b2bed68 100644
--- a/lxd/storage/drivers/driver_lvm_utils.go
+++ b/lxd/storage/drivers/driver_lvm_utils.go
@@ -56,7 +56,7 @@ func (d *lvm) openLoopFile(source string) (*os.File, error) {
 	}
 
 	if filepath.IsAbs(source) && !shared.IsBlockdevPath(source) {
-		unlock := locking.Lock(OperationLockName(d.name, "", ""))
+		unlock := locking.Lock(OperationLockName("openLoopFile", d.name, "", "", ""))
 		defer unlock()
 
 		// Try to prepare new loop device.

From 5df12c19e18dacd2603b688a082f61ada6bd73c5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 17:27:08 +0000
Subject: [PATCH 5/7] lxd/storage/backend/lxd: drivers.OperationLockName usage

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 5c0a0019f1..4d323ef8cb 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -2091,7 +2091,7 @@ func (b *lxdBackend) EnsureImage(fingerprint string, op *operations.Operation) e
 	// We need to lock this operation to ensure that the image is not being created multiple times.
 	// Uses a lock name of "EnsureImage_<fingerprint>" to avoid deadlocking with CreateVolume below that also
 	// establishes a lock on the volume type & name if it needs to mount the volume before filling.
-	unlock := locking.Lock(drivers.OperationLockName(b.name, string(drivers.VolumeTypeImage), fmt.Sprintf("EnsureImage_%v", fingerprint)))
+	unlock := locking.Lock(drivers.OperationLockName("EnsureImage", b.name, drivers.VolumeTypeImage, "", fingerprint))
 	defer unlock()
 
 	// Load image info from database.
@@ -2224,9 +2224,8 @@ func (b *lxdBackend) DeleteImage(fingerprint string, op *operations.Operation) e
 	logger.Debug("DeleteImage started")
 	defer logger.Debug("DeleteImage finished")
 
-	// We need to lock this operation to ensure that the image is not being
-	// deleted multiple times.
-	unlock := locking.Lock(drivers.OperationLockName(b.name, string(drivers.VolumeTypeImage), fingerprint))
+	// We need to lock this operation to ensure that the image is not being deleted multiple times.
+	unlock := locking.Lock(drivers.OperationLockName("DeleteImage", b.name, drivers.VolumeTypeImage, "", fingerprint))
 	defer unlock()
 
 	// Load image info from database.

From 547def947197373ae9cfdafe3027a71bbb1ed337 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 17:28:48 +0000
Subject: [PATCH 6/7] lxd/storage/drivers: Adds mount and unmount locking

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_btrfs_volumes.go  | 12 ++++++++
 lxd/storage/drivers/driver_ceph_volumes.go   | 12 ++++++++
 lxd/storage/drivers/driver_cephfs_volumes.go | 12 ++++++++
 lxd/storage/drivers/driver_dir_volumes.go    | 12 ++++++++
 lxd/storage/drivers/driver_lvm_volumes.go    | 12 ++++++++
 lxd/storage/drivers/driver_zfs_volumes.go    | 30 ++++++++++++++------
 6 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go
index c9bafdef88..e695b1e20c 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -758,6 +758,9 @@ func (d *btrfs) GetVolumeDiskPath(vol Volume) (string, error) {
 // MountVolume simulates mounting a volume. As the driver doesn't have volumes to mount it returns
 // false indicating that there is no need to issue an unmount.
 func (d *btrfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	// Don't attempt to modify the permission of an existing custom volume root.
 	// A user inside the instance may have modified this and we don't want to reset it on restart.
 	if !shared.PathExists(vol.MountPath()) || vol.volType != VolumeTypeCustom {
@@ -773,6 +776,9 @@ func (d *btrfs) MountVolume(vol Volume, op *operations.Operation) (bool, error)
 // UnmountVolume simulates unmounting a volume.
 // As driver doesn't have volumes to unmount it returns false indicating the volume was already unmounted.
 func (d *btrfs) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	return false, nil
 }
 
@@ -1276,6 +1282,9 @@ func (d *btrfs) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) e
 
 // MountVolumeSnapshot sets up a read-only mount on top of the snapshot to avoid accidental modifications.
 func (d *btrfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	snapPath := snapVol.MountPath()
 
 	// Don't attempt to modify the permission of an existing custom volume root.
@@ -1292,6 +1301,9 @@ func (d *btrfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (b
 
 // UnmountVolumeSnapshot removes the read-only mount placed on top of a snapshot.
 func (d *btrfs) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	snapPath := snapVol.MountPath()
 	return forceUnmount(snapPath)
 }
diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index 846a393489..dac36c8047 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -925,6 +925,9 @@ func (d *ceph) GetVolumeDiskPath(vol Volume) (string, error) {
 
 // MountVolume simulates mounting a volume.
 func (d *ceph) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	mountPath := vol.MountPath()
 
 	// Activate RBD volume if needed.
@@ -962,6 +965,9 @@ func (d *ceph) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
 // UnmountVolume simulates unmounting a volume.
 // keepBlockDev indicates if backing block device should be not be unmapped if volume is unmounted.
 func (d *ceph) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	// Attempt to unmount the volume.
 	mountPath := vol.MountPath()
 	if vol.contentType == ContentTypeFS && shared.IsMountPoint(mountPath) {
@@ -1274,6 +1280,9 @@ func (d *ceph) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) er
 
 // MountVolumeSnapshot simulates mounting a volume snapshot.
 func (d *ceph) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	mountPath := snapVol.MountPath()
 
 	if snapVol.contentType == ContentTypeFS && !shared.IsMountPoint(mountPath) {
@@ -1370,6 +1379,9 @@ func (d *ceph) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bo
 
 // UnmountVolume simulates unmounting a volume snapshot.
 func (d *ceph) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	mountPath := snapVol.MountPath()
 
 	if snapVol.contentType == ContentTypeFS && shared.IsMountPoint(mountPath) {
diff --git a/lxd/storage/drivers/driver_cephfs_volumes.go b/lxd/storage/drivers/driver_cephfs_volumes.go
index af9058d5d8..c7fb7066bd 100644
--- a/lxd/storage/drivers/driver_cephfs_volumes.go
+++ b/lxd/storage/drivers/driver_cephfs_volumes.go
@@ -339,12 +339,18 @@ func (d *cephfs) GetVolumeDiskPath(vol Volume) (string, error) {
 
 // MountVolume sets up the volume for use.
 func (d *cephfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	return false, nil
 }
 
 // UnmountVolume clears any runtime state for the volume.
 // As driver doesn't have volumes to unmount it returns false indicating the volume was already unmounted.
 func (d *cephfs) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	return false, nil
 }
 
@@ -508,11 +514,17 @@ func (d *cephfs) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation)
 
 // MountVolumeSnapshot makes the snapshot available for use.
 func (d *cephfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	return false, nil
 }
 
 // UnmountVolumeSnapshot clears any runtime state for the snapshot.
 func (d *cephfs) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	return false, nil
 }
 
diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go
index a189f908ed..f391ea841f 100644
--- a/lxd/storage/drivers/driver_dir_volumes.go
+++ b/lxd/storage/drivers/driver_dir_volumes.go
@@ -310,6 +310,9 @@ func (d *dir) GetVolumeDiskPath(vol Volume) (string, error) {
 // MountVolume simulates mounting a volume. As the driver doesn't have volumes to mount it returns
 // false indicating that there is no need to issue an unmount.
 func (d *dir) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	// Don't attempt to modify the permission of an existing custom volume root.
 	// A user inside the instance may have modified this and we don't want to reset it on restart.
 	if !shared.PathExists(vol.MountPath()) || vol.volType != VolumeTypeCustom {
@@ -325,6 +328,9 @@ func (d *dir) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
 // UnmountVolume simulates unmounting a volume.
 // As driver doesn't have volumes to unmount it returns false indicating the volume was already unmounted.
 func (d *dir) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	return false, nil
 }
 
@@ -405,6 +411,9 @@ func (d *dir) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) err
 
 // MountVolumeSnapshot sets up a read-only mount on top of the snapshot to avoid accidental modifications.
 func (d *dir) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	snapPath := snapVol.MountPath()
 
 	// Don't attempt to modify the permission of an existing custom volume root.
@@ -421,6 +430,9 @@ func (d *dir) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo
 
 // UnmountVolumeSnapshot removes the read-only mount placed on top of a snapshot.
 func (d *dir) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	snapPath := snapVol.MountPath()
 	return forceUnmount(snapPath)
 }
diff --git a/lxd/storage/drivers/driver_lvm_volumes.go b/lxd/storage/drivers/driver_lvm_volumes.go
index 518d2686a9..a96ec08ad0 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -433,6 +433,9 @@ func (d *lvm) GetVolumeDiskPath(vol Volume) (string, error) {
 
 // MountVolume mounts a volume. Returns true if this volume was our mount.
 func (d *lvm) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	// Activate LVM volume if needed.
 	volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], vol.volType, vol.contentType, vol.name)
 	activated, err := d.activateVolume(volDevPath)
@@ -470,6 +473,9 @@ func (d *lvm) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
 // UnmountVolume unmounts a volume. Returns true if we unmounted.
 // keepBlockDev indicates if backing block device should be not be deactivated if volume is unmounted.
 func (d *lvm) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	var err error
 	volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], vol.volType, vol.contentType, vol.name)
 	mountPath := vol.MountPath()
@@ -703,6 +709,9 @@ func (d *lvm) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) err
 
 // MountVolumeSnapshot sets up a read-only mount on top of the snapshot to avoid accidental modifications.
 func (d *lvm) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	var err error
 	mountPath := snapVol.MountPath()
 
@@ -810,6 +819,9 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo
 // UnmountVolumeSnapshot removes the read-only mount placed on top of a snapshot.
 // If a temporary snapshot volume exists then it will attempt to remove it.
 func (d *lvm) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	var err error
 	volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], snapVol.volType, snapVol.contentType, snapVol.name)
 	mountPath := snapVol.MountPath()
diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index 9f36b838a3..463c97114b 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -1043,6 +1043,9 @@ func (d *zfs) GetVolumeDiskPath(vol Volume) (string, error) {
 
 // MountVolume simulates mounting a volume.
 func (d *zfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	var err error
 	mountPath := vol.MountPath()
 	dataset := d.dataset(vol, false)
@@ -1109,6 +1112,9 @@ func (d *zfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
 // UnmountVolume simulates unmounting a volume.
 // keepBlockDev indicates if backing block device should be not be deactivated if volume is unmounted.
 func (d *zfs) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Operation) (bool, error) {
+	unlock := vol.MountLock()
+	defer unlock()
+
 	mountPath := vol.MountPath()
 	dataset := d.dataset(vol, false)
 
@@ -1592,6 +1598,9 @@ func (d *zfs) DeleteVolumeSnapshot(vol Volume, op *operations.Operation) error {
 
 // MountVolumeSnapshot simulates mounting a volume snapshot.
 func (d *zfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
 	var err error
 	mountPath := snapVol.MountPath()
 	snapshotDataset := d.dataset(snapVol, false)
@@ -1675,23 +1684,26 @@ func (d *zfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo
 }
 
 // UnmountVolume simulates unmounting a volume snapshot.
-func (d *zfs) UnmountVolumeSnapshot(vol Volume, op *operations.Operation) (bool, error) {
-	mountPath := vol.MountPath()
-	snapshotDataset := d.dataset(vol, false)
+func (d *zfs) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
+	unlock := snapVol.MountLock()
+	defer unlock()
+
+	mountPath := snapVol.MountPath()
+	snapshotDataset := d.dataset(snapVol, false)
 
 	// For VMs, also mount the filesystem dataset.
-	if vol.IsVMBlock() {
-		fsVol := vol.NewVMBlockFilesystemVolume()
-		_, err := d.UnmountVolumeSnapshot(fsVol, op)
+	if snapVol.IsVMBlock() {
+		fsSnapVol := snapVol.NewVMBlockFilesystemVolume()
+		_, err := d.UnmountVolumeSnapshot(fsSnapVol, op)
 		if err != nil {
 			return false, err
 		}
 	}
 
 	// For block devices, we make them disappear.
-	if vol.contentType == ContentTypeBlock {
-		parent, _, _ := shared.InstanceGetParentAndSnapshotName(vol.Name())
-		parentVol := NewVolume(d, d.Name(), vol.volType, vol.contentType, parent, vol.config, vol.poolConfig)
+	if snapVol.contentType == ContentTypeBlock {
+		parent, _, _ := shared.InstanceGetParentAndSnapshotName(snapVol.Name())
+		parentVol := NewVolume(d, d.Name(), snapVol.volType, snapVol.contentType, parent, snapVol.config, snapVol.poolConfig)
 		parentDataset := d.dataset(parentVol, false)
 
 		err := d.setDatasetProperties(parentDataset, "snapdev=hidden")

From e0e96afc6f7a61deaf62a7ac841f6b09001fb1a9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 9 Nov 2020 17:32:23 +0000
Subject: [PATCH 7/7] lxd/storage/drivers/volume: Removes locking from
 MountTask and UnmountTask

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

diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go
index 3ced59a0e0..e03873f1a7 100644
--- a/lxd/storage/drivers/volume.go
+++ b/lxd/storage/drivers/volume.go
@@ -192,40 +192,22 @@ func (v Volume) MountTask(task func(mountPath string, op *operations.Operation)
 	// If the volume is a snapshot then call the snapshot specific mount/unmount functions as
 	// these will mount the snapshot read only.
 	if v.IsSnapshot() {
-		unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
-
 		ourMount, err := v.driver.MountVolumeSnapshot(v, op)
 		if err != nil {
-			unlock()
 			return err
 		}
 
-		unlock()
-
 		if ourMount {
-			defer func() {
-				unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
-				v.driver.UnmountVolumeSnapshot(v, op)
-				unlock()
-			}()
+			defer v.driver.UnmountVolumeSnapshot(v, op)
 		}
 	} else {
-		unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
-
 		ourMount, err := v.driver.MountVolume(v, op)
 		if err != nil {
-			unlock()
 			return err
 		}
 
-		unlock()
-
 		if ourMount {
-			defer func() {
-				unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
-				v.driver.UnmountVolume(v, false, op)
-				unlock()
-			}()
+			defer v.driver.UnmountVolume(v, false, op)
 		}
 	}
 
@@ -239,40 +221,22 @@ func (v Volume) UnmountTask(task func(op *operations.Operation) error, keepBlock
 	// If the volume is a snapshot then call the snapshot specific mount/unmount functions as
 	// these will mount the snapshot read only.
 	if v.IsSnapshot() {
-		unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
-
 		ourUnmount, err := v.driver.UnmountVolumeSnapshot(v, op)
 		if err != nil {
-			unlock()
 			return err
 		}
 
-		unlock()
-
 		if ourUnmount {
-			defer func() {
-				unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
-				v.driver.MountVolumeSnapshot(v, op)
-				unlock()
-			}()
+			defer v.driver.MountVolumeSnapshot(v, op)
 		}
 	} else {
-		unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
-
 		ourUnmount, err := v.driver.UnmountVolume(v, keepBlockDev, op)
 		if err != nil {
-			unlock()
 			return err
 		}
 
-		unlock()
-
 		if ourUnmount {
-			defer func() {
-				unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
-				v.driver.MountVolume(v, op)
-				unlock()
-			}()
+			defer v.driver.MountVolume(v, op)
 		}
 	}
 


More information about the lxc-devel mailing list