The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/3568

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) ===
When a storage volume is attached through a profile we need to make sure that
all attachers specify the same id mapping for their next id map.

Closes #3548.

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From 2e0fe3e3bb800a21c293997f09f4efb659a78b62 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Tue, 18 Jul 2017 20:15:53 +0200
Subject: [PATCH 1/3] storage: storagePoolVolumeUsedByContainersGet()

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
 lxd/storage_volumes_utils.go | 58 +++++++++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index 64a81a187..7fe33b574 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -179,30 +179,14 @@ func storagePoolVolumeUpdate(d *Daemon, poolName string, 
volumeName string, volu
        return nil
 }
 
-func storagePoolVolumeUsedByGet(d *Daemon, volumeName string, volumeTypeName 
string) ([]string, error) {
-       // Handle container volumes
-       if volumeTypeName == "container" {
-               cName, sName, snap := 
containerGetParentAndSnapshotName(volumeName)
-
-               if snap {
-                       return 
[]string{fmt.Sprintf("/%s/containers/%s/snapshots/%s", version.APIVersion, 
cName, sName)}, nil
-               }
-
-               return []string{fmt.Sprintf("/%s/containers/%s", 
version.APIVersion, cName)}, nil
-       }
-
-       // Handle image volumes
-       if volumeTypeName == "image" {
-               return []string{fmt.Sprintf("/%s/images/%s", 
version.APIVersion, volumeName)}, nil
-       }
-
-       // Look for containers using the interface
+func storagePoolVolumeUsedByContainersGet(d *Daemon, volumeName string,
+       volumeTypeName string) ([]string, error) {
        cts, err := dbContainersList(d.db, cTypeRegular)
        if err != nil {
                return []string{}, err
        }
 
-       volumeUsedBy := []string{}
+       ctsUsingVolume := []string{}
        volumeNameWithType := fmt.Sprintf("%s/%s", volumeTypeName, volumeName)
        for _, ct := range cts {
                c, err := containerLoadByName(d, ct)
@@ -219,11 +203,45 @@ func storagePoolVolumeUsedByGet(d *Daemon, volumeName 
string, volumeTypeName str
                        // "container////bla" but only against "container/bla".
                        cleanSource := filepath.Clean(d["source"])
                        if cleanSource == volumeName || cleanSource == 
volumeNameWithType {
-                               volumeUsedBy = append(volumeUsedBy, 
fmt.Sprintf("/%s/containers/%s", version.APIVersion, ct))
+                               ctsUsingVolume = append(ctsUsingVolume, ct)
                        }
                }
        }
 
+       return ctsUsingVolume, nil
+}
+
+// volumeUsedBy = append(volumeUsedBy, fmt.Sprintf("/%s/containers/%s", 
version.APIVersion, ct))
+func storagePoolVolumeUsedByGet(d *Daemon, volumeName string, volumeTypeName 
string) ([]string, error) {
+       // Handle container volumes
+       if volumeTypeName == "container" {
+               cName, sName, snap := 
containerGetParentAndSnapshotName(volumeName)
+
+               if snap {
+                       return 
[]string{fmt.Sprintf("/%s/containers/%s/snapshots/%s", version.APIVersion, 
cName, sName)}, nil
+               }
+
+               return []string{fmt.Sprintf("/%s/containers/%s", 
version.APIVersion, cName)}, nil
+       }
+
+       // Handle image volumes
+       if volumeTypeName == "image" {
+               return []string{fmt.Sprintf("/%s/images/%s", 
version.APIVersion, volumeName)}, nil
+       }
+
+       // Look for containers using this volume
+       ctsUsingVolume, err := storagePoolVolumeUsedByContainersGet(d,
+               volumeName, volumeTypeName)
+       if err != nil {
+               return []string{}, err
+       }
+
+       volumeUsedBy := []string{}
+       for _, ct := range ctsUsingVolume {
+               volumeUsedBy = append(volumeUsedBy,
+                       fmt.Sprintf("/%s/containers/%s", version.APIVersion, 
ct))
+       }
+
        profiles, err := profilesUsingPoolVolumeGetNames(d.db, volumeName, 
volumeTypeName)
        if err != nil {
                return []string{}, err

From b3ac056c1ee24019b07787dbb255fc789a2cf72f Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Tue, 18 Jul 2017 20:16:24 +0200
Subject: [PATCH 2/3] storage: check idmaps of all attaching containers

When a storage volume is attached through a profile we need to make sure that
all attachers specify the same id mapping for their next id map.

Closes #3548.

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
 lxd/storage.go | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/lxd/storage.go b/lxd/storage.go
index de3578f3b..706b0077e 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -434,13 +434,32 @@ func storagePoolVolumeAttachInit(d *Daemon, poolName 
string, volumeName string,
 
        if !reflect.DeepEqual(nextIdmap, lastIdmap) {
                logger.Debugf("Shifting storage volume")
-               volumeUsedBy, err := storagePoolVolumeUsedByGet(d, volumeName, 
volumeTypeName)
+               volumeUsedBy, err := storagePoolVolumeUsedByContainersGet(d,
+                       volumeName, volumeTypeName)
                if err != nil {
                        return nil, err
                }
 
                if len(volumeUsedBy) > 1 {
-                       return nil, fmt.Errorf("idmaps of container and storage 
volume are not identical")
+                       for _, ct := range volumeUsedBy {
+                               c, err := containerLoadByName(d, ct)
+                               if err != nil {
+                                       continue
+                               }
+
+                               if c.IsRunning() {
+                                       return nil, fmt.Errorf("idmaps of 
container and storage volume are not identical")
+                               }
+
+                               ctNextIdmap, err := c.IdmapSet()
+                               if err != nil {
+                                       return nil, fmt.Errorf("idmaps of 
container and storage volume are not identical")
+                               }
+
+                               if !reflect.DeepEqual(nextIdmap, ctNextIdmap) {
+                                       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.

From aa04ee0be96b2bc463ae7764d734ffad79d6b8bf Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Tue, 18 Jul 2017 20:24:58 +0200
Subject: [PATCH 3/3] [DO NOT MERGE]

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
 test/main.sh | 76 ++++++++++++++++++++++++++++++------------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/test/main.sh b/test/main.sh
index 9fc18d525..dfe16cb42 100755
--- a/test/main.sh
+++ b/test/main.sh
@@ -596,44 +596,44 @@ if [ "$#" -gt 0 ]; then
   exit
 fi
 
-run_test test_check_deps "checking dependencies"
-run_test test_static_analysis "static analysis"
-run_test test_database_update "database schema updates"
-run_test test_remote_url "remote url handling"
-run_test test_remote_admin "remote administration"
-run_test test_remote_usage "remote usage"
-run_test test_basic_usage "basic usage"
-run_test test_security "security features"
-run_test test_image_expiry "image expiry"
-run_test test_image_list_all_aliases "image list all aliases"
-run_test test_image_auto_update "image auto-update"
-run_test test_image_prefer_cached "image prefer cached"
-run_test test_image_import_dir "import image from directory"
-run_test test_concurrent_exec "concurrent exec"
-run_test test_concurrent "concurrent startup"
-run_test test_snapshots "container snapshots"
-run_test test_snap_restore "snapshot restores"
-run_test test_config_profiles "profiles and configuration"
-run_test test_config_edit "container configuration edit"
-run_test test_config_edit_container_snapshot_pool_config "container and 
snapshot volume configuration edit"
-run_test test_server_config "server configuration"
-run_test test_filemanip "file manipulations"
-run_test test_network "network management"
-run_test test_idmap "id mapping"
-run_test test_template "file templating"
-run_test test_pki "PKI mode"
-run_test test_devlxd "/dev/lxd"
-run_test test_fuidshift "fuidshift"
-run_test test_migration "migration"
-run_test test_fdleak "fd leak"
-run_test test_cpu_profiling "CPU profiling"
-run_test test_mem_profiling "memory profiling"
-run_test test_storage "storage"
-run_test test_init_auto "lxd init auto"
-run_test test_init_interactive "lxd init interactive"
-run_test test_init_preseed "lxd init preseed"
-run_test test_storage_profiles "storage profiles"
-run_test test_container_import "container import"
+# run_test test_check_deps "checking dependencies"
+# run_test test_static_analysis "static analysis"
+# run_test test_database_update "database schema updates"
+# run_test test_remote_url "remote url handling"
+# run_test test_remote_admin "remote administration"
+# run_test test_remote_usage "remote usage"
+# run_test test_basic_usage "basic usage"
+# run_test test_security "security features"
+# run_test test_image_expiry "image expiry"
+# run_test test_image_list_all_aliases "image list all aliases"
+# run_test test_image_auto_update "image auto-update"
+# run_test test_image_prefer_cached "image prefer cached"
+# run_test test_image_import_dir "import image from directory"
+# run_test test_concurrent_exec "concurrent exec"
+# run_test test_concurrent "concurrent startup"
+# run_test test_snapshots "container snapshots"
+# run_test test_snap_restore "snapshot restores"
+# run_test test_config_profiles "profiles and configuration"
+# run_test test_config_edit "container configuration edit"
+# run_test test_config_edit_container_snapshot_pool_config "container and 
snapshot volume configuration edit"
+# run_test test_server_config "server configuration"
+# run_test test_filemanip "file manipulations"
+# run_test test_network "network management"
+# run_test test_idmap "id mapping"
+# run_test test_template "file templating"
+# run_test test_pki "PKI mode"
+# run_test test_devlxd "/dev/lxd"
+# run_test test_fuidshift "fuidshift"
+# run_test test_migration "migration"
+# run_test test_fdleak "fd leak"
+# run_test test_cpu_profiling "CPU profiling"
+# run_test test_mem_profiling "memory profiling"
+# run_test test_storage "storage"
+# run_test test_init_auto "lxd init auto"
+# run_test test_init_interactive "lxd init interactive"
+# run_test test_init_preseed "lxd init preseed"
+# run_test test_storage_profiles "storage profiles"
+# run_test test_container_import "container import"
 run_test test_storage_volume_attach "attaching storage volumes"
 
 TEST_RESULT=success
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to