[pve-devel] [PATCH qemu-server 04/12] drive: move vm_is_volid_owner() and try_deallocate_drive() helpers to Drive module

[Fiona Ebner]

While at it, avoid making the if conditions/lines for the
try_deallocate_drive() callers even longer, and bring the code in-line
with the style guide.

No functional change intended.

Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
---
 PVE/API2/Qemu.pm        |  8 ++++++--
 PVE/QemuServer.pm       | 42 +++--------------------------------------
 PVE/QemuServer/Drive.pm | 38 +++++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 41 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 5ac61aa5..fc0df860 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1972,13 +1972,17 @@ my $update_vm_api  = sub {
 		    my $drive = PVE::QemuServer::parse_drive($opt, $val);
 		    PVE::QemuConfig->check_protection($conf, "can't remove unused disk '$drive->{file}'");
 		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk']);
-		    if (PVE::QemuServer::try_deallocate_drive($storecfg, $vmid, $conf, $opt, $drive, $rpcenv, $authuser)) {
+		    my $remove_from_config = PVE::QemuServer::Drive::try_deallocate_drive(
+			$storecfg, $vmid, $conf, $opt, $drive, $rpcenv, $authuser);
+		    if ($remove_from_config) {
 			delete $conf->{$opt};
 			PVE::QemuConfig->write_config($vmid, $conf);
 		    }
 		} elsif ($opt eq 'vmstate') {
 		    PVE::QemuConfig->check_protection($conf, "can't remove vmstate '$val'");
-		    if (PVE::QemuServer::try_deallocate_drive($storecfg, $vmid, $conf, $opt, { file => $val }, $rpcenv, $authuser, 1)) {
+		    my $remove_from_config = PVE::QemuServer::Drive::try_deallocate_drive(
+			$storecfg, $vmid, $conf, $opt, { file => $val }, $rpcenv, $authuser, 1);
+		    if ($remove_from_config) {
 			delete $conf->{$opt};
 			PVE::QemuConfig->write_config($vmid, $conf);
 		    }
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 9d06ac8b..454ee64a 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1830,20 +1830,6 @@ sub add_random_macs {
     }
 }
 
-sub vm_is_volid_owner {
-    my ($storecfg, $vmid, $volid) = @_;
-
-    if ($volid !~  m|^/|) {
-	my ($path, $owner);
-	eval { ($path, $owner) = PVE::Storage::path($storecfg, $volid); };
-	if ($owner && ($owner == $vmid)) {
-	    return 1;
-	}
-    }
-
-    return;
-}
-
 sub vmconfig_register_unused_drive {
     my ($storecfg, $vmid, $conf, $drive) = @_;
 
@@ -1853,7 +1839,7 @@ sub vmconfig_register_unused_drive {
 	delete $conf->{cloudinit};
     } elsif (!drive_is_cdrom($drive)) {
 	my $volid = $drive->{file};
-	if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
+	if (PVE::QemuServer::Drive::vm_is_volid_owner($storecfg, $vmid, $volid)) {
 	    PVE::QemuConfig->add_unused_volume($conf, $volid, $vmid);
 	}
     }
@@ -5027,29 +5013,6 @@ sub vmconfig_hotplug_pending {
     }
 }
 
-sub try_deallocate_drive {
-    my ($storecfg, $vmid, $conf, $key, $drive, $rpcenv, $authuser, $force) = @_;
-
-    if (($force || $key =~ /^unused/) && !drive_is_cdrom($drive, 1)) {
-	my $volid = $drive->{file};
-	if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
-	    my $sid = PVE::Storage::parse_volume_id($volid);
-	    $rpcenv->check($authuser, "/storage/$sid", ['Datastore.AllocateSpace']);
-
-	    # check if the disk is really unused
-	    die "unable to delete '$volid' - volume is still in use (snapshot?)\n"
-		if PVE::QemuServer::Drive::is_volume_in_use($storecfg, $conf, $key, $volid);
-	    PVE::Storage::vdisk_free($storecfg, $volid);
-	    return 1;
-	} else {
-	    # If vm is not owner of this disk remove from config
-	    return 1;
-	}
-    }
-
-    return;
-}
-
 sub vmconfig_delete_or_detach_drive {
     my ($vmid, $storecfg, $conf, $opt, $force) = @_;
 
@@ -5060,7 +5023,8 @@ sub vmconfig_delete_or_detach_drive {
 
     if ($force) {
 	$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk']);
-	try_deallocate_drive($storecfg, $vmid, $conf, $opt, $drive, $rpcenv, $authuser, $force);
+	PVE::QemuServer::Drive::try_deallocate_drive(
+	    $storecfg, $vmid, $conf, $opt, $drive, $rpcenv, $authuser, $force);
     } else {
 	vmconfig_register_unused_drive($storecfg, $vmid, $conf, $drive);
     }
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index 1041c1dd..8ff44641 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -998,4 +998,42 @@ sub get_scsi_device_type {
 
     return $devicetype;
 }
+
+sub vm_is_volid_owner {
+    my ($storecfg, $vmid, $volid) = @_;
+
+    if ($volid !~  m|^/|) {
+	my ($path, $owner);
+	eval { ($path, $owner) = PVE::Storage::path($storecfg, $volid); };
+	if ($owner && ($owner == $vmid)) {
+	    return 1;
+	}
+    }
+
+    return;
+}
+
+sub try_deallocate_drive {
+    my ($storecfg, $vmid, $conf, $key, $drive, $rpcenv, $authuser, $force) = @_;
+
+    if (($force || $key =~ /^unused/) && !drive_is_cdrom($drive, 1)) {
+	my $volid = $drive->{file};
+	if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
+	    my $sid = PVE::Storage::parse_volume_id($volid);
+	    $rpcenv->check($authuser, "/storage/$sid", ['Datastore.AllocateSpace']);
+
+	    # check if the disk is really unused
+	    die "unable to delete '$volid' - volume is still in use (snapshot?)\n"
+		if is_volume_in_use($storecfg, $conf, $key, $volid);
+	    PVE::Storage::vdisk_free($storecfg, $volid);
+	    return 1;
+	} else {
+	    # If vm is not owner of this disk remove from config
+	    return 1;
+	}
+    }
+
+    return;
+}
+
 1;
-- 
2.39.5