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

[Fabian Grünbichler]

On October 11, 2019 11:55 am, Dominic Jäger wrote:
> Between destroying a VM (unlink config file) and removing it from user.cfg
> creating a new VM with the ID that is still in use in user.cfg was possible.
> VMs could go missing as a consequence.
> 
> Adding a lock solves this. This lock does not interfere with the one in
> vm_destroy as they are held by the same process.
> 
> Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
> ---
>  PVE/API2/Qemu.pm | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 267a08e..7dd83a9 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1492,9 +1492,11 @@ __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 {
> +		PVE::QemuServer::vm_destroy($storecfg, $vmid, $skiplock);
> +		PVE::AccessControl::remove_vm_access($vmid);
> +		PVE::Firewall::remove_vmfw_conf($vmid);
> +	    });

this is not enough - as soon as PVE::QemuServer::destroy_vm unlinks the 
VM config file, the lock_config becomes worthless in a clustered setup. 

you need to move the VM config unlinking to become the last step, e.g. 
by exposing destroy_vm's $keep_empty_config via vm_destroy as well, 
and setting it here.

the current situation is especially bad, since we vdisk_free unused 
disks AFTER unlinking the config file, so the delay between unlinking 
the config file, and removing from user.cfg/removing the firewall config 
can be quite long..

>  	};
>  
>  	return $rpcenv->fork_worker('qmdestroy', $vmid, $authuser, $realcmd);
> -- 
> 2.20.1
> 
> 
>