[lxc-devel] [lxd/master] Moves lxcContainerOperations to operationlock package

tomponline on Github lxc-bot at linuxcontainers.org
Fri Sep 27 15:04:01 UTC 2019


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/20190927/24697e78/attachment.bin>
-------------- next part --------------
From eb476bdacb91a77d263169b292ec9d25cd952e24 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at 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.parrott at 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.parrott at 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.parrott at 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
 


More information about the lxc-devel mailing list