[lxc-devel] [lxd/master] VM: Exec hanging fixes

tomponline on Github lxc-bot at linuxcontainers.org
Thu Feb 6 12:18:23 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 483 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200206/28311ca2/attachment.bin>
-------------- next part --------------
From bd28effdb629aafc1a667b5980fd6f13b583eeea Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 4 Feb 2020 09:53:11 +0000
Subject: [PATCH 01/19] lxd/storage/drivers/driver/lvm: Uses d.thinpoolName()
 rather than d.config["lvm.thinpool_name"]

During thin pool rename requests, use d.thinpoolName() rather than d.config["lvm.thinpool_name"] in case d.config["lvm.thinpool_name"] is not defined.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_lvm.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lxd/storage/drivers/driver_lvm.go b/lxd/storage/drivers/driver_lvm.go
index 04ccc4725c..09423c1ba1 100644
--- a/lxd/storage/drivers/driver_lvm.go
+++ b/lxd/storage/drivers/driver_lvm.go
@@ -466,11 +466,11 @@ func (d *lvm) Update(changedConfig map[string]string) error {
 	}
 
 	if changedConfig["lvm.thinpool_name"] != "" {
-		_, err := shared.TryRunCommand("lvrename", d.config["lvm.vg_name"], d.config["lvm.thinpool_name"], changedConfig["lvm.thinpool_name"])
+		_, err := shared.TryRunCommand("lvrename", d.config["lvm.vg_name"], d.thinpoolName(), changedConfig["lvm.thinpool_name"])
 		if err != nil {
-			return errors.Wrapf(err, "Error renaming LVM thin pool from %q to %q", d.config["lvm.thinpool_name"], changedConfig["lvm.thinpool_name"])
+			return errors.Wrapf(err, "Error renaming LVM thin pool from %q to %q", d.thinpoolName(), changedConfig["lvm.thinpool_name"])
 		}
-		d.logger.Debug("Thin pool volume renamed", log.Ctx{"vg_name": d.config["lvm.vg_name"], "thinpool": d.config["lvm.thinpool_name"], "new_thinpool": changedConfig["lvm.thinpool_name"]})
+		d.logger.Debug("Thin pool volume renamed", log.Ctx{"vg_name": d.config["lvm.vg_name"], "thinpool": d.thinpoolName(), "new_thinpool": changedConfig["lvm.thinpool_name"]})
 	}
 
 	return nil

From 4d97547265cb124b72e1a92aece3eb7da853020a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 5 Feb 2020 09:55:47 +0000
Subject: [PATCH 02/19] lxd/patches: setupStorageDriver usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/patches.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/patches.go b/lxd/patches.go
index 4ff0e23157..8fe0db7509 100644
--- a/lxd/patches.go
+++ b/lxd/patches.go
@@ -451,7 +451,7 @@ func patchStorageApi(name string, d *Daemon) error {
 		return err
 	}
 
-	return SetupStorageDriver(d.State(), true)
+	return setupStorageDriver(d.State(), true)
 }
 
 func upgradeFromStorageTypeBtrfs(name string, d *Daemon, defaultPoolName string, defaultStorageTypeName string, cRegular []string, cSnapshots []string, imgPublic []string, imgPrivate []string) error {

From 816d3f875714a495c91ab1cde6a36e2885ac1077 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 5 Feb 2020 09:58:10 +0000
Subject: [PATCH 03/19] lxd/storage: Renames SetupStorageDriver to
 setupStorageDriver for consistency

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/storage.go b/lxd/storage.go
index 38d30a404d..71d5b02a26 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -711,7 +711,7 @@ func resetContainerDiskIdmap(container *containerLXC, srcIdmap *idmap.IdmapSet)
 	return nil
 }
 
