The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/3352
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) === Signed-off-by: Christian Brauner <[email protected]>
From 677eb260485f114efd0fffe8f3d212963098cbc0 Mon Sep 17 00:00:00 2001 From: Christian Brauner <[email protected]> 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 <[email protected]> --- 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 <[email protected]> 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 <[email protected]> --- 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) } }
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
