The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6256
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) ===
From eb476bdacb91a77d263169b292ec9d25cd952e24 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 27 Sep 2019 16:00:23 +0100
Subject: [PATCH 1/2] lxd/instance/operationlock: Adds operationlock package
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/instance/operationlock/operationlock.go | 109 ++++++++++++++++++++
1 file changed, 109 insertions(+)
create mode 100644 lxd/instance/operationlock/operationlock.go
diff --git a/lxd/instance/operationlock/operationlock.go
b/lxd/instance/operationlock/operationlock.go
new file mode 100644
index 0000000000..a115506a1e
--- /dev/null
+++ b/lxd/instance/operationlock/operationlock.go
@@ -0,0 +1,109 @@
+package operationlock
+
+import (
+ "fmt"
+ "sync"
+ "time"
+)
+
+var instanceOperationsLock sync.Mutex
+var instanceOperations map[int]*InstanceOperation =
make(map[int]*InstanceOperation)
+
+// InstanceOperation operation locking.
+type InstanceOperation struct {
+ action string
+ chanDone chan error
+ chanReset chan bool
+ err error
+ id int
+ reusable bool
+}
+
+// Action returns operation's action.
+func (op InstanceOperation) Action() string {
+ return op.action
+}
+
+// Create creates a new operation lock for an Instance if one does not already
exist and returns it.
+// The lock will be released after 30s or when Done() is called, which ever
occurs first.
+// If reusable is set as true then future lock attempts can specify the reuse
argument as true which
+// will then trigger a reset of the 30s timeout on the existing lock and
return it.
+func Create(instanceID int, action string, reusable bool, reuse bool)
(*InstanceOperation, error) {
+ instanceOperationsLock.Lock()
+ defer instanceOperationsLock.Unlock()
+
+ op := instanceOperations[instanceID]
+ if op != nil {
+ if op.reusable && reuse {
+ op.Reset()
+ return op, nil
+ }
+
+ return nil, fmt.Errorf("Instance is busy running a %s
operation", op.action)
+ }
+
+ op = &InstanceOperation{}
+ op.id = instanceID
+ op.action = action
+ op.reusable = reusable
+ op.chanDone = make(chan error, 0)
+ op.chanReset = make(chan bool, 0)
+
+ go func(op *InstanceOperation) {
+ for {
+ select {
+ case <-op.chanReset:
+ continue
+ case <-time.After(time.Second * 30):
+ op.Done(fmt.Errorf("Instance %s operation timed
out after 30 seconds", op.action))
+ return
+ }
+ }
+ }(op)
+
+ instanceOperations[instanceID] = op
+
+ return op, nil
+}
+
+// Get retrieves an existing lock or returns nil if no lock exists.
+func Get(instanceID int) *InstanceOperation {
+ instanceOperationsLock.Lock()
+ defer instanceOperationsLock.Unlock()
+
+ return instanceOperations[instanceID]
+}
+
+// Reset resets an operation.
+func (op *InstanceOperation) Reset() error {
+ if !op.reusable {
+ return fmt.Errorf("Can't reset a non-reusable operation")
+ }
+
+ op.chanReset <- true
+ return nil
+}
+
+// Wait waits for an operation to finish.
+func (op *InstanceOperation) Wait() error {
+ <-op.chanDone
+
+ return op.err
+}
+
+// Done indicates the operation has finished.
+func (op *InstanceOperation) Done(err error) {
+ instanceOperationsLock.Lock()
+ defer instanceOperationsLock.Unlock()
+
+ // Check if already done
+ runningOp, ok := instanceOperations[op.id]
+ if !ok || runningOp != op {
+ return
+ }
+
+ op.err = err
+ close(op.chanDone)
+
+ delete(instanceOperations, op.id)
+}
From 938d7ef6a77a3ab8032a7a96bffa8b5e9a5634f0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 27 Sep 2019 16:00:41 +0100
Subject: [PATCH 2/2] lxd/container/lxc: Migrates container_lxc to use
operationlock package
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/container_lxc.go | 137 ++++---------------------------------------
1 file changed, 11 insertions(+), 126 deletions(-)
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 91e787796d..3ec915c73b 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -33,6 +33,7 @@ import (
"github.com/lxc/lxd/lxd/device/config"
"github.com/lxc/lxd/lxd/events"
"github.com/lxc/lxd/lxd/instance/instancetype"
+ "github.com/lxc/lxd/lxd/instance/operationlock"
"github.com/lxc/lxd/lxd/maas"
"github.com/lxc/lxd/lxd/operations"
"github.com/lxc/lxd/lxd/project"
@@ -53,72 +54,6 @@ import (
log "github.com/lxc/lxd/shared/log15"
)
-// Operation locking
-type lxcContainerOperation struct {
- action string
- chanDone chan error
- chanReset chan bool
- err error
- id int
- reusable bool
-}
-
-func (op *lxcContainerOperation) Create(id int, action string, reusable bool)
*lxcContainerOperation {
- op.id = id
- op.action = action
- op.reusable = reusable
- op.chanDone = make(chan error, 0)
- op.chanReset = make(chan bool, 0)
-
- go func(op *lxcContainerOperation) {
- for {
- select {
- case <-op.chanReset:
- continue
- case <-time.After(time.Second * 30):
- op.Done(fmt.Errorf("Container %s operation
timed out after 30 seconds", op.action))
- return
- }
- }
- }(op)
-
- return op
-}
-
-func (op *lxcContainerOperation) Reset() error {
- if !op.reusable {
- return fmt.Errorf("Can't reset a non-reusable operation")
- }
-
- op.chanReset <- true
- return nil
-}
-
-func (op *lxcContainerOperation) Wait() error {
- <-op.chanDone
-
- return op.err
-}
-
-func (op *lxcContainerOperation) Done(err error) {
- lxcContainerOperationsLock.Lock()
- defer lxcContainerOperationsLock.Unlock()
-
- // Check if already done
- runningOp, ok := lxcContainerOperations[op.id]
- if !ok || runningOp != op {
- return
- }
-
- op.err = err
- close(op.chanDone)
-
- delete(lxcContainerOperations, op.id)
-}
-
-var lxcContainerOperationsLock sync.Mutex
-var lxcContainerOperations map[int]*lxcContainerOperation =
make(map[int]*lxcContainerOperation)
-
// Helper functions
func lxcSetConfigItem(c *lxc.Container, key string, value string) error {
if c == nil {
@@ -618,56 +553,6 @@ func (c *containerLXC) Type() instancetype.Type {
return c.dbType
}
-func (c *containerLXC) createOperation(action string, reusable bool, reuse
bool) (*lxcContainerOperation, error) {
- op, _ := c.getOperation("")
- if op != nil {
- if reuse && op.reusable {
- op.Reset()
- return op, nil
- }
-
- return nil, fmt.Errorf("Container is busy running a %s
operation", op.action)
- }
-
- lxcContainerOperationsLock.Lock()
- defer lxcContainerOperationsLock.Unlock()
-
- op = &lxcContainerOperation{}
- op.Create(c.id, action, reusable)
- lxcContainerOperations[c.id] = op
-
- return lxcContainerOperations[c.id], nil
-}
-
-func (c *containerLXC) getOperation(action string) (*lxcContainerOperation,
error) {
- lxcContainerOperationsLock.Lock()
- defer lxcContainerOperationsLock.Unlock()
-
- op := lxcContainerOperations[c.id]
-
- if op == nil {
- return nil, fmt.Errorf("No running %s container operation",
action)
- }
-
- if action != "" && op.action != action {
- return nil, fmt.Errorf("Container is running a %s operation,
not a %s operation", op.action, action)
- }
-
- return op, nil
-}
-
-func (c *containerLXC) waitOperation() error {
- op, _ := c.getOperation("")
- if op != nil {
- err := op.Wait()
- if err != nil {
- return err
- }
- }
-
- return nil
-}
-
func idmapSize(state *state.State, isolatedStr string, size string) (int64,
error) {
isolated := false
if shared.IsTrue(isolatedStr) {
@@ -2527,7 +2412,7 @@ func (c *containerLXC) Start(stateful bool) error {
var ctxMap log.Ctx
// Setup a new operation
- op, err := c.createOperation("start", false, false)
+ op, err := operationlock.Create(c.id, "start", false, false)
if err != nil {
return errors.Wrap(err, "Create container start operation")
}
@@ -2553,7 +2438,7 @@ func (c *containerLXC) Start(stateful bool) error {
ctxMap = log.Ctx{
"project": c.project,
"name": c.name,
- "action": op.action,
+ "action": op.Action(),
"created": c.creationDate,
"ephemeral": c.ephemeral,
"used": c.lastUsedDate,
@@ -2774,7 +2659,7 @@ func (c *containerLXC) Stop(stateful bool) error {
}
// Setup a new operation
- op, err := c.createOperation("stop", false, true)
+ op, err := operationlock.Create(c.id, "stop", false, true)
if err != nil {
return err
}
@@ -2782,7 +2667,7 @@ func (c *containerLXC) Stop(stateful bool) error {
ctxMap = log.Ctx{
"project": c.project,
"name": c.name,
- "action": op.action,
+ "action": op.Action(),
"created": c.creationDate,
"ephemeral": c.ephemeral,
"used": c.lastUsedDate,
@@ -2908,7 +2793,7 @@ func (c *containerLXC) Shutdown(timeout time.Duration)
error {
}
// Setup a new operation
- op, err := c.createOperation("stop", true, true)
+ op, err := operationlock.Create(c.id, "stop", true, true)
if err != nil {
return err
}
@@ -2984,10 +2869,10 @@ func (c *containerLXC) OnStop(target string) error {
return fmt.Errorf("Invalid stop target: %s", target)
}
- // Get operation
- op, _ := c.getOperation("")
- if op != nil && op.action != "stop" {
- return fmt.Errorf("Container is already running a %s
operation", op.action)
+ // Pick up the existing stop operation lock created in Stop() function.
+ op := operationlock.Get(c.id)
+ if op != nil && op.Action() != "stop" {
+ return fmt.Errorf("Container is already running a %s
operation", op.Action())
}
// Make sure we can't call go-lxc functions by mistake
@@ -3042,7 +2927,7 @@ func (c *containerLXC) OnStop(target string) error {
logger.Error("Failed to set container state",
log.Ctx{"container": c.Name(), "err": err})
}
- go func(c *containerLXC, target string, op *lxcContainerOperation) {
+ go func(c *containerLXC, target string, op
*operationlock.InstanceOperation) {
c.fromHook = false
err = nil
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
