[pve-devel] [PATCH manager 4/6] backup: drop 'maxfiles' parameter

[Fiona Ebner <f.ebner@proxmox.com>]

The 'maxfiles' parameter has been deprecated since the addition of
'prune-backups' in the Proxmox VE 7 beta.

Drop the tests that only had maxfiles or both, but adapt the mixed
tests for CLI/backup/storage precedence.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/VZDump.pm      |  14 ++---
 PVE/VZDump.pm           |  23 ++------
 test/vzdump_new_test.pl | 120 ++--------------------------------------
 3 files changed, 16 insertions(+), 141 deletions(-)

diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index de48e108..a8f21eba 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -27,7 +27,7 @@ my sub assert_param_permission_vzdump {
 
     PVE::API2::Backup::assert_param_permission_common($rpcenv, $user, $param);
 
-    if (defined($param->{maxfiles}) || defined($param->{'prune-backups'})) {
+    if (defined($param->{'prune-backups'})) {
         if (my $storeid = PVE::VZDump::get_storage_param($param)) {
             $rpcenv->check($user, "/storage/$storeid", ['Datastore.Allocate']);
         }
@@ -40,12 +40,12 @@ __PACKAGE__->register_method({
     method => 'POST',
     description => "Create backup.",
     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 'maxfiles' and 'prune-backups' settings require "
-            . "'Datastore.Allocate' on the backup storage. The 'bwlimit', 'performance' and "
-            . "'ionice' parameters require 'Sys.Modify' on '/'.",
+        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 '/'.",
         user => 'all',
     },
     protected => 1,
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index cf71a9fa..defe9e0a 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -177,28 +177,16 @@ my sub merge_performance {
     return $res;
 }
 
-my $parse_prune_backups_maxfiles = sub {
+my $parse_prune_backups = sub {
     my ($param, $kind) = @_;
 
-    my $maxfiles = delete $param->{maxfiles};
     my $prune_backups = $param->{'prune-backups'};
 
-    debugmsg(
-        'warn',
-        "both 'maxfiles' and 'prune-backups' defined as ${kind} - ignoring 'maxfiles'",
-    ) if defined($maxfiles) && defined($prune_backups);
-
     if (defined($prune_backups)) {
         return $prune_backups if ref($prune_backups) eq 'HASH'; # already parsed
         $param->{'prune-backups'} = PVE::JSONSchema::parse_property_string(
             'prune-backups', $prune_backups,
         );
-    } elsif (defined($maxfiles)) {
-        if ($maxfiles) {
-            $param->{'prune-backups'} = { 'keep-last' => $maxfiles };
-        } else {
-            $param->{'prune-backups'} = { 'keep-all' => 1 };
-        }
     }
 
     return $param->{'prune-backups'};
@@ -335,7 +323,7 @@ sub read_vzdump_defaults {
             defined($default) ? ($_ => $default) : ()
         } keys $fleecing_fmt->%*
     };
-    $parse_prune_backups_maxfiles->($defaults, "defaults in VZDump schema");
+    $parse_prune_backups->($defaults, "defaults in VZDump schema");
 
     my $raw;
     eval { $raw = PVE::Tools::file_get_contents($fn); };
@@ -360,7 +348,7 @@ sub read_vzdump_defaults {
         my @mailto = split_list($res->{mailto});
         $res->{mailto} = [@mailto];
     }
-    $parse_prune_backups_maxfiles->($res, "options in '$fn'");
+    $parse_prune_backups->($res, "options in '$fn'");
     parse_fleecing($res);
     parse_performance($res);
 
@@ -1548,10 +1536,7 @@ sub verify_vzdump_parameters {
     raise_param_exc({ pool => "option conflicts with option 'vmid'" })
         if $param->{pool} && $param->{vmid};
 
-    raise_param_exc({ 'prune-backups' => "option conflicts with option 'maxfiles'" })
-        if defined($param->{'prune-backups'}) && defined($param->{maxfiles});
-
-    $parse_prune_backups_maxfiles->($param, 'CLI parameters');
+    $parse_prune_backups->($param, 'CLI parameters');
     parse_fleecing($param);
     parse_performance($param);
 
diff --git a/test/vzdump_new_test.pl b/test/vzdump_new_test.pl
index 36225ebb..db829e70 100755
--- a/test/vzdump_new_test.pl
+++ b/test/vzdump_new_test.pl
@@ -105,78 +105,6 @@ my @tests = (
         },
     },
     # TODO make parse error critical?
-    {
-        description => 'maxfiles vzdump 1',
-        vzdump_param => {
-            maxfiles => 0,
-        },
-        expected => {
-            'prune-backups' => {
-                'keep-all' => 1,
-            },
-            remove => 0,
-        },
-    },
-    {
-        description => 'maxfiles vzdump 2',
-        vzdump_param => {
-            maxfiles => 7,
-        },
-        expected => {
-            'prune-backups' => {
-                'keep-last' => 7,
-            },
-            remove => 1,
-        },
-    },
-    {
-        description => 'maxfiles storage 1',
-        storage_param => {
-            maxfiles => 0,
-        },
-        expected => {
-            'prune-backups' => {
-                'keep-all' => 1,
-            },
-            remove => 0,
-        },
-    },
-    {
-        description => 'maxfiles storage 2',
-        storage_param => {
-            maxfiles => 7,
-        },
-        expected => {
-            'prune-backups' => {
-                'keep-last' => 7,
-            },
-            remove => 1,
-        },
-    },
-    {
-        description => 'maxfiles CLI 1',
-        cli_param => {
-            maxfiles => 0,
-        },
-        expected => {
-            'prune-backups' => {
-                'keep-all' => 1,
-            },
-            remove => 0,
-        },
-    },
-    {
-        description => 'maxfiles CLI 2',
-        cli_param => {
-            maxfiles => 7,
-        },
-        expected => {
-            'prune-backups' => {
-                'keep-last' => 7,
-            },
-            remove => 1,
-        },
-    },
     {
         description => 'prune-backups vzdump 1',
         vzdump_param => {
@@ -219,19 +147,6 @@ my @tests = (
             remove => 0,
         },
     },
-    {
-        description => 'both vzdump 1',
-        vzdump_param => {
-            'prune-backups' => 'keep-all=1',
-            maxfiles => 7,
-        },
-        expected => {
-            'prune-backups' => {
-                'keep-all' => 1,
-            },
-            remove => 0,
-        },
-    },
     {
         description => 'prune-backups storage 1',
         storage_param => {
@@ -275,21 +190,6 @@ my @tests = (
             remove => 0,
         },
     },
-    {
-        description => 'both storage 1',
-        storage_param => {
-            'prune-backups' => 'keep-hourly=1,keep-monthly=2,keep-yearly=3',
-            maxfiles => 0,
-        },
-        expected => {
-            'prune-backups' => {
-                'keep-hourly' => 1,
-                'keep-monthly' => 2,
-                'keep-yearly' => 3,
-            },
-            remove => 1,
-        },
-    },
     {
         description => 'prune-backups CLI 1',
         cli_param => {
@@ -329,19 +229,10 @@ my @tests = (
         expected => "format error\n"
             . "foo: property is not defined in schema and the schema does not allow additional properties\n",
     },
-    {
-        description => 'both CLI 1',
-        cli_param => {
-            'prune-backups' => 'keep-hourly=1,keep-monthly=2,keep-yearly=3',
-            maxfiles => 4,
-        },
-        expected => "400 Parameter verification failed.\n"
-            . "prune-backups: option conflicts with option 'maxfiles'\n",
-    },
     {
         description => 'mixed 1',
         vzdump_param => {
-            maxfiles => 7,
+            'prune-backups' => 'keep-last=7',
         },
         storage_param => {
             'prune-backups' => 'keep-hourly=24',
@@ -357,7 +248,7 @@ my @tests = (
     {
         description => 'mixed 2',
         vzdump_param => {
-            maxfiles => 7,
+            'prune-backups' => 'keep-last=7',
         },
         storage_param => {
             'prune-backups' => 'keephourly=24',
@@ -372,7 +263,7 @@ my @tests = (
     {
         description => 'mixed 3',
         vzdump_param => {
-            maxfiles => 7,
+            'prune-backups' => 'keep-last=7',
         },
         cli_param => {
             'prune-backups' => 'keep-all=1',
@@ -387,7 +278,7 @@ my @tests = (
     {
         description => 'mixed 4',
         vzdump_param => {
-            maxfiles => 7,
+            'prune-backups' => 'keep-last=7',
         },
         storage_param => {
             'prune-backups' => 'keep-all=0,keep-last=10',
@@ -405,7 +296,7 @@ my @tests = (
     {
         description => 'mixed 5',
         vzdump_param => {
-            maxfiles => 7,
+            'prune-backups' => 'keep-last=7',
         },
         storage_param => {
             'prune-backups' => 'keep-all=0,keep-last=10',
@@ -650,7 +541,6 @@ foreach my $test (@tests) {
         my $vzdump = PVE::VZDump->new('fake cmdline', $test->{cli_param}, undef);
 
         my $opts = $vzdump->{opts} or die "did not get options\n";
-        die "maxfiles is defined" if defined($opts->{maxfiles});
 
         my $res = {};
         foreach my $opt (@{$tested_options}) {
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel