[pve-devel] [PATCH v4 manager 1/1] backup: move logic to include guests into method
Aaron Lauterer
a.lauterer at proxmox.com
Fri Apr 3 16:11:36 CEST 2020
This extracts the logic which guests are to be included in a backup job
into its own method 'get_included_guests'. This makes it possible to
develop other features around backup jobs.
Logic which was spread out accross the API2/VZDump.pm file and the
VZDump.pm file is refactored into the new method, most notably the
logic that dealt with excluded guests.
The new method returns either just the included VMIDs on the local node
or an array with with the local VMIDs and the VMIDs present on other
nodes in the cluster. The latter is used for 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.lauterer at proxmox.com>
---
v3 -> v4:
* reworked the whole logic according to the discussion with
fabian [0].
* re-added missing early exit
[0] https://pve.proxmox.com/pipermail/pve-devel/2020-March/042451.html
PVE/API2/VZDump.pm | 43 ++++++--------------------
PVE/VZDump.pm | 76 ++++++++++++++++++++++++++++++++--------------
2 files changed, 63 insertions(+), 56 deletions(-)
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index f01e4de0..88f53b82 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -69,43 +69,17 @@ __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'));
- }
- if($param->{stop}){
- PVE::VZDump::stop_running_backups();
- return 'OK' if !scalar(@vmids);
- }
+ my ($vmids, $skiplist) = PVE::VZDump->get_included_guests($param);
- 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);
- }
+ # silent exit if specified guests run on other nodes
+ return "OK" if !scalar(@$vmids);
- $param->{vmids} = PVE::VZDump::check_vmids(@vmids)
+ if($param->{stop}){
+ PVE::VZDump::stop_running_backups();
+ 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'} || '');
@@ -118,7 +92,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};
@@ -130,6 +104,7 @@ __PACKAGE__->register_method ({
die "interrupted by signal\n";
};
+ $param->{vmids} = $vmids;
my $vzdump = PVE::VZDump->new($cmdline, $param, $skiplist);
eval {
@@ -167,7 +142,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..3bcfd422 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);
@@ -1026,34 +1027,23 @@ sub exec_backup {
my $opts = $self->{opts};
+ my $nodename = PVE::INotify::nodename();
+
debugmsg ('info', "starting new backup job: $self->{cmdline}", undef, 1);
debugmsg ('info', "skip external VMs: " . join(', ', @{$self->{skiplist}}))
if scalar(@{$self->{skiplist}});
my $tasklist = [];
+ my $vzdump_plugins = {};
+ foreach my $p (@{$self->{plugins}}) {
+ $vzdump_plugins->{$p->type()} = $p;
+ }
- 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 $plugin = $vzdump_plugins->{$vmlist->{ids}->{$vmid}->{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.
@@ -1156,4 +1146,46 @@ sub stop_running_backups {
}
}
+sub get_included_guests {
+ my ($self, $job) = @_;
+
+ my $nodename = PVE::INotify::nodename();
+ my @vmids;
+ my $local_vmids = [];
+ my $foreign_vmids = [];
+
+ my $vmlist = PVE::Cluster::get_vmlist();
+
+ if ($job->{pool}) {
+ @vmids = @{PVE::API2Tools::get_resource_pool_guest_members($job->{pool})};
+ } elsif ($job->{vmid}) {
+ # selected VMIDs
+ @vmids = PVE::Tools::split_list(extract_param($job, 'vmid'));
+ } else {
+ # all or exclude
+ my @exclude = PVE::Tools::split_list(extract_param($job, 'exclude'));
+ @exclude = @{PVE::VZDump::check_vmids(@exclude)};
+ my $excludehash = { map { $_ => 1 } @exclude };
+
+ foreach my $id (keys %{ $vmlist->{ids} }) {
+ next if $excludehash->{$id};
+ push @vmids, $id
+ if !$job->{node} || $vmlist->{ids}->{$id}->{node} eq $job->{node};
+ }
+ }
+ @vmids = sort {$a <=> $b} @vmids;
+
+ @vmids = @{PVE::VZDump::check_vmids(@vmids)};
+
+ foreach my $vmid (@vmids) {
+ my $d = $vmlist->{ids}->{$vmid};
+ if ($d && ($d->{node} ne $nodename)) {
+ push @$foreign_vmids, $vmid;
+ } else {
+ push @$local_vmids, $vmid;
+ }
+ }
+ return wantarray ? ($local_vmids, $foreign_vmids) : $local_vmids;
+}
+
1;
--
2.20.1
More information about the pve-devel
mailing list