Allow prune-backups as an alternative to maxfiles

and make the two options mutally exclusive as long
as they are specified on the same level (e.g. both
from the storage configuration). Otherwise prefer
option > storage config > default (only maxfiles has a default currently).

Defines the backup limit for prune-backups as the sum of all
keep-values.

There is no perfect way to determine whether a
new backup would trigger a removal with prune later:
1. we would need a way to include the not yet existing backup
   in a 'prune --dry-run' check.
2. even if we had that check, if it's executed right before
   a full hour, and the actual backup happens after the full
   hour, the information from the check is not correct.

So in some cases, we allow backup jobs with remove=0, that
will lead to a removal when the next prune is executed.
Still, the job with remove=0 does not execute a prune, so:
1. There is a well-defined limit.
2. A job with remove=0 never removes an old backup.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>

diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 2eda973e59ce9c65bcae7efab5de53d4e93abedb..19fa1e3bb0300c9a6ec81732cfa546668140db80 100644 (file)
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -25,7 +25,7 @@ __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. The 'maxfiles', 'tmpdir', 'dumpdir', 'script', 'bwlimit' and 'ionice' parameters are restricted to the 'root\@pam' user.",
+       description => "The user needs 'VM.Backup' permissions on any VM, and 'Datastore.AllocateSpace' on the backup storage. The 'maxfiles', 'prune-backups', 'tmpdir', 'dumpdir', 'script', 'bwlimit' and 'ionice' parameters are restricted to the 'root\@pam' user.",
        user => 'all',
     },
     protected => 1,
