From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id B45471FF09C for ; Mon, 02 Nov 2026 00:22:47 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 29FE32176E; Mon, 02 Nov 2026 00:22:17 +0100 (CET) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Subject: [PATCH manager 3/5] vzdump: honor start window for guest backups Date: Mon, 2 Nov 2026 00:20:26 +0100 Message-ID: <20261101232205.976807-4-t.lamprecht@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261101232205.976807-1-t.lamprecht@proxmox.com> References: <20261101232205.976807-1-t.lamprecht@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793575330315 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.369 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 6WEXEQUEUJ5ECORRFJPRMENSL3MYYZ4F X-Message-ID-Hash: 6WEXEQUEUJ5ECORRFJPRMENSL3MYYZ4F X-MailFrom: t.lamprecht@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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