[lxc-devel] [lxd/master] Fix storage quota re-application issue

[stgraber on Github]

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/20180823/81de5ee2/attachment.bin>
-------------- next part --------------
From 2e2414d83eb3af2e555362d9c48a5431739bb0f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 23 Aug 2018 17:06:51 -0400
Subject: [PATCH 1/2] lxd/containers: Also use apply_quota for CEPH
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #4960

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 0beed0e234..9280c3cec2 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -4088,7 +4088,7 @@ func (c *containerLXC) Update(args db.ContainerArgs, userRequested bool) error {
 	if newRootDiskDeviceSize != oldRootDiskDeviceSize {
 		storageTypeName := c.storage.GetStorageTypeName()
 		storageIsReady := c.storage.ContainerStorageReady(c.Name())
-		if storageTypeName == "lvm" && isRunning || !storageIsReady {
+		if (storageTypeName == "lvm" || storageTypeName == "ceph") && isRunning || !storageIsReady {
 			c.localConfig["volatile.apply_quota"] = newRootDiskDeviceSize
 		} else {
 			size, err := shared.ParseByteSizeString(newRootDiskDeviceSize)

From a2d0e291076c39cba55a40e2318cb6bf614bbec9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 23 Aug 2018 17:22:55 -0400
Subject: [PATCH 2/2] lxd/containers: Simplify and fix pool update logic
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #4960

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 9280c3cec2..02deecd97c 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -4021,65 +4021,32 @@ func (c *containerLXC) Update(args db.ContainerArgs, userRequested bool) error {
 		c.idmapset = nil
 	}
 
-	// Retrieve old root disk devices.
-	oldLocalRootDiskDeviceKey, oldLocalRootDiskDevice, _ := shared.GetRootDiskDevice(oldLocalDevices)
-	var oldProfileRootDiskDevices []string
-	for k, v := range oldExpandedDevices {
-		if shared.IsRootDiskDevice(v) && k != oldLocalRootDiskDeviceKey && !shared.StringInSlice(k, oldProfileRootDiskDevices) {
-			oldProfileRootDiskDevices = append(oldProfileRootDiskDevices, k)
-		}
-	}
-
-	// Retrieve new root disk devices.
-	newLocalRootDiskDeviceKey, newLocalRootDiskDevice, _ := shared.GetRootDiskDevice(c.localDevices)
-	var newProfileRootDiskDevices []string
+	// Make sure we have a valid root disk device (and only one)
+	newRootDiskDeviceKey := ""
 	for k, v := range c.expandedDevices {
-		if shared.IsRootDiskDevice(v) && k != newLocalRootDiskDeviceKey && !shared.StringInSlice(k, newProfileRootDiskDevices) {
-			newProfileRootDiskDevices = append(newProfileRootDiskDevices, k)
+		if v["type"] == "disk" && v["path"] == "/" && v["pool"] != "" {
+			if newRootDiskDeviceKey != "" {
+				return fmt.Errorf("Containers may only have one root disk device")
+			}
+
+			newRootDiskDeviceKey = k
 		}
 	}
 
-	// Verify root disk devices. (Be specific with error messages.)
-	var oldRootDiskDeviceKey string
-	var newRootDiskDeviceKey string
-	if oldLocalRootDiskDevice["pool"] != "" {
-		oldRootDiskDeviceKey = oldLocalRootDiskDeviceKey
-		newRootDiskDeviceKey = newLocalRootDiskDeviceKey
-
-		if newLocalRootDiskDevice["pool"] == "" {
-			if len(newProfileRootDiskDevices) == 0 {
-				return fmt.Errorf("Update will cause the container to rely on a profile's root disk device but none was found")
-			} else if len(newProfileRootDiskDevices) > 1 {
-				return fmt.Errorf("Update will cause the container to rely on a profile's root disk device but conflicting devices were found")
-			} else if c.expandedDevices[newProfileRootDiskDevices[0]]["pool"] != oldLocalRootDiskDevice["pool"] {
-				newRootDiskDeviceKey = newProfileRootDiskDevices[0]
-				return fmt.Errorf("Using the profile's root disk device would change the storage pool of the container")
-			}
-		}
-	} else {
-		// This branch should allow us to cover cases where a container
-		// didn't have root disk device before for whatever reason. As
-		// long as there is a root disk device in one of the local or
-		// profile devices we're good.
-		if newLocalRootDiskDevice["pool"] != "" {
-			newRootDiskDeviceKey = newLocalRootDiskDeviceKey
+	if newRootDiskDeviceKey == "" {
+		return fmt.Errorf("Containers must have a root disk device (directly or inherited)")
+	}
 
-			if len(oldProfileRootDiskDevices) > 0 {
-				oldRootDiskDeviceKey = oldProfileRootDiskDevices[0]
-				if oldExpandedDevices[oldRootDiskDeviceKey]["pool"] != newLocalRootDiskDevice["pool"] {
-					return fmt.Errorf("The new local root disk device would change the storage pool of the container")
-				}
-			}
-		} else {
-			if len(newProfileRootDiskDevices) == 0 {
-				return fmt.Errorf("Update will cause the container to rely on a profile's root disk device but none was found")
-			} else if len(newProfileRootDiskDevices) > 1 {
-				return fmt.Errorf("Using the profile's root disk device would change the storage pool of the container")
-			}
-			newRootDiskDeviceKey = newProfileRootDiskDevices[0]
+	// Retrieve the old root disk device
+	oldRootDiskDeviceKey := ""
+	for k, v := range c.expandedDevices {
+		if v["type"] == "disk" && v["path"] == "/" && v["pool"] != "" {
+			oldRootDiskDeviceKey = k
+			break
 		}
 	}
 
+	// Deal with quota changes
 	oldRootDiskDeviceSize := oldExpandedDevices[oldRootDiskDeviceKey]["size"]
 	newRootDiskDeviceSize := c.expandedDevices[newRootDiskDeviceKey]["size"]