The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7489
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) ===
Whilst working on https://github.com/lxc/lxd/pull/7488 I noticed that when the mount failed the container failed to start and left orphaned veth pairs hanging around on the host.
This PR runs the device cleanup code when an instance fails to start.
Also makes a tentative start at introducing contextual debug logging to instances.
From fb9e4c3fc573107278bad724aaf8f168ef843fdb Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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)
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
