[lxc-devel] [lxd/master] Instance: Device cleanup on failed start

tomponline on Github lxc-bot at linuxcontainers.org
Fri Jun 5 11:14:19 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 638 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200605/8f6463c4/attachment.bin>
-------------- next part --------------
From fb9e4c3fc573107278bad724aaf8f168ef843fdb Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 5 Jun 2020 12:03:35 +0100
Subject: [PATCH 1/4] lxd/instance/drivers/driver/lxc: Adds debug logging to
 deviceStop

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

diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go
index ec483f9fe2..cccf2ec1d1 100644
--- a/lxd/instance/drivers/driver_lxc.go
+++ b/lxd/instance/drivers/driver_lxc.go
@@ -53,6 +53,7 @@ import (
 	"github.com/lxc/lxd/shared/instancewriter"
 	log "github.com/lxc/lxd/shared/log15"
 	"github.com/lxc/lxd/shared/logger"
+	"github.com/lxc/lxd/shared/logging"
 	"github.com/lxc/lxd/shared/netutils"
 	"github.com/lxc/lxd/shared/osarch"
 	"github.com/lxc/lxd/shared/units"
@@ -1535,6 +1536,8 @@ func (c *lxc) deviceUpdate(deviceName string, rawConfig deviceConfig.Device, old
 
 // deviceStop loads a new device and calls its Stop() function.
 func (c *lxc) deviceStop(deviceName string, rawConfig deviceConfig.Device, stopHookNetnsPath string) error {
+	logger := logging.AddContext(logger.Log, log.Ctx{"device": deviceName, "project": c.Project(), "instance": c.Name()})
+	logger.Debug("Stopping device")
 	d, configCopy, err := c.deviceLoad(deviceName, rawConfig)
 
 	// If deviceLoad fails with unsupported device type then return.
@@ -1552,7 +1555,7 @@ func (c *lxc) deviceStop(deviceName string, rawConfig deviceConfig.Device, stopH
 
 		}
 
-		logger.Errorf("Device stop validation failed for '%s': %v", deviceName, err)
+		logger.Error("Device stop validation failed for", log.Ctx{"err": err})
 	}
 
 	canHotPlug, _ := d.CanHotPlug()

From b0407c961fe6ef5b1a04139e6658fcf8dc80b470 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 5 Jun 2020 12:04:02 +0100
Subject: [PATCH 2/4] lxd/instance/drivers/driver/lxc: Adds driver revert on
 failed start in startCommon

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_lxc.go | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go
index cccf2ec1d1..fa943932c4 100644
--- a/lxd/instance/drivers/driver_lxc.go
+++ b/lxd/instance/drivers/driver_lxc.go
@@ -1899,6 +1899,10 @@ func (c *lxc) expandDevices(profiles []api.Profile) error {
 // Start functions
 func (c *lxc) startCommon() (string, []func() error, error) {
 	var ourStart bool
+
+	revert := revert.New()
+	defer revert.Fail()
+
 	postStartHooks := []func() error{}
 
 	// Load the go-lxc struct
@@ -2048,13 +2052,23 @@ func (c *lxc) startCommon() (string, []func() error, error) {
 	nicID := -1
 
 	// Setup devices in sorted order, this ensures that device mounts are added in path order.
-	for _, dev := range c.expandedDevices.Sorted() {
+	for _, d := range c.expandedDevices.Sorted() {
+		dev := d // Ensure device variable has local scope for revert.
+
 		// Start the device.
 		runConf, err := c.deviceStart(dev.Name, dev.Config, false)
 		if err != nil {
 			return "", postStartHooks, errors.Wrapf(err, "Failed to start device %q", dev.Name)
 		}
 
+		// Stop device on failure to setup container, pass non-empty stopHookNetnsPath so that stop code
+		// doesn't think container is running.
+		revert.Add(func() {
+			err := c.deviceStop(dev.Name, dev.Config, "startfailed")
+			if err != nil {
+				logger.Errorf("Failed to cleanup device %q: %v", dev.Name, err)
+			}
+		})
 		if runConf == nil {
 			continue
 		}
@@ -2253,6 +2267,7 @@ func (c *lxc) startCommon() (string, []func() error, error) {
 	// Unmount any previously mounted shiftfs
 	unix.Unmount(c.RootfsPath(), unix.MNT_DETACH)
 
+	revert.Success()
 	return configPath, postStartHooks, nil
 }
 

From f72c68444e584f4595a74b9fe68ba170b3f2ad9f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 5 Jun 2020 12:04:46 +0100
Subject: [PATCH 3/4] lxd/instance/drivers/driver/qemu: Adds debug logging to
 deviceStop

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

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 223758dd11..d34e71e175 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -54,6 +54,7 @@ import (
 	"github.com/lxc/lxd/shared/instancewriter"
 	log "github.com/lxc/lxd/shared/log15"
 	"github.com/lxc/lxd/shared/logger"
+	"github.com/lxc/lxd/shared/logging"
 	"github.com/lxc/lxd/shared/osarch"
 	"github.com/lxc/lxd/shared/termios"
 	"github.com/lxc/lxd/shared/units"
@@ -1100,6 +1101,8 @@ func (vm *qemu) deviceStart(deviceName string, rawConfig deviceConfig.Device, is
 
 // deviceStop loads a new device and calls its Stop() function.
 func (vm *qemu) deviceStop(deviceName string, rawConfig deviceConfig.Device) error {
+	logger := logging.AddContext(logger.Log, log.Ctx{"device": deviceName, "project": vm.Project(), "instance": vm.Name()})
+	logger.Debug("Stopping device")
 	d, _, err := vm.deviceLoad(deviceName, rawConfig)
 
 	// If deviceLoad fails with unsupported device type then return.
@@ -1117,7 +1120,7 @@ func (vm *qemu) deviceStop(deviceName string, rawConfig deviceConfig.Device) err
 
 		}
 
-		logger.Errorf("Device stop validation failed for '%s': %v", deviceName, err)
+		logger.Error("Device stop validation failed for", log.Ctx{"err": err})
 	}
 
 	canHotPlug, _ := d.CanHotPlug()

From a21768c3bfed2de2059172d213ab3485a9cb453d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 5 Jun 2020 12:05:37 +0100
Subject: [PATCH 4/4] lxd/instance/drivers/driver/qemu: Simplifies failed start
 device cleanup in Start

Makes consistent with container cleanup.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu.go | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index d34e71e175..a90e65fd5a 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -667,7 +667,9 @@ func (vm *qemu) Start(stateful bool) error {
 	devConfs := make([]*deviceConfig.RunConfig, 0, len(vm.expandedDevices))
 
 	// Setup devices in sorted order, this ensures that device mounts are added in path order.
-	for _, dev := range vm.expandedDevices.Sorted() {
+	for _, d := range vm.expandedDevices.Sorted() {
+		dev := d // Ensure device variable has local scope for revert.
+
 		// Start the device.
 		runConf, err := vm.deviceStart(dev.Name, dev.Config, false)
 		if err != nil {
@@ -679,15 +681,12 @@ func (vm *qemu) Start(stateful bool) error {
 			continue
 		}
 
-		// Use a local function argument to ensure the current device is added to the reverter.
-		func(localDev deviceConfig.DeviceNamed) {
-			revert.Add(func() {
-				err := vm.deviceStop(localDev.Name, localDev.Config)
-				if err != nil {
-					logger.Errorf("Failed to cleanup device %q: %v", localDev.Name, err)
-				}
-			})
-		}(dev)
+		revert.Add(func() {
+			err := vm.deviceStop(dev.Name, dev.Config)
+			if err != nil {
+				logger.Errorf("Failed to cleanup device %q: %v", dev.Name, err)
+			}
+		})
 
 		devConfs = append(devConfs, runConf)
 	}


More information about the lxc-devel mailing list