[pve-devel] [PATCH 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, and in some conditions,
node limits, is still in the PVE::VZDump->exec_backup() method.

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

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..018da270 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