The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7727
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Tweaks existing `locking` package to be generic so it can be used outside of the storage environment.
Intended to be used with OVN OVS bridge setup to ensure multiple networks sharing the same parent network don't race to perform connection setup.
From 81f6c3e67ea624a6f9dfab383e8c90517bbaa91c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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()
}()
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
