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).
> 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
