[PATCH manager 3/5] vzdump: honor start window for guest backups

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

A backup job can now limit how long after its scheduled start guest
backups may still begin, with the new 'start-window' option. For
manual backups, the window starts with the task.

The window also bounds waiting for the global lock and for backup
slots, and a job whose window ended before it got the lock does not
start at all. Guests not backed up because of this are listed as
skipped in the task log and in the notification, which then reports a
failure.

The job runner passes the scheduled time along, so a run that starts
late, for example a missed one repeated after the node was down, only
gets what is left of its window.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

Notes:
    NOTE: needs pve-guest-common with patch 1. Older versions die on the
    'job-scheduled-time' parameter that scheduled jobs now pass, so every
    scheduled backup would fail.

 PVE/API2/Backup.pm |  2 +-
 PVE/API2/VZDump.pm | 18 ++++++++++++----
 PVE/Jobs/VZDump.pm |  3 ++-
 PVE/VZDump.pm      | 51 ++++++++++++++++++++++++++++++++++++++--------
 4 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
index 3bd855939..f9ad25065 100644
--- a/PVE/API2/Backup.pm
+++ b/PVE/API2/Backup.pm
@@ -40,7 +40,7 @@ sub assert_param_permission_common {
     my ($rpcenv, $user, $param, $is_delete) = @_;
     return if $user eq 'root@pam'; # always OK
 
-    for my $key (qw(tmpdir dumpdir script job-id)) {
+    for my $key (qw(tmpdir dumpdir script job-id job-scheduled-time)) {
         raise_param_exc({ $key => "Only root may set this option." }) if exists $param->{$key};
     }
 
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 3e31505f3..ab45f04ab 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -42,10 +42,11 @@ __PACKAGE__->register_method({
     permissions => {
         description => "The user needs 'VM.Backup' permissions on any VM, and"
             . " 'Datastore.AllocateSpace' on the backup storage (and fleecing storage when fleecing"
-            . " is used). The 'tmpdir', 'dumpdir', 'script' and 'job-id' parameters are restricted"
-            . " to the 'root\@pam' user. The 'prune-backups' setting requires 'Datastore.Allocate'"
-            . " on the backup storage. The 'bwlimit', 'performance' and 'ionice' parameters require"
-            . " 'Sys.Modify' on '/'. The 'stop' parameter requires 'Sys.Modify' on '/nodes/{node}'",
+            . " is used). The 'tmpdir', 'dumpdir', 'script', 'job-id' and 'job-scheduled-time'"
+            . " parameters are restricted to the 'root\@pam' user. The 'prune-backups' setting"
+            . " requires 'Datastore.Allocate' on the backup storage. The 'bwlimit', 'performance'"
+            . " and 'ionice' parameters require 'Sys.Modify' on '/'. The 'stop' parameter requires"
+            . " 'Sys.Modify' on '/nodes/{node}'",
         user => 'all',
     },
     protected => 1,
@@ -63,6 +64,15 @@ __PACKAGE__->register_method({
                     optional => 1,
                 },
             ),
+            'job-scheduled-time' => {
+                type => 'integer',
+                description =>
+                    "The time the backup job was scheduled for, as UNIX epoch. Used as start"
+                    . " of the 'start-window' instead of the current time. Only root\@pam can set"
+                    . " this parameter.",
+                optional => 1,
+                minimum => 0,
+            },
             stdout => {
                 type => 'boolean',
                 description => "Write tar to stdout, not to a file.",
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index d7a2e08dd..fe3e0a0ec 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -12,7 +12,7 @@ use PVE::API2::VZDump;
 use base qw(PVE::VZDump::JobBase);
 
 sub run {
-    my ($class, $conf, $job_id) = @_;
+    my ($class, $conf, $job_id, $schedule, $scheduled_time) = @_;
 
     my $props = $class->properties();
     # remove all non vzdump related options
@@ -21,6 +21,7 @@ sub run {
     }
 
     $conf->{'job-id'} = $job_id;
+    $conf->{'job-scheduled-time'} = $scheduled_time if defined($scheduled_time);
 
     # Required as string parameters # FIXME why?! we could just check ref()
     for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 59809d85c..417cf9e53 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -692,9 +692,20 @@ sub new {
     };
     $self->{exclusive} = !$opts->{overlap} || $uses_provider;
 
+    if (my $window = $opts->{'start-window'}) {
+        $self->{'start-deadline'} = ($opts->{'job-scheduled-time'} // time()) + $window * 60;
+    }
+
     return $self;
 }
 
+sub start_window_ended {
+    my ($self) = @_;
+
+    my $deadline = $self->{'start-deadline'};
+    return defined($deadline) && time() >= $deadline;
+}
+
 sub init_job {
     my ($self) = @_;
 
@@ -788,7 +799,19 @@ sub getlock {
 
     my $fh;
 
-    my $maxwait = $self->{opts}->{lockwait} || $self->{lockwait};
+    my $maxwait = ($self->{opts}->{lockwait} || $self->{lockwait} || 0) * 60;
+    my $window_err = "start window ended before the job could start\n";
+    my $timeout_err = "got timeout\n";
+
+    if (defined(my $deadline = $self->{'start-deadline'})) {
+        my $left = $deadline - time();
+        if ($left <= 0) {
+            debugmsg('err', $window_err, undef, 1);
+            die $window_err;
+        } elsif ($left < $maxwait) {
+            ($maxwait, $timeout_err) = ($left, $window_err);
+        }
+    }
 
     die "missing UPID" if !$upid; # should not happen
     $self->{upid} = $upid;
@@ -824,9 +847,9 @@ sub getlock {
 
         debugmsg('info', "trying to get global lock - waiting...", undef, 1);
         eval {
-            alarm($maxwait * 60);
+            alarm($maxwait);
 
-            local $SIG{ALRM} = sub { alarm(0); die "got timeout\n"; };
+            local $SIG{ALRM} = sub { alarm(0); die $timeout_err; };
 
             if (!$acquire->(0)) {
                 my $err = $!;
@@ -841,8 +864,9 @@ sub getlock {
         my $err = $@;
 
         if ($err) {
-            debugmsg('err', "can't acquire lock '$lockfile' - $err", undef, 1);
-            die "can't acquire lock '$lockfile' - $err";
+            $err = "can't acquire lock '$lockfile' - $err" if $err ne $window_err;
+            debugmsg('err', $err, undef, 1);
+            die $err;
         }
 
         debugmsg('info', "got global lock", undef, 1);
@@ -1791,6 +1815,11 @@ my sub guest_moved_to {
 my sub claim_next_guest {
     my ($self, $queue, $deferred) = @_;
 
+    if ($self->start_window_ended()) {
+        skip_task($_, 'start window ended') for splice($queue->@*);
+        return;
+    }
+
     my $maxwait = ($self->{opts}->{lockwait} || 0) * 60;
 
     for (my $i = 0; $i < scalar($queue->@*); $i++) {
@@ -1935,7 +1964,7 @@ my sub next_guest_to_start {
         $pending->{'request-error'} = $err;
     } elsif (!defined($granted)) {
         $pending->{key} = undef; # vanished, for example if released by an admin
-    } elsif ($granted) {
+    } elsif ($granted && !$self->start_window_ended()) {
         delete $schedule->{pending};
         $schedule->{'cluster-wait'} += time() - $pending->{since};
         if (my $node = guest_moved_to($task->{vmid})) {
@@ -1948,14 +1977,18 @@ my sub next_guest_to_start {
         delete $pending->{'request-error'};
     }
 
+    my $window_ended = $self->start_window_ended();
     my $waited = $schedule->{'cluster-wait'} + time() - $pending->{since};
     my $maxwait = ($self->{opts}->{lockwait} || 0) * 60;
-    if ($waited >= $maxwait) {
+    if ($window_ended || $waited >= $maxwait) {
         release_cluster_slots($pending->{key});
         delete $schedule->{pending};
         $schedule->{'cluster-wait'} = $waited;
-        my $reason = 'timeout waiting for cluster-wide backup slot';
-        $reason .= " - $pending->{'request-error'}" if defined($pending->{'request-error'});
+        my $reason = 'start window ended';
+        if (!$window_ended) {
+            $reason = 'timeout waiting for cluster-wide backup slot';
+            $reason .= " - $pending->{'request-error'}" if defined($pending->{'request-error'});
+        }
         skip_task($task, $reason);
     } elsif (defined($pending->{key}) && !$pending->{logged}++) {
         my $reached = eval { PVE::VZDump::ClusterSlots::reached_limits($pending->{key}) };
-- 
2.47.3