[lxc-devel] [lxd/master] VM remove storage volume record on creation fail

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 536 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191119/dc2486d5/attachment.bin>
-------------- next part --------------
From a8797e59eb855ea207ed63b1f1c4986cee694e01 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 19 Nov 2019 12:02:30 +0000
Subject: [PATCH 1/2] lxd/vm/qemu: Comment ending consistency

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

diff --git a/lxd/vm_qemu.go b/lxd/vm_qemu.go
index 3b99e0f170..f3cdd00d32 100644
--- a/lxd/vm_qemu.go
+++ b/lxd/vm_qemu.go
@@ -168,7 +168,7 @@ func vmQemuCreate(s *state.State, args db.InstanceArgs) (Instance, error) {
 		return nil, err
 	}
 
-	// Validate expanded config
+	// Validate expanded config.
 	err = containerValidConfig(s.OS, vm.expandedConfig, false, true)
 	if err != nil {
 		logger.Error("Failed creating instance", ctxMap)
@@ -181,7 +181,7 @@ func vmQemuCreate(s *state.State, args db.InstanceArgs) (Instance, error) {
 		return nil, errors.Wrap(err, "Invalid devices")
 	}
 
-	// Retrieve the instance's storage pool
+	// Retrieve the instance's storage pool.
 	_, rootDiskDevice, err := shared.GetRootDiskDevice(vm.expandedDevices.CloneNative())
 	if err != nil {
 		return nil, err

From 9d3bdcd7a25c1f4247fde4f0873f5d42988f7c1e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 19 Nov 2019 12:01:42 +0000
Subject: [PATCH 2/2] lxd/vm/qemu: Handle deletion of storage volume DB record
 when reverting VM create

Fixes #6461.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/vm_qemu.go | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/lxd/vm_qemu.go b/lxd/vm_qemu.go
index f3cdd00d32..00bf6366c0 100644
--- a/lxd/vm_qemu.go
+++ b/lxd/vm_qemu.go
@@ -34,6 +34,7 @@ import (
 	"github.com/lxc/lxd/lxd/project"
 	"github.com/lxc/lxd/lxd/state"
 	storagePools "github.com/lxc/lxd/lxd/storage"
+	storageDrivers "github.com/lxc/lxd/lxd/storage/drivers"
 	"github.com/lxc/lxd/lxd/util"
 	"github.com/lxc/lxd/lxd/vsock"
 	"github.com/lxc/lxd/shared"
@@ -1845,24 +1846,32 @@ func (vm *vmQemu) Delete() error {
 	// Attempt to initialize storage interface for the instance.
 	pool, err := storagePools.GetPoolByInstance(vm.state, vm)
 	if err != nil {
-		logger.Warn("Failed to init storage pool", log.Ctx{"project": vm.Project(), "instance": vm.Name(), "err": err})
-
-		// Remove the volume record from the database. This deletion would
-		// normally be handled by DeleteInstance() but since the storage driver
-		// (new storage) is not implemented, we need to do it here manually.
-		poolName, err := vm.StoragePool()
-		if err != nil {
-			return err
-		}
+		logger.Error("Failed to init storage pool", log.Ctx{"project": vm.Project(), "instance": vm.Name(), "err": err})
+
+		// Because of the way vmQemuCreate creates the storage volume record before loading
+		// the storage pool driver, Delete() may be called as part of a revertion if the
+		// pool being used to create the VM on doesn't support VMs. This deletion will then
+		// fail too, so we need to detect this scenario and just remove the storage volume
+		// DB record.
+		// TODO: This can be removed once all pool drivers are ported to new storage layer.
+		if err == storageDrivers.ErrUnknownDriver || err == storageDrivers.ErrNotImplemented {
+			// Remove the volume record from the database. This deletion would
+			// normally be handled by DeleteInstance() call below but since the storage
+			// driver (new storage) is not implemented, we need to do it here manually.
+			poolName, err := vm.StoragePool()
+			if err != nil {
+				return err
+			}
 
-		poolID, err := vm.state.Cluster.StoragePoolGetID(poolName)
-		if err != nil {
-			return err
-		}
+			poolID, err := vm.state.Cluster.StoragePoolGetID(poolName)
+			if err != nil {
+				return err
+			}
 
-		err = vm.state.Cluster.StoragePoolVolumeDelete(vm.Project(), vm.Name(), db.StoragePoolVolumeTypeVM, poolID)
-		if err != nil {
-			return err
+			err = vm.state.Cluster.StoragePoolVolumeDelete(vm.Project(), vm.Name(), db.StoragePoolVolumeTypeVM, poolID)
+			if err != nil {
+				return err
+			}
 		}
 	}