[pve-devel] [PATCH v2 qemu-server] replace get_used_paths with is_volume_in_use

[Wolfgang Bumiller]

get_used_paths returned a hash of used paths for all the
volumes in a VM's config, which is not enough to figure out
whether there are snapshots, as snapshots often have
different paths.  Eg. on ZFS it is not enough to check for
/dev/zvol/tank/vm-123-disk-1 because the snapshot's path is
/dev/zvol/tank/vm-123-disk-1 at snap1 and thus we allowed
deleting the drive. Then when trying to delete the snapshot
later you get:
  zfs error: cannot open 'tank/vm-751-disk-1': dataset does not exist
and it refuses to delete the snapshot.

Since its only use was to check whether or not a drive is
still in use it is now renamed to is_volume_in_use and
beside checking paths now also checks volume-ids as those
should stay the same.

Fixes #841
---
 Changes to v1:
   -) added default return value for $scan_config()
   -) moved PVE::Storage::path call on the target volume into is_volume_in_use
   -) removed $scan_snapshots parameter as it's always 1
   -) removed $vmid parameter as it's unused
   -) don't set $skip_drive in move_disk
 PVE/API2/Qemu.pm  |  6 ++----
 PVE/QemuServer.pm | 28 +++++++++++++---------------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index c316031..23f4fa2 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2488,10 +2488,8 @@ __PACKAGE__->register_method({
                 }
 
 		if ($param->{delete}) {
-                    my $used_paths = PVE::QemuServer::get_used_paths($vmid, $storecfg, $conf, 1, 1);
-                    my $path = PVE::Storage::path($storecfg, $old_volid);
-		    if ($used_paths->{$path}){
-			warn "volume $old_volid have snapshots. Can't delete it\n";
+                    if (PVE::QemuServer::is_volume_in_use($storecfg, $conf, undef, $old_volid)) {
+			warn "volume $old_volid still has snapshots, can't delete it\n";
 			PVE::QemuServer::add_unused_volume($conf, $old_volid);
 			PVE::QemuServer::update_config_nolock($vmid, $conf, 1);
 		    } else {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 0eff1cc..95cea49 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4347,10 +4347,8 @@ sub try_deallocate_drive {
 	    $rpcenv->check($authuser, "/storage/$sid", ['Datastore.AllocateSpace']);
 
 	    # check if the disk is really unused
-	    my $used_paths = PVE::QemuServer::get_used_paths($vmid, $storecfg, $conf, 1, $key);
-	    my $path = PVE::Storage::path($storecfg, $volid);
 	    die "unable to delete '$volid' - volume is still in use (snapshot?)\n"
-		   if $used_paths->{$path};
+		if is_volume_in_use($storecfg, $conf, $key, $volid);
 	    PVE::Storage::vdisk_free($storecfg, $volid);
 	    return 1;
 	} else {
@@ -5375,10 +5373,10 @@ sub scan_volids {
     return $volid_hash;
 }
 
-sub get_used_paths {
-    my ($vmid, $storecfg, $conf, $scan_snapshots, $skip_drive) = @_;
+sub is_volume_in_use {
+    my ($storecfg, $conf, $skip_drive, $volid) = @_;
 
-    my $used_path = {};
+    my $path = PVE::Storage::path($storecfg, $volid);
 
     my $scan_config = sub {
 	my ($cref, $snapname) = @_;
@@ -5389,31 +5387,31 @@ sub get_used_paths {
 		next if $skip_drive && $key eq $skip_drive;
 		my $drive = parse_drive($key, $value);
 		next if !$drive || !$drive->{file} || drive_is_cdrom($drive);
+		return 1 if $volid eq $drive->{file};
 		if ($drive->{file} =~ m!^/!) {
-		    $used_path->{$drive->{file}}++; # = 1;
+		    return 1 if $drive->{file} eq $path;
 		} else {
 		    my ($storeid, $volname) = PVE::Storage::parse_volume_id($drive->{file}, 1);
 		    next if !$storeid;
 		    my $scfg = PVE::Storage::storage_config($storecfg, $storeid, 1);
 		    next if !$scfg;
-		    my $path = PVE::Storage::path($storecfg, $drive->{file}, $snapname);
-		    $used_path->{$path}++; # = 1;
+		    return 1 if $path eq PVE::Storage::path($storecfg, $drive->{file}, $snapname);
 		}
 	    }
 	}
+
+	return 0;
     };
 
-    &$scan_config($conf);
+    return 1 if &$scan_config($conf);
 
     undef $skip_drive;
 
-    if ($scan_snapshots) {
-	foreach my $snapname (keys %{$conf->{snapshots}}) {
-	    &$scan_config($conf->{snapshots}->{$snapname}, $snapname);
-	}
+    foreach my $snapname (keys %{$conf->{snapshots}}) {
+	return 1 if &$scan_config($conf->{snapshots}->{$snapname}, $snapname);
     }
 
-    return $used_path;
+    return 0;
 }
 
 sub update_disksize {
-- 
2.1.4