[prev in list] [next in list] [prev in thread] [next in thread]
List: lxc-devel
Subject: [lxc-devel] [lxd/master] storage: remove pool property on pool delete
From: brauner on Github <lxc-bot () linuxcontainers ! org>
Date: 2017-02-26 23:50:15
Message-ID: 20170226235015.BBC1D40625 () mailman01 ! srv ! dcmtl ! stgraber ! net
[Download RAW message or body]
[Attachment #2 (text/x-mailbox)]
The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/2961
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) ===
So this may be somewhat debatable. One could make a reasonable argument that we \
simply leave the profile devices that have a pool property set alone and simply let \
users figure this out. However, I feel that this might be a problem. Consider the \
case where a user has a profile that has a disk device entry like:
```
sensitive-device:
path: /opt
source: sensitive-vol1
pool: pool1
```
and the user deletes the volume and the pool but we leave the properties set in the \
device. Now the user creates a new pool of the same name and a new volume of the same \
name that contains sensitive data and creates a container on the profile that still \
contains this device. This could potentially be a problem. So wipe the pool property \
on pool delete. For disk devices outside of LXD's storage management abilities (host \
paths etc.) we can't do this. But for pools and volumes we created we should probably \
do this.
[Attachment #3 (text/plain)]
From 1d874d6deeefab25862975b3b48e90267c4b53c2 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner@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@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@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@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
+}
[Attachment #4 (text/plain)]
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic