and make the two options mutally exclusive. 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.eb...@proxmox.com> --- PVE/VZDump.pm | 83 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 58 insertions(+), 25 deletions(-) diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm index e8669665..e7191d8c 100644 --- a/PVE/VZDump.pm +++ b/PVE/VZDump.pm @@ -89,6 +89,9 @@ sub storage_info { maxfiles => $scfg->{maxfiles}, }; + $info->{'prune-backups'} = PVE::JSONSchema::parse_property_string('prune-backups', $scfg->{'prune-backups'}) + if defined($scfg->{'prune-backups'}); + if ($type eq 'pbs') { $info->{pbs} = 1; } else { @@ -459,12 +462,15 @@ 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}; + $opts->{maxfiles} //= $info->{maxfiles}; + $opts->{'prune-backups'} = $info->{'prune-backups'}; + } } elsif ($opts->{dumpdir}) { $errors .= "dumpdir '$opts->{dumpdir}' does not exist" if ! -d $opts->{dumpdir}; @@ -472,7 +478,12 @@ sub new { die "internal error"; } - $opts->{maxfiles} //= $defaults->{maxfiles}; + if (defined($opts->{maxfiles}) && defined($opts->{'prune-backups'})) { + $errors .= "\n" if $errors; + $errors .= "cannot have 'maxfiles' and 'prune-backups' configured at the same time"; + } elsif (!defined($opts->{'prune-backups'})) { + $opts->{maxfiles} //= $defaults->{maxfiles}; + } if ($opts->{tmpdir} && ! -d $opts->{tmpdir}) { $errors .= "\n" if $errors; @@ -651,6 +662,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(); @@ -703,8 +715,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); @@ -713,10 +735,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}) { @@ -928,22 +950,33 @@ sub exec_backup_task { } # purge older backup - if ($maxfiles && $opts->{remove}) { + 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->{tarfile}); + $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 ($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->{tarfile}); - $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 @prune_list = PVE::Storage::prune_backups($cfg, $opts->{storage}, $prune_options, $vmid, 0); + foreach my $prune_entry (@prune_list) { + if ($prune_entry->{mark} eq 'remove') { + my $volid = $prune_entry->{volid}; + debugmsg ('info', "delete old backup '$volid'", $logfd); + } } } } -- 2.20.1 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel