On 6/17/20 3:50 PM, Thomas Lamprecht wrote:
Am 6/17/20 um 2:13 PM schrieb Aaron Lauterer:
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>
---
v5 -> v6:
* incorporate suggested changes
* did not move sorting in PVE::VZDump::get_included_guests into the `if`
clause because that would sort them only for `all` and/or `exclude`
jobs but the VMIDs can be provided in random order for other job types
like manually selected VMIDs as well.
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 | 19 ++++++-----
PVE/VZDump.pm | 80 ++++++++++++++++++++++------------------------
2 files changed, 50 insertions(+), 49 deletions(-)
applied, thanks! made a few small followups, most importan pulling the sorting
inside check_vmid, as that generates a new array anyway so needs to iterate
over all, and a style fix (see below).
sounds good and sorry for that style slip, must have pressed undo one or
two times too many at some point without noticing it ;)
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index bdbf641e..2f477606 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -1044,29 +1044,23 @@ sub exec_backup {
if scalar(@{$self->{skiplist}});
my $tasklist = [];
+ my $vzdump_plugins = {};
+ foreach my $plugin (@{$self->{plugins}}) {
+ my $type = $plugin->type();
+ next if exists $vzdump_plugins->{$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} };
For hashes:
* always a trailing comma, allows to make insertions without touching any
existing line at the end
* closing brace on new line:
* optionally and not always wanted, sort keys alphabetically
push @$tasklist, {
vmid => $vmid,
state => 'todo',
plugin => $plugin,
mode => $opts->{mode},
};
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
