[lxc-devel] [lxd/master] Add shifting of custom storage volume

stgraber on Github lxc-bot at linuxcontainers.org
Fri Aug 9 03:04:09 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190808/6546ec9a/attachment-0001.bin>
-------------- next part --------------
From caa45853f8835cc6b1d15a177c2ff6869665e8bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Sun, 28 Jul 2019 00:42:43 -0400
Subject: [PATCH 1/7] doc/storage: Make descriptions consistent
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 doc/storage.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/storage.md b/doc/storage.md
index eb9df2a20f..10e028d21a 100644
--- a/doc/storage.md
+++ b/doc/storage.md
@@ -47,7 +47,7 @@ block.filesystem        | string    | block based driver (lvm)  | same as volume
 block.mount\_options    | string    | block based driver (lvm)  | same as volume.block.mount\_options   | storage           | Mount options for block devices
 security.unmapped       | bool      | custom volume             | false                                 | storage\_unmapped | Disable id mapping for the volume
 zfs.remove\_snapshots   | string    | zfs driver                | same as volume.zfs.remove\_snapshots  | storage           | Remove snapshots as needed
-zfs.use\_refquota       | string    | zfs driver                | same as volume.zfs.zfs\_requota       | storage           | Use refquota instead of quota for space.
+zfs.use\_refquota       | string    | zfs driver                | same as volume.zfs.zfs\_requota       | storage           | Use refquota instead of quota for space
 
 Storage volume configuration keys can be set using the lxc tool with:
 

From 97dba695783b5d59b045aae78b45710854625703 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Sun, 28 Jul 2019 00:52:58 -0400
Subject: [PATCH 2/7] api: Add storage_shifted extension
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 doc/api-extensions.md | 9 +++++++++
 shared/version/api.go | 1 +
 2 files changed, 10 insertions(+)

diff --git a/doc/api-extensions.md b/doc/api-extensions.md
index 3202ad3e1b..ab23f72efb 100644
--- a/doc/api-extensions.md
+++ b/doc/api-extensions.md
@@ -806,3 +806,12 @@ elevated permissions.
 
 ## container\_disk\_shift
 Adds the `shift` property on `disk` devices which controls the use of the shiftfs overlay.
+
+## storage\_shifted
+Introduces a new `security.shifted` boolean on storage volumes.
+
+Setting it to true will allow multiple isolated containers to attach the
+same storage volume while keeping the filesystem writable from all of
+them.
+
+This makes use of shiftfs as an overlay filesystem.
diff --git a/shared/version/api.go b/shared/version/api.go
index fdf8a5921f..e088dff56b 100644
--- a/shared/version/api.go
+++ b/shared/version/api.go
@@ -161,6 +161,7 @@ var APIExtensions = []string{
 	"container_exec_user_group_cwd",
 	"container_syscall_intercept",
 	"container_disk_shift",
+	"storage_shifted",
 }
 
 // APIExtensionsCount returns the number of available API extensions.

