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

[Fabian Grünbichler]

On October 15, 2019 12:17 pm, Dominic Jäger wrote:
> 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.

and for pve-container? ;)

>  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
> 
> 
>