[pve-devel] [PATCH qemu-server v3] Fix #2412: Missing VMs in pools

[Dominic Jäger]

Between calling vm_destroy and removing the ID from user.cfg (remove_vm_access)
creating a new VM with this ID was possible. VMs could go missing from pools as
a consequence.

Adding a lock solves this for clones from the same node. Additionally,
unlinking must happen at the very end of the deletion process to avoid that
other nodes use the ID in the meanwhile.

Co-developed-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
---
v1->v2: Move unlink to the end of the deletion process
v2->v3: Use skiplock parameter and add error handling for unlink

I'll send a separate patch for the discussed clean up.

 PVE/API2/Qemu.pm | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 267a08e..7e1d314 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1492,9 +1492,15 @@ __PACKAGE__->register_method({
 	    my $upid = shift;
 
 	    syslog('info', "destroy VM $vmid: $upid\n");
-	    PVE::QemuServer::vm_destroy($storecfg, $vmid, $skiplock);
-	    PVE::AccessControl::remove_vm_access($vmid);
-            PVE::Firewall::remove_vmfw_conf($vmid);
+	    PVE::QemuConfig->lock_config($vmid, sub {
+		die "VM $vmid is running - destroy failed\n"
+		    if (PVE::QemuServer::check_running($vmid));
+		PVE::QemuServer::destroy_vm($storecfg, $vmid, 1, $skiplock);
+		PVE::AccessControl::remove_vm_access($vmid);
+		PVE::Firewall::remove_vmfw_conf($vmid);
+		unlink PVE::QemuConfig->config_file($vmid)
+		    or die "Removal of VM $vmid config file failed: $!\n";
+	    });
 	};
 
 	return $rpcenv->fork_worker('qmdestroy', $vmid, $authuser, $realcmd);
-- 
2.20.1