The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6054
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) ===
From caa45853f8835cc6b1d15a177c2ff6869665e8bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@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 <stgra...@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?= <stgra...@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 <stgra...@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?= <stgra...@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 <stgra...@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?= <stgra...@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 <stgra...@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?= <stgra...@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 <stgra...@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?= <stgra...@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 <stgra...@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?= <stgra...@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 <stgra...@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 }
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel