[pve-devel] [PATCH qemu-server] Fix #1717: delete snapshot when vm running and drive not attached

[Wolfgang Bumiller]

On Sat, Jun 30, 2018 at 01:18:49PM +0200, Alexandre Derumier wrote:
> if we try to delete a snapshot, and that is disk from the snapshot
> is not attached anymore (unused), we can't delete the snapshot
> with qemu snapshot delete command (for storage which use it (qcow2,rbd,...))
> 
> example:
> 
> ...
> unused0: rbd:vm-107-disk-3
> 
> [snap1]
> ...
> scsi2: rbd:vm-107-disk-3,size=1G
> 
> -> die
>  qmp command 'delete-drive-snapshot' failed - Device 'drive-scsi2' not found
> 
> If drive is not attached, we need to use the storage snapshot delete command
> ---
>  PVE/QemuServer.pm | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index d6efb3a..31152b6 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4197,6 +4197,20 @@ sub qemu_volume_snapshot_delete {
>  
>      my $running = check_running($vmid);
>  
> +    if($running) {
> +
> +	my $running_volids = {};
> +	my $conf = PVE::QemuConfig->load_config($vmid);
> +
> +	foreach_drive($conf, sub {
> +	    my ($ds, $drive) = @_;
> +
> +	    return if drive_is_cdrom($drive);
> +	    my $running_volids->{$drive->{file}} = 1;
> +	});
> +	$running = undef if !$running_volids->{$volid};

Since we cannot cache or reuse the hash anyway building it is
unnecessary. The drive_is_cdrom() check is also not really necessary as
we perform this elsewhere already. In fact, it would be wrong, since
such drives would also be hooked into qemu when it's running.

So it should be sufficient to update $running directly, iow just do

    $running = 0 if $drive->{file} eq $volid;

in the loop and ditch the $running_volids hash. We could even 'break'
at that point, unfortunately we have no break-condition in
foreach_drive() (we could use an eval{} block and die() or a `goto`,
but anyway, that's not really important here).


> +    }
> +
>      if ($running && do_snapshots_with_qemu($storecfg, $volid)){
>  	vm_mon_cmd($vmid, "delete-drive-snapshot", device => $deviceid, name => $snap);
>      } else {
> -- 
> 2.11.0