[lxc-devel] [lxd/master] do mounts in the right order

[tych0 on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 609 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20161214/3dc2a26f/attachment.bin>
-------------- next part --------------
From 7507fa1c2fe65ef50f487d26877ec5f5a1515b5d Mon Sep 17 00:00:00 2001
From: Tycho Andersen <tycho.andersen at canonical.com>
Date: Wed, 14 Dec 2016 17:16:38 +0000
Subject: [PATCH] do mounts in the right order

In particular, if there are two paths to be mounted:

/mnt
/mnt/foo

We should always mount /mnt before we mount /mnt/foo, otherwise, the /mnt
mount will overmount /mnt/foo, which is almost certainly not what people
want.

Closes #2717

Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
---
 lxd/container_lxc.go  | 59 +++++++++++++++++++++++++++++++++++++++++++--------
 test/suites/config.sh | 20 +++++++++++++++++
 2 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 5561571..cbbe8c2 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1577,6 +1577,7 @@ func (c *containerLXC) startCommon() (string, error) {
 	var usbs []usbDevice
 	var gpus []gpuDevice
 	var nvidiaDevices []nvidiaGpuDevices
+	diskDevices := map[string]shared.Device{}
 
 	// Create the devices
 	for _, k := range c.expandedDevices.DeviceNames() {
@@ -1662,12 +1663,8 @@ func (c *containerLXC) startCommon() (string, error) {
 				}
 			}
 		} else if m["type"] == "disk" {
-			// Disk device
 			if m["path"] != "/" {
-				_, err := c.createDiskDevice(k, m)
-				if err != nil {
-					return "", err
-				}
+				diskDevices[k] = m
 			}
 		} else if m["type"] == "nic" {
 			if m["nictype"] == "bridged" && shared.IsTrue(m["security.mac_filtering"]) {
@@ -1710,6 +1707,14 @@ func (c *containerLXC) startCommon() (string, error) {
 		}
 	}
 
+	err = c.addDiskDevices(diskDevices, func(name string, d shared.Device) error {
+		_, err := c.createDiskDevice(name, d)
+		return err
+	})
+	if err != nil {
+		return "", err
+	}
+
 	// Create any missing directory
 	err = os.MkdirAll(c.LogPath(), 0700)
 	if err != nil {
@@ -3412,6 +3417,8 @@ func (c *containerLXC) Update(args containerArgs, userRequested bool) error {
 			}
 		}
 
+		diskDevices := map[string]shared.Device{}
+
 		for k, m := range addDevices {
 			if shared.StringInSlice(m["type"], []string{"unix-char", "unix-block"}) {
 				err = c.insertUnixDevice(m)
@@ -3419,10 +3426,7 @@ func (c *containerLXC) Update(args containerArgs, userRequested bool) error {
 					return err
 				}
 			} else if m["type"] == "disk" && m["path"] != "/" {
-				err = c.insertDiskDevice(k, m)
-				if err != nil {
-					return err
-				}
+				diskDevices[k] = m
 			} else if m["type"] == "nic" {
 				err = c.insertNetworkDevice(k, m)
 				if err != nil {
@@ -3497,6 +3501,11 @@ func (c *containerLXC) Update(args containerArgs, userRequested bool) error {
 			}
 		}
 
+		err = c.addDiskDevices(diskDevices, c.insertDiskDevice)
+		if err != nil {
+			return err
+		}
+
 		updateDiskLimit := false
 		for k, m := range updateDevices {
 			if m["type"] == "disk" {
@@ -5707,6 +5716,38 @@ func (c *containerLXC) insertDiskDevice(name string, m shared.Device) error {
 	return nil
 }
 
+type byPath []shared.Device
+
+func (a byPath) Len() int {
+	return len(a)
+}
+
+func (a byPath) Swap(i, j int) {
+	a[i], a[j] = a[j], a[i]
+}
+
+func (a byPath) Less(i, j int) bool {
+	return a[i]["path"] < a[j]["path"]
+}
+
+func (c *containerLXC) addDiskDevices(devices map[string]shared.Device, handler func(string, shared.Device)error) error {
+	ordered := byPath{}
+
+	for _, d := range devices {
+		ordered = append(ordered, d)
+	}
+
+	sort.Sort(ordered)
+	for _, d := range ordered {
+		err := handler(d["path"], d)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (c *containerLXC) removeDiskDevice(name string, m shared.Device) error {
 	// Check that the container is running
 	pid := c.InitPID()
diff --git a/test/suites/config.sh b/test/suites/config.sh
index 74c6d78..3bfbaea 100644
--- a/test/suites/config.sh
+++ b/test/suites/config.sh
@@ -79,6 +79,24 @@ testloopmounts() {
   sed -i "\|^${lpath}|d" "${TEST_DIR}/loops"
 }
 
+test_mount_order() {
+  mkdir -p "${TEST_DIR}/order/empty"
+  mkdir -p "${TEST_DIR}/order/full"
+  touch "${TEST_DIR}/order/full/filler"
+
+  # The idea here is that sometimes (depending on how golang randomizes the
+  # config) the empty dir will have the contents of full in it, but sometimes
+  # it won't depending on whether the devices below are processed in order or
+  # not. This should not be racy, and they should *always* be processed in path
+  # order, so the filler file should always be there.
+  lxc config device add foo order disk source="${TEST_DIR}/order" path=/mnt
+  lxc config device add foo orderFull disk source="${TEST_DIR}/order/full" path=/mnt/empty
+
+  lxc start foo
+  lxc exec foo -- cat /mnt/empty/filler
+  lxc stop foo --force
+}
+
 test_config_profiles() {
   ensure_import_testimage
 
@@ -185,6 +203,8 @@ test_config_profiles() {
 
   testloopmounts
 
+  test_mount_order
+
   lxc delete foo
 
   lxc init testimage foo