[lxc-devel] [lxd/master] Instance: Fixes crash when removing device that cant be loaded

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 450 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200902/0589aa59/attachment-0001.bin>
-------------- next part --------------
From b43d403fbe9865a3e635797121b8216bcecc2716 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 2 Sep 2020 17:10:50 +0100
Subject: [PATCH] lxd/instance/drivers: Fixes crash when removing device that
 cant be loaded

Also makes error logging and returned error messages consistent between lxc and qemu.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_lxc.go  | 12 +++++++++---
 lxd/instance/drivers/driver_qemu.go | 13 ++++++++++---
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go
index 196e70cdbc..98381f6775 100644
--- a/lxd/instance/drivers/driver_lxc.go
+++ b/lxd/instance/drivers/driver_lxc.go
@@ -1565,8 +1565,7 @@ func (c *lxc) deviceStop(deviceName string, rawConfig deviceConfig.Device, stopH
 	if err != nil {
 		// If there is no device returned, then we cannot proceed, so return as error.
 		if d == nil {
-			return fmt.Errorf("Device stop validation failed for '%s': %v", deviceName, err)
-
+			return fmt.Errorf("Device stop validation failed for %q: %v", deviceName, err)
 		}
 
 		logger.Error("Device stop validation failed for", log.Ctx{"err": err})
@@ -1729,6 +1728,8 @@ func (c *lxc) deviceHandleMounts(mounts []deviceConfig.MountEntryItem) error {
 
 // deviceRemove loads a new device and calls its Remove() function.
 func (c *lxc) deviceRemove(deviceName string, rawConfig deviceConfig.Device) error {
+	logger := logging.AddContext(logger.Log, log.Ctx{"device": deviceName, "project": c.Project(), "instance": c.Name()})
+
 	d, _, err := c.deviceLoad(deviceName, rawConfig)
 
 	// If deviceLoad fails with unsupported device type then return.
@@ -1740,7 +1741,12 @@ func (c *lxc) deviceRemove(deviceName string, rawConfig deviceConfig.Device) err
 	// scenario that a new version of LXD has additional validation restrictions than older
 	// versions we still need to allow previously valid devices to be stopped.
 	if err != nil {
-		logger.Errorf("Device remove validation failed for '%s': %v", deviceName, err)
+		// If there is no device returned, then we cannot proceed, so return as error.
+		if d == nil {
+			return fmt.Errorf("Device remove validation failed for %q: %v", deviceName, err)
+		}
+
+		logger.Error("Device remove validation failed for", log.Ctx{"err": err})
 	}
 
 	return d.Remove()
diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 1d14ca7555..eb9f71b7bf 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -1147,11 +1147,11 @@ func (vm *qemu) deviceStop(deviceName string, rawConfig deviceConfig.Device) err
 	if err != nil {
 		// If there is no device returned, then we cannot proceed, so return as error.
 		if d == nil {
-			return fmt.Errorf("Device stop validation failed for '%s': %v", deviceName, err)
+			return fmt.Errorf("Device stop validation failed for %q: %v", deviceName, err)
 
 		}
 
-		logger.Error("Device stop validation failed for", log.Ctx{"err": err})
+		logger.Error("Device stop validation failed", log.Ctx{"err": err})
 	}
 
 	canHotPlug, _ := d.CanHotPlug()
@@ -3366,6 +3366,8 @@ func (vm *qemu) deviceAdd(deviceName string, rawConfig deviceConfig.Device) erro
 }
 
 func (vm *qemu) deviceRemove(deviceName string, rawConfig deviceConfig.Device) error {
+	logger := logging.AddContext(logger.Log, log.Ctx{"device": deviceName, "project": vm.Project(), "instance": vm.Name()})
+
 	d, _, err := vm.deviceLoad(deviceName, rawConfig)
 
 	// If deviceLoad fails with unsupported device type then return.
@@ -3377,7 +3379,12 @@ func (vm *qemu) deviceRemove(deviceName string, rawConfig deviceConfig.Device) e
 	// scenario that a new version of LXD has additional validation restrictions than older
 	// versions we still need to allow previously valid devices to be stopped.
 	if err != nil {
-		logger.Errorf("Device remove validation failed for '%s': %v", deviceName, err)
+		// If there is no device returned, then we cannot proceed, so return as error.
+		if d == nil {
+			return fmt.Errorf("Device remove validation failed for %q: %v", deviceName, err)
+		}
+
+		logger.Error("Device remove validation failed", log.Ctx{"err": err})
 	}
 
 	return d.Remove()