[lxc-devel] [lxd/master] Device Disk

tomponline on Github lxc-bot at linuxcontainers.org
Thu Aug 29 10:02:56 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 422 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190829/2d3bb736/attachment-0001.bin>
-------------- next part --------------
From d7a8d0a12e5ae8d1fdb96341bdd372388472c4a7 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 12:04:57 +0100
Subject: [PATCH 01/30] test: Removes tmpfs references from gpu tests

Tests may break if not run on tmpfs otherwise.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 test/suites/container_devices_gpu.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/suites/container_devices_gpu.sh b/test/suites/container_devices_gpu.sh
index c761bf1126..ce496f8b94 100644
--- a/test/suites/container_devices_gpu.sh
+++ b/test/suites/container_devices_gpu.sh
@@ -14,7 +14,7 @@ test_container_devices_gpu() {
   startMountCount=$(lxc exec "${ctName}" -- mount | wc -l)
   startDevCount=$(find "${LXD_DIR}"/devices/"${ctName}" -type c | wc -l)
   lxc config device add "${ctName}" gpu-basic gpu mode=0600 id=0
-  lxc exec "${ctName}" -- mount | grep "tmpfs on /dev/dri/card0 type tmpfs"
+  lxc exec "${ctName}" -- mount | grep "/dev/dri/card0"
   lxc exec "${ctName}" -- stat -c '%a' /dev/dri/card0 | grep 600
   stat -c '%a' "${LXD_DIR}"/devices/"${ctName}"/unix.gpu--basic.dev-dri-card0 | grep 600
   lxc config device remove "${ctName}" gpu-basic

From 1241f2d9efb063f6eb75c8643ecc65a76e454113 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 16:02:09 +0100
Subject: [PATCH 02/30] device/device/utils/unix: Various small improvements

- Improves comments
- Removes duplicated path and source logic
- Removes duplicated default mode

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/device_utils_unix.go | 63 +++++++++++++++++----------------
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/lxd/device/device_utils_unix.go b/lxd/device/device_utils_unix.go
index de2cf57644..17a97bd704 100644
--- a/lxd/device/device_utils_unix.go
+++ b/lxd/device/device_utils_unix.go
@@ -17,6 +17,9 @@ import (
 	"github.com/lxc/lxd/shared/logger"
 )
 
+// unixDefaultMode default mode to create unix devices with if not specified in device config.
+const unixDefaultMode = 0660
+
 // unixDeviceInstanceAttributes returns the UNIX device attributes for an instance device.
 // Uses supplied device config for device properties, and if they haven't been set, falls back to
 // using UnixGetDeviceAttributes() to directly query an existing device file.
@@ -48,11 +51,7 @@ func unixDeviceInstanceAttributes(devicesPath string, prefix string, config conf
 		dType = "b"
 	}
 
-	// Figure out the paths
-	destPath := config["path"]
-	if destPath == "" {
-		destPath = config["source"]
-	}
+	destPath := unixDeviceDestPath(config)
 	relativeDestPath := strings.TrimPrefix(destPath, "/")
 	devName := fmt.Sprintf("%s.%s", strings.Replace(prefix, "/", "-", -1), strings.Replace(relativeDestPath, "/", "-", -1))
 	devPath := filepath.Join(devicesPath, devName)
@@ -93,13 +92,8 @@ func UnixDeviceAttributes(path string) (string, uint32, uint32, error) {
 	return dType, major, minor, nil
 }
 
+// unixDeviceModeOct converts a string unix octal mode to an int.
 func unixDeviceModeOct(strmode string) (int, error) {
-	// Default mode
-	if strmode == "" {
-		return 0600, nil
-	}
-
-	// Converted mode
 	i, err := strconv.ParseInt(strmode, 8, 32)
 	if err != nil {
 		return 0, fmt.Errorf("Bad device mode: %s", strmode)
@@ -120,8 +114,19 @@ type UnixDevice struct {
 	GID          int         // Owner GID.
 }
 
+// unixDeviceSourcePath returns the absolute path for a device on the host.
+// This is based on the "source" property of the device's config, or the "path" property if "source"
+// not define. This uses the shared.HostPath function so works when running in a snap environment.
+func unixDeviceSourcePath(m config.Device) string {
+	srcPath := m["source"]
+	if srcPath == "" {
+		srcPath = m["path"]
+	}
+	return shared.HostPath(srcPath)
+}
+
 // unixDeviceDestPath returns the absolute path for a device inside an instance.
-// This is based on the "path" property of the devices' config, or the "source" property if "path"
+// This is based on the "path" property of the device's config, or the "source" property if "path"
 // not defined.
 func unixDeviceDestPath(m config.Device) string {
 	destPath := m["path"]
@@ -153,51 +158,49 @@ func UnixDeviceCreate(s *state.State, idmapSet *idmap.IdmapSet, devicesPath stri
 		}
 	}
 
-	srcPath := m["source"]
-	if srcPath == "" {
-		srcPath = m["path"]
-	}
-	srcPath = shared.HostPath(srcPath)
+	srcPath := unixDeviceSourcePath(m)
 
 	// Get the major/minor of the device we want to create.
 	if m["major"] == "" && m["minor"] == "" {
 		// If no major and minor are set, use those from the device on the host.
 		_, d.Major, d.Minor, err = UnixDeviceAttributes(srcPath)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to get device attributes for %s: %s", m["path"], err)
+			return nil, fmt.Errorf("Failed to get device attributes for %s: %s", srcPath, err)
 		}
 	} else if m["major"] == "" || m["minor"] == "" {
-		return nil, fmt.Errorf("Both major and minor must be supplied for device: %s", m["path"])
+		return nil, fmt.Errorf("Both major and minor must be supplied for device: %s", srcPath)
 	} else {
 		tmp, err := strconv.ParseUint(m["major"], 10, 32)
 		if err != nil {
-			return nil, fmt.Errorf("Bad major %s in device %s", m["major"], m["path"])
+			return nil, fmt.Errorf("Bad major %s in device %s", m["major"], srcPath)
 		}
 		d.Major = uint32(tmp)
 
 		tmp, err = strconv.ParseUint(m["minor"], 10, 32)
 		if err != nil {
-			return nil, fmt.Errorf("Bad minor %s in device %s", m["minor"], m["path"])
+			return nil, fmt.Errorf("Bad minor %s in device %s", m["minor"], srcPath)
 		}
 		d.Minor = uint32(tmp)
 	}
 
-	// Get the device mode (defaults to 0660 if not supplied).
-	d.Mode = os.FileMode(0660)
+	// Get the device mode (defaults to unixDefaultMode if not supplied).
+	d.Mode = os.FileMode(unixDefaultMode)
 	if m["mode"] != "" {
 		tmp, err := unixDeviceModeOct(m["mode"])
 		if err != nil {
-			return nil, fmt.Errorf("Bad mode %s in device %s", m["mode"], m["path"])
+			return nil, fmt.Errorf("Bad mode %s in device %s", m["mode"], srcPath)
 		}
 		d.Mode = os.FileMode(tmp)
 	} else if !defaultMode {
+		// If not specified mode in device config, and default mode is false, then try and
+		// read the source device's mode and use that inside the instance.
 		d.Mode, err = shared.GetPathMode(srcPath)
 		if err != nil {
 			errno, isErrno := shared.GetErrno(err)
 			if !isErrno || errno != unix.ENOENT {
-				return nil, fmt.Errorf("Failed to retrieve mode of device %s: %s", m["path"], err)
+				return nil, fmt.Errorf("Failed to retrieve mode of device %s: %s", srcPath, err)
 			}
-			d.Mode = os.FileMode(0660)
+			d.Mode = os.FileMode(unixDefaultMode)
 		}
 	}
 
@@ -213,14 +216,14 @@ func UnixDeviceCreate(s *state.State, idmapSet *idmap.IdmapSet, devicesPath stri
 	if m["uid"] != "" {
 		d.UID, err = strconv.Atoi(m["uid"])
 		if err != nil {
-			return nil, fmt.Errorf("Invalid uid %s in device %s", m["uid"], m["path"])
+			return nil, fmt.Errorf("Invalid uid %s in device %s", m["uid"], srcPath)
 		}
 	}
 
 	if m["gid"] != "" {
 		d.GID, err = strconv.Atoi(m["gid"])
 		if err != nil {
-			return nil, fmt.Errorf("Invalid gid %s in device %s", m["gid"], m["path"])
+			return nil, fmt.Errorf("Invalid gid %s in device %s", m["gid"], srcPath)
 		}
 	}
 
@@ -242,7 +245,7 @@ func UnixDeviceCreate(s *state.State, idmapSet *idmap.IdmapSet, devicesPath stri
 		devNum := int(unix.Mkdev(d.Major, d.Minor))
 		err := unix.Mknod(devPath, uint32(d.Mode), devNum)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to create device %s for %s: %s", devPath, m["path"], err)
+			return nil, fmt.Errorf("Failed to create device %s for %s: %s", devPath, srcPath, err)
 		}
 
 		err = os.Chown(devPath, d.UID, d.GID)
@@ -260,7 +263,7 @@ func UnixDeviceCreate(s *state.State, idmapSet *idmap.IdmapSet, devicesPath stri
 			err := idmapSet.ShiftFile(devPath)
 			if err != nil {
 				// uidshift failing is weird, but not a big problem. Log and proceed.
-				logger.Debugf("Failed to uidshift device %s: %s\n", m["path"], err)
+				logger.Debugf("Failed to uidshift device %s: %s\n", srcPath, err)
 			}
 		}
 	} else {

From 98df6e48efae87bc2e19968d14208c0c9283802f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 16:03:24 +0100
Subject: [PATCH 03/30] device: Moves empty device type validation into device
 package

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

diff --git a/lxd/device/device.go b/lxd/device/device.go
index 83cc952537..a3bf232f61 100644
--- a/lxd/device/device.go
+++ b/lxd/device/device.go
@@ -129,6 +129,10 @@ func (d *deviceCommon) Remove() error {
 // is still returned with the validation error. If an unknown device is requested or the device is
 // not compatible with the instance type then an ErrUnsupportedDevType error is returned.
 func New(instance InstanceIdentifier, state *state.State, name string, conf config.Device, volatileGet VolatileGetter, volatileSet VolatileSetter) (Device, error) {
+	if conf["type"] == "" {
+		return nil, fmt.Errorf("Missing device type for device '%s'", name)
+	}
+
 	devFunc := devTypes[conf["type"]]
 
 	// Check if top-level type is recognised, if it is known type it will return a function.

From 47cc80d213bb1a4bf798c4c13b5097425b9ab455 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 16:13:14 +0100
Subject: [PATCH 04/30] container: Removes unused unix-char and unix-block
 validation

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container.go | 48 ------------------------------------------------
 1 file changed, 48 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index e7894faa44..39d46f72b1 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -108,27 +108,6 @@ func containerValidDeviceConfigKey(t, k string) bool {
 	}
 
 	switch t {
-	case "unix-char", "unix-block":
-		switch k {
-		case "gid":
-			return true
-		case "major":
-			return true
-		case "minor":
-			return true
-		case "mode":
-			return true
-		case "source":
-			return true
-		case "path":
-			return true
-		case "required":
-			return true
-		case "uid":
-			return true
-		default:
-			return false
-		}
 	case "disk":
 		switch k {
 		case "limits.max":
@@ -341,33 +320,6 @@ func containerValidDevices(state *state.State, cluster *db.Cluster, devices conf
 					return fmt.Errorf("The \"shift\" property cannot be used with custom storage volumes")
 				}
 			}
-		} else if shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
-			if m["source"] == "" && m["path"] == "" {
-				return fmt.Errorf("Unix device entry is missing the required \"source\" or \"path\" property")
-			}
-
-			if (m["required"] == "" || shared.IsTrue(m["required"])) && (m["major"] == "" || m["minor"] == "") {
-				srcPath, exist := m["source"]
-				if !exist {
-					srcPath = m["path"]
-				}
-				if !shared.PathExists(srcPath) {
-					return fmt.Errorf("The device path doesn't exist on the host and major/minor wasn't specified")
-				}
-
-				dType, _, _, err := device.UnixDeviceAttributes(srcPath)
-				if err != nil {
-					return err
-				}
-
-				if m["type"] == "unix-char" && dType != "c" {
-					return fmt.Errorf("Path specified for unix-char device is a block device")
-				}
-
-				if m["type"] == "unix-block" && dType != "b" {
-					return fmt.Errorf("Path specified for unix-block device is a character device")
-				}
-			}
 		} else if m["type"] == "none" {
 			continue
 		} else {

From 970677eb6bc7428b5a2a314251729e7b13f1352f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 16:13:43 +0100
Subject: [PATCH 05/30] container: Moves missing device type validation into
 device package

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

diff --git a/lxd/container.go b/lxd/container.go
index 39d46f72b1..25e02f88a7 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -223,21 +223,18 @@ func containerValidDevices(state *state.State, cluster *db.Cluster, devices conf
 	var diskDevicePaths []string
 	// Check each device individually
 	for name, m := range devices {
-		if m["type"] == "" {
-			return fmt.Errorf("Missing device type for device '%s'", name)
-		}
-
-		if !shared.StringInSlice(m["type"], []string{"disk", "gpu", "infiniband", "nic", "none", "proxy", "unix-block", "unix-char", "usb"}) {
-			return fmt.Errorf("Invalid device type for device '%s'", name)
-		}
-
 		// Validate config using device interface.
 		_, err := device.New(&containerLXC{}, state, name, config.Device(m), nil, nil)
 		if err != device.ErrUnsupportedDevType {
 			if err != nil {
 				return err
 			}
-			continue
+			continue // Validated device OK.
+		}
+
+		// Fallback to legacy validation for non device package devices.
+		if !shared.StringInSlice(m["type"], []string{"disk", "none"}) {
+			return fmt.Errorf("Invalid device type for device '%s'", name)
 		}
 
 		for k := range m {

From 3c2782b9441a70ecac6cc50a9736886a60b89913 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 16:15:55 +0100
Subject: [PATCH 06/30] container/lxc: Adds safety net for deviceStop() in case
 no device returned

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 81fa471d16..c571601076 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -2072,6 +2072,12 @@ func (c *containerLXC) deviceStop(deviceName string, rawConfig map[string]string
 	// 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 {
+		// 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)
+
+		}
+
 		logger.Errorf("Device stop validation failed for '%s': %v", deviceName, err)
 	}
 

From 98f87b44b30a2759143ae4c12ccd467ad4f1e85f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 16:16:35 +0100
Subject: [PATCH 07/30] container/lxc: Removes unused unix-char and unix-block
 code

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index c571601076..b25d526b52 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1639,36 +1639,7 @@ func (c *containerLXC) initLXC(config bool) error {
 	// Setup devices
 	for _, k := range c.expandedDevices.DeviceNames() {
 		m := c.expandedDevices[k]
-		if shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
-			// destination paths
-			destPath := m["path"]
-			if destPath == "" {
-				destPath = m["source"]
-			}
-
-			srcPath := m["source"]
-			if srcPath == "" {
-				srcPath = m["path"]
-			}
-
-			relativeDestPath := strings.TrimPrefix(destPath, "/")
-			sourceDevPath := filepath.Join(c.DevicesPath(), fmt.Sprintf("unix.%s.%s", strings.Replace(k, "/", "-", -1), strings.Replace(relativeDestPath, "/", "-", -1)))
-
-			// Don't add mount entry for devices that don't yet exist
-			if m["required"] != "" && !shared.IsTrue(m["required"]) && srcPath != "" && !shared.PathExists(srcPath) {
-				continue
-			}
-
-			// inform liblxc about the mount
-			err = lxcSetConfigItem(cc, "lxc.mount.entry",
-				fmt.Sprintf("%s %s %s",
-					shared.EscapePathFstab(sourceDevPath),
-					shared.EscapePathFstab(relativeDestPath),
-					"none bind,create=file 0 0"))
-			if err != nil {
-				return err
-			}
-		} else if m["type"] == "disk" {
+		if m["type"] == "disk" {
 			isRootfs := shared.IsRootDiskDevice(m)
 
 			// source paths
@@ -2520,21 +2491,6 @@ func (c *containerLXC) startCommon() (string, []func() error, error) {
 			if m["pool"] == "" && m["source"] != "" && !shared.IsTrue(m["optional"]) && !shared.PathExists(shared.HostPath(m["source"])) {
 				return "", postStartHooks, fmt.Errorf("Missing source '%s' for disk '%s'", m["source"], name)
 			}
-		case "unix-char", "unix-block":
-			srcPath, exist := m["source"]
-			if !exist {
-				srcPath = m["path"]
-			}
-
-			if srcPath != "" && m["required"] != "" && !shared.IsTrue(m["required"]) {
-				err = deviceInotifyAddClosestLivingAncestor(c.state, filepath.Dir(srcPath))
-				if err != nil {
-					logger.Errorf("Failed to add \"%s\" to inotify targets", srcPath)
-					return "", postStartHooks, fmt.Errorf("Failed to setup inotify watch for '%s': %v", srcPath, err)
-				}
-			} else if srcPath != "" && m["major"] == "" && m["minor"] == "" && !shared.PathExists(srcPath) {
-				return "", postStartHooks, fmt.Errorf("Missing source '%s' for device '%s'", srcPath, name)
-			}
 		}
 	}
 
@@ -2682,49 +2638,7 @@ func (c *containerLXC) startCommon() (string, []func() error, error) {
 	nicID := -1
 	for _, k := range c.expandedDevices.DeviceNames() {
 		m := c.expandedDevices[k]
-		if shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
-			idmapSet, err := c.CurrentIdmap()
-			if err != nil {
-				return "", postStartHooks, err
-			}
-
-			// Unix device
-			d, err := device.UnixDeviceCreate(c.state, idmapSet, c.DevicesPath(), fmt.Sprintf("unix.%s", k), m, true)
-			if err != nil {
-				// Deal with device hotplug
-				if m["required"] == "" || shared.IsTrue(m["required"]) {
-					return "", postStartHooks, err
-				}
-
-				srcPath := m["source"]
-				if srcPath == "" {
-					srcPath = m["path"]
-				}
-				srcPath = shared.HostPath(srcPath)
-
-				err = deviceInotifyAddClosestLivingAncestor(c.state, filepath.Dir(srcPath))
-				if err != nil {
-					logger.Errorf("Failed to add \"%s\" to inotify targets", srcPath)
-					return "", postStartHooks, err
-				}
-				continue
-			}
-			devPath := d.HostPath
-			if c.isCurrentlyPrivileged() && !c.state.OS.RunningInUserNS && c.state.OS.CGroupDevicesController {
-				// Add the new device cgroup rule
-				dType, dMajor, dMinor, err := device.UnixDeviceAttributes(devPath)
-				if err != nil {
-					if m["required"] == "" || shared.IsTrue(m["required"]) {
-						return "", postStartHooks, err
-					}
-				} else {
-					err = lxcSetConfigItem(c.c, "lxc.cgroup.devices.allow", fmt.Sprintf("%s %d:%d rwm", dType, dMajor, dMinor))
-					if err != nil {
-						return "", postStartHooks, fmt.Errorf("Failed to add cgroup rule for device")
-					}
-				}
-			}
-		} else if m["type"] == "disk" {
+		if m["type"] == "disk" {
 			if m["path"] != "/" {
 				diskDevices[k] = m
 			}
@@ -5068,22 +4982,7 @@ func (c *containerLXC) Update(args db.ContainerArgs, userRequested bool) error {
 
 		// Live update the devices
 		for k, m := range removeDevices {
-			if shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
-				prefix := fmt.Sprintf("unix.%s", k)
-				destPath := m["path"]
-				if destPath == "" {
-					destPath = m["source"]
-				}
-
-				if !device.UnixDeviceExists(c.DevicesPath(), prefix, destPath) && (m["required"] != "" && !shared.IsTrue(m["required"])) {
-					continue
-				}
-
-				err = c.removeUnixDevice(fmt.Sprintf("unix.%s", k), m, true)
-				if err != nil {
-					return err
-				}
-			} else if m["type"] == "disk" && m["path"] != "/" {
+			if m["type"] == "disk" && m["path"] != "/" {
 				err = c.removeDiskDevice(k, m)
 				if err != nil {
 					return err
@@ -5093,14 +4992,7 @@ func (c *containerLXC) Update(args db.ContainerArgs, userRequested bool) error {
 
 		diskDevices := map[string]config.Device{}
 		for k, m := range addDevices {
-			if shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
-				err = c.insertUnixDevice(fmt.Sprintf("unix.%s", k), m, true)
-				if err != nil {
-					if m["required"] == "" || shared.IsTrue(m["required"]) {
-						return err
-					}
-				}
-			} else if m["type"] == "disk" && m["path"] != "/" {
+			if m["type"] == "disk" && m["path"] != "/" {
 				diskDevices[k] = m
 			}
 		}
@@ -6826,73 +6718,6 @@ func (c *containerLXC) removeMount(mount string) error {
 	return nil
 }
 
-func (c *containerLXC) insertUnixDevice(prefix string, m config.Device, defaultMode bool) error {
-	// Check that the container is running
-	if !c.IsRunning() {
-		return fmt.Errorf("Can't insert device into stopped container")
-	}
-
-	idmapSet, err := c.CurrentIdmap()
-	if err != nil {
-		return err
-	}
-
-	// Create the device on the host
-	d, err := device.UnixDeviceCreate(c.state, idmapSet, c.DevicesPath(), prefix, m, defaultMode)
-	if err != nil {
-		return fmt.Errorf("Failed to setup device: %s", err)
-	}
-	devPath := d.HostPath
-	tgtPath := d.RelativePath
-
-	// Bind-mount it into the container
-	err = c.insertMount(devPath, tgtPath, "none", unix.MS_BIND, false)
-	if err != nil {
-		return fmt.Errorf("Failed to add mount for device: %s", err)
-	}
-
-	// Check if we've been passed major and minor numbers already.
-	var dMajor, dMinor uint32
-	if m["major"] != "" {
-		tmp, err := strconv.ParseUint(m["major"], 10, 32)
-		if err != nil {
-			return err
-		}
-		dMajor = uint32(tmp)
-	}
-
-	if m["minor"] != "" {
-		tmp, err := strconv.ParseUint(m["minor"], 10, 32)
-		if err != nil {
-			return err
-		}
-		dMinor = uint32(tmp)
-	}
-
-	dType := ""
-	if m["type"] == "unix-char" {
-		dType = "c"
-	} else if m["type"] == "unix-block" {
-		dType = "b"
-	}
-
-	if dType == "" || m["major"] == "" || m["minor"] == "" {
-		dType, dMajor, dMinor, err = device.UnixDeviceAttributes(devPath)
-		if err != nil {
-			return err
-		}
-	}
-
-	if c.isCurrentlyPrivileged() && !c.state.OS.RunningInUserNS && c.state.OS.CGroupDevicesController {
-		// Add the new device cgroup rule
-		if err := c.CGroupSet("devices.allow", fmt.Sprintf("%s %d:%d rwm", dType, dMajor, dMinor)); err != nil {
-			return fmt.Errorf("Failed to add cgroup rule for device")
-		}
-	}
-
-	return nil
-}
-
 func (c *containerLXC) InsertSeccompUnixDevice(prefix string, m config.Device, pid int) error {
 	if pid < 0 {
 		return fmt.Errorf("Invalid request PID specified")
@@ -6948,132 +6773,6 @@ func (c *containerLXC) InsertSeccompUnixDevice(prefix string, m config.Device, p
 	return c.insertMountLXD(devPath, tgtPath, "none", unix.MS_BIND, pid, false)
 }
 
-func (c *containerLXC) insertUnixDeviceNum(name string, m config.Device, major int, minor int, path string, defaultMode bool) error {
-	temp := config.Device{}
-	if err := shared.DeepCopy(&m, &temp); err != nil {
-		return err
-	}
-
-	temp["major"] = fmt.Sprintf("%d", major)
-	temp["minor"] = fmt.Sprintf("%d", minor)
-	temp["path"] = path
-
-	return c.insertUnixDevice(name, temp, defaultMode)
-}
-
-// removeUnixDevice does the following: retrieves the device type, major and minor numbers,
-// adds a cgroup deny rule for the device if container is privileged.
-// Then it removes the mount and file inside container.
-// Finally it removes the host-side mount if running in UserNS and removes host-side dev file.
-func (c *containerLXC) removeUnixDevice(prefix string, m config.Device, eject bool) error {
-	// Check that the container is running
-	pid := c.InitPID()
-	if pid == -1 {
-		return fmt.Errorf("Can't remove device from stopped container")
-	}
-
-	// Check if we've been passed major and minor numbers already.
-	var err error
-	var dMajor, dMinor uint32
-	if m["major"] != "" {
-		tmp, err := strconv.ParseUint(m["major"], 10, 32)
-		if err != nil {
-			return err
-		}
-
-		dMajor = uint32(tmp)
-	}
-
-	if m["minor"] != "" {
-		tmp, err := strconv.ParseUint(m["minor"], 10, 32)
-		if err != nil {
-			return err
-		}
-
-		dMinor = uint32(tmp)
-	}
-
-	dType := ""
-	if m["type"] == "unix-char" {
-		dType = "c"
-	} else if m["type"] == "unix-block" {
-		dType = "b"
-	}
-
-	// Figure out the paths
-	destPath := m["path"]
-	if destPath == "" {
-		destPath = m["source"]
-	}
-	relativeDestPath := strings.TrimPrefix(destPath, "/")
-	devName := fmt.Sprintf("%s.%s", strings.Replace(prefix, "/", "-", -1), strings.Replace(relativeDestPath, "/", "-", -1))
-	devPath := filepath.Join(c.DevicesPath(), devName)
-
-	if dType == "" || m["major"] == "" || m["minor"] == "" {
-		dType, dMajor, dMinor, err = device.UnixDeviceAttributes(devPath)
-		if err != nil {
-			return err
-		}
-	}
-
-	if c.isCurrentlyPrivileged() && !c.state.OS.RunningInUserNS && c.state.OS.CGroupDevicesController {
-		// Remove the device cgroup rule
-		err := c.CGroupSet("devices.deny", fmt.Sprintf("%s %d:%d rwm", dType, dMajor, dMinor))
-		if err != nil {
-			return err
-		}
-	}
-
-	if eject && c.FileExists(relativeDestPath) == nil {
-		err := c.removeMount(destPath)
-		if err != nil {
-			return fmt.Errorf("Error unmounting the device: %s", err)
-		}
-
-		err = c.FileRemove(relativeDestPath)
-		if err != nil {
-			return fmt.Errorf("Error removing the device: %s", err)
-		}
-	}
-
-	// Remove the host side
-	if c.state.OS.RunningInUserNS {
-		unix.Unmount(devPath, unix.MNT_DETACH)
-	}
-
-	err = os.Remove(devPath)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func (c *containerLXC) removeUnixDeviceNum(prefix string, m config.Device, major int, minor int, path string) error {
-	pid := c.InitPID()
-	if pid == -1 {
-		return fmt.Errorf("Can't remove device from stopped container")
-	}
-
-	temp := config.Device{}
-	if err := shared.DeepCopy(&m, &temp); err != nil {
-		return err
-	}
-
-	temp["major"] = fmt.Sprintf("%d", major)
-	temp["minor"] = fmt.Sprintf("%d", minor)
-	temp["path"] = path
-
-	err := c.removeUnixDevice(prefix, temp, true)
-	if err != nil {
-		logger.Error("Failed to remove device", log.Ctx{"err": err, m["type"]: path, "container": c.Name()})
-		return err
-	}
-
-	c.FileRemove(filepath.Dir(path))
-	return nil
-}
-
 func (c *containerLXC) removeUnixDevices() error {
 	// Check that we indeed have devices to remove
 	if !shared.PathExists(c.DevicesPath()) {

From 414275eedb90ea2365de53648572e78d305d91c2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 14:58:24 +0100
Subject: [PATCH 08/30] devices: Renames USBDevice to USBEvent

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/devices.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lxd/devices.go b/lxd/devices.go
index 6331214a9b..73eae2be86 100644
--- a/lxd/devices.go
+++ b/lxd/devices.go
@@ -48,7 +48,7 @@ func (c deviceTaskCPUs) Len() int           { return len(c) }
 func (c deviceTaskCPUs) Less(i, j int) bool { return *c[i].count < *c[j].count }
 func (c deviceTaskCPUs) Swap(i, j int)      { c[i], c[j] = c[j], c[i] }
 
-func deviceNetlinkListener() (chan []string, chan []string, chan device.USBDevice, error) {
+func deviceNetlinkListener() (chan []string, chan []string, chan device.USBEvent, error) {
 	NETLINK_KOBJECT_UEVENT := 15
 	UEVENT_BUFFER_SIZE := 2048
 
@@ -73,9 +73,9 @@ func deviceNetlinkListener() (chan []string, chan []string, chan device.USBDevic
 
 	chCPU := make(chan []string, 1)
 	chNetwork := make(chan []string, 0)
-	chUSB := make(chan device.USBDevice)
+	chUSB := make(chan device.USBEvent)
 
-	go func(chCPU chan []string, chNetwork chan []string, chUSB chan device.USBDevice) {
+	go func(chCPU chan []string, chNetwork chan []string, chUSB chan device.USBEvent) {
 		b := make([]byte, UEVENT_BUFFER_SIZE*2)
 		for {
 			r, err := unix.Read(fd, b)
@@ -170,7 +170,7 @@ func deviceNetlinkListener() (chan []string, chan []string, chan device.USBDevic
 					return strings.Repeat("0", l-len(s)) + s
 				}
 
-				usb, err := device.USBDeviceLoad(
+				usb, err := device.USBNewEvent(
 					props["ACTION"],
 					/* udev doesn't zero pad these, while
 					 * everything else does, so let's zero pad them

From 2e2fb3291e92ed2f864b7726568c166b386d7083 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 14:59:05 +0100
Subject: [PATCH 09/30] device/usb: Adds unexported usbIsOurDevice function and
 switches to USBEvent

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/usb.go | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/lxd/device/usb.go b/lxd/device/usb.go
index 30df3f3394..e2aa19b851 100644
--- a/lxd/device/usb.go
+++ b/lxd/device/usb.go
@@ -12,6 +12,21 @@ import (
 	"github.com/lxc/lxd/shared"
 )
 
+// usbDevPath is the path where USB devices can be enumerated.
+const usbDevPath = "/sys/bus/usb/devices"
+
+// usbIsOurDevice indicates whether the USB device event qualifies as part of our device.
+// This function is not defined against the usb struct type so that it can be used in event
+// callbacks without needing to keep a reference to the usb device struct.
+func usbIsOurDevice(config config.Device, usb *USBEvent) bool {
+	// Check if event matches criteria for this device, if not return.
+	if (config["vendorid"] != "" && config["vendorid"] != usb.Vendor) || (config["productid"] != "" && config["productid"] != usb.Product) {
+		return false
+	}
+
+	return true
+}
+
 type usb struct {
 	deviceCommon
 }
@@ -54,20 +69,20 @@ func (d *usb) Register() error {
 	state := d.state
 
 	// Handler for when a USB event occurs.
-	f := func(usb USBDevice) (*RunConfig, error) {
-		if !USBIsOurDevice(deviceConfig, &usb) {
+	f := func(e USBEvent) (*RunConfig, error) {
+		if !usbIsOurDevice(deviceConfig, &e) {
 			return nil, nil
 		}
 
 		runConf := RunConfig{}
 
-		if usb.Action == "add" {
-			err := unixDeviceSetupCharNum(state, devicesPath, "unix", deviceName, deviceConfig, usb.Major, usb.Minor, usb.Path, false, &runConf)
+		if e.Action == "add" {
+			err := unixDeviceSetupCharNum(state, devicesPath, "unix", deviceName, deviceConfig, e.Major, e.Minor, e.Path, false, &runConf)
 			if err != nil {
 				return nil, err
 			}
-		} else if usb.Action == "remove" {
-			relativeTargetPath := strings.TrimPrefix(usb.Path, "/")
+		} else if e.Action == "remove" {
+			relativeTargetPath := strings.TrimPrefix(e.Path, "/")
 			err := unixDeviceRemove(devicesPath, "unix", deviceName, relativeTargetPath, &runConf)
 			if err != nil {
 				return nil, err
@@ -84,7 +99,7 @@ func (d *usb) Register() error {
 			}}
 		}
 
-		runConf.Uevents = append(runConf.Uevents, usb.UeventParts)
+		runConf.Uevents = append(runConf.Uevents, e.UeventParts)
 
 		return &runConf, nil
 	}
@@ -109,7 +124,7 @@ func (d *usb) Start() (*RunConfig, error) {
 	runConf := RunConfig{}
 
 	for _, usb := range usbs {
-		if !USBIsOurDevice(d.config, &usb) {
+		if !usbIsOurDevice(d.config, &usb) {
 			continue
 		}
 
@@ -155,8 +170,8 @@ func (d *usb) postStop() error {
 }
 
 // loadUsb scans the host machine for USB devices.
-func (d *usb) loadUsb() ([]USBDevice, error) {
-	result := []USBDevice{}
+func (d *usb) loadUsb() ([]USBEvent, error) {
+	result := []USBEvent{}
 
 	ents, err := ioutil.ReadDir(usbDevPath)
 	if err != nil {
@@ -175,15 +190,15 @@ func (d *usb) loadUsb() ([]USBDevice, error) {
 				continue
 			}
 
-			return []USBDevice{}, err
+			return []USBEvent{}, err
 		}
 
 		parts := strings.Split(values["dev"], ":")
 		if len(parts) != 2 {
-			return []USBDevice{}, fmt.Errorf("invalid device value %s", values["dev"])
+			return []USBEvent{}, fmt.Errorf("invalid device value %s", values["dev"])
 		}
 
-		usb, err := USBDeviceLoad(
+		usb, err := USBNewEvent(
 			"add",
 			values["idVendor"],
 			values["idProduct"],

From 2eacc4b92ab6866a7e98069ff795e20d3e497208 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 14:59:44 +0100
Subject: [PATCH 10/30] device/device/utils/usb: Removed and moved into usb and
 device_utils_usb_events

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/device_utils_usb.go | 149 ---------------------------------
 1 file changed, 149 deletions(-)
 delete mode 100644 lxd/device/device_utils_usb.go

diff --git a/lxd/device/device_utils_usb.go b/lxd/device/device_utils_usb.go
deleted file mode 100644
index 21a348c2ed..0000000000
--- a/lxd/device/device_utils_usb.go
+++ /dev/null
@@ -1,149 +0,0 @@
-package device
-
-import (
-	"fmt"
-	"path/filepath"
-	"strconv"
-	"strings"
-	"sync"
-
-	"github.com/lxc/lxd/lxd/device/config"
-	"github.com/lxc/lxd/lxd/state"
-	log "github.com/lxc/lxd/shared/log15"
-	"github.com/lxc/lxd/shared/logger"
-)
-
-// usbDevPath is the path where USB devices can be enumerated.
-const usbDevPath = "/sys/bus/usb/devices"
-
-// USBDevice represents the properties of a USB device and a USB uevent.
-type USBDevice struct {
-	Action string
-
-	Vendor  string
-	Product string
-
-	Path        string
-	Major       uint32
-	Minor       uint32
-	UeventParts []string
-	UeventLen   int
-}
-
-// usbHandlers stores the event handler callbacks for USB events.
-var usbHandlers = map[string]func(USBDevice) (*RunConfig, error){}
-
-// usbMutex controls access to the usbHandlers map.
-var usbMutex sync.Mutex
-
-// USBRegisterHandler registers a handler function to be called whenever a USB device event occurs.
-func USBRegisterHandler(instance InstanceIdentifier, deviceName string, handler func(USBDevice) (*RunConfig, error)) {
-	usbMutex.Lock()
-	defer usbMutex.Unlock()
-
-	// Null delimited string of project name, instance name and device name.
-	key := fmt.Sprintf("%s\000%s\000%s", instance.Project(), instance.Name(), deviceName)
-	usbHandlers[key] = handler
-}
-
-// USBUnregisterHandler removes a registered USB handler function for a device.
-func USBUnregisterHandler(instance InstanceIdentifier, deviceName string) {
-	usbMutex.Lock()
-	defer usbMutex.Unlock()
-
-	// Null delimited string of project name, instance name and device name.
-	key := fmt.Sprintf("%s\000%s\000%s", instance.Project(), instance.Name(), deviceName)
-	delete(usbHandlers, key)
-}
-
-// USBRunHandlers executes any handlers registered for USB events.
-func USBRunHandlers(state *state.State, event *USBDevice) {
-	usbMutex.Lock()
-	defer usbMutex.Unlock()
-
-	for key, hook := range usbHandlers {
-		keyParts := strings.SplitN(key, "\000", 3)
-		projectName := keyParts[0]
-		instanceName := keyParts[1]
-		deviceName := keyParts[2]
-
-		if hook == nil {
-			delete(usbHandlers, key)
-			continue
-		}
-
-		runConf, err := hook(*event)
-		if err != nil {
-			logger.Error("USB event hook failed", log.Ctx{"err": err, "project": projectName, "instance": instanceName, "device": deviceName})
-			continue
-		}
-
-		// If runConf supplied, load instance and call its USB event handler function so
-		// any instance specific device actions can occur.
-		if runConf != nil {
-			instance, err := InstanceLoadByProjectAndName(state, projectName, instanceName)
-			if err != nil {
-				logger.Error("USB event loading instance failed", log.Ctx{"err": err, "project": projectName, "instance": instanceName, "device": deviceName})
-				continue
-			}
-
-			err = instance.DeviceEventHandler(runConf)
-			if err != nil {
-				logger.Error("USB event instance handler failed", log.Ctx{"err": err, "project": projectName, "instance": instanceName, "device": deviceName})
-				continue
-			}
-		}
-	}
-}
-
-// USBDeviceLoad instantiates a new USBDevice struct.
-func USBDeviceLoad(action string, vendor string, product string, major string, minor string, busnum string, devnum string, devname string, ueventParts []string, ueventLen int) (USBDevice, error) {
-	majorInt, err := strconv.ParseUint(major, 10, 32)
-	if err != nil {
-		return USBDevice{}, err
-	}
-
-	minorInt, err := strconv.ParseUint(minor, 10, 32)
-	if err != nil {
-		return USBDevice{}, err
-	}
-
-	path := devname
-	if devname == "" {
-		busnumInt, err := strconv.Atoi(busnum)
-		if err != nil {
-			return USBDevice{}, err
-		}
-
-		devnumInt, err := strconv.Atoi(devnum)
-		if err != nil {
-			return USBDevice{}, err
-		}
-		path = fmt.Sprintf("/dev/bus/usb/%03d/%03d", busnumInt, devnumInt)
-	} else {
-		if !filepath.IsAbs(devname) {
-			path = fmt.Sprintf("/dev/%s", devname)
-		}
-	}
-
-	return USBDevice{
-		action,
-		vendor,
-		product,
-		path,
-		uint32(majorInt),
-		uint32(minorInt),
-		ueventParts,
-		ueventLen,
-	}, nil
-}
-
-// USBIsOurDevice indicates whether the USB device event qualifies as part of our device.
-func USBIsOurDevice(config config.Device, usb *USBDevice) bool {
-	// Check if event matches criteria for this device, if not return.
-	if (config["vendorid"] != "" && config["vendorid"] != usb.Vendor) || (config["productid"] != "" && config["productid"] != usb.Product) {
-		return false
-	}
-
-	return true
-}

From 11bf0f1bed05cf1ef572f37172a4686bca048779 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 15:00:20 +0100
Subject: [PATCH 11/30] device/device/utils/usb/events: Adds USB event handler
 functions

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/device_utils_usb_events.go | 135 ++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)
 create mode 100644 lxd/device/device_utils_usb_events.go

diff --git a/lxd/device/device_utils_usb_events.go b/lxd/device/device_utils_usb_events.go
new file mode 100644
index 0000000000..08807d0c83
--- /dev/null
+++ b/lxd/device/device_utils_usb_events.go
@@ -0,0 +1,135 @@
+package device
+
+import (
+	"fmt"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"sync"
+
+	"github.com/lxc/lxd/lxd/state"
+	log "github.com/lxc/lxd/shared/log15"
+	"github.com/lxc/lxd/shared/logger"
+)
+
+// USBEvent represents the properties of a USB device uevent.
+type USBEvent struct {
+	Action string
+
+	Vendor  string
+	Product string
+
+	Path        string
+	Major       uint32
+	Minor       uint32
+	UeventParts []string
+	UeventLen   int
+}
+
+// usbHandlers stores the event handler callbacks for USB events.
+var usbHandlers = map[string]func(USBEvent) (*RunConfig, error){}
+
+// usbMutex controls access to the usbHandlers map.
+var usbMutex sync.Mutex
+
+// usbRegisterHandler registers a handler function to be called whenever a USB device event occurs.
+func usbRegisterHandler(instance InstanceIdentifier, deviceName string, handler func(USBEvent) (*RunConfig, error)) {
+	usbMutex.Lock()
+	defer usbMutex.Unlock()
+
+	// Null delimited string of project name, instance name and device name.
+	key := fmt.Sprintf("%s\000%s\000%s", instance.Project(), instance.Name(), deviceName)
+	usbHandlers[key] = handler
+}
+
+// usbUnregisterHandler removes a registered USB handler function for a device.
+func usbUnregisterHandler(instance InstanceIdentifier, deviceName string) {
+	usbMutex.Lock()
+	defer usbMutex.Unlock()
+
+	// Null delimited string of project name, instance name and device name.
+	key := fmt.Sprintf("%s\000%s\000%s", instance.Project(), instance.Name(), deviceName)
+	delete(usbHandlers, key)
+}
+
+// USBRunHandlers executes any handlers registered for USB events.
+func USBRunHandlers(state *state.State, event *USBEvent) {
+	usbMutex.Lock()
+	defer usbMutex.Unlock()
+
+	for key, hook := range usbHandlers {
+		keyParts := strings.SplitN(key, "\000", 3)
+		projectName := keyParts[0]
+		instanceName := keyParts[1]
+		deviceName := keyParts[2]
+
+		if hook == nil {
+			delete(usbHandlers, key)
+			continue
+		}
+
+		runConf, err := hook(*event)
+		if err != nil {
+			logger.Error("USB event hook failed", log.Ctx{"err": err, "project": projectName, "instance": instanceName, "device": deviceName})
+			continue
+		}
+
+		// If runConf supplied, load instance and call its USB event handler function so
+		// any instance specific device actions can occur.
+		if runConf != nil {
+			instance, err := InstanceLoadByProjectAndName(state, projectName, instanceName)
+			if err != nil {
+				logger.Error("USB event loading instance failed", log.Ctx{"err": err, "project": projectName, "instance": instanceName, "device": deviceName})
+				continue
+			}
+
+			err = instance.DeviceEventHandler(runConf)
+			if err != nil {
+				logger.Error("USB event instance handler failed", log.Ctx{"err": err, "project": projectName, "instance": instanceName, "device": deviceName})
+				continue
+			}
+		}
+	}
+}
+
+// USBNewEvent instantiates a new USBEvent struct.
+func USBNewEvent(action string, vendor string, product string, major string, minor string, busnum string, devnum string, devname string, ueventParts []string, ueventLen int) (USBEvent, error) {
+	majorInt, err := strconv.ParseUint(major, 10, 32)
+	if err != nil {
+		return USBEvent{}, err
+	}
+
+	minorInt, err := strconv.ParseUint(minor, 10, 32)
+	if err != nil {
+		return USBEvent{}, err
+	}
+
+	path := devname
+	if devname == "" {
+		busnumInt, err := strconv.Atoi(busnum)
+		if err != nil {
+			return USBEvent{}, err
+		}
+
+		devnumInt, err := strconv.Atoi(devnum)
+		if err != nil {
+			return USBEvent{}, err
+		}
+		path = fmt.Sprintf("/dev/bus/usb/%03d/%03d", busnumInt, devnumInt)
+	} else {
+		if !filepath.IsAbs(devname) {
+			path = fmt.Sprintf("/dev/%s", devname)
+		}
+	}
+
+	return USBEvent{
+		action,
+		vendor,
+		product,
+		path,
+		uint32(majorInt),
+		uint32(minorInt),
+		ueventParts,
+		ueventLen,
+	}, nil
+}

From bdec2cd89791fcbad2f0c08ffaeb50f9d4729189 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 15:02:19 +0100
Subject: [PATCH 12/30] device/usb: Removes unused function

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/usb.go | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/lxd/device/usb.go b/lxd/device/usb.go
index e2aa19b851..137983fc50 100644
--- a/lxd/device/usb.go
+++ b/lxd/device/usb.go
@@ -54,11 +54,6 @@ func (d *usb) validateConfig() error {
 	return nil
 }
 
-// validateEnvironment checks the runtime environment for correctness.
-func (d *usb) validateEnvironment() error {
-	return nil
-}
-
 // Register is run after the device is started or when LXD starts.
 func (d *usb) Register() error {
 	// Extract variables needed to run the event hook so that the reference to this device
@@ -111,11 +106,6 @@ func (d *usb) Register() error {
 
 // Start is run when the device is added to the instance.
 func (d *usb) Start() (*RunConfig, error) {
-	err := d.validateEnvironment()
-	if err != nil {
-		return nil, err
-	}
-
 	usbs, err := d.loadUsb()
 	if err != nil {
 		return nil, err

From 9f5e13c7db589a15044acf981dfc6f4132568ac5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 16:03:02 +0100
Subject: [PATCH 13/30] device: Links up unix-char and unix-block devices

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

diff --git a/lxd/device/device.go b/lxd/device/device.go
index a3bf232f61..a2eba93088 100644
--- a/lxd/device/device.go
+++ b/lxd/device/device.go
@@ -14,6 +14,8 @@ var devTypes = map[string]func(config.Device) device{
 	"proxy":      func(c config.Device) device { return &proxy{} },
 	"gpu":        func(c config.Device) device { return &gpu{} },
 	"usb":        func(c config.Device) device { return &usb{} },
+	"unix-char":  func(c config.Device) device { return &unixCommon{} },
+	"unix-block": func(c config.Device) device { return &unixCommon{} },
 }
 
 // VolatileSetter is a function that accepts one or more key/value strings to save into the LXD

From c4191b6c9299492f8b69dcd8c7f32d54a2765151 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 16:28:28 +0100
Subject: [PATCH 14/30] devices: Removes inotify code

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/devices.go | 473 -------------------------------------------------
 1 file changed, 473 deletions(-)

diff --git a/lxd/devices.go b/lxd/devices.go
index 73eae2be86..02279b8e2b 100644
--- a/lxd/devices.go
+++ b/lxd/devices.go
@@ -13,13 +13,11 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"unsafe"
 
 	"golang.org/x/sys/unix"
 
 	"github.com/lxc/lxd/lxd/device"
 	"github.com/lxc/lxd/lxd/state"
-	"github.com/lxc/lxd/lxd/sys"
 	"github.com/lxc/lxd/lxd/util"
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/logger"
@@ -820,474 +818,3 @@ func deviceParseDiskLimit(readSpeed string, writeSpeed string) (int64, int64, in
 
 	return readBps, readIops, writeBps, writeIops, nil
 }
-
-func deviceInotifyInit(s *state.State) (int, error) {
-	s.OS.InotifyWatch.Lock()
-	defer s.OS.InotifyWatch.Unlock()
-
-	if s.OS.InotifyWatch.Fd >= 0 {
-		return s.OS.InotifyWatch.Fd, nil
-	}
-
-	inFd, err := unix.InotifyInit1(unix.IN_CLOEXEC)
-	if err != nil {
-		logger.Errorf("Failed to initialize inotify")
-		return -1, err
-	}
-	logger.Debugf("Initialized inotify with file descriptor %d", inFd)
-
-	s.OS.InotifyWatch.Fd = inFd
-	return inFd, nil
-}
-
-func findClosestLivingAncestor(cleanPath string) (bool, string) {
-	if shared.PathExists(cleanPath) {
-		return true, cleanPath
-	}
-
-	s := cleanPath
-	for {
-		s = filepath.Dir(s)
-		if s == cleanPath {
-			return false, s
-		}
-		if shared.PathExists(s) {
-			return true, s
-		}
-	}
-}
-
-func deviceInotifyAddClosestLivingAncestor(s *state.State, path string) error {
-	cleanPath := filepath.Clean(path)
-	// Find first existing ancestor directory and add it to the target.
-	exists, watchDir := findClosestLivingAncestor(cleanPath)
-	if !exists {
-		return fmt.Errorf("No existing ancestor directory found for \"%s\"", path)
-	}
-
-	err := deviceInotifyAddTarget(s, watchDir)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func deviceInotifyAddTarget(s *state.State, path string) error {
-	s.OS.InotifyWatch.Lock()
-	defer s.OS.InotifyWatch.Unlock()
-
-	inFd := s.OS.InotifyWatch.Fd
-	if inFd < 0 {
-		return fmt.Errorf("Inotify instance not intialized")
-	}
-
-	// Do not add the same target twice.
-	_, ok := s.OS.InotifyWatch.Targets[path]
-	if ok {
-		logger.Debugf("Inotify is already watching \"%s\"", path)
-		return nil
-	}
-
-	mask := uint32(0)
-	mask |= unix.IN_ONLYDIR
-	mask |= unix.IN_CREATE
-	mask |= unix.IN_DELETE
-	mask |= unix.IN_DELETE_SELF
-	wd, err := unix.InotifyAddWatch(inFd, path, mask)
-	if err != nil {
-		return err
-	}
-
-	s.OS.InotifyWatch.Targets[path] = &sys.InotifyTargetInfo{
-		Mask: mask,
-		Path: path,
-		Wd:   wd,
-	}
-
-	// Add a second key based on the watch file descriptor to the map that
-	// points to the same allocated memory. This is used to reverse engineer
-	// the absolute path when an event happens in the watched directory.
-	// We prefix the key with a \0 character as this is disallowed in
-	// directory and file names and thus guarantees uniqueness of the key.
-	wdString := fmt.Sprintf("\000:%d", wd)
-	s.OS.InotifyWatch.Targets[wdString] = s.OS.InotifyWatch.Targets[path]
-	return nil
-}
-
-func deviceInotifyDel(s *state.State) {
-	s.OS.InotifyWatch.Lock()
-	unix.Close(s.OS.InotifyWatch.Fd)
-	s.OS.InotifyWatch.Fd = -1
-	s.OS.InotifyWatch.Unlock()
-}
-
-const LXD_BATCH_IN_EVENTS uint = 100
-const LXD_SINGLE_IN_EVENT_SIZE uint = (unix.SizeofInotifyEvent + unix.PathMax)
-const LXD_BATCH_IN_BUFSIZE uint = LXD_BATCH_IN_EVENTS * LXD_SINGLE_IN_EVENT_SIZE
-
-func deviceInotifyWatcher(s *state.State) (chan sys.InotifyTargetInfo, error) {
-	targetChan := make(chan sys.InotifyTargetInfo)
-	go func(target chan sys.InotifyTargetInfo) {
-		for {
-			buf := make([]byte, LXD_BATCH_IN_BUFSIZE)
-			n, errno := unix.Read(s.OS.InotifyWatch.Fd, buf)
-			if errno != nil {
-				if errno == unix.EINTR {
-					continue
-				}
-
-				deviceInotifyDel(s)
-				return
-			}
-
-			if n < unix.SizeofInotifyEvent {
-				continue
-			}
-
-			var offset uint32
-			for offset <= uint32(n-unix.SizeofInotifyEvent) {
-				name := ""
-				event := (*unix.InotifyEvent)(unsafe.Pointer(&buf[offset]))
-
-				nameLen := uint32(event.Len)
-				if nameLen > 0 {
-					bytes := (*[unix.PathMax]byte)(unsafe.Pointer(&buf[offset+unix.SizeofInotifyEvent]))
-					name = strings.TrimRight(string(bytes[0:nameLen]), "\000")
-				}
-
-				target <- sys.InotifyTargetInfo{
-					Mask: uint32(event.Mask),
-					Path: name,
-					Wd:   int(event.Wd),
-				}
-
-				offset += (unix.SizeofInotifyEvent + nameLen)
-			}
-		}
-	}(targetChan)
-
-	return targetChan, nil
-}
-
-func deviceInotifyDelWatcher(s *state.State, path string) error {
-	s.OS.InotifyWatch.Lock()
-	defer s.OS.InotifyWatch.Unlock()
-
-	if s.OS.InotifyWatch.Fd < 0 {
-		return nil
-	}
-
-	target, ok := s.OS.InotifyWatch.Targets[path]
-	if !ok {
-		logger.Debugf("Inotify target \"%s\" not present", path)
-		return nil
-	}
-
-	ret, err := unix.InotifyRmWatch(s.OS.InotifyWatch.Fd, uint32(target.Wd))
-	if ret != 0 {
-		// When a file gets deleted the wd for that file will
-		// automatically be deleted from the inotify instance. So
-		// ignore errors here.
-		logger.Debugf("Inotify syscall returned %s for \"%s\"", err, path)
-	}
-	delete(s.OS.InotifyWatch.Targets, path)
-	wdString := fmt.Sprintf("\000:%d", target.Wd)
-	delete(s.OS.InotifyWatch.Targets, wdString)
-	return nil
-}
-
-func createAncestorPaths(cleanPath string) []string {
-	components := strings.Split(cleanPath, "/")
-	ancestors := []string{}
-	newPath := "/"
-	ancestors = append(ancestors, newPath)
-	for _, v := range components[1:] {
-		newPath = filepath.Join(newPath, v)
-		ancestors = append(ancestors, newPath)
-	}
-
-	return ancestors
-}
-
-func deviceInotifyEvent(s *state.State, target *sys.InotifyTargetInfo) {
-	if (target.Mask & unix.IN_ISDIR) > 0 {
-		if (target.Mask & unix.IN_CREATE) > 0 {
-			deviceInotifyDirCreateEvent(s, target)
-		} else if (target.Mask & unix.IN_DELETE) > 0 {
-			deviceInotifyDirDeleteEvent(s, target)
-		}
-		deviceInotifyDirRescan(s)
-	} else if (target.Mask & unix.IN_DELETE_SELF) > 0 {
-		deviceInotifyDirDeleteEvent(s, target)
-		deviceInotifyDirRescan(s)
-	} else {
-		deviceInotifyFileEvent(s, target)
-	}
-}
-
-func deviceInotifyDirDeleteEvent(s *state.State, target *sys.InotifyTargetInfo) {
-	parentKey := fmt.Sprintf("\000:%d", target.Wd)
-	s.OS.InotifyWatch.RLock()
-	parent, ok := s.OS.InotifyWatch.Targets[parentKey]
-	s.OS.InotifyWatch.RUnlock()
-	if !ok {
-		return
-	}
-
-	// The absolute path of the file for which we received an event?
-	targetName := filepath.Join(parent.Path, target.Path)
-	targetName = filepath.Clean(targetName)
-	err := deviceInotifyDelWatcher(s, targetName)
-	if err != nil {
-		logger.Errorf("Failed to remove \"%s\" from inotify targets: %s", targetName, err)
-	} else {
-		logger.Errorf("Removed \"%s\" from inotify targets", targetName)
-	}
-}
-
-func deviceInotifyDirRescan(s *state.State) {
-	containers, err := containerLoadNodeAll(s)
-	if err != nil {
-		logger.Errorf("Failed to load containers: %s", err)
-		return
-	}
-
-	for _, containerIf := range containers {
-		c, ok := containerIf.(*containerLXC)
-		if !ok {
-			logger.Errorf("Received device event on non-LXC container")
-			return
-		}
-
-		if !c.IsRunning() {
-			continue
-		}
-
-		devices := c.ExpandedDevices()
-		for _, name := range devices.DeviceNames() {
-			m := devices[name]
-			if !shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
-				continue
-			}
-
-			if m["required"] == "" || shared.IsTrue(m["required"]) {
-				continue
-			}
-
-			cmp := m["source"]
-			if cmp == "" {
-				cmp = m["path"]
-			}
-			cleanDevPath := filepath.Clean(cmp)
-			if shared.PathExists(cleanDevPath) {
-				c.insertUnixDevice(fmt.Sprintf("unix.%s", name), m, false)
-			} else {
-				c.removeUnixDevice(fmt.Sprintf("unix.%s", name), m, true)
-			}
-
-			// and add its nearest existing ancestor.
-			err = deviceInotifyAddClosestLivingAncestor(s, filepath.Dir(cleanDevPath))
-			if err != nil {
-				logger.Errorf("Failed to add \"%s\" to inotify targets: %s", filepath.Dir(cleanDevPath), err)
-			} else {
-				logger.Debugf("Added \"%s\" to inotify targets", filepath.Dir(cleanDevPath))
-			}
-		}
-	}
-}
-
-func deviceInotifyDirCreateEvent(s *state.State, target *sys.InotifyTargetInfo) {
-	parentKey := fmt.Sprintf("\000:%d", target.Wd)
-	s.OS.InotifyWatch.RLock()
-	parent, ok := s.OS.InotifyWatch.Targets[parentKey]
-	s.OS.InotifyWatch.RUnlock()
-	if !ok {
-		return
-	}
-
-	containers, err := containerLoadNodeAll(s)
-	if err != nil {
-		logger.Errorf("Failed to load containers: %s", err)
-		return
-	}
-
-	// The absolute path of the file for which we received an event?
-	targetName := filepath.Join(parent.Path, target.Path)
-	targetName = filepath.Clean(targetName)
-
-	// ancestors
-	del := createAncestorPaths(targetName)
-	keep := []string{}
-	for _, containerIf := range containers {
-		c, ok := containerIf.(*containerLXC)
-		if !ok {
-			logger.Errorf("Received device event on non-LXC container")
-			return
-		}
-
-		if !c.IsRunning() {
-			continue
-		}
-
-		devices := c.ExpandedDevices()
-		for _, name := range devices.DeviceNames() {
-			m := devices[name]
-			if !shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
-				continue
-			}
-
-			if m["required"] == "" || shared.IsTrue(m["required"]) {
-				continue
-			}
-
-			cmp := m["source"]
-			if cmp == "" {
-				cmp = m["path"]
-			}
-			cleanDevPath := filepath.Clean(cmp)
-
-			for i := len(del) - 1; i >= 0; i-- {
-				// Only keep paths that can be deleted.
-				if strings.HasPrefix(cleanDevPath, del[i]) {
-					if shared.StringInSlice(del[i], keep) {
-						break
-					}
-
-					keep = append(keep, del[i])
-					break
-				}
-			}
-		}
-	}
-
-	for i, v := range del {
-		if shared.StringInSlice(v, keep) {
-			del[i] = ""
-		}
-	}
-
-	for _, v := range del {
-		if v == "" {
-			continue
-		}
-
-		err := deviceInotifyDelWatcher(s, v)
-		if err != nil {
-			logger.Errorf("Failed to remove \"%s\" from inotify targets: %s", v, err)
-		} else {
-			logger.Debugf("Removed \"%s\" from inotify targets", v)
-		}
-	}
-
-	for _, v := range keep {
-		if v == "" {
-			continue
-		}
-
-		err = deviceInotifyAddClosestLivingAncestor(s, v)
-		if err != nil {
-			logger.Errorf("Failed to add \"%s\" to inotify targets: %s", v, err)
-		} else {
-			logger.Debugf("Added \"%s\" to inotify targets", v)
-		}
-	}
-}
-
-func deviceInotifyFileEvent(s *state.State, target *sys.InotifyTargetInfo) {
-	parentKey := fmt.Sprintf("\000:%d", target.Wd)
-	s.OS.InotifyWatch.RLock()
-	parent, ok := s.OS.InotifyWatch.Targets[parentKey]
-	s.OS.InotifyWatch.RUnlock()
-	if !ok {
-		return
-	}
-
-	containers, err := containerLoadNodeAll(s)
-	if err != nil {
-		logger.Errorf("Failed to load containers: %s", err)
-		return
-	}
-
-	// Does the current file have watchers?
-	hasWatchers := false
-	// The absolute path of the file for which we received an event?
-	targetName := filepath.Join(parent.Path, target.Path)
-	for _, containerIf := range containers {
-		c, ok := containerIf.(*containerLXC)
-		if !ok {
-			logger.Errorf("Received device event on non-LXC container")
-			return
-		}
-
-		if !c.IsRunning() {
-			continue
-		}
-
-		devices := c.ExpandedDevices()
-		for _, name := range devices.DeviceNames() {
-			m := devices[name]
-			if !shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
-				continue
-			}
-
-			cmp := m["source"]
-			if cmp == "" {
-				cmp = m["path"]
-			}
-
-			if m["required"] == "" || shared.IsTrue(m["required"]) {
-				continue
-			}
-
-			cleanDevPath := filepath.Clean(cmp)
-			cleanInotPath := filepath.Clean(targetName)
-			if !hasWatchers && strings.HasPrefix(cleanDevPath, cleanInotPath) {
-				hasWatchers = true
-			}
-
-			if cleanDevPath != cleanInotPath {
-				continue
-			}
-
-			if (target.Mask & unix.IN_CREATE) > 0 {
-				err := c.insertUnixDevice(fmt.Sprintf("unix.%s", name), m, false)
-				if err != nil {
-					logger.Error("Failed to create unix device", log.Ctx{"err": err, "dev": m, "container": c.Name()})
-					continue
-				}
-			} else if (target.Mask & unix.IN_DELETE) > 0 {
-				err := c.removeUnixDevice(fmt.Sprintf("unix.%s", name), m, true)
-				if err != nil {
-					logger.Error("Failed to remove unix device", log.Ctx{"err": err, "dev": m, "container": c.Name()})
-					continue
-				}
-			} else {
-				logger.Error("Uknown action for unix device", log.Ctx{"dev": m, "container": c.Name()})
-			}
-		}
-	}
-
-	if !hasWatchers {
-		err := deviceInotifyDelWatcher(s, targetName)
-		if err != nil {
-			logger.Errorf("Failed to remove \"%s\" from inotify targets: %s", targetName, err)
-		} else {
-			logger.Debugf("Removed \"%s\" from inotify targets", targetName)
-		}
-	}
-}
-
-func deviceInotifyHandler(s *state.State) {
-	watchChan, err := deviceInotifyWatcher(s)
-	if err != nil {
-		return
-	}
-
-	for {
-		select {
-		case v := <-watchChan:
-			deviceInotifyEvent(s, &v)
-		}
-	}
-}

From 898cd2bc6a41952de0cb19f447d0a1d6767c4c51 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 15:38:32 +0100
Subject: [PATCH 15/30] device/device/utils/unix/events: Adds unix event
 handling functions

Moves inotify related functions into this file too as they were tightly coupled to the unix event handlers.

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/device_utils_unix_events.go | 567 +++++++++++++++++++++++++
 1 file changed, 567 insertions(+)
 create mode 100644 lxd/device/device_utils_unix_events.go

diff --git a/lxd/device/device_utils_unix_events.go b/lxd/device/device_utils_unix_events.go
new file mode 100644
index 0000000000..7c2e37d58c
--- /dev/null
+++ b/lxd/device/device_utils_unix_events.go
@@ -0,0 +1,567 @@
+package device
+
+import (
+	"fmt"
+	"path/filepath"
+	"strings"
+	"sync"
+	"unsafe"
+
+	"golang.org/x/sys/unix"
+
+	"github.com/lxc/lxd/lxd/state"
+	"github.com/lxc/lxd/lxd/sys"
+	"github.com/lxc/lxd/shared"
+	log "github.com/lxc/lxd/shared/log15"
+	"github.com/lxc/lxd/shared/logger"
+)
+
+const inotifyBatchInEvents uint = 100
+const inotifySingleInEventSize uint = (unix.SizeofInotifyEvent + unix.PathMax)
+const inotifyBatchInBufSize uint = inotifyBatchInEvents * inotifySingleInEventSize
+
+// UnixEvent represents the properties of a Unix device inotify event.
+type UnixEvent struct {
+	Action string // The type of event, either add or remove.
+	Path   string // The absolute source path on the host.
+}
+
+// UnixSubscription used to subcribe to specific events.
+type UnixSubscription struct {
+	Path    string                              // The absolute source path on the host.
+	Handler func(UnixEvent) (*RunConfig, error) // The function to run when an event occurs.
+}
+
+// unixHandlers stores the event handler callbacks for Unix events.
+var unixHandlers = map[string]UnixSubscription{}
+
+// unixMutex controls access to the unixHandlers map.
+var unixMutex sync.Mutex
+
+// UnixInotifyInit initialises the inotify internal structures.
+func UnixInotifyInit(s *state.State) (int, error) {
+	s.OS.InotifyWatch.Lock()
+	defer s.OS.InotifyWatch.Unlock()
+
+	if s.OS.InotifyWatch.Fd >= 0 {
+		return s.OS.InotifyWatch.Fd, nil
+	}
+
+	inFd, err := unix.InotifyInit1(unix.IN_CLOEXEC)
+	if err != nil {
+		logger.Errorf("Failed to initialize inotify")
+		return -1, err
+	}
+	logger.Debugf("Initialized inotify with file descriptor %d", inFd)
+
+	s.OS.InotifyWatch.Fd = inFd
+
+	return inFd, nil
+}
+
+// UnixInotifyHandler starts watching for inotify events.
+func UnixInotifyHandler(s *state.State) {
+	watchChan, err := inotifyWatcher(s)
+	if err != nil {
+		return
+	}
+
+	for {
+		select {
+		case v := <-watchChan:
+			inotifyEvent(s, &v)
+		}
+	}
+}
+
+// unixRegisterHandler registers a handler function to be called whenever a Unix device event occurs.
+func unixRegisterHandler(s *state.State, instance InstanceIdentifier, deviceName, path string, handler func(UnixEvent) (*RunConfig, error)) error {
+	if path == "" || handler == nil {
+		return fmt.Errorf("Invalid subscription")
+	}
+
+	unixMutex.Lock()
+	defer unixMutex.Unlock()
+
+	// Null delimited string of project name, instance name and device name.
+	key := fmt.Sprintf("%s\000%s\000%s", instance.Project(), instance.Name(), deviceName)
+	unixHandlers[key] = UnixSubscription{
+		Path:    path,
+		Handler: handler,
+	}
+
+	// Add inotify watcher to its nearest existing ancestor.
+	cleanDevDirPath := filepath.Dir(filepath.Clean(path))
+	err := inotifyAddClosestLivingAncestor(s, cleanDevDirPath)
+	if err != nil {
+		return fmt.Errorf("Failed to add \"%s\" to inotify targets: %s", cleanDevDirPath, err)
+	}
+
+	logger.Debugf("Added \"%s\" to inotify targets", cleanDevDirPath)
+	return nil
+}
+
+// unixUnregisterHandler removes a registered Unix handler function for a device.
+func unixUnregisterHandler(s *state.State, instance InstanceIdentifier, deviceName string) error {
+	unixMutex.Lock()
+	defer unixMutex.Unlock()
+
+	// Null delimited string of project name, instance name and device name.
+	key := fmt.Sprintf("%s\000%s\000%s", instance.Project(), instance.Name(), deviceName)
+
+	sub, exists := unixHandlers[key]
+	if !exists {
+		return nil
+	}
+
+	// Remove active subscription for this device.
+	delete(unixHandlers, key)
+
+	// Create a map of all unique living ancestor paths for all active subscriptions and count
+	// how many subscriptions are using each living ancestor path.
+	subsLivingAncestors := make(map[string]uint)
+	for _, sub := range unixHandlers {
+
+		exists, path := inotifyFindClosestLivingAncestor(filepath.Dir(sub.Path))
+		if !exists {
+			continue
+		}
+
+		subsLivingAncestors[path]++ // Count how many subscriptions are sharing a watcher.
+	}
+
+	// Identify which living ancestor path the subscription we just deleted was using.
+	exists, ourSubPath := inotifyFindClosestLivingAncestor(filepath.Dir(sub.Path))
+
+	// If we were the only subscription using the living ancestor path, then remove the watcher.
+	if exists && subsLivingAncestors[ourSubPath] == 0 {
+		err := inotifyDelWatcher(s, ourSubPath)
+		if err != nil {
+			return fmt.Errorf("Failed to remove \"%s\" from inotify targets: %s", ourSubPath, err)
+		}
+		logger.Debugf("Removed \"%s\" from inotify targets", ourSubPath)
+	}
+
+	return nil
+}
+
+// unixRunHandlers executes any handlers registered for Unix events.
+func unixRunHandlers(state *state.State, event *UnixEvent) {
+	unixMutex.Lock()
+	defer unixMutex.Unlock()
+
+	for key, sub := range unixHandlers {
+		keyParts := strings.SplitN(key, "\000", 3)
+		projectName := keyParts[0]
+		instanceName := keyParts[1]
+		deviceName := keyParts[2]
+
+		// Delete subscription if no handler function defined.
+		if sub.Handler == nil {
+			delete(unixHandlers, key)
+			continue
+		}
+
+		// Don't execute handler if subscription path and event paths don't match.
+		if sub.Path != event.Path {
+			continue
+		}
+
+		// Run handler function.
+		runConf, err := sub.Handler(*event)
+		if err != nil {
+			logger.Error("Unix event hook failed", log.Ctx{"err": err, "project": projectName, "instance": instanceName, "device": deviceName, "path": sub.Path})
+			continue
+		}
+
+		// If runConf supplied, load instance and call its Unix event handler function so
+		// any instance specific device actions can occur.
+		if runConf != nil {
+			instance, err := InstanceLoadByProjectAndName(state, projectName, instanceName)
+			if err != nil {
+				logger.Error("Unix event loading instance failed", log.Ctx{"err": err, "project": projectName, "instance": instanceName, "device": deviceName})
+				continue
+			}
+
+			err = instance.DeviceEventHandler(runConf)
+			if err != nil {
+				logger.Error("Unix event instance handler failed", log.Ctx{"err": err, "project": projectName, "instance": instanceName, "device": deviceName})
+				continue
+			}
+		}
+	}
+}
+
+// unixGetSubcribedPaths returns all the subcribed paths as a map keyed on path.
+func unixGetSubcribedPaths() map[string]struct{} {
+	unixMutex.Lock()
+	defer unixMutex.Unlock()
+
+	paths := make(map[string]struct{})
+
+	for _, sub := range unixHandlers {
+		paths[sub.Path] = struct{}{}
+	}
+
+	return paths
+}
+
+// unixNewEvent returns a newly created Unix device event struct.
+// If an empty action is supplied then the action of the event is derived from whether the path
+// exists (add) or not (removed). This allows the peculiarities of the inotify API to be somewhat
+// masked by the consuming event handler functions.
+func unixNewEvent(action string, path string) UnixEvent {
+	if action == "" {
+		if shared.PathExists(path) {
+			action = "add"
+		} else {
+			action = "remove"
+		}
+	}
+
+	return UnixEvent{
+		Action: action,
+		Path:   path,
+	}
+}
+
+// inotifyDirRescan tries to add inotify watchers for all subscribed paths. It generates pseudo
+// events to any registered handler with either add or remove action based on whether or not the
+// file exists.
+func inotifyDirRescan(s *state.State) {
+	// Because we don't know what sort of event actually occurred, lets generate a pseudo event
+	// for each of the unique paths that devices for all instances on this host are subcribed
+	// to. This will test for whether each file actually exists or not and allow the handler
+	// functions to add/remove as necessary. It also allows inotify watchers to be setup for
+	// all desired paths.
+	subs := unixGetSubcribedPaths()
+	for subPath := range subs {
+		cleanDevPath := filepath.Clean(subPath)
+		e := unixNewEvent("", subPath)
+		unixRunHandlers(s, &e)
+
+		// Add its nearest existing ancestor.
+		err := inotifyAddClosestLivingAncestor(s, filepath.Dir(cleanDevPath))
+		if err != nil {
+			logger.Errorf("Failed to add \"%s\" to inotify targets: %s", filepath.Dir(cleanDevPath), err)
+		} else {
+			logger.Debugf("Added \"%s\" to inotify targets", filepath.Dir(cleanDevPath))
+		}
+	}
+}
+
+func inotifyFindClosestLivingAncestor(cleanPath string) (bool, string) {
+	if shared.PathExists(cleanPath) {
+		return true, cleanPath
+	}
+
+	s := cleanPath
+	for {
+		s = filepath.Dir(s)
+		if s == cleanPath {
+			return false, s
+		}
+		if shared.PathExists(s) {
+			return true, s
+		}
+	}
+}
+
+func inotifyAddClosestLivingAncestor(s *state.State, path string) error {
+	cleanPath := filepath.Clean(path)
+	// Find first existing ancestor directory and add it to the target.
+	exists, watchDir := inotifyFindClosestLivingAncestor(cleanPath)
+	if !exists {
+		return fmt.Errorf("No existing ancestor directory found for \"%s\"", path)
+	}
+
+	err := inotifyAddTarget(s, watchDir)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func inotifyAddTarget(s *state.State, path string) error {
+	s.OS.InotifyWatch.Lock()
+	defer s.OS.InotifyWatch.Unlock()
+
+	inFd := s.OS.InotifyWatch.Fd
+	if inFd < 0 {
+		return fmt.Errorf("Inotify instance not intialized")
+	}
+
+	// Do not add the same target twice.
+	_, ok := s.OS.InotifyWatch.Targets[path]
+	if ok {
+		logger.Debugf("Inotify is already watching \"%s\"", path)
+		return nil
+	}
+
+	mask := uint32(0)
+	mask |= unix.IN_ONLYDIR
+	mask |= unix.IN_CREATE
+	mask |= unix.IN_DELETE
+	mask |= unix.IN_DELETE_SELF
+	wd, err := unix.InotifyAddWatch(inFd, path, mask)
+	if err != nil {
+		return err
+	}
+
+	s.OS.InotifyWatch.Targets[path] = &sys.InotifyTargetInfo{
+		Mask: mask,
+		Path: path,
+		Wd:   wd,
+	}
+
+	// Add a second key based on the watch file descriptor to the map that
+	// points to the same allocated memory. This is used to reverse engineer
+	// the absolute path when an event happens in the watched directory.
+	// We prefix the key with a \0 character as this is disallowed in
+	// directory and file names and thus guarantees uniqueness of the key.
+	wdString := fmt.Sprintf("\000:%d", wd)
+	s.OS.InotifyWatch.Targets[wdString] = s.OS.InotifyWatch.Targets[path]
+	return nil
+}
+
+func inotifyDel(s *state.State) {
+	s.OS.InotifyWatch.Lock()
+	unix.Close(s.OS.InotifyWatch.Fd)
+	s.OS.InotifyWatch.Fd = -1
+	s.OS.InotifyWatch.Unlock()
+}
+
+func inotifyWatcher(s *state.State) (chan sys.InotifyTargetInfo, error) {
+	targetChan := make(chan sys.InotifyTargetInfo)
+	go func(target chan sys.InotifyTargetInfo) {
+		for {
+			buf := make([]byte, inotifyBatchInBufSize)
+			n, errno := unix.Read(s.OS.InotifyWatch.Fd, buf)
+			if errno != nil {
+				if errno == unix.EINTR {
+					continue
+				}
+
+				inotifyDel(s)
+				return
+			}
+
+			if n < unix.SizeofInotifyEvent {
+				continue
+			}
+
+			var offset uint32
+			for offset <= uint32(n-unix.SizeofInotifyEvent) {
+				name := ""
+				event := (*unix.InotifyEvent)(unsafe.Pointer(&buf[offset]))
+
+				nameLen := uint32(event.Len)
+				if nameLen > 0 {
+					bytes := (*[unix.PathMax]byte)(unsafe.Pointer(&buf[offset+unix.SizeofInotifyEvent]))
+					name = strings.TrimRight(string(bytes[0:nameLen]), "\000")
+				}
+
+				target <- sys.InotifyTargetInfo{
+					Mask: uint32(event.Mask),
+					Path: name,
+					Wd:   int(event.Wd),
+				}
+
+				offset += (unix.SizeofInotifyEvent + nameLen)
+			}
+		}
+	}(targetChan)
+
+	return targetChan, nil
+}
+
+func inotifyDelWatcher(s *state.State, path string) error {
+	s.OS.InotifyWatch.Lock()
+	defer s.OS.InotifyWatch.Unlock()
+
+	if s.OS.InotifyWatch.Fd < 0 {
+		return nil
+	}
+
+	target, ok := s.OS.InotifyWatch.Targets[path]
+	if !ok {
+		logger.Debugf("Inotify target \"%s\" not present", path)
+		return nil
+	}
+
+	ret, err := unix.InotifyRmWatch(s.OS.InotifyWatch.Fd, uint32(target.Wd))
+	if ret != 0 {
+		// When a file gets deleted the wd for that file will
+		// automatically be deleted from the inotify instance. So
+		// ignore errors here.
+		logger.Debugf("Inotify syscall returned %s for \"%s\"", err, path)
+	}
+	delete(s.OS.InotifyWatch.Targets, path)
+	wdString := fmt.Sprintf("\000:%d", target.Wd)
+	delete(s.OS.InotifyWatch.Targets, wdString)
+	return nil
+}
+
+func inotifyCreateAncestorPaths(cleanPath string) []string {
+	components := strings.Split(cleanPath, "/")
+	ancestors := []string{}
+	newPath := "/"
+	ancestors = append(ancestors, newPath)
+	for _, v := range components[1:] {
+		newPath = filepath.Join(newPath, v)
+		ancestors = append(ancestors, newPath)
+	}
+
+	return ancestors
+}
+
+func inotifyEvent(s *state.State, target *sys.InotifyTargetInfo) {
+	if (target.Mask & unix.IN_ISDIR) > 0 {
+		if (target.Mask & unix.IN_CREATE) > 0 {
+			inotifyDirCreateEvent(s, target)
+		} else if (target.Mask & unix.IN_DELETE) > 0 {
+			inotifyDirDeleteEvent(s, target)
+		}
+		inotifyDirRescan(s)
+	} else if (target.Mask & unix.IN_DELETE_SELF) > 0 {
+		inotifyDirDeleteEvent(s, target)
+		inotifyDirRescan(s)
+	} else {
+		inotifyFileEvent(s, target)
+	}
+}
+
+func inotifyDirDeleteEvent(s *state.State, target *sys.InotifyTargetInfo) {
+	parentKey := fmt.Sprintf("\000:%d", target.Wd)
+	s.OS.InotifyWatch.RLock()
+	parent, ok := s.OS.InotifyWatch.Targets[parentKey]
+	s.OS.InotifyWatch.RUnlock()
+	if !ok {
+		return
+	}
+
+	// The absolute path of the file for which we received an event?
+	targetName := filepath.Join(parent.Path, target.Path)
+	targetName = filepath.Clean(targetName)
+	err := inotifyDelWatcher(s, targetName)
+	if err != nil {
+		logger.Errorf("Failed to remove \"%s\" from inotify targets: %s", targetName, err)
+	} else {
+		logger.Debugf("Removed \"%s\" from inotify targets", targetName)
+	}
+}
+
+func inotifyDirCreateEvent(s *state.State, target *sys.InotifyTargetInfo) {
+	parentKey := fmt.Sprintf("\000:%d", target.Wd)
+	s.OS.InotifyWatch.RLock()
+	parent, ok := s.OS.InotifyWatch.Targets[parentKey]
+	s.OS.InotifyWatch.RUnlock()
+	if !ok {
+		return
+	}
+
+	// The absolute path of the file for which we received an event?
+	targetName := filepath.Join(parent.Path, target.Path)
+	targetName = filepath.Clean(targetName)
+
+	// ancestors
+	del := inotifyCreateAncestorPaths(targetName)
+	keep := []string{}
+
+	subs := unixGetSubcribedPaths()
+	for subPath := range subs {
+		cleanDevPath := filepath.Clean(subPath)
+
+		for i := len(del) - 1; i >= 0; i-- {
+			// Only keep paths that can be deleted.
+			if strings.HasPrefix(cleanDevPath, del[i]) {
+				if shared.StringInSlice(del[i], keep) {
+					break
+				}
+
+				keep = append(keep, del[i])
+				break
+			}
+		}
+	}
+
+	for i, v := range del {
+		if shared.StringInSlice(v, keep) {
+			del[i] = ""
+		}
+	}
+
+	for _, v := range del {
+		if v == "" {
+			continue
+		}
+
+		err := inotifyDelWatcher(s, v)
+		if err != nil {
+			logger.Errorf("Failed to remove \"%s\" from inotify targets: %s", v, err)
+		} else {
+			logger.Debugf("Removed \"%s\" from inotify targets", v)
+		}
+	}
+
+	for _, v := range keep {
+		if v == "" {
+			continue
+		}
+
+		err := inotifyAddClosestLivingAncestor(s, v)
+		if err != nil {
+			logger.Errorf("Failed to add \"%s\" to inotify targets: %s", v, err)
+		} else {
+			logger.Debugf("Added \"%s\" to inotify targets", v)
+		}
+	}
+}
+
+func inotifyFileEvent(s *state.State, target *sys.InotifyTargetInfo) {
+	parentKey := fmt.Sprintf("\000:%d", target.Wd)
+	s.OS.InotifyWatch.RLock()
+	parent, ok := s.OS.InotifyWatch.Targets[parentKey]
+	s.OS.InotifyWatch.RUnlock()
+	if !ok {
+		return
+	}
+
+	// Does the current file have watchers?
+	hasWatchers := false
+	// The absolute path of the file for which we received an event?
+	targetName := filepath.Join(parent.Path, target.Path)
+
+	subs := unixGetSubcribedPaths()
+	for subPath := range subs {
+		cleanDevPath := filepath.Clean(subPath)
+		cleanInotPath := filepath.Clean(targetName)
+		if !hasWatchers && strings.HasPrefix(cleanDevPath, cleanInotPath) {
+			hasWatchers = true
+		}
+
+		if cleanDevPath != cleanInotPath {
+			continue
+		}
+
+		if (target.Mask & unix.IN_CREATE) > 0 {
+			e := unixNewEvent("add", cleanInotPath)
+			unixRunHandlers(s, &e)
+		} else if (target.Mask & unix.IN_DELETE) > 0 {
+			e := unixNewEvent("remove", cleanInotPath)
+			unixRunHandlers(s, &e)
+		} else {
+			logger.Error("Uknown action for unix device", log.Ctx{"dev": subPath, "target": cleanInotPath})
+		}
+	}
+
+	if !hasWatchers {
+		err := inotifyDelWatcher(s, targetName)
+		if err != nil {
+			logger.Errorf("Failed to remove \"%s\" from inotify targets: %s", targetName, err)
+		} else {
+			logger.Debugf("Removed \"%s\" from inotify targets", targetName)
+		}
+	}
+}

From f5287bf2aa1e3b01d13ba91297de7fc267724fb1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 15:59:39 +0100
Subject: [PATCH 16/30] device/unixCommon: Adds implementation for unix-char
 and unix-block devices

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/unixCommon.go | 207 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 207 insertions(+)
 create mode 100644 lxd/device/unixCommon.go

diff --git a/lxd/device/unixCommon.go b/lxd/device/unixCommon.go
new file mode 100644
index 0000000000..398fa089e0
--- /dev/null
+++ b/lxd/device/unixCommon.go
@@ -0,0 +1,207 @@
+package device
+
+import (
+	"fmt"
+	"path/filepath"
+	"strings"
+
+	"github.com/lxc/lxd/lxd/device/config"
+	"github.com/lxc/lxd/lxd/instance"
+	"github.com/lxc/lxd/shared"
+)
+
+// unixIsOurDeviceType checks that device file type matches what we are expecting in the config.
+func unixIsOurDeviceType(config config.Device, dType string) bool {
+	if config["type"] == "unix-char" && dType == "c" {
+		return true
+	}
+
+	if config["type"] == "unix-block" && dType == "b" {
+		return true
+	}
+
+	return false
+}
+
+type unixCommon struct {
+	deviceCommon
+}
+
+// validateConfig checks the supplied config for correctness.
+func (d *unixCommon) validateConfig() error {
+	if d.instance.Type() != instance.TypeContainer {
+		return ErrUnsupportedDevType
+	}
+
+	rules := map[string]func(string) error{
+		"source":   shared.IsAny,
+		"path":     shared.IsAny,
+		"major":    unixValidDeviceNum,
+		"minor":    unixValidDeviceNum,
+		"uid":      unixValidUserID,
+		"gid":      unixValidUserID,
+		"mode":     unixValidOctalFileMode,
+		"required": shared.IsBool,
+	}
+
+	err := config.ValidateDevice(rules, d.config)
+	if err != nil {
+		return err
+	}
+
+	if d.config["source"] == "" && d.config["path"] == "" {
+		return fmt.Errorf("Unix device entry is missing the required \"source\" or \"path\" property")
+	}
+
+	return nil
+}
+
+// Register is run after the device is started or when LXD starts.
+func (d *unixCommon) Register() error {
+	// Don't register for hot plug events if the device is required.
+	if d.config["required"] == "" || shared.IsTrue(d.config["required"]) {
+		return nil
+	}
+
+	// Extract variables needed to run the event hook so that the reference to this device
+	// struct is not needed to be kept in memory.
+	devicesPath := d.instance.DevicesPath()
+	deviceConfig := d.config
+	deviceName := d.name
+	state := d.state
+
+	// Handler for when a Unix event occurs.
+	f := func(e UnixEvent) (*RunConfig, error) {
+		// Check if the event is for a device file that this device wants.
+		if unixDeviceSourcePath(deviceConfig) != e.Path {
+			return nil, nil
+		}
+
+		// Derive the host side path for the instance device file.
+		ourPrefix := unixDeviceJoinPath("unix", deviceName)
+		relativeDestPath := strings.TrimPrefix(unixDeviceDestPath(deviceConfig), "/")
+		devName := unixDeviceEncode(unixDeviceJoinPath(ourPrefix, relativeDestPath))
+		devPath := filepath.Join(devicesPath, devName)
+
+		runConf := RunConfig{}
+
+		if e.Action == "add" {
+			// Skip if host side instance device file already exists.
+			if shared.PathExists(devPath) {
+				return nil, nil
+			}
+
+			// Get the file type and sanity check it matches what the user was expecting.
+			dType, _, _, err := UnixDeviceAttributes(e.Path)
+			if err != nil {
+				return nil, err
+			}
+
+			if !unixIsOurDeviceType(d.config, dType) {
+				return nil, fmt.Errorf("Path specified is not a %s device", d.config["type"])
+			}
+
+			err = unixDeviceSetup(state, devicesPath, "unix", deviceName, deviceConfig, true, &runConf)
+			if err != nil {
+				return nil, err
+			}
+		} else if e.Action == "remove" {
+			// Skip if host side instance device file doesn't exist.
+			if !shared.PathExists(devPath) {
+				return nil, nil
+			}
+
+			err := unixDeviceRemove(devicesPath, "unix", deviceName, relativeDestPath, &runConf)
+			if err != nil {
+				return nil, err
+			}
+
+			// Add a post hook function to remove the specific USB device file after unmount.
+			runConf.PostHooks = []func() error{func() error {
+				err := unixDeviceDeleteFiles(state, devicesPath, "unix", deviceName, relativeDestPath)
+				if err != nil {
+					return fmt.Errorf("Failed to delete files for device '%s': %v", deviceName, err)
+				}
+
+				return nil
+			}}
+		}
+
+		return &runConf, nil
+	}
+
+	// Register the handler function against the device's source path.
+	subPath := unixDeviceSourcePath(deviceConfig)
+	err := unixRegisterHandler(d.state, d.instance, d.name, subPath, f)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// Start is run when the device is added to the container.
+func (d *unixCommon) Start() (*RunConfig, error) {
+	runConf := RunConfig{}
+	runConf.PostHooks = []func() error{d.Register}
+	srcPath := unixDeviceSourcePath(d.config)
+
+	// If device file already exists on system, proceed to add it whether its required or not.
+	dType, _, _, err := UnixDeviceAttributes(srcPath)
+	if err == nil {
+		// Sanity check device type matches what the device config is expecting.
+		if !unixIsOurDeviceType(d.config, dType) {
+			return nil, fmt.Errorf("Path specified is not a %s device", d.config["type"])
+		}
+
+		err = unixDeviceSetup(d.state, d.instance.DevicesPath(), "unix", d.name, d.config, true, &runConf)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		// If the device file doesn't exist on the system, but major & minor numbers have
+		// been provided in the config then we can go ahead and create the device anyway.
+		if d.config["major"] != "" && d.config["minor"] != "" {
+			err := unixDeviceSetup(d.state, d.instance.DevicesPath(), "unix", d.name, d.config, true, &runConf)
+			if err != nil {
+				return nil, err
+			}
+		} else if d.config["required"] == "" || shared.IsTrue(d.config["required"]) {
+			// If the file is missing and the device is required then we cannot proceed.
+			return nil, fmt.Errorf("The required device path doesn't exist and the major and minor settings are not specified")
+		}
+	}
+
+	return &runConf, nil
+}
+
+// Stop is run when the device is removed from the instance.
+func (d *unixCommon) Stop() (*RunConfig, error) {
+	// Unregister any Unix event handlers for this device.
+	err := unixUnregisterHandler(d.state, d.instance, d.name)
+	if err != nil {
+		return nil, err
+	}
+
+	runConf := RunConfig{
+		PostHooks: []func() error{d.postStop},
+	}
+
+	err = unixDeviceRemove(d.instance.DevicesPath(), "unix", d.name, "", &runConf)
+	if err != nil {
+		return nil, err
+	}
+
+	return &runConf, nil
+}
+
+// postStop is run after the device is removed from the instance.
+func (d *unixCommon) postStop() error {
+	// Remove host files for this device.
+	err := unixDeviceDeleteFiles(d.state, d.instance.DevicesPath(), "unix", d.name, "")
+	if err != nil {
+		return fmt.Errorf("Failed to delete files for device '%s': %v", d.name, err)
+	}
+
+	return nil
+}

From 9dd310127054d9ad69085733b6a639a144e3fc9a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Tue, 27 Aug 2019 16:02:32 +0100
Subject: [PATCH 17/30] container/lxc: Removes device Register() after device
 start

Moves to using post start hooks so that Register is optional, controlled by the device, and always occurs after the instance has started or the device has started (when hot plugging).

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/container_lxc.go | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index b25d526b52..0dab11ff35 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1931,12 +1931,6 @@ func (c *containerLXC) deviceStart(deviceName string, rawConfig map[string]strin
 		}
 	}
 
-	// Check whether device wants to register for any events irrespective of instance run state.
-	err = d.Register()
-	if err != nil {
-		return nil, err
-	}
-
 	return runConf, nil
 }
 

From ace538b8f9158626685ca6b26e3d86e63852887d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Tue, 27 Aug 2019 16:03:55 +0100
Subject: [PATCH 18/30] daemon: Reorganised the uevent and inotify event
 handler startup to occur before device registration.

This is to allow devices to trigger new inotify watches as devices are registered, and to ensure that instances are started before executing their event handlers.

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/daemon.go | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index 61170d95eb..49b704356b 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -30,6 +30,7 @@ import (
 
 	"github.com/lxc/lxd/lxd/cluster"
 	"github.com/lxc/lxd/lxd/db"
+	"github.com/lxc/lxd/lxd/device"
 	"github.com/lxc/lxd/lxd/endpoints"
 	"github.com/lxc/lxd/lxd/maas"
 	"github.com/lxc/lxd/lxd/node"
@@ -843,20 +844,20 @@ func (d *Daemon) init() error {
 	}
 
 	if !d.os.MockMode {
-		// Register devices on running instances to receive events.
-		devicesRegister(d.State())
-
 		// Start the scheduler
 		go deviceEventListener(d.State())
 
-		// Setup inotify watches
-		_, err := deviceInotifyInit(d.State())
+		// Setup unix inotify watches
+		_, err := device.UnixInotifyInit(d.State())
 		if err != nil {
 			return err
 		}
 
-		deviceInotifyDirRescan(d.State())
-		go deviceInotifyHandler(d.State())
+		go device.UnixInotifyHandler(d.State())
+
+		// Register devices on running instances to receive events.
+		// This should come after the event handler go routines have been started.
+		devicesRegister(d.State())
 
 		// Setup seccomp handler
 		if d.os.SeccompListener {

From d47267e4034eef2f192c30131007a3167aff292b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Tue, 27 Aug 2019 16:05:18 +0100
Subject: [PATCH 19/30] device/device/utils/unix: Moves some config validation
 functions into device package

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/device_utils_unix.go | 42 +++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/lxd/device/device_utils_unix.go b/lxd/device/device_utils_unix.go
index 17a97bd704..a2c686255d 100644
--- a/lxd/device/device_utils_unix.go
+++ b/lxd/device/device_utils_unix.go
@@ -550,3 +550,45 @@ func unixDeviceDeleteFiles(s *state.State, devicesPath string, typePrefix string
 
 	return nil
 }
+
+// unixValidDeviceNum validates the major and minor numbers for a UNIX device.
+func unixValidDeviceNum(value string) error {
+	if value == "" {
+		return nil
+	}
+
+	_, err := strconv.ParseUint(value, 10, 32)
+	if err != nil {
+		return fmt.Errorf("Invalid value for a UNIX device number")
+	}
+
+	return nil
+}
+
+// unixValidUserID validates the UNIX UID and GID values for ownership.
+func unixValidUserID(value string) error {
+	if value == "" {
+		return nil
+	}
+
+	_, err := strconv.ParseUint(value, 10, 32)
+	if err != nil {
+		return fmt.Errorf("Invalid value for a UNIX ID")
+	}
+
+	return nil
+}
+
+// unixValidOctalFileMode validates the UNIX file mode.
+func unixValidOctalFileMode(value string) error {
+	if value == "" {
+		return nil
+	}
+
+	_, err := strconv.ParseUint(value, 8, 32)
+	if err != nil {
+		return fmt.Errorf("Invalid value for an octal file mode")
+	}
+
+	return nil
+}

From e2a480e7dfff38418f5d1817b79c411b2974843b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Tue, 27 Aug 2019 16:06:30 +0100
Subject: [PATCH 20/30] device/gpu: Used device package validation functions

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/gpu.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lxd/device/gpu.go b/lxd/device/gpu.go
index 67a8a233cd..1b8f5d895a 100644
--- a/lxd/device/gpu.go
+++ b/lxd/device/gpu.go
@@ -41,9 +41,9 @@ func (d *gpu) validateConfig() error {
 		"productid": shared.IsDeviceID,
 		"id":        shared.IsAny,
 		"pci":       shared.IsAny,
-		"uid":       shared.IsUnixUserID,
-		"gid":       shared.IsUnixUserID,
-		"mode":      shared.IsOctalFileMode,
+		"uid":       unixValidUserID,
+		"gid":       unixValidUserID,
+		"mode":      unixValidOctalFileMode,
 	}
 
 	err := config.ValidateDevice(rules, d.config)

From 0d2d915cb9eede77b70bdfc066bc550f75ae8e6c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Tue, 27 Aug 2019 16:07:02 +0100
Subject: [PATCH 21/30] device/proxy: Used device package validation functions

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/proxy.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lxd/device/proxy.go b/lxd/device/proxy.go
index c9f00270f7..f27d544642 100644
--- a/lxd/device/proxy.go
+++ b/lxd/device/proxy.go
@@ -64,12 +64,12 @@ func (d *proxy) validateConfig() error {
 		"listen":         validateAddr,
 		"connect":        validateAddr,
 		"bind":           validateBind,
-		"mode":           shared.IsOctalFileMode,
+		"mode":           unixValidOctalFileMode,
 		"nat":            shared.IsBool,
-		"gid":            shared.IsUnixUserID,
-		"uid":            shared.IsUnixUserID,
-		"security.uid":   shared.IsUnixUserID,
-		"security.gid":   shared.IsUnixUserID,
+		"gid":            unixValidUserID,
+		"uid":            unixValidUserID,
+		"security.uid":   unixValidUserID,
+		"security.gid":   unixValidUserID,
 		"proxy_protocol": shared.IsBool,
 	}
 

From 1d0f61a2e2839d3d66def5dcbc70a34afe4f87c4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Tue, 27 Aug 2019 16:07:30 +0100
Subject: [PATCH 22/30] device/usb: Updates Register() to be called by post
 start hook

Also updates util function names.

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/usb.go | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lxd/device/usb.go b/lxd/device/usb.go
index 137983fc50..ad5a6ba16d 100644
--- a/lxd/device/usb.go
+++ b/lxd/device/usb.go
@@ -40,9 +40,9 @@ func (d *usb) validateConfig() error {
 	rules := map[string]func(string) error{
 		"vendorid":  shared.IsDeviceID,
 		"productid": shared.IsDeviceID,
-		"uid":       shared.IsUnixUserID,
-		"gid":       shared.IsUnixUserID,
-		"mode":      shared.IsOctalFileMode,
+		"uid":       unixValidUserID,
+		"gid":       unixValidUserID,
+		"mode":      unixValidOctalFileMode,
 		"required":  shared.IsBool,
 	}
 
@@ -99,7 +99,7 @@ func (d *usb) Register() error {
 		return &runConf, nil
 	}
 
-	USBRegisterHandler(d.instance, d.name, f)
+	usbRegisterHandler(d.instance, d.name, f)
 
 	return nil
 }
@@ -112,6 +112,7 @@ func (d *usb) Start() (*RunConfig, error) {
 	}
 
 	runConf := RunConfig{}
+	runConf.PostHooks = []func() error{d.Register}
 
 	for _, usb := range usbs {
 		if !usbIsOurDevice(d.config, &usb) {
@@ -134,7 +135,7 @@ func (d *usb) Start() (*RunConfig, error) {
 // Stop is run when the device is removed from the instance.
 func (d *usb) Stop() (*RunConfig, error) {
 	// Unregister any USB event handlers for this device.
-	USBUnregisterHandler(d.instance, d.name)
+	usbUnregisterHandler(d.instance, d.name)
 
 	runConf := RunConfig{
 		PostHooks: []func() error{d.postStop},

From dc80b64b8896c8052b9c361b703cdef67ee0a5df Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Tue, 27 Aug 2019 16:08:51 +0100
Subject: [PATCH 23/30] shared/container: Removes device related validation
 functions

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 shared/container.go | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/shared/container.go b/shared/container.go
index 89ce027e55..bc5e63b837 100644
--- a/shared/container.go
+++ b/shared/container.go
@@ -115,32 +115,6 @@ func IsNotEmpty(value string) error {
 	return nil
 }
 
-func IsUnixUserID(value string) error {
-	if value == "" {
-		return nil
-	}
-
-	_, err := strconv.ParseUint(value, 10, 32)
-	if err != nil {
-		return fmt.Errorf("Invalid value for a UNIX ID")
-	}
-
-	return nil
-}
-
-func IsOctalFileMode(value string) error {
-	if value == "" {
-		return nil
-	}
-
-	_, err := strconv.ParseUint(value, 8, 32)
-	if err != nil {
-		return fmt.Errorf("Invalid value for an octal file mode")
-	}
-
-	return nil
-}
-
 // IsDeviceID validates string is four lowercase hex characters suitable as Vendor or Device ID.
 func IsDeviceID(value string) error {
 	if value == "" {

From 43a819ff3cd16564b964036e2de2c863ea2a2845 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 22 Aug 2019 15:59:17 +0100
Subject: [PATCH 24/30] test: Adds tests for unix-char and unix-block devices

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 test/main.sh                          |   2 +
 test/suites/container_devices_unix.sh | 169 ++++++++++++++++++++++++++
 2 files changed, 171 insertions(+)
 create mode 100644 test/suites/container_devices_unix.sh

diff --git a/test/main.sh b/test/main.sh
index 33e60d42d2..b7c4cf9dfc 100755
--- a/test/main.sh
+++ b/test/main.sh
@@ -200,6 +200,8 @@ run_test test_container_devices_infiniband_physical "container devices - infinib
 run_test test_container_devices_infiniband_sriov "container devices - infiniband - sriov"
 run_test test_container_devices_proxy "container devices - proxy"
 run_test test_container_devices_gpu "container devices - gpu"
+run_test test_container_devices_unix_char "container devices - unix-char"
+run_test test_container_devices_unix_block "container devices - unix-block"
 run_test test_security "security features"
 run_test test_security_protection "container protection"
 run_test test_image_expiry "image expiry"
diff --git a/test/suites/container_devices_unix.sh b/test/suites/container_devices_unix.sh
new file mode 100644
index 0000000000..0a91a6c8c2
--- /dev/null
+++ b/test/suites/container_devices_unix.sh
@@ -0,0 +1,169 @@
+test_container_devices_unix_block() {
+  test_container_devices_unix "unix-block"
+}
+
+test_container_devices_unix_char() {
+  test_container_devices_unix "unix-char"
+}
+
+test_container_devices_unix() {
+  deviceType=$1
+  deviceTypeCode=""
+  deviceTypeDesc=""
+
+  if [ "$deviceType" = "unix-block" ]; then
+    deviceTypeCode="b"
+    deviceTypeDesc="block special file"
+  fi
+
+  if [ "$deviceType" = "unix-char" ]; then
+    deviceTypeCode="c"
+    deviceTypeDesc="character special file"
+  fi
+
+  if [ "$deviceTypeCode" = "" ]; then
+    echo "invalid device type specified in test"
+    false
+  fi
+
+  ensure_import_testimage
+  ensure_has_localhost_remote "${LXD_ADDR}"
+  ctName="ct$$"
+  lxc launch testimage "${ctName}"
+
+  # Create a test unix device.
+  testDev="${TEST_DIR}"/testdev-"${ctName}"
+  mknod "${testDev}" "${deviceTypeCode}" 0 0
+
+  # Check adding a device without source or path fails.
+  ! lxc config device add "${ctName}" test-dev-invalid "${deviceType}"
+  ! lxc config device add "${ctName}" test-dev-invalid "${deviceType}" required=false
+
+  # Check adding a device with missing source and no major/minor numbers fails.
+  ! lxc config device add "${ctName}" test-dev-invalid "${deviceType}" path=/tmp/testdevmissing
+
+  # Check adding a required (default) missing device fails.
+  ! lxc config device add "${ctName}" test-dev-invalid "${deviceType}" path=/tmp/testdevmissing
+  ! lxc config device add "${ctName}" test-dev-invalid "${deviceType}" path=/tmp/testdevmissing required=true
+
+  # Add device based on existing device, check its host-side name, default mode, major/minor inherited, and mounted in container.
+  lxc config device add "${ctName}" test-dev1 "${deviceType}" source="${testDev}" path=/tmp/testdev
+  lxc exec "${ctName}" -- mount | grep "/tmp/testdev"
+  lxc exec "${ctName}" -- stat -c '%F %a %t %T' /tmp/testdev | grep "${deviceTypeDesc} 660 0 0"
+  stat -c '%F %a %t %T' "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev1.tmp-testdev | grep "${deviceTypeDesc} 660 0 0"
+
+  # Add device with same dest path as existing device, but with different mode and major/minor and check original isn't replaced inside instance.
+  lxc config device add "${ctName}" test-dev2 "${deviceType}" source="${testDev}" path=/tmp/testdev major=1 minor=1 mode=600
+  lxc exec "${ctName}" -- mount | grep "/tmp/testdev"
+  lxc exec "${ctName}" -- stat -c '%F %a %t %T' /tmp/testdev | grep "${deviceTypeDesc} 660 0 0"
+
+  # Check a new host side file was created with correct attributes.
+  stat -c '%F %a %t %T' "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev2.tmp-testdev | grep "${deviceTypeDesc} 600 1 1"
+
+  # Remove dupe device and check the original is still mounted.
+  lxc config device remove "${ctName}" test-dev2
+  lxc exec "${ctName}" -- mount | grep "/tmp/testdev"
+  lxc exec "${ctName}" -- stat -c '%F %a %t %T' /tmp/testdev | grep "${deviceTypeDesc} 660 0 0"
+
+  # Check dupe device host side file is removed though.
+  if ls "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev2.tmp-testdev; then
+    echo "test-dev2 host side file not removed"
+    false
+  fi
+
+  # Add new device with custom mode and check it creates correctly on boot.
+  lxc stop -f "${ctName}"
+  lxc config device add "${ctName}" test-dev3 "${deviceType}" source="${testDev}" path=/tmp/testdev3 major=1 minor=1 mode=600
+  lxc start "${ctName}"
+  lxc exec "${ctName}" -- mount | grep "/tmp/testdev3"
+  lxc exec "${ctName}" -- stat -c '%F %a %t %T' /tmp/testdev3 | grep "${deviceTypeDesc} 600 1 1"
+  stat -c '%F %a %t %T' "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev3.tmp-testdev3 | grep "${deviceTypeDesc} 600 1 1"
+  lxc config device remove "${ctName}" test-dev3
+
+  # Add new device without a source, but with a path and major and minor numbers.
+  lxc config device add "${ctName}" test-dev4 "${deviceType}" path=/tmp/testdev4 major=0 minor=2 mode=777
+  lxc exec "${ctName}" -- mount | grep "/tmp/testdev4"
+  lxc exec "${ctName}" -- stat -c '%F %a %t %T' /tmp/testdev4 | grep "${deviceTypeDesc} 777 0 2"
+  stat -c '%F %a %t %T' "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev4.tmp-testdev4 | grep "${deviceTypeDesc} 777 0 2"
+  lxc config device remove "${ctName}" test-dev4
+
+  lxc stop -f "${ctName}"
+  lxc config device remove "${ctName}" test-dev1
+  rm "${testDev}"
+
+  # Add a device that is missing, but not required, start instance and then add it.
+  lxc config device add "${ctName}" test-dev-dynamic "${deviceType}" required=false source="${testDev}" path=/tmp/testdev
+  lxc start "${ctName}"
+  ! ls "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev--dynamic.tmp-testdev
+  mknod "${testDev}" "${deviceTypeCode}" 0 0
+  sleep 0.5
+  lxc exec "${ctName}" -- mount | grep "/tmp/testdev"
+  lxc exec "${ctName}" -- stat -c '%F %a %t %T' /tmp/testdev | grep "${deviceTypeDesc} 660 0 0"
+  stat -c '%F %a %t %T' "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev--dynamic.tmp-testdev | grep "${deviceTypeDesc} 660 0 0"
+
+  # Remove host side device and check it is dynamically removed from instance.
+  rm "${testDev}"
+  sleep 0.5
+  ! lxc exec "${ctName}" -- mount | grep "/tmp/testdev"
+  ! lxc exec "${ctName}" -- ls /tmp/testdev
+  ! ls "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev--dynamic.tmp-testdev
+
+  # Leave instance running, restart LXD, then add device back to check LXD start time inotify works.
+  shutdown_lxd "${LXD_DIR}"
+  respawn_lxd "${LXD_DIR}" true
+  mknod "${testDev}" "${deviceTypeCode}" 0 0
+  sleep 0.5
+  lxc exec "${ctName}" -- mount | grep "/tmp/testdev"
+  lxc exec "${ctName}" -- stat -c '%F %a %t %T' /tmp/testdev | grep "${deviceTypeDesc} 660 0 0"
+  stat -c '%F %a %t %T' "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev--dynamic.tmp-testdev | grep "${deviceTypeDesc} 660 0 0"
+
+  # Update device's source, check old instance device is removed and new watchers set up.
+  rm "${testDev}"
+  testDevSubDir="${testDev}"/subdev
+  ls -la "${TEST_DIR}"
+  lxc config device set "${ctName}" test-dev-dynamic source="${testDevSubDir}"
+  ! lxc exec "${ctName}" -- mount | grep "/tmp/testdev"
+  ! lxc exec "${ctName}" -- ls /tmp/testdev
+  ! ls "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev--dynamic.tmp-testdev
+
+  mkdir "${testDev}"
+  mknod "${testDevSubDir}" "${deviceTypeCode}" 0 0
+  sleep 0.5
+  lxc exec "${ctName}" -- mount | grep "/tmp/testdev"
+  lxc exec "${ctName}" -- stat -c '%F %a %t %T' /tmp/testdev | grep "${deviceTypeDesc} 660 0 0"
+  stat -c '%F %a %t %T' "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev--dynamic.tmp-testdev | grep "${deviceTypeDesc} 660 0 0"
+
+  # Cleanup.
+  rm -rvf "${testDev}"
+  sleep 0.5
+  ! lxc exec "${ctName}" -- mount | grep "/tmp/testdev"
+  ! lxc exec "${ctName}" -- ls /tmp/testdev
+  ! ls "${LXD_DIR}"/devices/"${ctName}"/unix.test--dev--dynamic.tmp-testdev
+  lxc delete -f "${ctName}"
+
+  # Check multiple instances sharing same watcher.
+  lxc launch testimage "${ctName}1"
+  lxc config device add "${ctName}1" test-dev-dynamic "${deviceType}" required=false source="${testDev}" path=/tmp/testdev1
+  lxc launch testimage "${ctName}2"
+  lxc config device add "${ctName}2" test-dev-dynamic "${deviceType}" required=false source="${testDev}" path=/tmp/testdev2
+  mknod "${testDev}" "${deviceTypeCode}" 0 0
+  sleep 0.5
+  lxc exec "${ctName}1" -- mount | grep "/tmp/testdev1"
+  lxc exec "${ctName}1" -- stat -c '%F %a %t %T' /tmp/testdev1 | grep "${deviceTypeDesc} 660 0 0"
+  stat -c '%F %a %t %T' "${LXD_DIR}"/devices/"${ctName}"1/unix.test--dev--dynamic.tmp-testdev1 | grep "${deviceTypeDesc} 660 0 0"
+  lxc exec "${ctName}2" -- mount | grep "/tmp/testdev2"
+  lxc exec "${ctName}2" -- stat -c '%F %a %t %T' /tmp/testdev2 | grep "${deviceTypeDesc} 660 0 0"
+  stat -c '%F %a %t %T' "${LXD_DIR}"/devices/"${ctName}"2/unix.test--dev--dynamic.tmp-testdev2 | grep "${deviceTypeDesc} 660 0 0"
+
+  # Stop one instance, then remove the host device to check the watcher still works after first
+  # instance was stopped. This checks the removal logic when multiple containers share watch path.
+  lxc stop -f "${ctName}1"
+  rm "${testDev}"
+  sleep 0.5
+  ! lxc exec "${ctName}2" -- mount | grep "/tmp/testdev2"
+  ! lxc exec "${ctName}2" -- ls /tmp/testdev2
+  ! ls "${LXD_DIR}"/devices/"${ctName}"2/unix.test--dev--dynamic.tmp-testdev2
+  lxc delete -f "${ctName}1"
+  lxc delete -f "${ctName}2"
+}
+

From 4fded498f97042a0432bc706d834b4065cbfeb30 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Thu, 29 Aug 2019 10:57:54 +0100
Subject: [PATCH 25/30] container: Removes unused disk device code

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/container.go | 108 +----------------------------------------------
 1 file changed, 2 insertions(+), 106 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index 25e02f88a7..823e201d25 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -108,35 +108,6 @@ func containerValidDeviceConfigKey(t, k string) bool {
 	}
 
 	switch t {
-	case "disk":
-		switch k {
-		case "limits.max":
-			return true
-		case "limits.read":
-			return true
-		case "limits.write":
-			return true
-		case "optional":
-			return true
-		case "path":
-			return true
-		case "readonly":
-			return true
-		case "size":
-			return true
-		case "source":
-			return true
-		case "recursive":
-			return true
-		case "pool":
-			return true
-		case "propagation":
-			return true
-		case "shift":
-			return true
-		default:
-			return false
-		}
 	case "none":
 		return false
 	default:
@@ -220,7 +191,6 @@ func containerValidDevices(state *state.State, cluster *db.Cluster, devices conf
 		return nil
 	}
 
-	var diskDevicePaths []string
 	// Check each device individually
 	for name, m := range devices {
 		// Validate config using device interface.
@@ -233,7 +203,7 @@ func containerValidDevices(state *state.State, cluster *db.Cluster, devices conf
 		}
 
 		// Fallback to legacy validation for non device package devices.
-		if !shared.StringInSlice(m["type"], []string{"disk", "none"}) {
+		if !shared.StringInSlice(m["type"], []string{"none"}) {
 			return fmt.Errorf("Invalid device type for device '%s'", name)
 		}
 
@@ -243,81 +213,7 @@ func containerValidDevices(state *state.State, cluster *db.Cluster, devices conf
 			}
 		}
 
-		if m["type"] == "infiniband" {
-			if m["nictype"] == "" {
-				return fmt.Errorf("Missing nic type")
-			}
-
-			if !shared.StringInSlice(m["nictype"], []string{"physical", "sriov"}) {
-				return fmt.Errorf("Bad nic type: %s", m["nictype"])
-			}
-
-			if m["parent"] == "" {
-				return fmt.Errorf("Missing parent for %s type nic", m["nictype"])
-			}
-		} else if m["type"] == "disk" {
-			if !expanded && !shared.StringInSlice(m["path"], diskDevicePaths) {
-				diskDevicePaths = append(diskDevicePaths, m["path"])
-			} else if !expanded {
-				return fmt.Errorf("More than one disk device uses the same path: %s", m["path"])
-			}
-
-			if m["path"] == "" {
-				return fmt.Errorf("Disk entry is missing the required \"path\" property")
-			}
-
-			if m["source"] == "" && m["path"] != "/" {
-				return fmt.Errorf("Disk entry is missing the required \"source\" property")
-			}
-
-			if m["path"] == "/" && m["source"] != "" {
-				return fmt.Errorf("Root disk entry may not have a \"source\" property set")
-			}
-
-			if m["size"] != "" && m["path"] != "/" {
-				return fmt.Errorf("Only the root disk may have a size quota")
-			}
-
-			if (m["path"] == "/" || !shared.IsDir(shared.HostPath(m["source"]))) && m["recursive"] != "" {
-				return fmt.Errorf("The recursive option is only supported for additional bind-mounted paths")
-			}
-
-			if m["pool"] != "" {
-				if filepath.IsAbs(m["source"]) {
-					return fmt.Errorf("Storage volumes cannot be specified as absolute paths")
-				}
-
-				_, err := cluster.StoragePoolGetID(m["pool"])
-				if err != nil {
-					return fmt.Errorf("The \"%s\" storage pool doesn't exist", m["pool"])
-				}
-
-				if !profile && expanded && m["source"] != "" && m["path"] != "/" {
-					isAvailable, err := cluster.StorageVolumeIsAvailable(
-						m["pool"], m["source"])
-					if err != nil {
-						return errors.Wrap(err, "Check if volume is available")
-					}
-					if !isAvailable {
-						return fmt.Errorf(
-							"Storage volume %q is already attached to a container "+
-								"on a different node", m["source"])
-					}
-				}
-			}
-
-			if m["propagation"] != "" {
-				if !shared.StringInSlice(m["propagation"], []string{"private", "shared", "slave", "unbindable", "rprivate", "rshared", "rslave", "runbindable"}) {
-					return fmt.Errorf("Invalid propagation mode '%s'", m["propagation"])
-				}
-			}
-
-			if m["shift"] != "" {
-				if m["pool"] != "" {
-					return fmt.Errorf("The \"shift\" property cannot be used with custom storage volumes")
-				}
-			}
-		} else if m["type"] == "none" {
+		if m["type"] == "none" {
 			continue
 		} else {
 			return fmt.Errorf("Invalid device type: %s", m["type"])

From 41084c30307d8f90aed2312b3875ad9bfc15e563 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Thu, 29 Aug 2019 10:58:59 +0100
Subject: [PATCH 26/30] container/lxc: Moves disk device to use device package

Also improves error messages when device setup fails to indicate which part of device setup has failed.

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/container_lxc.go | 277 ++++++++++++-------------------------------
 1 file changed, 75 insertions(+), 202 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 0dab11ff35..ef7ab2f1c7 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1636,149 +1636,6 @@ func (c *containerLXC) initLXC(config bool) error {
 		}
 	}
 
-	// Setup devices
-	for _, k := range c.expandedDevices.DeviceNames() {
-		m := c.expandedDevices[k]
-		if m["type"] == "disk" {
-			isRootfs := shared.IsRootDiskDevice(m)
-
-			// source paths
-			srcPath := shared.HostPath(m["source"])
-
-			// destination paths
-			destPath := m["path"]
-			relativeDestPath := strings.TrimPrefix(destPath, "/")
-
-			sourceDevPath := filepath.Join(c.DevicesPath(), fmt.Sprintf("disk.%s.%s", strings.Replace(k, "/", "-", -1), strings.Replace(relativeDestPath, "/", "-", -1)))
-
-			// Various option checks
-			isOptional := shared.IsTrue(m["optional"])
-			isReadOnly := shared.IsTrue(m["readonly"])
-			isRecursive := shared.IsTrue(m["recursive"])
-
-			// If we want to mount a storage volume from a storage
-			// pool we created via our storage api, we are always
-			// mounting a directory.
-			isFile := false
-			if m["pool"] == "" {
-				isFile = !shared.IsDir(srcPath) && !device.IsBlockdev(srcPath)
-			}
-
-			// Deal with a rootfs
-			if isRootfs {
-				if !util.RuntimeLiblxcVersionAtLeast(2, 1, 0) {
-					// Set the rootfs backend type if supported (must happen before any other lxc.rootfs)
-					err := lxcSetConfigItem(cc, "lxc.rootfs.backend", "dir")
-					if err == nil {
-						value := cc.ConfigItem("lxc.rootfs.backend")
-						if len(value) == 0 || value[0] != "dir" {
-							lxcSetConfigItem(cc, "lxc.rootfs.backend", "")
-						}
-					}
-				}
-
-				// Set the rootfs path
-				if util.RuntimeLiblxcVersionAtLeast(2, 1, 0) {
-					rootfsPath := fmt.Sprintf("dir:%s", c.RootfsPath())
-					err = lxcSetConfigItem(cc, "lxc.rootfs.path", rootfsPath)
-				} else {
-					rootfsPath := c.RootfsPath()
-					err = lxcSetConfigItem(cc, "lxc.rootfs", rootfsPath)
-				}
-				if err != nil {
-					return err
-				}
-
-				// Read-only rootfs (unlikely to work very well)
-				if isReadOnly {
-					err = lxcSetConfigItem(cc, "lxc.rootfs.options", "ro")
-					if err != nil {
-						return err
-					}
-				}
-			} else {
-				shift := false
-				if !c.IsPrivileged() {
-					shift = shared.IsTrue(m["shift"])
-					if !shift && m["pool"] != "" {
-						poolID, _, err := c.state.Cluster.StoragePoolGet(m["pool"])
-						if err != nil {
-							return err
-						}
-
-						_, volume, err := c.state.Cluster.StoragePoolNodeVolumeGetTypeByProject(c.project, m["source"], storagePoolVolumeTypeCustom, poolID)
-						if err != nil {
-							return err
-						}
-
-						if shared.IsTrue(volume.Config["security.shifted"]) {
-							shift = true
-						}
-					}
-				}
-
-				if shift {
-					if !c.state.OS.Shiftfs {
-						return fmt.Errorf("shiftfs is required by disk entry '%s' but isn't supported on system", k)
-					}
-
-					err = lxcSetConfigItem(cc, "lxc.hook.pre-start", fmt.Sprintf("/bin/mount -t shiftfs -o mark,passthrough=3 %s %s", sourceDevPath, sourceDevPath))
-					if err != nil {
-						return err
-					}
-
-					err = lxcSetConfigItem(cc, "lxc.hook.pre-mount", fmt.Sprintf("/bin/mount -t shiftfs -o passthrough=3 %s %s", sourceDevPath, sourceDevPath))
-					if err != nil {
-						return err
-					}
-
-					err = lxcSetConfigItem(cc, "lxc.hook.start-host", fmt.Sprintf("/bin/umount -l %s", sourceDevPath))
-					if err != nil {
-						return err
-					}
-				}
-
-				rbind := ""
-				options := []string{}
-				if isReadOnly {
-					options = append(options, "ro")
-				}
-
-				if isOptional {
-					options = append(options, "optional")
-				}
-
-				if isRecursive {
-					rbind = "r"
-				}
-
-				if m["propagation"] != "" {
-					if !util.RuntimeLiblxcVersionAtLeast(3, 0, 0) {
-						return fmt.Errorf("liblxc 3.0 is required for mount propagation configuration")
-					}
-
-					options = append(options, m["propagation"])
-				}
-
-				if isFile {
-					options = append(options, "create=file")
-				} else {
-					options = append(options, "create=dir")
-				}
-
-				err = lxcSetConfigItem(cc, "lxc.mount.entry",
-					fmt.Sprintf("%s %s none %sbind,%s 0 0",
-						shared.EscapePathFstab(sourceDevPath),
-						shared.EscapePathFstab(relativeDestPath),
-						rbind,
-						strings.Join(options, ",")))
-				if err != nil {
-					return err
-				}
-			}
-		}
-	}
-
 	// Setup shmounts
 	if c.state.OS.LXCFeatures["mount_injection_file"] {
 		err = lxcSetConfigItem(cc, "lxc.mount.auto", fmt.Sprintf("shmounts:%s:/dev/.lxd-mounts", c.ShmountsPath()))
@@ -2473,21 +2330,6 @@ func (c *containerLXC) startCommon() (string, []func() error, error) {
 		return "", postStartHooks, fmt.Errorf("The container is already running")
 	}
 
-	// Sanity checks for devices
-	for name, m := range c.expandedDevices {
-		switch m["type"] {
-		case "disk":
-			// When we want to attach a storage volume created via
-			// the storage api m["source"] only contains the name of
-			// the storage volume, not the path where it is mounted.
-			// So do only check for the existence of m["source"]
-			// when m["pool"] is empty.
-			if m["pool"] == "" && m["source"] != "" && !shared.IsTrue(m["optional"]) && !shared.PathExists(shared.HostPath(m["source"])) {
-				return "", postStartHooks, fmt.Errorf("Missing source '%s' for disk '%s'", m["source"], name)
-			}
-		}
-	}
-
 	// Load any required kernel modules
 	kernelModules := c.expandedConfig["linux.kernel_modules"]
 	if kernelModules != "" {
@@ -2632,67 +2474,98 @@ func (c *containerLXC) startCommon() (string, []func() error, error) {
 	nicID := -1
 	for _, k := range c.expandedDevices.DeviceNames() {
 		m := c.expandedDevices[k]
-		if m["type"] == "disk" {
-			if m["path"] != "/" {
-				diskDevices[k] = m
+
+		// Start the device.
+		runConf, err := c.deviceStart(k, m, false)
+		if err != nil {
+			return "", postStartHooks, errors.Wrapf(err, "Failed to start device '%s'", k)
+		}
+
+		// Process rootfs setup.
+		if runConf.RootFS.Path != "" {
+			if !util.RuntimeLiblxcVersionAtLeast(2, 1, 0) {
+				// Set the rootfs backend type if supported (must happen before any other lxc.rootfs)
+				err := lxcSetConfigItem(c.c, "lxc.rootfs.backend", "dir")
+				if err == nil {
+					value := c.c.ConfigItem("lxc.rootfs.backend")
+					if len(value) == 0 || value[0] != "dir" {
+						lxcSetConfigItem(c.c, "lxc.rootfs.backend", "")
+					}
+				}
 			}
-		} else {
-			// Use new Device interface if supported.
-			runConf, err := c.deviceStart(k, m, false)
-			if err != device.ErrUnsupportedDevType {
+
+			if util.RuntimeLiblxcVersionAtLeast(2, 1, 0) {
+				rootfsPath := fmt.Sprintf("dir:%s", runConf.RootFS.Path)
+				err = lxcSetConfigItem(c.c, "lxc.rootfs.path", rootfsPath)
+			} else {
+				err = lxcSetConfigItem(c.c, "lxc.rootfs", runConf.RootFS.Path)
+			}
+
+			if err != nil {
+				return "", postStartHooks, errors.Wrapf(err, "Failed to setup device rootfs '%s'", k)
+			}
+
+			if len(runConf.RootFS.Opts) > 0 {
+				err = lxcSetConfigItem(c.c, "lxc.rootfs.options", strings.Join(runConf.RootFS.Opts, ","))
 				if err != nil {
-					return "", postStartHooks, errors.Wrapf(err, "Failed to start device '%s'", k)
+					return "", postStartHooks, errors.Wrapf(err, "Failed to setup device rootfs '%s'", k)
 				}
+			}
+		}
 
-				// Pass any cgroups rules into LXC.
-				if len(runConf.CGroups) > 0 {
-					for _, rule := range runConf.CGroups {
-						err = lxcSetConfigItem(c.c, fmt.Sprintf("lxc.cgroup.%s", rule.Key), rule.Value)
-						if err != nil {
-							return "", postStartHooks, err
-						}
-					}
+		// Pass any cgroups rules into LXC.
+		if len(runConf.CGroups) > 0 {
+			for _, rule := range runConf.CGroups {
+				err = lxcSetConfigItem(c.c, fmt.Sprintf("lxc.cgroup.%s", rule.Key), rule.Value)
+				if err != nil {
+					return "", postStartHooks, errors.Wrapf(err, "Failed to setup device cgroup '%s'", k)
 				}
+			}
+		}
 
-				// Pass any mounts into LXC.
-				if len(runConf.Mounts) > 0 {
-					for _, mount := range runConf.Mounts {
-						mntVal := fmt.Sprintf("%s %s %s %s %d %d", shared.EscapePathFstab(mount.DevPath), shared.EscapePathFstab(mount.TargetPath), mount.FSType, strings.Join(mount.Opts, ","), mount.Freq, mount.PassNo)
-						err = lxcSetConfigItem(c.c, "lxc.mount.entry", mntVal)
-						if err != nil {
-							return "", postStartHooks, err
-						}
-					}
+		// Pass any mounts into LXC.
+		if len(runConf.Mounts) > 0 {
+			for _, mount := range runConf.Mounts {
+				mntVal := fmt.Sprintf("%s %s %s %s %d %d", shared.EscapePathFstab(mount.DevPath), shared.EscapePathFstab(mount.TargetPath), mount.FSType, strings.Join(mount.Opts, ","), mount.Freq, mount.PassNo)
+				err = lxcSetConfigItem(c.c, "lxc.mount.entry", mntVal)
+				if err != nil {
+					return "", postStartHooks, errors.Wrapf(err, "Failed to setup device mount '%s'", k)
 				}
+			}
+		}
 
-				// Pass any network setup config into LXC.
-				if len(runConf.NetworkInterface) > 0 {
-					// Increment nicID so that LXC network index is unique per device.
-					nicID++
+		// Pass any network setup config into LXC.
+		if len(runConf.NetworkInterface) > 0 {
+			// Increment nicID so that LXC network index is unique per device.
+			nicID++
 
-					networkKeyPrefix := "lxc.net"
-					if !util.RuntimeLiblxcVersionAtLeast(2, 1, 0) {
-						networkKeyPrefix = "lxc.network"
-					}
+			networkKeyPrefix := "lxc.net"
+			if !util.RuntimeLiblxcVersionAtLeast(2, 1, 0) {
+				networkKeyPrefix = "lxc.network"
+			}
 
-					for _, dev := range runConf.NetworkInterface {
-						err = lxcSetConfigItem(c.c, fmt.Sprintf("%s.%d.%s", networkKeyPrefix, nicID, dev.Key), dev.Value)
-						if err != nil {
-							return "", postStartHooks, err
-						}
-					}
+			for _, dev := range runConf.NetworkInterface {
+				err = lxcSetConfigItem(c.c, fmt.Sprintf("%s.%d.%s", networkKeyPrefix, nicID, dev.Key), dev.Value)
+				if err != nil {
+					return "", postStartHooks, errors.Wrapf(err, "Failed to setup device network interface '%s'", k)
 				}
+			}
+		}
 
-				// Add any post start hooks.
-				if len(runConf.PostHooks) > 0 {
-					postStartHooks = append(postStartHooks, runConf.PostHooks...)
-				}
+		// Add any post start hooks.
+		if len(runConf.PostHooks) > 0 {
+			postStartHooks = append(postStartHooks, runConf.PostHooks...)
+		}
 
-				continue
+		// tomp TODO move this into device package.
+		if m["type"] == "disk" {
+			if m["path"] != "/" {
+				diskDevices[k] = m
 			}
 		}
 	}
 
+	// tomp TODO figure out how this can work with device package.
 	err = c.addDiskDevices(diskDevices, func(name string, d config.Device) error {
 		_, err := c.createDiskDevice(name, d)
 		return err

From d15d42c3d5e7e550cc662b02a664f30b4f7f3116 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Thu, 29 Aug 2019 10:59:52 +0100
Subject: [PATCH 27/30] device: Links up disk device

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/device.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lxd/device/device.go b/lxd/device/device.go
index a2eba93088..3fa80dc4f1 100644
--- a/lxd/device/device.go
+++ b/lxd/device/device.go
@@ -16,6 +16,7 @@ var devTypes = map[string]func(config.Device) device{
 	"usb":        func(c config.Device) device { return &usb{} },
 	"unix-char":  func(c config.Device) device { return &unixCommon{} },
 	"unix-block": func(c config.Device) device { return &unixCommon{} },
+	"disk":       func(c config.Device) device { return &disk{} },
 }
 
 // VolatileSetter is a function that accepts one or more key/value strings to save into the LXD

From b68c2b085d5f92d3293973eaf673fd31414e2ee4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Thu, 29 Aug 2019 11:00:09 +0100
Subject: [PATCH 28/30] device/device/instance/id: Adds RootfsPath() to
 InstanceIdentifier interface

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/device_instance_id.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lxd/device/device_instance_id.go b/lxd/device/device_instance_id.go
index 3dc35cf5f7..555feb00ba 100644
--- a/lxd/device/device_instance_id.go
+++ b/lxd/device/device_instance_id.go
@@ -12,6 +12,7 @@ type InstanceIdentifier interface {
 	Type() string
 	Project() string
 	DevicesPath() string
+	RootfsPath() string
 	LogPath() string
 	ExpandedConfig() map[string]string
 	ExpandedDevices() config.Devices

From 0994ee160822ee0ca1f67dc4af7b88e7d1a86cd1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Thu, 29 Aug 2019 11:00:53 +0100
Subject: [PATCH 29/30] device/device/runconfig: Adds RootFS support

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/device_runconfig.go | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lxd/device/device_runconfig.go b/lxd/device/device_runconfig.go
index b38d7f37b0..49f91f8922 100644
--- a/lxd/device/device_runconfig.go
+++ b/lxd/device/device_runconfig.go
@@ -17,8 +17,15 @@ type MountEntryItem struct {
 	Shift      bool     // Whether or not to use shiftfs with this mount.
 }
 
+// RootFSEntryItem represents the root filesystem options for an Instance.
+type RootFSEntryItem struct {
+	Path string   // Describes the root file system source.
+	Opts []string // Describes the mount options associated with the filesystem.
+}
+
 // RunConfig represents LXD defined run-time config used for device setup/cleanup.
 type RunConfig struct {
+	RootFS           RootFSEntryItem  // RootFS to setup.
 	NetworkInterface []RunConfigItem  // Network interface configuration settings.
 	CGroups          []RunConfigItem  // Cgroup rules to setup.
 	Mounts           []MountEntryItem // Mounts to setup/remove.

From ee752883a01d3fbccf57a465f5c2f4d6786675f6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Thu, 29 Aug 2019 11:01:17 +0100
Subject: [PATCH 30/30] device/disk: Adds disk device implementation

Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
 lxd/device/disk.go | 246 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 246 insertions(+)
 create mode 100644 lxd/device/disk.go

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
new file mode 100644
index 0000000000..49f2dd139c
--- /dev/null
+++ b/lxd/device/disk.go
@@ -0,0 +1,246 @@
+package device
+
+import (
+	"fmt"
+	"path/filepath"
+	"strings"
+
+	"github.com/lxc/lxd/lxd/device/config"
+	"github.com/lxc/lxd/lxd/instance"
+	//"github.com/lxc/lxd/lxd/util"
+	"github.com/lxc/lxd/shared"
+)
+
+type disk struct {
+	deviceCommon
+}
+
+// validateConfig checks the supplied config for correctness.
+func (d *disk) validateConfig() error {
+	if d.instance.Type() != instance.TypeContainer {
+		return ErrUnsupportedDevType
+	}
+
+	// Supported propagation types.
+	// If an empty value is supplied the default behavior is to assume "private" mode.
+	// These come from https://www.kernel.org/doc/Documentation/filesystems/sharedsubtree.txt
+	propagationTypes := []string{"", "private", "shared", "slave", "unbindable", "rshared", "rslave", "runbindable", "rprivate"}
+	validatePropagation := func(input string) error {
+		if !shared.StringInSlice(d.config["bind"], propagationTypes) {
+			return fmt.Errorf("Invalid propagation value. Must be one of: %s", strings.Join(propagationTypes, ", "))
+		}
+
+		return nil
+	}
+
+	rules := map[string]func(string) error{
+		"path":         shared.IsNotEmpty,
+		"optional":     shared.IsBool,
+		"readonly":     shared.IsBool,
+		"recursive":    shared.IsBool,
+		"shift":        shared.IsBool,
+		"source":       shared.IsAny,
+		"limits.read":  shared.IsAny,
+		"limits.write": shared.IsAny,
+		"limits.max":   shared.IsAny,
+		"size":         shared.IsAny,
+		"pool":         shared.IsAny,
+		"propagation":  validatePropagation,
+	}
+
+	err := config.ValidateDevice(rules, d.config)
+	if err != nil {
+		return err
+	}
+
+	// tomp TODO figure out what all of this is about (why does expanded or not matter?).
+	/*if !expanded && !shared.StringInSlice(m["path"], diskDevicePaths) {
+		diskDevicePaths = append(diskDevicePaths, m["path"])
+	} else if !expanded {
+		return fmt.Errorf("More than one disk device uses the same path: %s", m["path"])
+	}*/
+
+	if d.config["source"] == "" && d.config["path"] != "/" {
+		return fmt.Errorf("Disk entry is missing the required \"source\" property")
+	}
+
+	if d.config["path"] == "/" && d.config["source"] != "" {
+		return fmt.Errorf("Root disk entry may not have a \"source\" property set")
+	}
+
+	if d.config["size"] != "" && d.config["path"] != "/" {
+		return fmt.Errorf("Only the root disk may have a size quota")
+	}
+
+	if d.config["recursive"] != "" && (d.config["path"] == "/" || !shared.IsDir(shared.HostPath(d.config["source"]))) {
+		return fmt.Errorf("The recursive option is only supported for additional bind-mounted paths")
+	}
+
+	if d.config["pool"] != "" {
+		if d.config["shift"] != "" {
+			return fmt.Errorf("The \"shift\" property cannot be used with custom storage volumes")
+		}
+
+		if filepath.IsAbs(d.config["source"]) {
+			return fmt.Errorf("Storage volumes cannot be specified as absolute paths")
+		}
+
+		_, err := d.state.Cluster.StoragePoolGetID(d.config["pool"])
+		if err != nil {
+			return fmt.Errorf("The \"%s\" storage pool doesn't exist", d.config["pool"])
+		}
+
+		// tomp TODO figure this storage pool validation bit out.
+		/*if !profile && expanded && d.config["source"] != "" && d.config["path"] != "/" {
+			isAvailable, err := d.state.Cluster.StorageVolumeIsAvailable(
+				d.config["pool"], d.config["source"])
+			if err != nil {
+				return errors.Wrap(err, "Check if volume is available")
+			}
+			if !isAvailable {
+				return fmt.Errorf("Storage volume %q is already attached to a container on a different node", d.config["source"])
+			}
+		}*/
+	}
+
+	return nil
+}
+
+// Start is run when the device is added to the container.
+func (d *disk) Start() (*RunConfig, error) {
+	runConf := RunConfig{}
+
+	// When we want to attach a storage volume created via the storage api m["source"] only
+	// contains the name of the storage volume, not the path where it is mounted.
+	// So do only check for the existence of m["source"] when m["pool"] is empty.
+	if d.config["pool"] == "" && d.config["source"] != "" && !shared.IsTrue(d.config["optional"]) && !shared.PathExists(shared.HostPath(d.config["source"])) {
+		return nil, fmt.Errorf("Missing source '%s' for disk '%s'", d.config["source"], d.name)
+	}
+
+	isRootfs := shared.IsRootDiskDevice(d.config)
+	isReadOnly := shared.IsTrue(d.config["readonly"])
+
+	// Deal with a rootfs.
+	if isRootfs {
+		// Set the rootfs path.
+		rootfs := RootFSEntryItem{
+			Path: d.instance.RootfsPath(),
+		}
+
+		// Read-only rootfs (unlikely to work very well)
+		if isReadOnly {
+			rootfs.Opts = append(rootfs.Opts, "ro")
+		}
+
+		runConf.RootFS = rootfs
+	} /*else {
+		// source paths
+		srcPath := shared.HostPath(d.config["source"])
+
+		// destination paths
+		destPath := d.config["path"]
+		relativeDestPath := strings.TrimPrefix(destPath, "/")
+
+		sourceDevPath := filepath.Join(d.instance.DevicesPath(), fmt.Sprintf("disk.%s.%s", strings.Replace(d.name, "/", "-", -1), strings.Replace(relativeDestPath, "/", "-", -1)))
+
+		// Various option checks
+		isOptional := shared.IsTrue(d.config["optional"])
+		isRecursive := shared.IsTrue(d.config["recursive"])
+
+		// If we want to mount a storage volume from a storage
+		// pool we created via our storage api, we are always
+		// mounting a directory.
+		isFile := false
+		if d.config["pool"] == "" {
+			isFile = !shared.IsDir(srcPath) && !IsBlockdev(srcPath)
+		}
+
+		shift := false
+		if !c.IsPrivileged() {
+			shift = shared.IsTrue(m["shift"])
+			if !shift && m["pool"] != "" {
+				poolID, _, err := c.state.Cluster.StoragePoolGet(m["pool"])
+				if err != nil {
+					return err
+				}
+
+				_, volume, err := c.state.Cluster.StoragePoolNodeVolumeGetTypeByProject(c.project, m["source"], storagePoolVolumeTypeCustom, poolID)
+				if err != nil {
+					return err
+				}
+
+				if shared.IsTrue(volume.Config["security.shifted"]) {
+					shift = true
+				}
+			}
+		}
+
+		if shift {
+			if !c.state.OS.Shiftfs {
+				return fmt.Errorf("shiftfs is required by disk entry '%s' but isn't supported on system", k)
+			}
+
+			err = lxcSetConfigItem(cc, "lxc.hook.pre-start", fmt.Sprintf("/bin/mount -t shiftfs -o mark,passthrough=3 %s %s", sourceDevPath, sourceDevPath))
+			if err != nil {
+				return err
+			}
+
+			err = lxcSetConfigItem(cc, "lxc.hook.pre-mount", fmt.Sprintf("/bin/mount -t shiftfs -o passthrough=3 %s %s", sourceDevPath, sourceDevPath))
+			if err != nil {
+				return err
+			}
+
+			err = lxcSetConfigItem(cc, "lxc.hook.start-host", fmt.Sprintf("/bin/umount -l %s", sourceDevPath))
+			if err != nil {
+				return err
+			}
+		}
+
+		rbind := ""
+		options := []string{}
+		if isReadOnly {
+			options = append(options, "ro")
+		}
+
+		if isOptional {
+			options = append(options, "optional")
+		}
+
+		if isRecursive {
+			rbind = "r"
+		}
+
+		if m["propagation"] != "" {
+			if !util.RuntimeLiblxcVersionAtLeast(3, 0, 0) {
+				return fmt.Errorf("liblxc 3.0 is required for mount propagation configuration")
+			}
+
+			options = append(options, m["propagation"])
+		}
+
+		if isFile {
+			options = append(options, "create=file")
+		} else {
+			options = append(options, "create=dir")
+		}
+
+		err = lxcSetConfigItem(cc, "lxc.mount.entry",
+			fmt.Sprintf("%s %s none %sbind,%s 0 0",
+				shared.EscapePathFstab(sourceDevPath),
+				shared.EscapePathFstab(relativeDestPath),
+				rbind,
+				strings.Join(options, ",")))
+		if err != nil {
+			return err
+		}
+	}*/
+
+	return &runConf, nil
+}
+
+// Stop is run when the device is removed from the instance.
+func (d *disk) Stop() (*RunConfig, error) {
+	runConf := RunConfig{}
+
+	return &runConf, nil
+}


More information about the lxc-devel mailing list