From c3b4d7c87f6cc03e81bceb22f8c586278f06ca5e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Sun, 28 Jul 2019 00:58:03 -0400
Subject: [PATCH 3/7] bash: Update bash completion for shifted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 scripts/bash/lxd-client | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/bash/lxd-client b/scripts/bash/lxd-client
index c412418a5d..b8613c4e0c 100644
--- a/scripts/bash/lxd-client
+++ b/scripts/bash/lxd-client
@@ -127,7 +127,7 @@ _have lxc && {
       volume.zfs.use_refquota zfs.clone_copy zfs.pool_name"
 
     storage_volume_keys="size block.filesystem block.mount_options \
-      security.unmapped zfs.remove_snapshots zfs.use_refquota"
+      security.unmapped security.shifted zfs.remove_snapshots zfs.use_refquota"
 
     if [ $COMP_CWORD -eq 1 ]; then
       COMPREPLY=( $(compgen -W "$lxc_cmds" -- ${COMP_WORDS[COMP_CWORD]}) )

From 008d1912eb394acbf881b73aeca05d868958c5cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Sun, 28 Jul 2019 01:13:20 -0400
Subject: [PATCH 4/7] doc/storage: Add security.shifted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 doc/storage.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/storage.md b/doc/storage.md
index 10e028d21a..cb533a7a5a 100644
--- a/doc/storage.md
+++ b/doc/storage.md
@@ -45,6 +45,7 @@ Key                     | Type      | Condition                 | Default
 size                    | string    | appropriate driver        | same as volume.size                   | storage           | Size of the storage volume
 block.filesystem        | string    | block based driver (lvm)  | same as volume.block.filesystem       | storage           | Filesystem of the storage volume
 block.mount\_options    | string    | block based driver (lvm)  | same as volume.block.mount\_options   | storage           | Mount options for block devices
+security.shifted        | bool      | custom volume             | false                                 | storage\_shifted  | Enable id shifting overlay (allows attach by multiple isolated containers)
 security.unmapped       | bool      | custom volume             | false                                 | storage\_unmapped | Disable id mapping for the volume
 zfs.remove\_snapshots   | string    | zfs driver                | same as volume.zfs.remove\_snapshots  | storage           | Remove snapshots as needed
 zfs.use\_refquota       | string    | zfs driver                | same as volume.zfs.zfs\_requota       | storage           | Use refquota instead of quota for space

From 58e9d06841ca2fce08b5927c094bd2c03a906a19 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Sun, 28 Jul 2019 01:13:35 -0400
Subject: [PATCH 5/7] lxd/storage: Add support for security.shifted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/storage.go                | 87 ++++++++++++++++++-----------------
 lxd/storage_volumes_config.go |  9 ++++
 lxd/storage_volumes_utils.go  | 17 +++++++
 3 files changed, 72 insertions(+), 41 deletions(-)

diff --git a/lxd/storage.go b/lxd/storage.go
index eb28c0bb37..6786c190db 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -447,23 +447,25 @@ func storagePoolVolumeAttachInit(s *state.State, poolName string, volumeName str
 		}
 	}
 
-	// Get the container's idmap
 	var nextIdmap *idmap.IdmapSet
-	if c.IsRunning() {
-		nextIdmap, err = c.CurrentIdmap()
-	} else {
-		nextIdmap, err = c.NextIdmap()
-	}
-	if err != nil {
-		return nil, err
-	}
-
 	nextJsonMap := "[]"
