[lxc-devel] [lxd/master] Device Unix
tomponline on Github
lxc-bot at linuxcontainers.org
Thu Aug 22 16:03:11 UTC 2019
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 521 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190822/0dc9ee4b/attachment-0001.bin>
-------------- next part --------------
From f7b65a8ee12ab496f94167715f78df8c454c3188 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/11] 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 b739c61f86372f4738588a4f5e77c2346c870ea9 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 02/11] 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 | 88 +++++++++++++++++++++++++++
2 files changed, 90 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..d5999a07df
--- /dev/null
+++ b/test/suites/container_devices_unix.sh
@@ -0,0 +1,88 @@
+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 char 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}"
+
+ # 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.
+ 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
+
+ # Check adding a device with no source and no major/minor numbers fails.
+ ! lxc config device add "${ctName}" test-dev-invalid5 "${deviceType}" path=/tmp/testdev5
+
+ lxc delete -f "${ctName}"
+ rm "${testDev}"
+}
+
From 7b32741e3865ad1f9ee153d983b05f50acd53924 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 03/11] device: Adds implementation for unix-char and
unix-block devices
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/unixCommon.go | 109 +++++++++++++++++++++++++++++++++++++++
1 file changed, 109 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..05d87766d6
--- /dev/null
+++ b/lxd/device/unixCommon.go
@@ -0,0 +1,109 @@
+package device
+
+import (
+ "fmt"
+
+ "github.com/lxc/lxd/lxd/device/config"
+ "github.com/lxc/lxd/lxd/instance"
+ "github.com/lxc/lxd/shared"
+)
+
+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": shared.IsAny, // TODO: Validate this
+ "minor": shared.IsAny, // TODO: Validate this
+ "uid": shared.IsUnixUserID,
+ "gid": shared.IsUnixUserID,
+ "mode": shared.IsOctalFileMode,
+ "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
+}
+
+// validateEnvironment checks the runtime environment for correctness.
+func (d *unixCommon) validateEnvironment() error {
+ if (d.config["required"] == "" || shared.IsTrue(d.config["required"])) && (d.config["major"] == "" || d.config["minor"] == "") {
+ srcPath := unixDeviceSourcePath(d.config)
+ if !shared.PathExists(srcPath) {
+ return fmt.Errorf("The device path doesn't exist on the host and major/minor wasn't specified")
+ }
+
+ dType, _, _, err := UnixDeviceAttributes(srcPath)
+ if err != nil {
+ return err
+ }
+
+ if d.config["type"] == "unix-char" && dType != "c" {
+ return fmt.Errorf("Path specified for unix-char device is a block device")
+ }
+
+ if d.config["type"] == "unix-block" && dType != "b" {
+ return fmt.Errorf("Path specified for unix-block device is a character device")
+ }
+ }
+
+ return nil
+}
+
+// Start is run when the device is added to the container.
+func (d *unixCommon) Start() (*RunConfig, error) {
+ err := d.validateEnvironment()
+ if err != nil {
+ return nil, err
+ }
+
+ runConf := RunConfig{}
+
+ err = unixDeviceSetup(d.state, d.instance.DevicesPath(), "unix", d.name, d.config, true, &runConf)
+ if err != nil {
+ return nil, err
+ }
+
+ return &runConf, nil
+}
+
+// Stop is run when the device is removed from the instance.
+func (d *unixCommon) Stop() (*RunConfig, error) {
+ 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 cba0b15245e97ebc4d7d9fa138e87a5d63809129 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 04/11] 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 41451860bebd37cafd39a70f424d2d96dd7028f4 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 05/11] 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 83cc952537..eaaaa3807c 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 0e399200d2163fa84b40d458539c3cb510fb808f 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 06/11] 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 eaaaa3807c..a2eba93088 100644
--- a/lxd/device/device.go
+++ b/lxd/device/device.go
@@ -131,6 +131,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 7185fb2d087c46fad0e1b49d58dd4f64284039ed 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 07/11] 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 f6f84007bf..2440867c0a 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 c82a228b6d82fa3589ce78e9487e9472b70cc9a5 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 08/11] 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 2440867c0a..6469ee7e26 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 bc90c8baad2611f990f183c8bda1bf4b1143beab 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 09/11] 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 6df108d6b5..b179add7ce 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 451a3be5499eafd259cee4f4ee363b9e421a916a 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 10/11] container/lxc: Removes unused unix-char and unix-block
code
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/container_lxc.go | 258 ++-----------------------------------------
1 file changed, 8 insertions(+), 250 deletions(-)
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index b179add7ce..0e0dc76c6c 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,7 +2491,7 @@ 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":
+ /*case "unix-char", "unix-block":
srcPath, exist := m["source"]
if !exist {
srcPath = m["path"]
@@ -2535,6 +2506,7 @@ func (c *containerLXC) startCommon() (string, []func() error, error) {
} else if srcPath != "" && m["major"] == "" && m["minor"] == "" && !shared.PathExists(srcPath) {
return "", postStartHooks, fmt.Errorf("Missing source '%s' for device '%s'", srcPath, name)
}
+ */
}
}
@@ -2682,7 +2654,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"}) {
+ /*if shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
idmapSet, err := c.CurrentIdmap()
if err != nil {
return "", postStartHooks, err
@@ -2724,7 +2696,8 @@ func (c *containerLXC) startCommon() (string, []func() error, error) {
}
}
}
- } else if m["type"] == "disk" {
+ } else */
+ if m["type"] == "disk" {
if m["path"] != "/" {
diskDevices[k] = m
}
@@ -5049,22 +5022,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
@@ -5074,14 +5032,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
}
}
@@ -6794,73 +6745,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")
@@ -6916,132 +6800,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 a00e1e12dcfb591ebec163535fd3ec7ff3bbc735 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 11/11] devices: Comments out inotify device management for now
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/devices.go | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lxd/devices.go b/lxd/devices.go
index 730764f5a6..7b26602678 100644
--- a/lxd/devices.go
+++ b/lxd/devices.go
@@ -1080,11 +1080,11 @@ func deviceInotifyDirRescan(s *state.State) {
cmp = m["path"]
}
cleanDevPath := filepath.Clean(cmp)
- if shared.PathExists(cleanDevPath) {
+ /*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, cleanDevPath)
@@ -1251,17 +1251,17 @@ func deviceInotifyFileEvent(s *state.State, target *sys.InotifyTargetInfo) {
}
if (target.Mask & unix.IN_CREATE) > 0 {
- err := c.insertUnixDevice(fmt.Sprintf("unix.%s", name), m, false)
+ /*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)
+ /*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()})
}
More information about the lxc-devel
mailing list