[lxc-devel] [lxd/master] Bugfixes

stgraber on Github lxc-bot at linuxcontainers.org
Mon Feb 27 06:12:15 UTC 2017


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/20170227/3f4527bc/attachment.bin>
-------------- next part --------------
From 3e042c9f6131f514a75a4737352d74906cbe7045 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Sun, 26 Feb 2017 23:43:04 -0500
Subject: [PATCH 1/9] Remove wrong error message
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_shared.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lxd/storage_shared.go b/lxd/storage_shared.go
index 799ff79..31ae73a 100644
--- a/lxd/storage_shared.go
+++ b/lxd/storage_shared.go
@@ -78,7 +78,6 @@ func (s *storageShared) setUnprivUserAcl(c container, destPath string) error {
 	acl := fmt.Sprintf("%d:rx", uid)
 	err := exec.Command("setfacl", "-m", acl, destPath).Run()
 	if err == nil {
-		shared.LogDebugf("Failed to set acl permission on container path: %s", err)
 		return nil
 	}
 

From 441e95c496b4feeec9a21de74d2aa4586db15886 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Sun, 26 Feb 2017 23:47:48 -0500
Subject: [PATCH 2/9] storage: Fix logging on image operations
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_btrfs.go | 12 ++++++------
 lxd/storage_lvm.go   | 16 ++++++++--------
 lxd/storage_zfs.go   |  8 ++++----
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/lxd/storage_btrfs.go b/lxd/storage_btrfs.go
index e6a6cd6..101ef9e 100644
--- a/lxd/storage_btrfs.go
+++ b/lxd/storage_btrfs.go
@@ -1163,7 +1163,7 @@ func (s *storageBtrfs) ContainerSnapshotCreateEmpty(snapshotContainer container)
 }
 
 func (s *storageBtrfs) ImageCreate(fingerprint string) error {
-	shared.LogDebugf("Creating BTRFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Creating BTRFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
 	// Create the subvolume.
 	source := s.pool.Config["source"]
@@ -1240,12 +1240,12 @@ func (s *storageBtrfs) ImageCreate(fingerprint string) error {
 
 	undo = false
 
-	shared.LogDebugf("Created BTRFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Created BTRFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 	return nil
 }
 
 func (s *storageBtrfs) ImageDelete(fingerprint string) error {
-	shared.LogDebugf("Deleting BTRFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Deleting BTRFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
 	_, err := s.StoragePoolMount()
 	if err != nil {
@@ -1274,12 +1274,12 @@ func (s *storageBtrfs) ImageDelete(fingerprint string) error {
 		}
 	}
 
-	shared.LogDebugf("Deleted BTRFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Deleted BTRFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 	return nil
 }
 
 func (s *storageBtrfs) ImageMount(fingerprint string) (bool, error) {
-	shared.LogDebugf("Mounting BTRFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Mounting BTRFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
 	// The storage pool must be mounted.
 	_, err := s.StoragePoolMount()
@@ -1287,7 +1287,7 @@ func (s *storageBtrfs) ImageMount(fingerprint string) (bool, error) {
 		return false, err
 	}
 
-	shared.LogDebugf("Mounted BTRFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Mounted BTRFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 	return true, nil
 }
 
diff --git a/lxd/storage_lvm.go b/lxd/storage_lvm.go
index f363536..afac382 100644
--- a/lxd/storage_lvm.go
+++ b/lxd/storage_lvm.go
@@ -1519,7 +1519,7 @@ func (s *storageLvm) ContainerSnapshotCreateEmpty(snapshotContainer container) e
 }
 
 func (s *storageLvm) ImageCreate(fingerprint string) error {
-	shared.LogDebugf("Creating LVM storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Creating LVM storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
 	err := s.StoragePoolCheck()
 	if err != nil {
@@ -1576,12 +1576,12 @@ func (s *storageLvm) ImageCreate(fingerprint string) error {
 
 	tryUndo = false
 
-	shared.LogDebugf("Created LVM storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Created LVM storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 	return nil
 }
 
 func (s *storageLvm) ImageDelete(fingerprint string) error {
-	shared.LogDebugf("Deleting LVM storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Deleting LVM storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
 	err := s.StoragePoolCheck()
 	if err != nil {
@@ -1612,12 +1612,12 @@ func (s *storageLvm) ImageDelete(fingerprint string) error {
 		}
 	}
 
-	shared.LogDebugf("Deleted LVM storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Deleted LVM storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 	return nil
 }
 
 func (s *storageLvm) ImageMount(fingerprint string) (bool, error) {
-	shared.LogDebugf("Mounting LVM storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Mounting LVM storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
 	imageMntPoint := getImageMountPoint(s.pool.Name, fingerprint)
 	if shared.IsMountPoint(imageMntPoint) {
@@ -1639,12 +1639,12 @@ func (s *storageLvm) ImageMount(fingerprint string) (bool, error) {
 		return false, fmt.Errorf("Error mounting image LV: %v", err)
 	}
 
-	shared.LogDebugf("Mounted LVM storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Mounted LVM storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 	return true, nil
 }
 
 func (s *storageLvm) ImageUmount(fingerprint string) (bool, error) {
-	shared.LogDebugf("Unmounting LVM storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Unmounting LVM storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
 	imageMntPoint := getImageMountPoint(s.pool.Name, fingerprint)
 	if !shared.IsMountPoint(imageMntPoint) {
@@ -1656,7 +1656,7 @@ func (s *storageLvm) ImageUmount(fingerprint string) (bool, error) {
 		return false, err
 	}
 
-	shared.LogDebugf("Unmounted LVM storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Unmounted LVM storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 	return true, nil
 }
 
diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index d9b4d11..0573638 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -1230,7 +1230,7 @@ func (s *storageZfs) ContainerSnapshotCreateEmpty(snapshotContainer container) e
 // - remove mountpoint property from zfs volume images/<fingerprint>
 // - create read-write snapshot from zfs volume images/<fingerprint>
 func (s *storageZfs) ImageCreate(fingerprint string) error {
-	shared.LogDebugf("Creating ZFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Creating ZFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
 	imageMntPoint := getImageMountPoint(s.pool.Name, fingerprint)
 	fs := fmt.Sprintf("images/%s", fingerprint)
@@ -1350,12 +1350,12 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 
 	revert = false
 
-	shared.LogDebugf("Created ZFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Created ZFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 	return nil
 }
 
 func (s *storageZfs) ImageDelete(fingerprint string) error {
-	shared.LogDebugf("Deleting ZFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Deleting ZFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
 	fs := fmt.Sprintf("images/%s", fingerprint)
 
@@ -1403,7 +1403,7 @@ func (s *storageZfs) ImageDelete(fingerprint string) error {
 		}
 	}
 
-	shared.LogDebugf("Deleted ZFS storage volume for image \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
+	shared.LogDebugf("Deleted ZFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 	return nil
 }
 

From ef9cf0fe568dda8002acc3c8bc7cada7a64b6833 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 27 Feb 2017 00:12:24 -0500
Subject: [PATCH 3/9] Validate the expanded config at container create
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As only the unexpanded config is validated prior to that.

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 9367d67..40febc8 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -210,6 +210,21 @@ func containerLXCCreate(d *Daemon, args containerArgs) (container, error) {
 		return nil, err
 	}
 
+	// Validate expanded config
+	err = containerValidConfig(d, c.expandedConfig, false, true)
+	if err != nil {
+		c.Delete()
+		shared.LogError("Failed creating container", ctxMap)
+		return nil, err
+	}
+
+	err = containerValidDevices(c.expandedDevices, false, true)
+	if err != nil {
+		c.Delete()
+		shared.LogError("Failed creating container", ctxMap)
+		return nil, err
+	}
+
 	// Retrieve the container's storage pool
 	_, rootDiskDevice, err := containerGetRootDiskDevice(c.expandedDevices)
 	if err != nil {

From 87ec1da613769c30d03e3df3f96eff680b8017c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 27 Feb 2017 00:31:24 -0500
Subject: [PATCH 4/9] Validate container idmap as early as possible
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.go     | 4 ++++
 lxd/container_lxc.go | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index 5e80799..d2eaf73 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -218,6 +218,10 @@ func containerValidConfig(d *Daemon, config map[string]string, profile bool, exp
 		return fmt.Errorf("security.syscalls.whitelist is mutually exclusive with security.syscalls.blacklist*")
 	}
 
+	if expanded && (config["security.privileged"] == "" || !shared.IsTrue(config["security.privileged"])) && d.IdmapSet == nil {
+		return fmt.Errorf("LXD doesn't have a uid/gid allocation. In this mode, only privileged containers are supported.")
+	}
+
 	return nil
 }
 
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 40febc8..253d464 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -694,10 +694,6 @@ func (c *containerLXC) init() error {
 
 	// Setup the Idmap
 	if !c.IsPrivileged() {
-		if c.daemon.IdmapSet == nil {
-			return fmt.Errorf("LXD doesn't have a uid/gid allocation. In this mode, only privileged containers are supported.")
-		}
-
 		c.idmapset, err = c.NextIdmapSet()
 		if err != nil {
 			return err

From 78877d8c639a5b763a4db8a94364bd1df0d1b380 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 27 Feb 2017 00:32:01 -0500
Subject: [PATCH 5/9] storage: Simplify container storage init
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.go          |  2 +-
 lxd/container_lxc.go      | 46 +++++++++++-----------------------------------
 lxd/container_snapshot.go |  7 ++++++-
 3 files changed, 18 insertions(+), 37 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index d2eaf73..6278efe 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -478,7 +478,7 @@ type container interface {
 	LogFilePath() string
 	LogPath() string
 
-	StoragePool() string
+	StoragePool() (string, error)
 
 	// FIXME: Those should be internal functions
 	// Needed for migration for now.
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 253d464..dc7e39e 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -237,10 +237,10 @@ func containerLXCCreate(d *Daemon, args containerArgs) (container, error) {
 		return nil, fmt.Errorf("The container's root device is missing the pool property.")
 	}
 
-	c.storagePool = rootDiskDevice["pool"]
+	storagePool := rootDiskDevice["pool"]
 
 	// Get the storage pool ID for the container
-	poolID, pool, err := dbStoragePoolGet(d.db, c.storagePool)
+	poolID, pool, err := dbStoragePoolGet(d.db, storagePool)
 	if err != nil {
 		c.Delete()
 		return nil, err
@@ -248,7 +248,7 @@ func containerLXCCreate(d *Daemon, args containerArgs) (container, error) {
 
 	// Fill in any default volume config
 	volumeConfig := map[string]string{}
-	err = storageVolumeFillDefault(c.storagePool, volumeConfig, pool)
+	err = storageVolumeFillDefault(storagePool, volumeConfig, pool)
 	if err != nil {
 		return nil, err
 	}
@@ -261,7 +261,7 @@ func containerLXCCreate(d *Daemon, args containerArgs) (container, error) {
 	}
 
 	// Initialize the container storage
-	cStorage, err := storagePoolVolumeContainerCreateInit(d, c.storagePool, args.Name)
+	cStorage, err := storagePoolVolumeContainerCreateInit(d, storagePool, args.Name)
 	if err != nil {
 		c.Delete()
 		shared.LogError("Failed to initialize container storage", ctxMap)
@@ -393,7 +393,6 @@ type containerLXC struct {
 	idmapset *shared.IdmapSet
 
 	// Storage
-	storagePool string
 	storage     storage
 }
 
@@ -700,11 +699,6 @@ func (c *containerLXC) init() error {
 		}
 	}
 
-	// Try to retrieve the container's storage pool from the storage volumes
-	// database but do not fail so that users can recover from storage
-	// breakage.
-	c.initStoragePool()
-
 	return nil
 }
 
@@ -1386,16 +1380,6 @@ func (c *containerLXC) initLXC() error {
 // container's storage volume will already exist in the database and so we can
 // retrieve it.
 func (c *containerLXC) initStoragePool() error {
-	if c.storagePool != "" {
-		return nil
-	}
-
-	poolName, err := dbContainerPool(c.daemon.db, c.Name())
-	if err != nil {
-		return err
-	}
-	c.storagePool = poolName
-
 	return nil
 }
 
@@ -1410,19 +1394,6 @@ func (c *containerLXC) initStorageInterface() error {
 		return err
 	}
 
-	// args.StoragePool can be empty here. In that case we simply set
-	// c.storagePool to the storage pool we detected based on the containers
-	// name. (Container names are globally unique.)
-	_, storagePool := s.GetContainerPoolInfo()
-	if c.storagePool == "" {
-		c.storagePool = storagePool
-	} else if c.storagePool != storagePool {
-		// If the storage pool passed in does not match the storage pool
-		// we reverse engineered based on the containers name we know
-		// something is messed up.
-		return fmt.Errorf("Container is supposed to exist on storage pool \"%s\", but it actually exists on \"%s\".", c.storagePool, storagePool)
-	}
-
 	c.storage = s
 
 	return nil
@@ -6643,6 +6614,11 @@ func (c *containerLXC) StatePath() string {
 	return filepath.Join(c.Path(), "state")
 }
 
-func (c *containerLXC) StoragePool() string {
-	return c.storagePool
+func (c *containerLXC) StoragePool() (string, error) {
+	poolName, err := dbContainerPool(c.daemon.db, c.Name())
+	if err != nil {
+		return "", err
+	}
+
+	return poolName, nil
 }
diff --git a/lxd/container_snapshot.go b/lxd/container_snapshot.go
index ce71b10..51d184e 100644
--- a/lxd/container_snapshot.go
+++ b/lxd/container_snapshot.go
@@ -109,7 +109,12 @@ func containerSnapshotsPost(d *Daemon, r *http.Request) Response {
 
 	// Check whether the container will be mounted exclusively by us or if
 	// it already was by someone else. In the latter case don't unmount it.
-	containerPoolVolumeMntPoint := getContainerMountPoint(c.StoragePool(), name)
+	storagePool, err := c.StoragePool()
+	if err != nil {
+		return SmartError(err)
+	}
+
+	containerPoolVolumeMntPoint := getContainerMountPoint(storagePool, name)
 	mountedBefore := shared.IsMountPoint(containerPoolVolumeMntPoint)
 	err = c.StorageStart()
 	if err != nil {

From e7ff35e99039192ef35ff082a8c56fb28da5538f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 27 Feb 2017 00:33:34 -0500
Subject: [PATCH 6/9] storage: Remove unused function
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 | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index dc7e39e..8e0d26a 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1376,13 +1376,6 @@ func (c *containerLXC) initLXC() error {
 	return nil
 }
 
-// This function is called on all non-create operations where an entry for the
-// container's storage volume will already exist in the database and so we can
-// retrieve it.
-func (c *containerLXC) initStoragePool() error {
-	return nil
-}
-
 // Initialize storage interface for this container.
 func (c *containerLXC) initStorageInterface() error {
 	if c.storage != nil {

From 030aa5224780add6e4b5f0b52cff0960cc7a63a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 27 Feb 2017 00:35:09 -0500
Subject: [PATCH 7/9] storage: Rename initStorageInterface to initStorage
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 | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 8e0d26a..4187bcf 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1376,8 +1376,8 @@ func (c *containerLXC) initLXC() error {
 	return nil
 }
 
-// Initialize storage interface for this container.
-func (c *containerLXC) initStorageInterface() error {
+// Initialize storage interface for this container
+func (c *containerLXC) initStorage() error {
 	if c.storage != nil {
 		return nil
 	}
@@ -2530,7 +2530,7 @@ func (c *containerLXC) Restore(sourceContainer container) error {
 	var ctxMap log.Ctx
 
 	// Initialize storage interface for the container.
-	err := c.initStorageInterface()
+	err := c.initStorage()
 	if err != nil {
 		return err
 	}
@@ -2658,7 +2658,7 @@ func (c *containerLXC) Delete() error {
 	shared.LogInfo("Deleting container", ctxMap)
 
 	// Attempt to initialize storage interface for the container.
-	c.initStorageInterface()
+	c.initStorage()
 
 	if c.IsSnapshot() {
 		// Remove the snapshot
@@ -2738,7 +2738,7 @@ func (c *containerLXC) Rename(newName string) error {
 	shared.LogInfo("Renaming container", ctxMap)
 
 	// Initialize storage interface for the container.
-	err := c.initStorageInterface()
+	err := c.initStorage()
 	if err != nil {
 		return err
 	}
@@ -3153,7 +3153,7 @@ func (c *containerLXC) Update(args containerArgs, userRequested bool) error {
 	}
 
 	// Initialize storage interface for the container.
-	err = c.initStorageInterface()
+	err = c.initStorage()
 	if err != nil {
 		return err
 	}
@@ -4040,7 +4040,7 @@ func (c *containerLXC) Migrate(cmd uint, stateDir string, function string, stop
 	shared.LogInfo("Migrating container", ctxMap)
 
 	// Initialize storage interface for the container.
-	err = c.initStorageInterface()
+	err = c.initStorage()
 	if err != nil {
 		return err
 	}
@@ -4753,7 +4753,7 @@ func (c *containerLXC) diskState() map[string]api.ContainerStateDisk {
 	disk := map[string]api.ContainerStateDisk{}
 
 	// Initialize storage interface for the container.
-	err := c.initStorageInterface()
+	err := c.initStorage()
 	if err != nil {
 		return disk
 	}
@@ -4993,7 +4993,7 @@ func (c *containerLXC) Storage() storage {
 
 func (c *containerLXC) StorageStart() error {
 	// Initialize storage interface for the container.
-	err := c.initStorageInterface()
+	err := c.initStorage()
 	if err != nil {
 		return err
 	}
@@ -5011,7 +5011,7 @@ func (c *containerLXC) StorageStart() error {
 
 func (c *containerLXC) StorageStop() error {
 	// Initialize storage interface for the container.
-	err := c.initStorageInterface()
+	err := c.initStorage()
 	if err != nil {
 		return err
 	}

From 2674001dad57c7c7cb13e5c9f62c63116a578a9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 27 Feb 2017 00:51:33 -0500
Subject: [PATCH 8/9] container: Initialize idmap on demand
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.go      |  2 +-
 lxd/container_exec.go |  8 +++++-
 lxd/container_lxc.go  | 75 +++++++++++++++++++++++++++++++++++++--------------
 lxd/migrate.go        |  6 ++++-
 lxd/storage.go        |  6 ++++-
 lxd/storage_shared.go | 14 +++++++---
 6 files changed, 83 insertions(+), 28 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index 6278efe..96257cc 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -485,7 +485,7 @@ type container interface {
 	StorageStart() error
 	StorageStop() error
 	Storage() storage
-	IdmapSet() *shared.IdmapSet
+	IdmapSet() (*shared.IdmapSet, error)
 	LastIdmapSet() (*shared.IdmapSet, error)
 	TemplateApply(trigger string) error
 	Daemon() *Daemon
diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index 2c86950..4e2ab6d 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -383,10 +383,16 @@ func containerExecPost(d *Daemon, r *http.Request) Response {
 	if post.WaitForWS {
 		ws := &execWs{}
 		ws.fds = map[int]string{}
-		idmapset := c.IdmapSet()
+
+		idmapset, err := c.IdmapSet()
+		if err != nil {
+			return InternalError(err)
+		}
+
 		if idmapset != nil {
 			ws.rootUid, ws.rootGid = idmapset.ShiftIntoNs(0, 0)
 		}
+
 		ws.conns = map[int]*websocket.Conn{}
 		ws.conns[-1] = nil
 		ws.conns[0] = nil
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 4187bcf..d172e8a 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -393,7 +393,7 @@ type containerLXC struct {
 	idmapset *shared.IdmapSet
 
 	// Storage
-	storage     storage
+	storage storage
 }
 
 func (c *containerLXC) createOperation(action string, reusable bool, reuse bool) (*lxcContainerOperation, error) {
@@ -691,14 +691,6 @@ func (c *containerLXC) init() error {
 		return err
 	}
 
-	// Setup the Idmap
-	if !c.IsPrivileged() {
-		c.idmapset, err = c.NextIdmapSet()
-		if err != nil {
-			return err
-		}
-	}
-
 	return nil
 }
 
@@ -965,8 +957,13 @@ func (c *containerLXC) initLXC() error {
 	}
 
 	// Setup idmap
-	if c.idmapset != nil {
-		lines := c.idmapset.ToLxcString()
+	idmapset, err := c.IdmapSet()
+	if err != nil {
+		return err
+	}
+
+	if idmapset != nil {
+		lines := idmapset.ToLxcString()
 		for _, line := range lines {
 			err := lxcSetConfigItem(cc, "lxc.id_map", strings.TrimSuffix(line, "\n"))
 			if err != nil {
@@ -1528,7 +1525,10 @@ func (c *containerLXC) startCommon() (string, error) {
 	}
 
 	/* Deal with idmap changes */
-	idmap := c.IdmapSet()
+	idmap, err := c.IdmapSet()
+	if err != nil {
+		return "", err
+	}
 
 	lastIdmap, err := c.LastIdmapSet()
 	if err != nil {
@@ -4086,12 +4086,17 @@ func (c *containerLXC) Migrate(cmd uint, stateDir string, function string, stop
 		 * namespace.
 		 */
 		if !c.IsPrivileged() {
+			idmapset, err := c.IdmapSet()
+			if err != nil {
+				return err
+			}
+
 			err = c.StorageStart()
 			if err != nil {
 				return err
 			}
 
-			err = c.IdmapSet().ShiftRootfs(stateDir)
+			err = idmapset.ShiftRootfs(stateDir)
 			err2 := c.StorageStop()
 			if err != nil {
 				return err
@@ -4253,7 +4258,12 @@ func (c *containerLXC) templateApplyNow(trigger string) error {
 
 			// Get the right uid and gid for the container
 			if !c.IsPrivileged() {
-				uid, gid = c.idmapset.ShiftIntoNs(0, 0)
+				idmapset, err := c.IdmapSet()
+				if err != nil {
+					return err
+				}
+
+				uid, gid = idmapset.ShiftIntoNs(0, 0)
 			}
 
 			// Create the directories leading to the file
@@ -4935,7 +4945,12 @@ func (c *containerLXC) tarStoreFile(linkmap map[uint64]string, offset int, tw *t
 
 	// Unshift the id under /rootfs/ for unpriv containers
 	if !c.IsPrivileged() && strings.HasPrefix(hdr.Name, "/rootfs") {
-		huid, hgid := c.idmapset.ShiftFromNs(int64(hdr.Uid), int64(hdr.Gid))
+		idmapset, err := c.IdmapSet()
+		if err != nil {
+			return err
+		}
+
+		huid, hgid := idmapset.ShiftFromNs(int64(hdr.Uid), int64(hdr.Gid))
 		hdr.Uid = int(huid)
 		hdr.Gid = int(hgid)
 		if hdr.Uid == -1 || hdr.Gid == -1 {
@@ -5234,8 +5249,13 @@ func (c *containerLXC) createUnixDevice(m types.Device) ([]string, error) {
 			return nil, fmt.Errorf("Failed to chmod device %s: %s", devPath, err)
 		}
 
-		if c.idmapset != nil {
-			if err := c.idmapset.ShiftFile(devPath); err != nil {
+		idmapset, err := c.IdmapSet()
+		if err != nil {
+			return nil, err
+		}
+
+		if idmapset != nil {
+			if err := idmapset.ShiftFile(devPath); err != nil {
 				// uidshift failing is weird, but not a big problem.  Log and proceed
 				shared.LogDebugf("Failed to uidshift device %s: %s\n", m["path"], err)
 			}
@@ -6485,8 +6505,23 @@ func (c *containerLXC) Id() int {
 	return c.id
 }
 
-func (c *containerLXC) IdmapSet() *shared.IdmapSet {
-	return c.idmapset
+func (c *containerLXC) IdmapSet() (*shared.IdmapSet, error) {
+	var err error
+
+	if c.idmapset != nil {
+		return c.idmapset, nil
+	}
+
+	if c.IsPrivileged() {
+		return nil, nil
+	}
+
+	c.idmapset, err = c.NextIdmapSet()
+	if err != nil {
+		return nil, err
+	}
+
+	return c.idmapset, nil
 }
 
 func (c *containerLXC) InitPID() int {
@@ -6511,7 +6546,7 @@ func (c *containerLXC) idmapsetFromConfig(k string) (*shared.IdmapSet, error) {
 	lastJsonIdmap := c.LocalConfig()[k]
 
 	if lastJsonIdmap == "" {
-		return c.IdmapSet(), nil
+		return c.IdmapSet()
 	}
 
 	lastIdmap := new(shared.IdmapSet)
diff --git a/lxd/migrate.go b/lxd/migrate.go
index 4ba9291..15f722c 100644
--- a/lxd/migrate.go
+++ b/lxd/migrate.go
@@ -304,7 +304,11 @@ func (s *migrationSourceWs) Do(migrateOp *operation) error {
 
 	idmaps := make([]*IDMapType, 0)
 
-	idmapset := s.container.IdmapSet()
+	idmapset, err := s.container.IdmapSet()
+	if err != nil {
+		return err
+	}
+
 	if idmapset != nil {
 		for _, ctnIdmap := range idmapset.Idmap {
 			idmap := IDMapType{
diff --git a/lxd/storage.go b/lxd/storage.go
index c85b5b6..828bcaa 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -615,7 +615,11 @@ func deleteSnapshotMountpoint(snapshotMountpoint string, snapshotsSymlinkTarget
 }
 
 func ShiftIfNecessary(container container, srcIdmap *shared.IdmapSet) error {
-	dstIdmap := container.IdmapSet()
+	dstIdmap, err := container.IdmapSet()
+	if err != nil {
+		return err
+	}
+
 	if dstIdmap == nil {
 		dstIdmap = new(shared.IdmapSet)
 	}
diff --git a/lxd/storage_shared.go b/lxd/storage_shared.go
index 31ae73a..c9cc01a 100644
--- a/lxd/storage_shared.go
+++ b/lxd/storage_shared.go
@@ -39,13 +39,16 @@ func (s *storageShared) shiftRootfs(c container) error {
 
 	shared.LogDebugf("Shifting root filesystem \"%s\" for \"%s\".", rpath, c.Name())
 
-	idmapset := c.IdmapSet()
+	idmapset, err := c.IdmapSet()
+	if err != nil {
+		return err
+	}
 
 	if idmapset == nil {
 		return fmt.Errorf("IdmapSet of container '%s' is nil", c.Name())
 	}
 
-	err := idmapset.ShiftRootfs(rpath)
+	err = idmapset.ShiftRootfs(rpath)
 	if err != nil {
 		shared.LogDebugf("Shift of rootfs %s failed: %s", rpath, err)
 		return err
@@ -58,7 +61,10 @@ func (s *storageShared) shiftRootfs(c container) error {
 }
 
 func (s *storageShared) setUnprivUserAcl(c container, destPath string) error {
-	idmapset := c.IdmapSet()
+	idmapset, err := c.IdmapSet()
+	if err != nil {
+		return err
+	}
 
 	// Skip for privileged containers
 	if idmapset == nil {
@@ -76,7 +82,7 @@ func (s *storageShared) setUnprivUserAcl(c container, destPath string) error {
 
 	// Attempt to set a POSIX ACL first.
 	acl := fmt.Sprintf("%d:rx", uid)
-	err := exec.Command("setfacl", "-m", acl, destPath).Run()
+	err = exec.Command("setfacl", "-m", acl, destPath).Run()
 	if err == nil {
 		return nil
 	}

From 806d55c88562ab82b32d2d8563c20a8ec9bdac98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 27 Feb 2017 01:07:54 -0500
Subject: [PATCH 9/9] images: Properly handled non-optimized stores
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/images.go | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lxd/images.go b/lxd/images.go
index d483d2e..7b95ee3 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -908,11 +908,17 @@ func autoUpdateImages(d *Daemon) {
 			continue
 		}
 
+		// If no optimized pools at least update the base store
+		if len(poolNames) == 0 {
+			poolNames = append(poolNames, "")
+		}
+
 		shared.LogDebug("Processing image", log.Ctx{"fp": fp, "server": source.Server, "protocol": source.Protocol, "alias": source.Alias})
 
 		// Update the image on each pool where it currently exists.
+		var hash string
 		for _, poolName := range poolNames {
-			hash, err := d.ImageDownload(nil, source.Server, source.Protocol, "", "", source.Alias, false, true, poolName)
+			hash, err = d.ImageDownload(nil, source.Server, source.Protocol, "", "", source.Alias, false, true, poolName)
 			if hash == fp {
 				shared.LogDebug("Already up to date", log.Ctx{"fp": fp})
 				continue
@@ -945,12 +951,8 @@ func autoUpdateImages(d *Daemon) {
 			}
 		}
 
-		nImg, err := dbImageGetPools(d.db, fp)
-		if err != nil {
-			continue
-		}
-
-		if len(nImg) > 0 {
+		// Image didn't change, move on
+		if hash == fp {
 			continue
 		}
 


More information about the lxc-devel mailing list