[lxc-devel] [lxd/master] storage: remove pool property on pool delete

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1222 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170226/dccba3ba/attachment.bin>
-------------- next part --------------
From 1d874d6deeefab25862975b3b48e90267c4b53c2 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 27 Feb 2017 00:37:34 +0100
Subject: [PATCH 1/2] storage: minimal improvements

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage.go       | 5 ++---
 lxd/storage_btrfs.go | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lxd/storage.go b/lxd/storage.go
index e11328f..e3f6f40 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -486,10 +486,9 @@ func createContainerMountpoint(mountPoint string, mountPointSymlink string, priv
 			return err
 		}
 
-		err = os.Chmod(mountPoint, mode)
-	} else {
-		err = os.Chmod(mountPoint, mode)
 	}
+
+	err = os.Chmod(mountPoint, mode)
 	if err != nil {
 		return err
 	}
diff --git a/lxd/storage_btrfs.go b/lxd/storage_btrfs.go
index e6a6cd6..36bd447 100644
--- a/lxd/storage_btrfs.go
+++ b/lxd/storage_btrfs.go
@@ -1730,9 +1730,9 @@ func (s *storageBtrfs) MigrationType() MigrationFSType {
 func (s *storageBtrfs) PreservesInodes() bool {
 	if runningInUserns {
 		return false
-	} else {
-		return true
 	}
+
+	return true
 }
 
 func (s *storageBtrfs) MigrationSource(c container) (MigrationStorageSourceDriver, error) {

From 39e32a12894f3653caa45996534f8b79f997b9b6 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 27 Feb 2017 00:37:52 +0100
Subject: [PATCH 2/2] storage-pools: remove pool property on pool delete

When a storage pool is deleted, remove all pool properties referencing it. This
should be safe since a storage pool can only be deleted when all storage volumes
have been deleted. So it shouldn't be possible that the removal of the pool
property causes inconsistencies. (Note, users can still add back a non-existing
pool to a profile.)

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_pools.go       | 41 ++---------------------------------
 lxd/storage_pools_utils.go | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go
index da2b874..db0849f 100644
--- a/lxd/storage_pools.go
+++ b/lxd/storage_pools.go
@@ -302,46 +302,9 @@ func storagePoolDelete(d *Daemon, r *http.Request) Response {
 		return InternalError(err)
 	}
 
-	// In case we deleted the default storage pool, try to update the
-	// default profile.
-	defaultID, defaultProfile, err := dbProfileGet(d.db, "default")
+	err = removePoolFromProfiles(d.db, poolName)
 	if err != nil {
-		return EmptySyncResponse
-	}
-	for k, v := range defaultProfile.Devices {
-		if v["type"] == "disk" && v["path"] == "/" {
-			if v["pool"] == poolName {
-				defaultProfile.Devices[k]["pool"] = ""
-
-				tx, err := dbBegin(d.db)
-				if err != nil {
-					return EmptySyncResponse
-				}
-
-				err = dbProfileConfigClear(tx, defaultID)
-				if err != nil {
-					tx.Rollback()
-					return EmptySyncResponse
-				}
-
-				err = dbProfileConfigAdd(tx, defaultID, defaultProfile.Config)
-				if err != nil {
-					tx.Rollback()
-					return EmptySyncResponse
-				}
-
-				err = dbDevicesAdd(tx, "profile", defaultID, defaultProfile.Devices)
-				if err != nil {
-					tx.Rollback()
-					return EmptySyncResponse
-				}
-
-				err = txCommit(tx)
-				if err != nil {
-					return EmptySyncResponse
-				}
-			}
-		}
+		return InternalError(err)
 	}
 
 	return EmptySyncResponse
diff --git a/lxd/storage_pools_utils.go b/lxd/storage_pools_utils.go
index 6d70bfc..b4e122c 100644
--- a/lxd/storage_pools_utils.go
+++ b/lxd/storage_pools_utils.go
@@ -103,3 +103,56 @@ func storagePoolUsedByGet(db *sql.DB, poolID int64, poolName string) ([]string,
 
 	return poolUsedBy, err
 }
+
+// When a storage pool is deleted, remove all pool properties referencing it.
+// This should be safe since a storage pool can only be deleted when all storage
+// volumes have been deleted. So it shouldn't be possible that the removal of
+// the pool property causes inconsistencies.
+func removePoolFromProfiles(db *sql.DB, poolName string) error {
+	profiles, err := dbProfiles(db)
+	if err != nil {
+		return err
+	}
+
+	for _, pName := range profiles {
+		pID, profile, err := dbProfileGet(db, pName)
+		if err != nil {
+			return err
+		}
+		for k, v := range profile.Devices {
+			if v["pool"] == poolName {
+				delete(profile.Devices[k], "pool")
+
+				tx, err := dbBegin(db)
+				if err != nil {
+					return err
+				}
+
+				err = dbProfileConfigClear(tx, pID)
+				if err != nil {
+					tx.Rollback()
+					return err
+				}
+
+				err = dbProfileConfigAdd(tx, pID, profile.Config)
+				if err != nil {
+					tx.Rollback()
+					return err
+				}
+
+				err = dbDevicesAdd(tx, "profile", pID, profile.Devices)
+				if err != nil {
+					tx.Rollback()
+					return err
+				}
+
+				err = txCommit(tx)
+				if err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	return nil
+}