[pve-devel] [PATCH v2 manager 1/2] vzdump: make guest include logic testable

[Aaron Lauterer]

As a first step to make the whole guest include logic more testable the
part from the API endpoint has been moved to its own method with as
little changes as possible.

Everything concerning `all` and `exclude` logic is still in the
PVE::VZDump->exec_backup() method.

Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
---

v1 -> v2:
* fixed return value. Array refs inside an array lead to nested
  arrays not working with `my ($foo, $bar) = method();`


As talked with thomas on[0] and off list, this patch series is meant to
have more confidence in the ongoing changes.

My other ongoing patch series [1] will move the all the logic, even the
one in the `exec_backup()` method into one single method.

[0] https://pve.proxmox.com/pipermail/pve-devel/2020-April/042795.html
[1] https://pve.proxmox.com/pipermail/pve-devel/2020-April/042753.html

 PVE/API2/VZDump.pm | 36 ++++++------------------------------
 PVE/VZDump.pm      | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index f01e4de0..68a3de89 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -69,39 +69,15 @@ __PACKAGE__->register_method ({
 	return 'OK' if $param->{node} && $param->{node} ne $nodename;
 
 	my $cmdline = PVE::VZDump::Common::command_line($param);
-	my @vmids;
-	# convert string lists to arrays
-	if ($param->{pool}) {
-	    @vmids = @{PVE::API2Tools::get_resource_pool_guest_members($param->{pool})};
-	} else {
-	    @vmids = PVE::Tools::split_list(extract_param($param, 'vmid'));
-	}
+	my ($vmids, $skiplist) = PVE::VZDump->get_included_guests($param);
 
 	if($param->{stop}){
 	    PVE::VZDump::stop_running_backups();
-	    return 'OK' if !scalar(@vmids);
+	    return 'OK' if !scalar(@{$vmids});
 	}
 
-	my $skiplist = [];
-	if (!$param->{all}) {
-	    if (!$param->{node} || $param->{node} eq $nodename) {
-		my $vmlist = PVE::Cluster::get_vmlist();
-		my @localvmids = ();
-		foreach my $vmid (@vmids) {
-		    my $d = $vmlist->{ids}->{$vmid};
-		    if ($d && ($d->{node} ne $nodename)) {
-			push @$skiplist, $vmid;
-		    } else {
-			push @localvmids, $vmid;
-		    }
-		}
-		@vmids = @localvmids;
-		# silent exit if specified VMs run on other nodes
-		return "OK" if !scalar(@vmids);
-	    }
-
-	    $param->{vmids} = PVE::VZDump::check_vmids(@vmids)
-	}
+	# silent exit if specified VMs run on other nodes
+	return "OK" if !scalar(@{$vmids});
 
 	my @exclude = PVE::Tools::split_list(extract_param($param, 'exclude'));
 	$param->{exclude} = PVE::VZDump::check_vmids(@exclude);
@@ -118,7 +94,7 @@ __PACKAGE__->register_method ({
 	}
 
 	die "you can only backup a single VM with option --stdout\n"
-	    if $param->{stdout} && scalar(@vmids) != 1;
+	    if $param->{stdout} && scalar(@{$vmids}) != 1;
 
 	$rpcenv->check($user, "/storage/$param->{storage}", [ 'Datastore.AllocateSpace' ])
 	    if $param->{storage};
@@ -167,7 +143,7 @@ __PACKAGE__->register_method ({
 	}
 
 	my $taskid;
-	$taskid = $vmids[0] if scalar(@vmids) == 1;
+	$taskid = ${$vmids}[0] if scalar(@{$vmids}) == 1;
 
 	return $rpcenv->fork_worker('vzdump', $taskid, $user, $worker);
    }});
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index f3274196..73ad9088 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -21,6 +21,7 @@ use PVE::RPCEnvironment;
 use PVE::Storage;
 use PVE::VZDump::Common;
 use PVE::VZDump::Plugin;
+use PVE::Tools qw(extract_param);
 
 my @posix_filesystems = qw(ext3 ext4 nfs nfs4 reiserfs xfs);
 
@@ -1156,4 +1157,39 @@ sub stop_running_backups {
     }
 }
 
+sub get_included_guests {
+    my ($self, $job) = @_;
+
+    my $nodename = PVE::INotify::nodename();
+    my $vmids = [];
+
+    # convert string lists to arrays
+    if ($job->{pool}) {
+	$vmids = PVE::API2Tools::get_resource_pool_guest_members($job->{pool});
+    } else {
+	$vmids = [ PVE::Tools::split_list(extract_param($job, 'vmid')) ];
+    }
+
+    my $skiplist = [];
+    if (!$job->{all}) {
+	if (!$job->{node} || $job->{node} eq $nodename) {
+	    my $vmlist = PVE::Cluster::get_vmlist();
+	    my $localvmids = [];
+	    foreach my $vmid (@{$vmids}) {
+		my $d = $vmlist->{ids}->{$vmid};
+		if ($d && ($d->{node} ne $nodename)) {
+		    push @{$skiplist}, $vmid;
+		} else {
+		    push @{$localvmids}, $vmid;
+		}
+	    }
+	    $vmids = $localvmids;
+	}
+
+	$job->{vmids} = PVE::VZDump::check_vmids(@{$vmids})
+    }
+
+    return ($vmids, $skiplist);
+}
+
 1;
-- 
2.20.1