[lxc-devel] [lxd/master] lvm: bugfixes

brauner on Github lxc-bot at linuxcontainers.org
Fri May 26 08:22:31 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170526/9b207af1/attachment.bin>
-------------- next part --------------
From 677eb260485f114efd0fffe8f3d212963098cbc0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 26 May 2017 09:52:18 +0200
Subject: [PATCH 1/2] lvm: only delete vg when empty

Although this goes against our policy which explicitly claims that LXD will
assume it is the only user of the storage pool let's check in any case and only
the delete the volume group when it is completely empty.

Closes #3351.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_lvm.go       | 34 +++++++++++++++++++++++++++++++---
 lxd/storage_lvm_utils.go | 14 ++++++++++++++
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/lxd/storage_lvm.go b/lxd/storage_lvm.go
index 0ebc10c19..3f1b4ed18 100644
--- a/lxd/storage_lvm.go
+++ b/lxd/storage_lvm.go
@@ -274,10 +274,38 @@ func (s *storageLvm) StoragePoolDelete() error {
 	}
 
 	poolName := s.getOnDiskPoolName()
-	// Remove the volume group.
-	output, err := shared.TryRunCommand("vgremove", "-f", poolName)
+	// Delete the thinpool.
+	if s.useThinpool {
+		// Check that the thinpool actually exists. For example, it
+		// won't when the user has never created a storage volume in the
+		// storage pool.
+		devPath := getLvmDevPath(poolName, "", s.thinPoolName)
+		ok, _ := storageLVExists(devPath)
+		if ok {
+			msg, err := shared.TryRunCommand("lvremove", "-f", devPath)
+			if err != nil {
+				logger.Errorf("failed to delete thinpool \"%s\" from volume group \"%s\": %s", s.thinPoolName, poolName, msg)
+				return err
+			}
+		}
+	}
+
+	// Check that the count in the volume group is zero. If not, we need to
+	// assume that other users are using the volume group, so don't remove
+	// it. This actually goes against policy since we explicitly state: our
+	// pool, and nothing but our pool but still, let's not hurt users.
+	count, err := lvmGetLVCount(poolName)
 	if err != nil {
-		return fmt.Errorf("failed to destroy the volume group for the lvm storage pool: %s", output)
+		return err
+	}
+
+	// Remove the volume group.
+	if count == 0 {
+		output, err := shared.TryRunCommand("vgremove", "-f", poolName)
+		if err != nil {
+			logger.Errorf("failed to destroy the volume group for the lvm storage pool: %s", output)
+			return err
+		}
 	}
 
 	if s.loopInfo != nil {
diff --git a/lxd/storage_lvm_utils.go b/lxd/storage_lvm_utils.go
index ed9d12669..f6bbc2ffe 100644
--- a/lxd/storage_lvm_utils.go
+++ b/lxd/storage_lvm_utils.go
@@ -483,6 +483,16 @@ func (s *storageLvm) containerCreateFromImageThinLv(c container, fp string) erro
 	return nil
 }
 
+func lvmGetLVCount(vgName string) (int, error) {
+	output, err := shared.TryRunCommand("vgs", "--noheadings", "-o", "lv_count", vgName)
+	if err != nil {
+		return -1, err
+	}
+
+	output = strings.TrimSpace(output)
+	return strconv.Atoi(output)
+}
+
 func lvmLvIsWritable(lvName string) (bool, error) {
 	output, err := shared.TryRunCommand("lvs", "--noheadings", "-o", "lv_attr", lvName)
 	if err != nil {
@@ -698,6 +708,10 @@ func containerNameToLVName(containerName string) string {
 }
 
 func getLvmDevPath(lvmPool string, volumeType string, lvmVolume string) string {
+	if volumeType == "" {
+		return fmt.Sprintf("/dev/%s/%s", lvmPool, lvmVolume)
+	}
+
 	return fmt.Sprintf("/dev/%s/%s_%s", lvmPool, volumeType, lvmVolume)
 }
 

From f0e3f0de7993bc47967d8d2442213cf42443184f Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 26 May 2017 10:19:57 +0200
Subject: [PATCH 2/2] lvm: disallow using non-empty volume groups

This is in line with what we do for the other drivers. I might loosen this
restriction in a follow up commit when the following two conditions are met:
- the volume group uses a thin pool
- the thin pool is the only logical volume that exists on pool creation

 Closes #3351.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_lvm.go | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/lxd/storage_lvm.go b/lxd/storage_lvm.go
index 3f1b4ed18..e0f2e6ba4 100644
--- a/lxd/storage_lvm.go
+++ b/lxd/storage_lvm.go
@@ -118,6 +118,7 @@ func (s *storageLvm) StoragePoolCreate() error {
 		}
 	}()
 
+	vgExisted := false
 	poolName := s.getOnDiskPoolName()
 	source := s.pool.Config["source"]
 	if source == "" {
@@ -187,6 +188,7 @@ func (s *storageLvm) StoragePoolCreate() error {
 				return fmt.Errorf("failed to create the volume group for the lvm storage pool: %s", output)
 			}
 		}
+		vgExisted = ok
 	} else {
 		s.pool.Config["size"] = ""
 		if filepath.IsAbs(source) {
@@ -225,6 +227,7 @@ func (s *storageLvm) StoragePoolCreate() error {
 					return fmt.Errorf("failed to create the volume group for the lvm storage pool: %s", output)
 				}
 			}
+			vgExisted = ok
 		} else {
 			if s.pool.Config["lvm.vg_name"] != "" {
 				// User gave us something weird.
@@ -241,6 +244,22 @@ func (s *storageLvm) StoragePoolCreate() error {
 				// Volume group does not exist.
 				return fmt.Errorf("the requested volume group \"%s\" does not exist", source)
 			}
+			vgExisted = ok
+		}
+	}
+
+	if vgExisted {
+		// Check that the volume group is empty.
+		// Otherwise we will refuse to use it.
+		count, err := lvmGetLVCount(poolName)
+		if err != nil {
+			logger.Errorf("failed to determine whether the volume group \"%s\" is empty", poolName)
+			return err
+		}
+		if count != 0 {
+			msg := fmt.Sprintf("volume group \"%s\" is not empty", poolName)
+			logger.Errorf(msg)
+			return fmt.Errorf(msg)
 		}
 	}
 


More information about the lxc-devel mailing list