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

Reply via email to