[lxc-devel] [lxd/master] Reverts root disk device non-empty pool check

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 780 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190905/62a63be1/attachment.bin>
-------------- next part --------------
From d5f290ef563877c85d7809dffc25742f72104a8b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Sep 2019 16:28:26 +0100
Subject: [PATCH 1/3] Revert "shared/container: Updates IsRootDiskDevice to use
 same definition of rootfs as container_lxc"

This reverts commit b1842a979d1fa46b04b39cc4a38fe3c826c733b5.
---
 shared/container.go | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/shared/container.go b/shared/container.go
index 811f690780..bc5e63b837 100644
--- a/shared/container.go
+++ b/shared/container.go
@@ -133,10 +133,11 @@ func IsDeviceID(value string) error {
 	return nil
 }
 
-// IsRootDiskDevice returns true if the given device representation is configured as root disk for
-// a container. It typically get passed a specific entry of api.Container.Devices.
+// IsRootDiskDevice returns true if the given device representation is
+// configured as root disk for a container. It typically get passed a specific
+// entry of api.Container.Devices.
 func IsRootDiskDevice(device map[string]string) bool {
-	if device["type"] == "disk" && device["path"] == "/" && device["source"] == "" && device["pool"] != "" {
+	if device["type"] == "disk" && device["path"] == "/" && device["source"] == "" {
 		return true
 	}
 

From a64d7fc590b97ffa311ebad53b6c561e62e28f78 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Sep 2019 16:29:03 +0100
Subject: [PATCH 2/3] shared/container: Improves comments on IsRootDiskDevice

Adds comment specifically about not validating non-empty pool property on root disk devices.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 shared/container.go | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/shared/container.go b/shared/container.go
index bc5e63b837..4d78aa6118 100644
--- a/shared/container.go
+++ b/shared/container.go
@@ -133,10 +133,13 @@ func IsDeviceID(value string) error {
 	return nil
 }
 
-// IsRootDiskDevice returns true if the given device representation is
-// configured as root disk for a container. It typically get passed a specific
-// entry of api.Container.Devices.
+// IsRootDiskDevice returns true if the given device representation is configured as root disk for
+// a container. It typically get passed a specific entry of api.Container.Devices.
 func IsRootDiskDevice(device map[string]string) bool {
+	// Root disk devices also need a non-empty "pool" property, but we can't check that here
+	// because this function is used with clients talking to older servers where there was no
+	// concept of a storage pool, and also it is used for migrating from old to new servers.
+	// The validation of the non-empty "pool" property is done inside the disk device itself.
 	if device["type"] == "disk" && device["path"] == "/" && device["source"] == "" {
 		return true
 	}

From a0b46acc81f2873cd9441c30e197b0d7afa32037 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Sep 2019 16:33:48 +0100
Subject: [PATCH 3/3] device/disk: Adds validation for root disk having a pool
 property

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/disk.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 54abf64064..a695d0a0c4 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -93,6 +93,10 @@ func (d *disk) validateConfig() error {
 		return fmt.Errorf("Root disk entry may not have a \"source\" property set")
 	}
 
+	if d.config["path"] == "/" && d.config["pool"] == "" {
+		return fmt.Errorf("Root disk entry must have a \"pool\" property set")
+	}
+
 	if d.config["size"] != "" && d.config["path"] != "/" {
 		return fmt.Errorf("Only the root disk may have a size quota")
 	}