[lxc-devel] [lxd/master] Locking: Moves generic locking package out of storage namespace
tomponline on Github
lxc-bot at linuxcontainers.org
Wed Aug 5 15:42:21 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 551 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200805/db3950dc/attachment-0001.bin>
-------------- next part --------------
From 81f6c3e67ea624a6f9dfab383e8c90517bbaa91c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 5 Aug 2020 16:22:50 +0100
Subject: [PATCH 1/4] lxd/storage/locking: Moves package to lxd/locking
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/{storage => }/locking/lock.go | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename lxd/{storage => }/locking/lock.go (100%)
diff --git a/lxd/storage/locking/lock.go b/lxd/locking/lock.go
similarity index 100%
rename from lxd/storage/locking/lock.go
rename to lxd/locking/lock.go
From 5630989206e2e18dc2b33d1da1fcc7606e2217ed Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 5 Aug 2020 16:34:20 +0100
Subject: [PATCH 2/4] lxd/locking: Renames variables to make them generic
And updates comment width.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/locking/lock.go | 44 +++++++++++++++++++-------------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/lxd/locking/lock.go b/lxd/locking/lock.go
index 727974484c..02c0ec992d 100644
--- a/lxd/locking/lock.go
+++ b/lxd/locking/lock.go
@@ -1,44 +1,38 @@
package locking
import (
- "fmt"
"sync"
)
-// ongoingOperationMap is a hashmap that allows functions to check whether the
-// operation they are about to perform is already in progress. If it is the
-// channel can be used to wait for the operation to finish. If it is not, the
-// function that wants to perform the operation should store its code in the
-// hashmap.
+// locks is a hashmap that allows functions to check whether the operation they are about to perform
+// is already in progress. If it is the channel can be used to wait for the operation to finish. If it is not, the
+// function that wants to perform the operation should store its code in the hashmap.
// Note that any access to this map must be done while holding a lock.
-var ongoingOperationMap = map[string]chan struct{}{}
+var locks = map[string]chan struct{}{}
-// ongoingOperationMapLock is used to access ongoingOperationMap.
-var ongoingOperationMapLock sync.Mutex
-
-// Lock creates a lock for a specific storage volume to allow activities that
-// require exclusive access to take place. 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(poolName string, volType string, volName string) func() {
- lockID := fmt.Sprintf("%s/%s/%s", poolName, volType, volName)
+// locksMutex is used to access locks safely.
+var locksMutex sync.Mutex
+// 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() {
for {
// Get exclusive access to the map and see if there is already an operation ongoing.
- ongoingOperationMapLock.Lock()
- waitCh, ok := ongoingOperationMap[lockID]
+ locksMutex.Lock()
+ waitCh, ok := locks[lockName]
if !ok {
// No ongoing operation, create a new channel to indicate our new operation.
waitCh = make(chan struct{})
- ongoingOperationMap[lockID] = waitCh
- ongoingOperationMapLock.Unlock()
+ locks[lockName] = waitCh
+ locksMutex.Unlock()
// Return a function that will complete the operation.
return func() {
// Get exclusive access to the map.
- ongoingOperationMapLock.Lock()
- doneCh, ok := ongoingOperationMap[lockID]
+ locksMutex.Lock()
+ doneCh, ok := locks[lockName]
// Load our existing operation.
if ok {
@@ -47,19 +41,19 @@ func Lock(poolName string, volType string, volName string) func() {
close(doneCh)
// Remove our existing operation entry from the map.
- delete(ongoingOperationMap, lockID)
+ delete(locks, lockName)
}
// Release the lock now that the done channel is closed and the
// map entry has been deleted, this will allow any waiting users
// to try and get access to the map to create a new operation.
- ongoingOperationMapLock.Unlock()
+ locksMutex.Unlock()
}
}
// An existing operation is ongoing, lets wait for that to finish and then try
// to get exlusive access to create a new operation again.
- ongoingOperationMapLock.Unlock()
+ locksMutex.Unlock()
<-waitCh
}
}
From 0fa1c760732ac7d944132e9c404013423267ad3c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 5 Aug 2020 16:35:10 +0100
Subject: [PATCH 3/4] lxd/storage/drivers/utils: Adds OperationLockName
function
Used with lxd/locking package to generate storage specific lock names.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/utils.go | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lxd/storage/drivers/utils.go b/lxd/storage/drivers/utils.go
index d4539b693b..055fd73e2d 100644
--- a/lxd/storage/drivers/utils.go
+++ b/lxd/storage/drivers/utils.go
@@ -791,3 +791,8 @@ func PathNameDecode(text string) string {
// converted back to "/" before making a final pass to convert "\0" back to original "-".
return strings.Replace(strings.Replace(strings.Replace(text, "--", "\000", -1), "-", "/", -1), "\000", "-", -1)
}
+
+// 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)
+}
From e826b254e772de67a5c5b72f97d41a1bfc0dd284 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 5 Aug 2020 16:35:46 +0100
Subject: [PATCH 4/4] lxd/storage: locking.Lock usage with OperationLockName
wrapper
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/backend_lxd.go | 6 +++---
lxd/storage/drivers/driver_lvm_utils.go | 4 ++--
lxd/storage/drivers/volume.go | 18 +++++++++---------
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 03bbfa1c92..312aee0a7a 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -22,7 +22,7 @@ import (
"github.com/lxc/lxd/lxd/revert"
"github.com/lxc/lxd/lxd/state"
"github.com/lxc/lxd/lxd/storage/drivers"
- "github.com/lxc/lxd/lxd/storage/locking"
+ "github.com/lxc/lxd/lxd/locking"
"github.com/lxc/lxd/lxd/storage/memorypipe"
"github.com/lxc/lxd/shared"
"github.com/lxc/lxd/shared/api"
@@ -2028,7 +2028,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(b.name, string(drivers.VolumeTypeImage), fmt.Sprintf("EnsureImage_%v", fingerprint))
+ unlock := locking.Lock(drivers.OperationLockName(b.name, string(drivers.VolumeTypeImage), fmt.Sprintf("EnsureImage_%v", fingerprint)))
defer unlock()
// Load image info from database.
@@ -2124,7 +2124,7 @@ func (b *lxdBackend) DeleteImage(fingerprint string, op *operations.Operation) e
// We need to lock this operation to ensure that the image is not being
// deleted multiple times.
- unlock := locking.Lock(b.name, string(drivers.VolumeTypeImage), fingerprint)
+ unlock := locking.Lock(drivers.OperationLockName(b.name, string(drivers.VolumeTypeImage), fingerprint))
defer unlock()
// Load image info from database.
diff --git a/lxd/storage/drivers/driver_lvm_utils.go b/lxd/storage/drivers/driver_lvm_utils.go
index e8ad0216cf..310cb724d6 100644
--- a/lxd/storage/drivers/driver_lvm_utils.go
+++ b/lxd/storage/drivers/driver_lvm_utils.go
@@ -12,7 +12,7 @@ import (
"github.com/lxc/lxd/lxd/operations"
"github.com/lxc/lxd/lxd/revert"
- "github.com/lxc/lxd/lxd/storage/locking"
+ "github.com/lxc/lxd/lxd/locking"
"github.com/lxc/lxd/shared"
log "github.com/lxc/lxd/shared/log15"
"github.com/lxc/lxd/shared/units"
@@ -56,7 +56,7 @@ func (d *lvm) openLoopFile(source string) (*os.File, error) {
}
if filepath.IsAbs(source) && !shared.IsBlockdevPath(source) {
- unlock := locking.Lock(d.name, "", "")
+ unlock := locking.Lock(OperationLockName(d.name, "", ""))
defer unlock()
// Try to prepare new loop device.
diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go
index 0c5cd7e88b..ae989c6991 100644
--- a/lxd/storage/drivers/volume.go
+++ b/lxd/storage/drivers/volume.go
@@ -8,7 +8,7 @@ import (
"github.com/lxc/lxd/lxd/operations"
"github.com/lxc/lxd/lxd/revert"
- "github.com/lxc/lxd/lxd/storage/locking"
+ "github.com/lxc/lxd/lxd/locking"
"github.com/lxc/lxd/shared"
"github.com/lxc/lxd/shared/units"
)
@@ -187,7 +187,7 @@ 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(v.pool, string(v.volType), v.name)
+ unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
ourMount, err := v.driver.MountVolumeSnapshot(v, op)
if err != nil {
@@ -199,13 +199,13 @@ func (v Volume) MountTask(task func(mountPath string, op *operations.Operation)
if ourMount {
defer func() {
- unlock := locking.Lock(v.pool, string(v.volType), v.name)
+ unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
v.driver.UnmountVolumeSnapshot(v, op)
unlock()
}()
}
} else {
- unlock := locking.Lock(v.pool, string(v.volType), v.name)
+ unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
ourMount, err := v.driver.MountVolume(v, op)
if err != nil {
@@ -217,7 +217,7 @@ func (v Volume) MountTask(task func(mountPath string, op *operations.Operation)
if ourMount {
defer func() {
- unlock := locking.Lock(v.pool, string(v.volType), v.name)
+ unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
v.driver.UnmountVolume(v, op)
unlock()
}()
@@ -233,7 +233,7 @@ func (v Volume) UnmountTask(task func(op *operations.Operation) error, op *opera
// 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(v.pool, string(v.volType), v.name)
+ unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
ourUnmount, err := v.driver.UnmountVolumeSnapshot(v, op)
if err != nil {
@@ -245,13 +245,13 @@ func (v Volume) UnmountTask(task func(op *operations.Operation) error, op *opera
if ourUnmount {
defer func() {
- unlock := locking.Lock(v.pool, string(v.volType), v.name)
+ unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
v.driver.MountVolumeSnapshot(v, op)
unlock()
}()
}
} else {
- unlock := locking.Lock(v.pool, string(v.volType), v.name)
+ unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
ourUnmount, err := v.driver.UnmountVolume(v, op)
if err != nil {
@@ -263,7 +263,7 @@ func (v Volume) UnmountTask(task func(op *operations.Operation) error, op *opera
if ourUnmount {
defer func() {
- unlock := locking.Lock(v.pool, string(v.volType), v.name)
+ unlock := locking.Lock(OperationLockName(v.pool, string(v.volType), v.name))
v.driver.MountVolume(v, op)
unlock()
}()
More information about the lxc-devel
mailing list