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 2D5291FF09B for ; Mon, 28 Sep 2026 09:16:12 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 7EB4B21831; Mon, 28 Sep 2026 09:15:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dualfroz.com; s=dkim; t=1790314217; h=from:subject:date:message-id:to:mime-version: content-transfer-encoding; bh=jbXJdYsDStKpecjG3WfndOUG3RteiuYGwJJN9gZCKuI=; b=Zb3iDP4mUkZ8/svYQL9a5nqZBRWCuP2NbEJRdVA2OQ4vQQuv2wh7O1DGe1HwDoNsd41LEi ibY7oYNDDJUdXnNOkMc+YTKID5z489ugy7o5ODiPvXK33JeIsxQ58xLiagiLhMGYfATRTg 8bnQKRnSNo5bY3hyQgiDsa+89PNlRLZSayg0ngLFjpQGJWclL0vMX8MMyX9Ei58foE8UpX slAAbrxGQlaXrbF4clpV1AKkOF7t0/fAOfNrUedn05lW1qHDfzXvYadaCwg3o5o99LsqLv 8asWvVaCMHl9vaKlTzbaqK3L98HidnWWqoFhmWbqCVuoszBp7ajBKoxho3Uc/A== From: Michal Fox To: pve-devel@lists.proxmox.com Subject: [PATCH manager] fix #7860: api: backup info: skip jobs with invalid guest selection Date: Fri, 25 Sep 2026 05:30:16 +0000 Message-ID: <20260925053016.7-1-me@dualfroz.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Last-TLS-Session-Version: TLSv1.3 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.143 Adjusted score from AWL reputation of From: address DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain DMARC_PASS -0.1 DMARC pass policy POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record X-MailFrom: me@dualfroz.com X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation Message-ID-Hash: IMP2FANRIQLTGIZQE74S5ZQBU7BB2YFK X-Message-ID-Hash: IMP2FANRIQLTGIZQE74S5ZQBU7BB2YFK X-Mailman-Approved-At: Mon, 28 Sep 2026 09:14:43 +0200 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 in pool mode keeps referencing its pool after the pool got deleted. Resolving the guests of such a job dies with "pool '' does not exist", and since get_included_vmids() did not catch this, the whole /cluster/backup-info/not-backed-up call failed. The GUI then hides the "Show guests without backup job" button, so users lose exactly the overview that would tell them some guests are no longer backed up. Catch the error per job, warn about it and continue with the remaining jobs. The broken job does not back up anything, so its guests correctly show up as not covered by any backup job. Add a test for get_included_vmids() covering this case. Signed-off-by: Michal Fox --- PVE/API2/Cluster/BackupInfo.pm | 7 +++- test/Makefile | 6 ++- test/backup_info_test.pl | 71 ++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) create mode 100755 test/backup_info_test.pl diff --git a/PVE/API2/Cluster/BackupInfo.pm b/PVE/API2/Cluster/BackupInfo.pm index 4ef59ac3..1ac59662 100644 --- a/PVE/API2/Cluster/BackupInfo.pm +++ b/PVE/API2/Cluster/BackupInfo.pm @@ -25,7 +25,12 @@ sub get_included_vmids { my $all_vmids = {}; for my $job ($legacy_jobs->@*, grep { $_->{type} eq 'vzdump' } values $jobs->{ids}->%*) { - my $job_included_guests = PVE::VZDump::get_included_guests($job); + # a job referencing e.g. a deleted pool backs up nothing, but must not break the overview + my $job_included_guests = eval { PVE::VZDump::get_included_guests($job) }; + if (my $err = $@) { + warn "ignoring backup job with invalid guest selection - $err"; + next; + } $all_vmids->{$_} = 1 for map { $_->@* } values %{$job_included_guests}; } diff --git a/test/Makefile b/test/Makefile index 2dfd20a4..adc1f17b 100644 --- a/test/Makefile +++ b/test/Makefile @@ -18,7 +18,7 @@ replication%.t: replication_test%.pl ./$< .PHONY: test-vzdump -test-vzdump: test-vzdump-guest-included test-vzdump-new +test-vzdump: test-vzdump-guest-included test-vzdump-new test-vzdump-backup-info .PHONY: test-vzdump-guest-included test-vzdump-guest-included: @@ -28,6 +28,10 @@ test-vzdump-guest-included: test-vzdump-new: ./vzdump_new_test.pl +.PHONY: test-vzdump-backup-info +test-vzdump-backup-info: + ./backup_info_test.pl + .PHONY: test-osd test-osd: ./OSD_test.pl diff --git a/test/backup_info_test.pl b/test/backup_info_test.pl new file mode 100755 index 00000000..54aece2e --- /dev/null +++ b/test/backup_info_test.pl @@ -0,0 +1,71 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use lib '..'; + +use Test::More; +use Test::MockModule; + +use PVE::API2::Cluster::BackupInfo; + +my $vmlist = { + ids => { + 100 => { type => 'qemu', node => 'node1' }, + 101 => { type => 'qemu', node => 'node1' }, + 102 => { type => 'lxc', node => 'node1' }, + }, +}; + +my $pools = { + testpool => [100], +}; + +my $jobs_cfg = { + ids => { + 'backup-pool' => { type => 'vzdump', pool => 'testpool' }, + 'backup-vmid' => { type => 'vzdump', vmid => '101' }, + }, +}; + +my $cluster_module = Test::MockModule->new('PVE::Cluster'); +$cluster_module->mock(get_vmlist => sub { return $vmlist }); + +my $api2tools_module = Test::MockModule->new('PVE::API2Tools'); +$api2tools_module->mock( + get_resource_pool_guest_members => sub { + my ($pool) = @_; + die "pool '$pool' does not exist\n" if !$pools->{$pool}; + return $pools->{$pool}; + }, +); + +my $backup_info_module = Test::MockModule->new('PVE::API2::Cluster::BackupInfo'); +$backup_info_module->mock( + cfs_read_file => sub { + my ($filename) = @_; + return { jobs => [] } if $filename eq 'vzdump.cron'; + return $jobs_cfg if $filename eq 'jobs.cfg'; + die "unexpected file '$filename'\n"; + }, +); + +is_deeply( + PVE::API2::Cluster::BackupInfo::get_included_vmids(), + { 100 => 1, 101 => 1 }, + 'guests of all backup jobs are included', +); + +$jobs_cfg->{ids}->{'backup-deleted-pool'} = { type => 'vzdump', pool => 'deletedpool' }; + +my @warnings; +my $included = eval { + local $SIG{__WARN__} = sub { push @warnings, $_[0] }; + PVE::API2::Cluster::BackupInfo::get_included_vmids(); +}; +is($@, '', 'job referencing a deleted pool does not make the lookup fail'); +is_deeply($included, { 100 => 1, 101 => 1 }, 'guests of the other jobs are still included'); +like($warnings[0] // '', qr/pool 'deletedpool' does not exist/, 'broken job is warned about'); + +done_testing(); -- 2.43.0