-	if nextIdmap != nil {
-		nextJsonMap, err = idmapsetToJSON(nextIdmap)
+	if !shared.IsTrue(poolVolumePut.Config["security.shifted"]) {
+		// Get the container's idmap
+		if c.IsRunning() {
+			nextIdmap, err = c.CurrentIdmap()
+		} else {
+			nextIdmap, err = c.NextIdmap()
+		}
 		if err != nil {
 			return nil, err
 		}
+
+		if nextIdmap != nil {
+			nextJsonMap, err = idmapsetToJSON(nextIdmap)
+			if err != nil {
+				return nil, err
+			}
+		}
 	}
 	poolVolumePut.Config["volatile.idmap.next"] = nextJsonMap
 
@@ -478,40 +480,43 @@ func storagePoolVolumeAttachInit(s *state.State, poolName string, volumeName str
 
 	if !nextIdmap.Equals(lastIdmap) {
 		logger.Debugf("Shifting storage volume")
-		volumeUsedBy, err := storagePoolVolumeUsedByContainersGet(s,
-			"default", volumeName, volumeTypeName)
-		if err != nil {
-			return nil, err
-		}
 
-		if len(volumeUsedBy) > 1 {
-			for _, ctName := range volumeUsedBy {
-				ct, err := containerLoadByProjectAndName(s, c.Project(), ctName)
-				if err != nil {
-					continue
-				}
+		if !shared.IsTrue(poolVolumePut.Config["security.shifted"]) {
+			volumeUsedBy, err := storagePoolVolumeUsedByContainersGet(s,
+				"default", volumeName, volumeTypeName)
+			if err != nil {
+				return nil, err
+			}
 
-				var ctNextIdmap *idmap.IdmapSet
-				if ct.IsRunning() {
-					ctNextIdmap, err = ct.CurrentIdmap()
-				} else {
-					ctNextIdmap, err = ct.NextIdmap()
-				}
-				if err != nil {
-					return nil, fmt.Errorf("Failed to retrieve idmap of container")
+			if len(volumeUsedBy) > 1 {
+				for _, ctName := range volumeUsedBy {
+					ct, err := containerLoadByProjectAndName(s, c.Project(), ctName)
+					if err != nil {
+						continue
+					}
+
+					var ctNextIdmap *idmap.IdmapSet
+					if ct.IsRunning() {
+						ctNextIdmap, err = ct.CurrentIdmap()
+					} else {
+						ctNextIdmap, err = ct.NextIdmap()
+					}
+					if err != nil {
+						return nil, fmt.Errorf("Failed to retrieve idmap of container")
+					}
+
+					if !nextIdmap.Equals(ctNextIdmap) {
+						return nil, fmt.Errorf("Idmaps of container %v and storage volume %v are not identical", ctName, volumeName)
+					}
 				}
-
-				if !nextIdmap.Equals(ctNextIdmap) {
-					return nil, fmt.Errorf("Idmaps of container %v and storage volume %v are not identical", ctName, volumeName)
+			} else if len(volumeUsedBy) == 1 {
+				// If we're the only one who's attached that container
+				// we can shift the storage volume.
+				// I'm not sure if we want some locking here.
+				if volumeUsedBy[0] != c.Name() {
+					return nil, fmt.Errorf("idmaps of container and storage volume are not identical")
 				}
 			}
-		} else if len(volumeUsedBy) == 1 {
-			// If we're the only one who's attached that container
-			// we can shift the storage volume.
-			// I'm not sure if we want some locking here.
-			if volumeUsedBy[0] != c.Name() {
-				return nil, fmt.Errorf("idmaps of container and storage volume are not identical")
-			}
 		}
 
 		// mount storage volume
diff --git a/lxd/storage_volumes_config.go b/lxd/storage_volumes_config.go
index 51e497d154..76dca2b558 100644
--- a/lxd/storage_volumes_config.go
+++ b/lxd/storage_volumes_config.go
@@ -51,32 +51,38 @@ func updateStoragePoolVolumeError(unchangeable []string, driverName string) erro
 // property.
 var changeableStoragePoolVolumeProperties = map[string][]string{
 	"btrfs": {
+		"security.shifted",
 		"security.unmapped",
 		"size",
 	},
 
 	"ceph": {
+		"security.shifted",
 		"block.mount_options",
 		"security.unmapped",
 		"size"},
 
 	"cephfs": {
+		"security.shifted",
 		"security.unmapped",
 		"size",
 	},
 
 	"dir": {
+		"security.shifted",
 		"security.unmapped",
 		"size",
 	},
 
 	"lvm": {
 		"block.mount_options",
+		"security.shifted",
 		"security.unmapped",
 		"size",
 	},
 
 	"zfs": {
+		"security.shifted",
 		"security.unmapped",
 		"size",
 		"zfs.remove_snapshots",
@@ -97,6 +103,9 @@ var storageVolumeConfigKeys = map[string]func(value string) ([]string, error){
 	"block.mount_options": func(value string) ([]string, error) {
 		return []string{"ceph", "lvm"}, shared.IsAny(value)
 	},
+	"security.shifted": func(value string) ([]string, error) {
+		return supportedPoolTypes, shared.IsBool(value)
+	},
 	"security.unmapped": func(value string) ([]string, error) {
 		return supportedPoolTypes, shared.IsBool(value)
 	},
diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index d314a29efb..ca695152c7 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -176,6 +176,23 @@ func storagePoolVolumeUpdate(state *state.State, poolName string, volumeName str
 		s.SetStoragePoolVolumeWritable(&newWritable)
 	}
 
+	// Check that security.unmapped and security.shifted aren't set together
+	if shared.IsTrue(newConfig["security.unmapped"]) && shared.IsTrue(newConfig["security.shifted"]) {
+		return fmt.Errorf("security.unmapped and security.shifted are mutually exclusive")
+	}
+
+	// Confirm that no containers are running when changing shifted state
+	if newConfig["security.shifted"] != oldConfig["security.shifted"] {
+		ctsUsingVolume, err := storagePoolVolumeUsedByRunningContainersWithProfilesGet(state, poolName, volumeName, storagePoolVolumeTypeNameCustom, true)
+		if err != nil {
+			return err
+		}
+
+		if len(ctsUsingVolume) != 0 {
+			return fmt.Errorf("Cannot modify shifting with running containers using the volume")
+		}
+	}
+
 	// Unset idmap keys if volume is unmapped
 	if shared.IsTrue(newConfig["security.unmapped"]) {
 		delete(newConfig, "volatile.idmap.last")

From 48ebf103943541c4da72bfb0a8ee1add773d74c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 8 Aug 2019 22:48:20 -0400
Subject: [PATCH 6/7] lxd/containers: Add support for shifted custom volumes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/container_lxc.go | 53 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 14948f7244..d59356fc5c 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1725,7 +1725,31 @@ func (c *containerLXC) initLXC(config bool) error {
 					}
 				}
 			} else {
-				if shared.IsTrue(m["shift"]) {
+				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
@@ -7532,9 +7556,34 @@ func (c *containerLXC) insertDiskDevice(name string, m config.Device) error {
 		flags |= unix.MS_REC
 	}
 
+	// Detect shifting
+	shift := false
+	if !c.isCurrentlyPrivileged() {
+		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 && !c.state.OS.Shiftfs {
+		return fmt.Errorf("shiftfs is required by disk entry '%s' but isn't supported on system", name)
+	}
+
 	// Bind-mount it into the container
 	destPath := strings.TrimSuffix(m["path"], "/")
-	err = c.insertMount(devPath, destPath, "none", flags, shared.IsTrue(m["shift"]))
+	err = c.insertMount(devPath, destPath, "none", flags, shift)
 	if err != nil {
 		return fmt.Errorf("Failed to add mount for device: %s", err)
 	}

From a1aa9d2cb530d55868aa43de9a75af5dff09a588 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 8 Aug 2019 23:02:40 -0400
Subject: [PATCH 7/7] tests: Add test for security.shifted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 test/suites/container_devices_disk.sh | 29 +++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/test/suites/container_devices_disk.sh b/test/suites/container_devices_disk.sh
index e0368b25eb..91ef518b0f 100644
--- a/test/suites/container_devices_disk.sh
+++ b/test/suites/container_devices_disk.sh
@@ -14,6 +14,7 @@ test_container_devices_disk_shift() {
     return
   fi
 
+  # Test basic shiftfs
   mkdir -p "${TEST_DIR}/shift-source"
   touch "${TEST_DIR}/shift-source/a"
   chown 123:456 "${TEST_DIR}/shift-source/a"
@@ -28,5 +29,33 @@ test_container_devices_disk_shift() {
   lxc stop foo -f
   lxc start foo
   [ "$(lxc exec foo -- stat /mnt/a -c '%u:%g')" = "123:456" ] || false
+  lxc config device remove foo shiftfs
+  lxc stop foo -f
+
+  # Test shifted custom volumes
+  POOL=$(lxc profile device get default root pool)
+  lxc storage volume create "${POOL}" foo-shift security.shifted=true
+
+  lxc start foo
+  lxc launch testimage foo-priv -c security.privileged=true
+  lxc launch testimage foo-isol1 -c security.idmap.isolated=true
+  lxc launch testimage foo-isol2 -c security.idmap.isolated=true
+
+  lxc config device add foo shifted disk pool="${POOL}" source=foo-shift path=/mnt
+  lxc config device add foo-priv shifted disk pool="${POOL}" source=foo-shift path=/mnt
+  lxc config device add foo-isol1 shifted disk pool="${POOL}" source=foo-shift path=/mnt
+  lxc config device add foo-isol2 shifted disk pool="${POOL}" source=foo-shift path=/mnt
+
+  lxc exec foo -- touch /mnt/a
+  lxc exec foo -- chown 123:456 /mnt/a
+
+  [ "$(lxc exec foo -- stat /mnt/a -c '%u:%g')" = "123:456" ] || false
+  [ "$(lxc exec foo-priv -- stat /mnt/a -c '%u:%g')" = "123:456" ] || false
+  [ "$(lxc exec foo-isol1 -- stat /mnt/a -c '%u:%g')" = "123:456" ] || false
+  [ "$(lxc exec foo-isol2 -- stat /mnt/a -c '%u:%g')" = "123:456" ] || false
+
+  lxc delete -f foo-priv foo-isol1 foo-isol2
+  lxc config device remove foo shifted
+  lxc storage volume delete "${POOL}" foo-shift
   lxc stop foo -f
 }


More information about the lxc-devel mailing list