-func SetupStorageDriver(s *state.State, forceCheck bool) error {
+func setupStorageDriver(s *state.State, forceCheck bool) error {
 	pools, err := s.Cluster.StoragePoolsNotPending()
 	if err != nil {
 		if err == db.ErrNoSuchObject {

From 5f2468b5cb72e07ce2c689db1d81ebd5026e2217 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 5 Feb 2020 10:02:09 +0000
Subject: [PATCH 04/19] lxd/storage/drivers/driver/zfs: Adds zfs kernel module
 load fail detection

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

diff --git a/lxd/storage/drivers/driver_zfs.go b/lxd/storage/drivers/driver_zfs.go
index 4071ec1a26..8b390a835e 100644
--- a/lxd/storage/drivers/driver_zfs.go
+++ b/lxd/storage/drivers/driver_zfs.go
@@ -50,7 +50,10 @@ func (d *zfs) load() error {
 	}
 
 	// Load the kernel module.
-	util.LoadModule("zfs")
+	err := util.LoadModule("zfs")
+	if err != nil {
+		return errors.Wrapf(err, "Error loading %q module", "zfs")
+	}
 
 	// Validate the needed tools are present.
 	for _, tool := range []string{"zpool", "zfs"} {

From 7deb9534fd3d4c50a968cdb659ec8c52c19352be Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 09:05:20 +0000
Subject: [PATCH 05/19] lxd/daemon: setupStorageDriver usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/daemon.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index fd0e4b57fc..77b074220a 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -815,7 +815,7 @@ func (d *Daemon) init() error {
 
 	/* Read the storage pools */
 	logger.Infof("Initializing storage pools")
-	err = SetupStorageDriver(d.State(), false)
+	err = setupStorageDriver(d.State(), false)
 	if err != nil {
 		return err
 	}

From 5009b2fc77ef0a39b3f895f3d117a4b62a7c9ae0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 09:05:33 +0000
Subject: [PATCH 06/19] lxd/daemon: Comment consistency

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/daemon.go | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index 77b074220a..6bdb4958b7 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -808,41 +808,41 @@ func (d *Daemon) init() error {
 		containersRestart(s)
 	}
 
-	// Setup the user-agent
+	// Setup the user-agent.
 	if clustered {
 		version.UserAgentFeatures([]string{"cluster"})
 	}
 
-	/* Read the storage pools */
+	// Read the storage pools.
 	logger.Infof("Initializing storage pools")
 	err = setupStorageDriver(d.State(), false)
 	if err != nil {
 		return err
 	}
 
-	// Mount any daemon storage
+	// Mount any daemon storage.
 	err = daemonStorageMount(d.State())
 	if err != nil {
 		return err
 	}
 
-	/* Apply all patches */
+	// Apply all patches.
 	err = patchesApplyAll(d)
 	if err != nil {
 		return err
 	}
 
-	/* Setup the networks */
+	// Setup the networks.
 	logger.Infof("Initializing networks")
 	err = networkStartup(d.State())
 	if err != nil {
 		return err
 	}
 
-	// Cleanup leftover images
+	// Cleanup leftover images.
 	pruneLeftoverImages(d)
 
-	/* Setup the proxy handler, external authentication and MAAS */
+	// Setup the proxy handler, external authentication and MAAS.
 	candidAPIURL := ""
 	candidAPIKey := ""
 	candidDomains := ""

From d01d09942dfcb8e20e2dd3796f8b4789eadf54e6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 09:15:17 +0000
Subject: [PATCH 07/19] lxd/storage/drivers/driver/lvm: Makes lvm.vg_name
 required for mounting

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

diff --git a/lxd/storage/drivers/driver_lvm.go b/lxd/storage/drivers/driver_lvm.go
index 09423c1ba1..34a3294aee 100644
--- a/lxd/storage/drivers/driver_lvm.go
+++ b/lxd/storage/drivers/driver_lvm.go
@@ -479,6 +479,10 @@ func (d *lvm) Update(changedConfig map[string]string) error {
 // Mount mounts the storage pool (this does nothing for external LVM pools, but for loopback image
 // LVM pools this creates a loop device).
 func (d *lvm) Mount() (bool, error) {
+	if d.config["lvm.vg_name"] == "" {
+		return false, fmt.Errorf("Cannot mount pool as %q is not specified", "lvm.vg_name")
+	}
+
 	// Open the loop file if the LVM device doesn't exist yet and the source points to a file.
 	if !shared.IsDir(fmt.Sprintf("/dev/%s", d.config["lvm.vg_name"])) && filepath.IsAbs(d.config["source"]) && !shared.IsBlockdevPath(d.config["source"]) {
 		loopFile, err := d.openLoopFile(d.config["source"])

From 112be17b0df238d4919a07f8fbb4d21191b0eb89 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 09:15:43 +0000
Subject: [PATCH 08/19] lxd/db/cluster/update: Adds updateFromV23 for ensuring
 lvm.vg_name key is set

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/db/cluster/update.go | 43 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/lxd/db/cluster/update.go b/lxd/db/cluster/update.go
index 1466f9f13e..6b3d353a9a 100644
--- a/lxd/db/cluster/update.go
+++ b/lxd/db/cluster/update.go
@@ -59,6 +59,49 @@ var updates = map[int]schema.Update{
 	21: updateFromV20,
 	22: updateFromV21,
 	23: updateFromV22,
+	24: updateFromV23,
+}
+
+// The lvm.vg_name config key is required for LVM to function.
+func updateFromV23(tx *sql.Tx) error {
+	// Fetch the IDs of all existing nodes.
+	nodeIDs, err := query.SelectIntegers(tx, "SELECT id FROM nodes")
+	if err != nil {
+		return errors.Wrap(err, "Failed to get IDs of current nodes")
+	}
+
+	// Fetch the IDs of all existing lvm pools.
+	poolIDs, err := query.SelectIntegers(tx, `SELECT id FROM storage_pools WHERE driver='lvm'`)
+	if err != nil {
+		return errors.Wrap(err, "Failed to get IDs of current lvm pools")
+	}
+
+	for _, poolID := range poolIDs {
+		for _, nodeID := range nodeIDs {
+			// Fetch the config for this lvm pool.
+			config, err := query.SelectConfig(tx, "storage_pools_config", "storage_pool_id=? AND node_id=?", poolID, nodeID)
+			if err != nil {
+				return errors.Wrap(err, "Failed to fetch of lvm pool config")
+			}
+
+			// Check if already set.
+			_, ok := config["lvm.vg_name"]
+			if ok {
+				continue
+			}
+
+			// Add lvm.vg_name config entry.
+			_, err = tx.Exec(`
+INSERT INTO storage_pools_config(storage_pool_id, node_id, key, value)
+SELECT ?, ?, 'lvm.vg_name', name FROM storage_pools WHERE id=?
+`, poolID, nodeID, poolID)
+			if err != nil {
+				return errors.Wrap(err, "Failed to create lvm.vg_name node config")
+			}
+		}
+	}
+
+	return nil
 }
 
 // The zfs.pool_name config key is required for ZFS to function.

From 2463ccea11ff21c8b2bb9d02a3750eda6c364840 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 09:16:03 +0000
Subject: [PATCH 09/19] lxd/db/cluster/update: Superfluous trailing whitespace

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/db/cluster/update.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lxd/db/cluster/update.go b/lxd/db/cluster/update.go
index 6b3d353a9a..7b9763a238 100644
--- a/lxd/db/cluster/update.go
+++ b/lxd/db/cluster/update.go
@@ -174,12 +174,12 @@ CREATE TABLE images_profiles (
 	FOREIGN KEY (profile_id) REFERENCES profiles (id) ON DELETE CASCADE,
 	UNIQUE (image_id, profile_id)
 );
-INSERT INTO images_profiles (image_id, profile_id) 
+INSERT INTO images_profiles (image_id, profile_id)
 	SELECT images.id, profiles.id FROM images
 	JOIN profiles ON images.project_id = profiles.project_id
 	WHERE profiles.name = 'default';
 INSERT INTO images_profiles (image_id, profile_id)
-	SELECT images.id, profiles.id FROM projects_config AS R 
+	SELECT images.id, profiles.id FROM projects_config AS R
 	JOIN projects_config AS S ON R.project_id = S.project_id
 	JOIN images ON images.project_id = R.project_id
 	JOIN profiles ON profiles.project_id = 1 AND profiles.name = "default"

From d10dd39459732167a4f542430e1dd629b2108877 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 09:17:23 +0000
Subject: [PATCH 10/19] lxd/db/cluster/schema: v24 update

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/db/cluster/schema.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/db/cluster/schema.go b/lxd/db/cluster/schema.go
index a7bd5e2b05..02f2057d10 100644
--- a/lxd/db/cluster/schema.go
+++ b/lxd/db/cluster/schema.go
@@ -495,5 +495,5 @@ CREATE TABLE storage_volumes_config (
     FOREIGN KEY (storage_volume_id) REFERENCES storage_volumes (id) ON DELETE CASCADE
 );
 
-INSERT INTO schema (version, updated_at) VALUES (23, strftime("%s"))
+INSERT INTO schema (version, updated_at) VALUES (24, strftime("%s"))
 `

From ecde4cc3a3b1da1471e38c934d837d9779c20162 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 10:19:50 +0000
Subject: [PATCH 11/19] lxd/device/config/devices: Adds NICType function on
 Device type

This function centralises the logic used to derive the nictype property from both the network and nictype properties.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/config/devices.go | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lxd/device/config/devices.go b/lxd/device/config/devices.go
index f6ec31c1e4..d574e19adc 100644
--- a/lxd/device/config/devices.go
+++ b/lxd/device/config/devices.go
@@ -19,6 +19,21 @@ func (device Device) Clone() Device {
 	return copy
 }
 
+// NICType returns the derived NIC Type for a NIC device.
+// If the "network" property is specified then this implicitly (at least for now) means the nictype is "bridged".
+// Otherwise the "nictype" property is returned. If the device type is not a NIC then an empty string is returned.
+func (device Device) NICType() string {
+	if device["type"] == "nic" {
+		if device["network"] != "" {
+			return "bridged"
+		}
+
+		return device["nictype"]
+	}
+
+	return ""
+}
+
 // Validate accepts a map of field/validation functions to run against the device's config.
 func (device Device) Validate(rules map[string]func(value string) error) error {
 	checkedFields := map[string]struct{}{}

From bd6aca8614e1634a80b0aa1ebea5502cfffa8141 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 10:21:16 +0000
Subject: [PATCH 12/19] lxd: Device.NICType usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_lxc.go                | 6 +++---
 lxd/device/device_utils_network.go  | 4 ++--
 lxd/device/infiniband.go            | 2 +-
 lxd/device/nic.go                   | 2 +-
 lxd/device/proxy.go                 | 2 +-
 lxd/instance/drivers/driver_qemu.go | 8 ++++----
 lxd/network/network_utils.go        | 4 ++--
 lxd/networks.go                     | 2 +-
 8 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index f958f87ffc..5f2f972754 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1724,7 +1724,7 @@ func (c *containerLXC) deviceResetVolatile(devName string, oldConfig, newConfig
 	// If the device type has changed, remove all old volatile keys.
 	// This will occur if the newConfig is empty (i.e the device is actually being removed) or
 	// if the device type is being changed but keeping the same name.
-	if newConfig["type"] != oldConfig["type"] || newConfig["nictype"] != oldConfig["nictype"] {
+	if newConfig["type"] != oldConfig["type"] || newConfig.NICType() != oldConfig.NICType() {
 		for k := range c.localConfig {
 			if !strings.HasPrefix(k, devicePrefix) {
 				continue
@@ -4094,7 +4094,7 @@ func (c *containerLXC) Update(args db.InstanceArgs, userRequested bool) error {
 		// between oldDevice and newDevice. The result of this is that as long as the
 		// devices are otherwise identical except for the fields returned here, then the
 		// device is considered to be being "updated" rather than "added & removed".
-		if oldDevice["type"] != newDevice["type"] || oldDevice["nictype"] != newDevice["nictype"] {
+		if oldDevice["type"] != newDevice["type"] || oldDevice.NICType() != newDevice.NICType() {
 			return []string{} // Device types aren't the same, so this cannot be an update.
 		}
 
@@ -6443,7 +6443,7 @@ func (c *containerLXC) FillNetworkDevice(name string, m deviceConfig.Device) (de
 	}
 
 	// Fill in the MAC address
-	if !shared.StringInSlice(m["nictype"], []string{"physical", "ipvlan", "sriov"}) && m["hwaddr"] == "" {
+	if !shared.StringInSlice(m.NICType(), []string{"physical", "ipvlan", "sriov"}) && m["hwaddr"] == "" {
 		configKey := fmt.Sprintf("volatile.%s.hwaddr", name)
 		volatileHwaddr := c.localConfig[configKey]
 		if volatileHwaddr == "" {
diff --git a/lxd/device/device_utils_network.go b/lxd/device/device_utils_network.go
index 4c9c5e757d..a8850aa5c8 100644
--- a/lxd/device/device_utils_network.go
+++ b/lxd/device/device_utils_network.go
@@ -388,7 +388,7 @@ func networkSetupHostVethDevice(device deviceConfig.Device, oldDevice deviceConf
 func networkSetVethRoutes(m deviceConfig.Device) error {
 	// Decide whether the route should point to the veth parent or the bridge parent.
 	routeDev := m["host_name"]
-	if m["nictype"] == "bridged" {
+	if m.NICType() == "bridged" {
 		routeDev = m["parent"]
 	}
 
@@ -426,7 +426,7 @@ func networkSetVethRoutes(m deviceConfig.Device) error {
 func networkRemoveVethRoutes(m deviceConfig.Device) {
 	// Decide whether the route should point to the veth parent or the bridge parent
 	routeDev := m["host_name"]
-	if m["nictype"] == "bridged" {
+	if m.NICType() == "bridged" {
 		routeDev = m["parent"]
 	}
 
diff --git a/lxd/device/infiniband.go b/lxd/device/infiniband.go
index 9fc586d941..adcb411c87 100644
--- a/lxd/device/infiniband.go
+++ b/lxd/device/infiniband.go
@@ -12,7 +12,7 @@ var infinibandTypes = map[string]func() device{
 
 // infinibandLoadByType returns an Infiniband device instantiated with supplied config.
 func infinibandLoadByType(c deviceConfig.Device) device {
-	f := infinibandTypes[c["nictype"]]
+	f := infinibandTypes[c.NICType()]
 	if f != nil {
 		return f()
 	}
diff --git a/lxd/device/nic.go b/lxd/device/nic.go
index f8b5bd3522..d33a8820a3 100644
--- a/lxd/device/nic.go
+++ b/lxd/device/nic.go
@@ -18,7 +18,7 @@ var nicTypes = map[string]func() device{
 
 // nicLoadByType returns a NIC device instantiated with supplied config.
 func nicLoadByType(c deviceConfig.Device) device {
-	f := nicTypes[c["nictype"]]
+	f := nicTypes[c.NICType()]
 	if f != nil {
 		return f()
 	}
diff --git a/lxd/device/proxy.go b/lxd/device/proxy.go
index 77a1394575..3e6f38b558 100644
--- a/lxd/device/proxy.go
+++ b/lxd/device/proxy.go
@@ -267,7 +267,7 @@ func (d *proxy) setupNAT() error {
 	var IPv6Addr net.IP
 
 	for _, devConfig := range d.inst.ExpandedDevices() {
-		if devConfig["type"] != "nic" || (devConfig["type"] == "nic" && devConfig["nictype"] != "bridged") {
+		if devConfig["type"] != "nic" || (devConfig["type"] == "nic" && devConfig.NICType() != "bridged") {
 			continue
 		}
 
diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index d85468be2b..efb2276fbb 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -2160,7 +2160,7 @@ func (vm *qemu) Update(args db.InstanceArgs, userRequested bool) error {
 		// between oldDevice and newDevice. The result of this is that as long as the
 		// devices are otherwise identical except for the fields returned here, then the
 		// device is considered to be being "updated" rather than "added & removed".
-		if oldDevice["type"] != newDevice["type"] || oldDevice["nictype"] != newDevice["nictype"] {
+		if oldDevice["type"] != newDevice["type"] || oldDevice.NICType() != newDevice.NICType() {
 			return []string{} // Device types aren't the same, so this cannot be an update.
 		}
 
@@ -2371,7 +2371,7 @@ func (vm *qemu) deviceResetVolatile(devName string, oldConfig, newConfig deviceC
 	// If the device type has changed, remove all old volatile keys.
 	// This will occur if the newConfig is empty (i.e the device is actually being removed) or
 	// if the device type is being changed but keeping the same name.
-	if newConfig["type"] != oldConfig["type"] || newConfig["nictype"] != oldConfig["nictype"] {
+	if newConfig["type"] != oldConfig["type"] || newConfig.NICType() != oldConfig.NICType() {
 		for k := range vm.localConfig {
 			if !strings.HasPrefix(k, devicePrefix) {
 				continue
@@ -3079,7 +3079,7 @@ func (vm *qemu) RenderState() (*api.InstanceState, error) {
 			networks := map[string]api.InstanceStateNetwork{}
 			for k, m := range vm.ExpandedDevices() {
 				// We only care about nics.
-				if m["type"] != "nic" || m["nictype"] != "bridged" {
+				if m["type"] != "nic" || m.NICType() != "bridged" {
 					continue
 				}
 
@@ -3451,7 +3451,7 @@ func (vm *qemu) FillNetworkDevice(name string, m deviceConfig.Device) (deviceCon
 	}
 
 	// Fill in the MAC address
-	if !shared.StringInSlice(m["nictype"], []string{"physical", "ipvlan", "sriov"}) && m["hwaddr"] == "" {
+	if !shared.StringInSlice(m.NICType(), []string{"physical", "ipvlan", "sriov"}) && m["hwaddr"] == "" {
 		configKey := fmt.Sprintf("volatile.%s.hwaddr", name)
 		volatileHwaddr := vm.localConfig[configKey]
 		if volatileHwaddr == "" {
diff --git a/lxd/network/network_utils.go b/lxd/network/network_utils.go
index 176abbfc8e..d911cbdf1b 100644
--- a/lxd/network/network_utils.go
+++ b/lxd/network/network_utils.go
@@ -34,7 +34,7 @@ func IsInUse(c instance.Instance, networkName string) bool {
 			continue
 		}
 
-		if !shared.StringInSlice(d["nictype"], []string{"bridged", "macvlan", "ipvlan", "physical", "sriov"}) {
+		if !shared.StringInSlice(d.NICType(), []string{"bridged", "macvlan", "ipvlan", "physical", "sriov"}) {
 			continue
 		}
 
@@ -236,7 +236,7 @@ func UpdateDNSMasqStatic(s *state.State, networkName string) error {
 		// Go through all its devices (including profiles
 		for k, d := range inst.ExpandedDevices() {
 			// Skip uninteresting entries
-			if d["type"] != "nic" || d["nictype"] != "bridged" || !shared.StringInSlice(d["parent"], networks) {
+			if d["type"] != "nic" || d.NICType() != "bridged" || !shared.StringInSlice(d["parent"], networks) {
 				continue
 			}
 
diff --git a/lxd/networks.go b/lxd/networks.go
index 4c71abd70c..44063b1a5f 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -699,7 +699,7 @@ func networkLeasesGet(d *Daemon, r *http.Request) response.Response {
 			// Go through all its devices (including profiles
 			for k, d := range inst.ExpandedDevices() {
 				// Skip uninteresting entries
-				if d["type"] != "nic" || d["nictype"] != "bridged" || d["parent"] != name {
+				if d["type"] != "nic" || d.NICType() != "bridged" || d["parent"] != name {
 					continue
 				}
 

From e4f521ac535d10eec7f8b80d56a294547afc0f12 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 10:22:36 +0000
Subject: [PATCH 13/19] lxd/device/nic/bridged: Bans use of nictype when using
 network property

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_bridged.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index af982f0a87..ad089ae747 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -76,7 +76,7 @@ func (d *nicBridged) validateConfig(instConf instance.ConfigReader) error {
 	if d.config["network"] != "" {
 		requiredFields = append(requiredFields, "network")
 
-		bannedKeys := []string{"parent", "mtu", "maas.subnet.ipv4", "maas.subnet.ipv6"}
+		bannedKeys := []string{"nictype", "parent", "mtu", "maas.subnet.ipv4", "maas.subnet.ipv6"}
 		for _, bannedKey := range bannedKeys {
 			if d.config[bannedKey] != "" {
 				return fmt.Errorf("Cannot use %q property in conjunction with %q property", bannedKey, "network")

From 0af1c3869d6d8ea3e2fde64df462774ca5f27f15 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 10:22:59 +0000
Subject: [PATCH 14/19] test: Updates nic bridged tests for NICType logic

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 test/suites/container_devices_nic_bridged.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/test/suites/container_devices_nic_bridged.sh b/test/suites/container_devices_nic_bridged.sh
index f78a7a7959..dbde3897dd 100644
--- a/test/suites/container_devices_nic_bridged.sh
+++ b/test/suites/container_devices_nic_bridged.sh
@@ -446,13 +446,14 @@ test_container_devices_nic_bridged() {
   # Can't use network property when parent is set.
   ! lxc profile device set "${ctName}" eth0 network="${brName}"
 
-  # Reset mtu and parent settings and assign network in one command.
-  lxc profile device set "${ctName}" eth0 mtu="" parent="" network="${brName}"
+  # Remove mtu, nictype and parent settings and assign network in one command.
+  lxc profile device set "${ctName}" eth0 mtu="" parent="" nictype="" network="${brName}"
 
   # Can't remove network if parent not specified.
   ! lxc profile device unset "${ctName}" eth0 network
 
   # Can't use some settings when network is set.
+  ! lxc profile device set "${ctName}" eth0 nictype="bridged"
   ! lxc profile device set "${ctName}" eth0 mtu="1400"
   ! lxc profile device set "${ctName}" eth0 maas.subnet.ipv4="test"
   ! lxc profile device set "${ctName}" eth0 maas.subnet.ipv6="test"
@@ -469,6 +470,17 @@ test_container_devices_nic_bridged() {
     echo "container mtu has not been inherited from network"
     false
   fi
+
+  # Check profile routes are applied on boot (as these use derived nictype).
+  if ! ip -4 r list dev "${brName}" | grep "192.0.2.1${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ! ip -6 r list dev "${brName}" | grep "2001:db8::1${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
   lxc stop -f "${ctName}"
   lxc network unset "${brName}" bridge.mtu
 

From 2360fa7f0ddd38b65c1581048c396a4552b39890 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 10:51:44 +0000
Subject: [PATCH 15/19] lxd/network/network/utils: Fix network in use detection

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

diff --git a/lxd/network/network_utils.go b/lxd/network/network_utils.go
index d911cbdf1b..4499bc8ed9 100644
--- a/lxd/network/network_utils.go
+++ b/lxd/network/network_utils.go
@@ -38,11 +38,15 @@ func IsInUse(c instance.Instance, networkName string) bool {
 			continue
 		}
 
+		if d["network"] == networkName {
+			return true
+		}
+
 		if d["parent"] == "" {
 			continue
 		}
 
-		if GetHostDevice(d["parent"], d["vlan"]) == networkName || d["network"] == networkName {
+		if GetHostDevice(d["parent"], d["vlan"]) == networkName {
 			return true
 		}
 	}

From 08c173228a85a5f788dec1bdd26d8a722be56c67 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 11:10:34 +0000
Subject: [PATCH 16/19] lxd-agent/exec: Logs signal forwarding as info rather
 than error

It isn't an error.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd-agent/exec.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd-agent/exec.go b/lxd-agent/exec.go
index 2908882d2f..1feddb8325 100644
--- a/lxd-agent/exec.go
+++ b/lxd-agent/exec.go
@@ -339,7 +339,7 @@ func (s *execWs) Do(op *operations.Operation) error {
 						logger.Errorf("Failed forwarding signal '%d' to PID %d", command.Signal, attachedChildPid)
 						continue
 					}
-					logger.Errorf("Forwarded signal '%d' to PID %d", command.Signal, attachedChildPid)
+					logger.Infof("Forwarded signal '%d' to PID %d", command.Signal, attachedChildPid)
 				}
 			}
 		}()

From ee18203d66c821567c8c83f483068143560ad679 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 12:15:30 +0000
Subject: [PATCH 17/19] lxd/container/exec: Only log finished mirror websocket
 when go routine exits

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_exec.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index c35fab967f..2302ee54b3 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -241,12 +241,11 @@ func (s *execWs) Do(op *operations.Operation) error {
 			s.connsLock.Unlock()
 
 			logger.Debugf("Started mirroring websocket")
+			defer logger.Debugf("Finished mirroring websocket")
 			readDone, writeDone := netutils.WebsocketExecMirror(conn, ptys[0], ptys[0], attachedChildIsDead, int(ptys[0].Fd()))
 
 			<-readDone
 			<-writeDone
-			logger.Debugf("Finished mirroring websocket")
-
 			conn.Close()
 			wgEOF.Done()
 		}()

From 10f9cb93206c82cbe825b58db720eda218c5d59d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 12:15:51 +0000
Subject: [PATCH 18/19] lxd/instance/drivers/driver/qemu: Fix go routine leak
 and hanging lxc clients

Ensure that websocket mirror process ends when client terminal exits.

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

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index efb2276fbb..83a7522315 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -2904,7 +2904,14 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 			return nil, err
 		}
 
-		revert.Add(func() { termios.Restore(int(stdin.Fd()), oldttystate) })
+		revert.Add(func() {
+			termios.Restore(int(stdin.Fd()), oldttystate)
+
+			// Write a reset escape sequence to the console to cancel any ongoing reads to the handle
+			// and then close it.
+			stdout.Write([]byte("\x1bc"))
+			stdout.Close()
+		})
 	}
 
 	dataDone := make(chan bool)

From 41bb1e879fef6f8682287df14cee6ee13f034454 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 6 Feb 2020 12:16:42 +0000
Subject: [PATCH 19/19] shared: Upper case first character of some debug
 messages

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 shared/netutils/network_linux.go | 2 +-
 shared/network.go                | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/shared/netutils/network_linux.go b/shared/netutils/network_linux.go
index f634aac8ed..cbea0423b3 100644
--- a/shared/netutils/network_linux.go
+++ b/shared/netutils/network_linux.go
@@ -183,7 +183,7 @@ func WebsocketExecMirror(conn *websocket.Conn, w io.WriteCloser, r io.ReadCloser
 			buf, ok := <-in
 			if !ok {
 				r.Close()
-				logger.Debugf("sending write barrier")
+				logger.Debugf("Sending write barrier")
 				err := conn.WriteMessage(websocket.TextMessage, []byte{})
 				if err != nil {
 					logger.Debugf("Got err writing barrier %s", err)
diff --git a/shared/network.go b/shared/network.go
index f8604d2232..71e5f09551 100644
--- a/shared/network.go
+++ b/shared/network.go
@@ -206,7 +206,7 @@ func WebsocketRecvStream(w io.Writer, conn *websocket.Conn) chan bool {
 			}
 
 			if mt == websocket.TextMessage {
-				logger.Debugf("got message barrier")
+				logger.Debugf("Got message barrier")
 				break
 			}
 
@@ -296,7 +296,7 @@ func defaultReader(conn *websocket.Conn, r io.ReadCloser, readDone chan<- bool)
 		buf, ok := <-in
 		if !ok {
 			r.Close()
-			logger.Debugf("sending write barrier")
+			logger.Debugf("Sending write barrier")
 			conn.WriteMessage(websocket.TextMessage, []byte{})
 			readDone <- true
 			return
@@ -460,7 +460,7 @@ func WebsocketConsoleMirror(conn *websocket.Conn, w io.WriteCloser, r io.ReadClo
 			buf, ok := <-in
 			if !ok {
 				r.Close()
-				logger.Debugf("sending write barrier")
+				logger.Debugf("Sending write barrier")
 				conn.WriteMessage(websocket.BinaryMessage, []byte("\r"))
 				conn.WriteMessage(websocket.TextMessage, []byte{})
 				readDone <- true


More information about the lxc-devel mailing list