@@ -58,7 +58,7 @@ __PACKAGE__->register_method ({
                if $param->{stdout};
        }
 
-       foreach my $key (qw(maxfiles tmpdir dumpdir script bwlimit ionice)) {
+       foreach my $key (qw(maxfiles prune-backups tmpdir dumpdir script bwlimit ionice)) {
            raise_param_exc({ $key => "Only root may set this option."})
                if defined($param->{$key}) && ($user ne 'root@pam');
        }

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index aea7389beed34a636b95f3a48d80d7341a71c293..ecf0b7deff5442ed0d743581491aa4bb2ce9b386 100644 (file)
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -89,6 +89,12 @@ sub storage_info {
        maxfiles => $scfg->{maxfiles},
     };
 
+    $info->{'prune-backups'} = PVE::JSONSchema::parse_property_string('prune-backups', $scfg->{'prune-backups'})
+       if defined($scfg->{'prune-backups'});
+
+    die "cannot have 'maxfiles' and 'prune-backups' configured at the same time\n"
+       if defined($info->{'prune-backups'}) && defined($info->{maxfiles});
+
     if ($type eq 'pbs') {
        $info->{pbs} = 1;
     } else {
@@ -459,12 +465,18 @@ sub new {
 
     if ($opts->{storage}) {
        my $info = eval { storage_info ($opts->{storage}) };
-       $errors .= "could not get storage information for '$opts->{storage}': $@"
-           if ($@);
-       $opts->{dumpdir} = $info->{dumpdir};
-       $opts->{scfg} = $info->{scfg};
-       $opts->{pbs} = $info->{pbs};
-       $opts->{maxfiles} //= $info->{maxfiles};
+       if (my $err = $@) {
+           $errors .= "could not get storage information for '$opts->{storage}': $err";
+       } else {
+           $opts->{dumpdir} = $info->{dumpdir};
+           $opts->{scfg} = $info->{scfg};
+           $opts->{pbs} = $info->{pbs};
+
+           if (!defined($opts->{'prune-backups'}) && !defined($opts->{maxfiles})) {
+               $opts->{'prune-backups'} = $info->{'prune-backups'};
+               $opts->{maxfiles} = $info->{maxfiles};
+           }
+       }
     } elsif ($opts->{dumpdir}) {
        $errors .= "dumpdir '$opts->{dumpdir}' does not exist"
            if ! -d $opts->{dumpdir};
@@ -472,7 +484,9 @@ sub new {
        die "internal error";
     }
 
-    $opts->{maxfiles} //= $defaults->{maxfiles};
+    if (!defined($opts->{'prune-backups'}) && !defined($opts->{maxfiles})) {
+       $opts->{maxfiles} = $defaults->{maxfiles};
+    }
 
     if ($opts->{tmpdir} && ! -d $opts->{tmpdir}) {
        $errors .= "\n" if $errors;
@@ -653,6 +667,7 @@ sub exec_backup_task {
 
     my $opts = $self->{opts};
 
+    my $cfg = PVE::Storage::config();
     my $vmid = $task->{vmid};
     my $plugin = $task->{plugin};
     my $vmtype = $plugin->type();
@@ -706,8 +721,18 @@ sub exec_backup_task {
        my $basename = $bkname . strftime("-%Y_%m_%d-%H_%M_%S", localtime($task->{backup_time}));
 
        my $maxfiles = $opts->{maxfiles};
+       my $prune_options = $opts->{'prune-backups'};
+
+       my $backup_limit = 0;
+       if (defined($maxfiles)) {
+           $backup_limit = $maxfiles;
+       } elsif (defined($prune_options)) {
+           foreach my $keep (values %{$prune_options}) {
+               $backup_limit += $keep;
+           }
+       }
 
-       if ($maxfiles && !$opts->{remove}) {
+       if ($backup_limit && !$opts->{remove}) {
            my $count;
            if ($self->{opts}->{pbs}) {
                my $res = PVE::Storage::PBSPlugin::run_client_cmd($opts->{scfg}, $opts->{storage}, 'snapshots', $pbs_group_name);
@@ -716,10 +741,10 @@ sub exec_backup_task {
                my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname);
                $count = scalar(@$bklist);
            }
-           die "There is a max backup limit of ($maxfiles) enforced by the".
+           die "There is a max backup limit of $backup_limit enforced by the".
                " target storage or the vzdump parameters.".
                " Either increase the limit or delete old backup(s).\n"
-               if $count >= $maxfiles;
+               if $count >= $backup_limit;
        }
 
        if (!$self->{opts}->{pbs}) {
@@ -926,23 +951,28 @@ sub exec_backup_task {
        }
 
        # purge older backup
-       if ($maxfiles && $opts->{remove}) {
-
-           if ($self->{opts}->{pbs}) {
-               my $args = [$pbs_group_name, '--quiet', '1', '--keep-last', $maxfiles];
-               my $logfunc = sub { my $line = shift; debugmsg ('info', $line, $logfd); };
-               PVE::Storage::PBSPlugin::run_raw_client_cmd(
-                   $opts->{scfg}, $opts->{storage}, 'prune', $args, logfunc => $logfunc);
-           } else {
-               my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname, $task->{target});
-               $bklist = [ sort { $b->{ctime} <=> $a->{ctime} } @$bklist ];
-
-               while (scalar (@$bklist) >= $maxfiles) {
-                   my $d = pop @$bklist;
-                   my $archive_path = $d->{path};
-                   debugmsg ('info', "delete old backup '$archive_path'", $logfd);
-                   PVE::Storage::archive_remove($archive_path);
+       if ($opts->{remove}) {
+           if ($maxfiles) {
+
+               if ($self->{opts}->{pbs}) {
+                   my $args = [$pbs_group_name, '--quiet', '1', '--keep-last', $maxfiles];
+                   my $logfunc = sub { my $line = shift; debugmsg ('info', $line, $logfd); };
+                   PVE::Storage::PBSPlugin::run_raw_client_cmd(
+                       $opts->{scfg}, $opts->{storage}, 'prune', $args, logfunc => $logfunc);
+               } else {
+                   my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname, $task->{target});
+                   $bklist = [ sort { $b->{ctime} <=> $a->{ctime} } @$bklist ];
+
+                   while (scalar (@$bklist) >= $maxfiles) {
+                       my $d = pop @$bklist;
+                       my $archive_path = $d->{path};
+                       debugmsg ('info', "delete old backup '$archive_path'", $logfd);
+                       PVE::Storage::archive_remove($archive_path);
+                   }
                }
+           } elsif (defined($prune_options)) {
+               my $logfunc = sub { debugmsg($_[0], $_[1], $logfd) };
+               PVE::Storage::prune_backups($cfg, $opts->{storage}, $prune_options, $vmid, $vmtype, 0, $logfunc);
            }
        }
 
@@ -1129,6 +1159,12 @@ 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});
+
+    $param->{'prune-backups'} = PVE::JSONSchema::parse_property_string('prune-backups', $param->{'prune-backups'})
+       if defined($param->{'prune-backups'});
+
     $param->{all} = 1 if (defined($param->{exclude}) && !$param->{pool});
 
     warn "option 'size' is deprecated and will be removed in a future " .