[PATCH manager 0/2] fix #7860: api: pool: do not delete when used by backup job

[PATCH manager 0/2] fix #7860: api: pool: do not delete when used by backup job

[Elias Huhsovitz]

Deleting a pool that is still used by a backup job leaves the job in
an invalid state. DELETE `/cluster/backup-info/not-backed-up` then
fails with "pool '<name>' does not exist", which causes the GUI to
hide the "Show guests without backup job" button (bug #7860).

This small series aims to prevent the invalid configuration state.

Summary of changes
------------------
Elias Huhsovitz (2):
  fix #7860: api: pool: do not delete when used by backup job
  api: pool: update delete enpoint description to match behaviour

 PVE/API2/Pool.pm | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

-- 
2.47.3

[PATCH manager 1/2] fix #7860: api: pool: do not delete when used by backup job

[Elias Huhsovitz]

Currently, deleting a pool that is used by a backup is allowed. This
results in a suboptimal configuration where calling the API endpoint
`/cluster/backup-info/not-backed-up` fails, because it tries to query
the VMs from the non-existent pool.

Mitigate this issue by not allowing pools used by backups to be deleted.

Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
 PVE/API2/Pool.pm | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/PVE/API2/Pool.pm b/PVE/API2/Pool.pm
index 63aff5bb..a0ff8a31 100644
--- a/PVE/API2/Pool.pm
+++ b/PVE/API2/Pool.pm
@@ -475,6 +475,19 @@ __PACKAGE__->register_method({
     },
 });
 
+my sub check_pool_used_in_backup_job {
+    my ($pool) = @_;
+
+    my $backup_jobs = cfs_read_file('jobs.cfg');
+
+    for my $job_id (keys $backup_jobs->{ids}->%*) {
+        my $referenced_pool = $backup_jobs->{ids}->{$job_id}->{pool};
+        next if !defined($referenced_pool);
+        next if $referenced_pool ne $pool;
+        die "pool '$pool' is used in backup job '$job_id'\n";
+    }
+}
+
 __PACKAGE__->register_method({
     name => 'delete_pool',
     protected => 1,
@@ -524,6 +537,8 @@ __PACKAGE__->register_method({
                     die "pool '$pool' is not empty (contains storage '$storeid')\n";
                 }
 
+                check_pool_used_in_backup_job($pool);
+
                 delete($usercfg->{pools}->{$pool});
                 PVE::AccessControl::delete_pool_acl($pool, $usercfg);
 
-- 
2.47.3

[PATCH manager 2/2] api: pool: update delete enpoint description to match behaviour

[Elias Huhsovitz]

Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
 PVE/API2/Pool.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Pool.pm b/PVE/API2/Pool.pm
index a0ff8a31..2cb10fb1 100644
--- a/PVE/API2/Pool.pm
+++ b/PVE/API2/Pool.pm
@@ -494,10 +494,11 @@ __PACKAGE__->register_method({
     path => '',
     method => 'DELETE',
     permissions => {
-        description => "You can only delete empty pools (no members).",
+        description => "Requires 'Pool.Allocate' on '/pool/{poolid}'.",
         check => ['perm', '/pool/{poolid}', ['Pool.Allocate']],
     },
-    description => "Delete pool.",
+    description => "Delete pool. A pool can only be deleted if it is empty "
+        . "(no sub-pools, VMs, or storage) and is not referenced by any backup job.",
     parameters => {
         additionalProperties => 0,
         properties => {
-- 
2.47.3