The `guest include` logic handling `all` and `exclude` parameters was in the `PVE::VZDump->exec_backup()` method. Moving this logic into the `get_included_guests` method allows us to simplify and generalize it.
This helps to make the overall logic easier to test and develop other features around vzdump backup jobs. The method now returns a hash with node names as keys mapped to arrays of VMIDs on these nodes that are included in the vzdump job. The VZDump API call to create a new backup is adapted to use the new method to create the list of local VMIDs and the skiplist. Permission checks are kept where they are to be able to handle missing permissions according to the current context. The old behavior to die on a backup job when the user is missing the permission to a guest and the job is not an 'all' or 'exclude' job is kept. Signed-off-by: Aaron Lauterer <a.laute...@proxmox.com> --- v4 -> v5: * move handling of `all` and `exlcude` cases from `PVE::VZDump->exec_backup` to `PVE::VZDump->get_included_guests` * change return value to hash of node names with arrays of included VMIDs to be more general * adapt API call to start a VZDump job accordingly. Creating the list of local VMIDs and the skiplist is handled right there again as so far this is the only place where that kind of separation is needed v3 -> v4: * reworked the whole logic according to the discussion with fabian [1]. * re-added missing early exit [1] https://pve.proxmox.com/pipermail/pve-devel/2020-March/042451.html PVE/API2/VZDump.pm | 18 ++++++++--- PVE/VZDump.pm | 76 +++++++++++++++++++++------------------------- 2 files changed, 48 insertions(+), 46 deletions(-) diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm index 68a3de89..77dac1b1 100644 --- a/PVE/API2/VZDump.pm +++ b/PVE/API2/VZDump.pm @@ -69,19 +69,26 @@ __PACKAGE__->register_method ({ return 'OK' if $param->{node} && $param->{node} ne $nodename; my $cmdline = PVE::VZDump::Common::command_line($param); - my ($vmids, $skiplist) = PVE::VZDump->get_included_guests($param); + + my $vmids_per_node = PVE::VZDump->get_included_guests($param); + + my $vmids = $vmids_per_node->{$nodename} // []; + + my $skiplist = []; + + for my $current_node (keys %{$vmids_per_node}) { + push @{$skiplist}, @{$vmids_per_node->{$current_node}} + if $current_node ne $nodename; + } if($param->{stop}){ PVE::VZDump::stop_running_backups(); return 'OK' if !scalar(@{$vmids}); } - # silent exit if specified VMs run on other nodes + # silent exit if specified guests 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); - # exclude-path list need to be 0 separated if (defined($param->{'exclude-path'})) { my @expaths = split(/\0/, $param->{'exclude-path'} || ''); @@ -106,6 +113,7 @@ __PACKAGE__->register_method ({ die "interrupted by signal\n"; }; + $param->{vmids} = $vmids; my $vzdump = PVE::VZDump->new($cmdline, $param, $skiplist); eval { diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm index 68c08f47..cd278f4d 100644 --- a/PVE/VZDump.pm +++ b/PVE/VZDump.pm @@ -1039,29 +1039,18 @@ sub exec_backup { if scalar(@{$self->{skiplist}}); my $tasklist = []; + my $vzdump_plugins = {}; + foreach my $plugin (@{$self->{plugins}}) { + my $type = $plugin->type(); + $vzdump_plugins->{$type} = $plugin; + } - if ($opts->{all}) { - foreach my $plugin (@{$self->{plugins}}) { - my $vmlist = $plugin->vmlist(); - foreach my $vmid (sort @$vmlist) { - next if grep { $_ eq $vmid } @{$opts->{exclude}}; - next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Backup' ], 1); - push @$tasklist, { vmid => $vmid, state => 'todo', plugin => $plugin, mode => $opts->{mode} }; - } - } - } else { - foreach my $vmid (sort @{$opts->{vmids}}) { - my $plugin; - foreach my $pg (@{$self->{plugins}}) { - my $vmlist = $pg->vmlist(); - if (grep { $_ eq $vmid } @$vmlist) { - $plugin = $pg; - last; - } - } - $rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Backup' ]); - push @$tasklist, { vmid => $vmid, state => 'todo', plugin => $plugin, mode => $opts->{mode} }; - } + my $vmlist = PVE::Cluster::get_vmlist(); + foreach my $vmid (sort @{$opts->{vmids}}) { + my $guest_type = $vmlist->{ids}->{$vmid}->{type}; + my $plugin = $vzdump_plugins->{$guest_type}; + next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Backup' ], $opts->{all}); + push @$tasklist, { vmid => $vmid, state => 'todo', plugin => $plugin, mode => $opts->{mode} }; } # Use in-memory files for the outer hook logs to pass them to sendmail. @@ -1169,34 +1158,39 @@ sub get_included_guests { my $nodename = PVE::INotify::nodename(); my $vmids = []; + my $vmids_per_node = {}; + + my $vmlist = PVE::Cluster::get_vmlist(); - # convert string lists to arrays if ($job->{pool}) { $vmids = PVE::API2Tools::get_resource_pool_guest_members($job->{pool}); + } elsif ($job->{vmid}) { + $vmids = [ PVE::Tools::split_list($job->{vmid}) ]; } else { - $vmids = [ PVE::Tools::split_list(extract_param($job, 'vmid')) ]; + # all or exclude + my @exclude = PVE::Tools::split_list($job->{exclude}); + @exclude = @{PVE::VZDump::check_vmids(@exclude)}; + my $excludehash = { map { $_ => 1 } @exclude }; + + for my $id (keys %{ $vmlist->{ids} }) { + next if $excludehash->{$id}; + push @$vmids, $id; + } } + $vmids = [ sort {$a <=> $b} @$vmids]; - 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; - } + $vmids = PVE::VZDump::check_vmids(@$vmids); + + foreach my $vmid (@$vmids) { + my $vmid_data = $vmlist->{ids}->{$vmid}; + my $node = $vmid_data->{node}; + + next if (defined $job->{node} && $job->{node} ne $node); - $job->{vmids} = PVE::VZDump::check_vmids(@{$vmids}) + push @{$vmids_per_node->{$node}}, $vmid; } - return ($vmids, $skiplist); + return $vmids_per_node; } 1; -- 2.20.1 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
