* [pve-devel] [PATCH v4 storage 1/7] Introduce prune-backups property for directory-based storages
2020-07-09 12:45 [pve-devel] [PATCH-SERIES v4] fix #2649: introduce prune-backups property for storages supporting backups Fabian Ebner
@ 2020-07-09 12:45 ` Fabian Ebner
2020-07-24 17:07 ` [pve-devel] applied: " Thomas Lamprecht
2020-07-09 12:45 ` [pve-devel] [PATCH v4 storage 2/7] Add prune_backups to storage API Fabian Ebner
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Fabian Ebner @ 2020-07-09 12:45 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
Changes from v3:
* use property string validator
PVE/Storage/CIFSPlugin.pm | 1 +
PVE/Storage/CephFSPlugin.pm | 1 +
PVE/Storage/DirPlugin.pm | 5 +--
PVE/Storage/GlusterfsPlugin.pm | 5 +--
PVE/Storage/NFSPlugin.pm | 5 +--
PVE/Storage/PBSPlugin.pm | 1 +
PVE/Storage/Plugin.pm | 59 +++++++++++++++++++++++++++++++++-
7 files changed, 70 insertions(+), 7 deletions(-)
diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index 72ec757..6edbc9d 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -134,6 +134,7 @@ sub options {
nodes => { optional => 1 },
disable => { optional => 1 },
maxfiles => { optional => 1 },
+ 'prune-backups' => { optional => 1 },
content => { optional => 1 },
format => { optional => 1 },
username => { optional => 1 },
diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
index 6575f4f..880ec05 100644
--- a/PVE/Storage/CephFSPlugin.pm
+++ b/PVE/Storage/CephFSPlugin.pm
@@ -150,6 +150,7 @@ sub options {
fuse => { optional => 1 },
bwlimit => { optional => 1 },
maxfiles => { optional => 1 },
+ 'prune-backups' => { optional => 1 },
};
}
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index 39760a8..3c81d24 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -49,10 +49,11 @@ sub properties {
sub options {
return {
path => { fixed => 1 },
- nodes => { optional => 1 },
+ nodes => { optional => 1 },
shared => { optional => 1 },
disable => { optional => 1 },
- maxfiles => { optional => 1 },
+ maxfiles => { optional => 1 },
+ 'prune-backups' => { optional => 1 },
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
index 70ea4fc..2dd414d 100644
--- a/PVE/Storage/GlusterfsPlugin.pm
+++ b/PVE/Storage/GlusterfsPlugin.pm
@@ -129,9 +129,10 @@ sub options {
server2 => { optional => 1 },
volume => { fixed => 1 },
transport => { optional => 1 },
- nodes => { optional => 1 },
+ nodes => { optional => 1 },
disable => { optional => 1 },
- maxfiles => { optional => 1 },
+ maxfiles => { optional => 1 },
+ 'prune-backups' => { optional => 1 },
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index 82b0c5f..6abb24b 100644
--- a/PVE/Storage/NFSPlugin.pm
+++ b/PVE/Storage/NFSPlugin.pm
@@ -79,9 +79,10 @@ sub options {
path => { fixed => 1 },
server => { fixed => 1 },
export => { fixed => 1 },
- nodes => { optional => 1 },
+ nodes => { optional => 1 },
disable => { optional => 1 },
- maxfiles => { optional => 1 },
+ maxfiles => { optional => 1 },
+ 'prune-backups' => { optional => 1 },
options => { optional => 1 },
content => { optional => 1 },
format => { optional => 1 },
diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index b236f6c..87b09f2 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -55,6 +55,7 @@ sub options {
password => { optional => 1 },
'encryption-key' => { optional => 1 },
maxfiles => { optional => 1 },
+ 'prune-backups' => { optional => 1 },
fingerprint => { optional => 1 },
};
}
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 7f04e85..6e321ae 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -10,7 +10,7 @@ use File::Basename;
use File::stat qw();
use PVE::Tools qw(run_command);
-use PVE::JSONSchema qw(get_standard_option);
+use PVE::JSONSchema qw(get_standard_option register_standard_option);
use PVE::Cluster qw(cfs_register_file);
use JSON;
@@ -43,6 +43,62 @@ cfs_register_file ('storage.cfg',
sub { __PACKAGE__->parse_config(@_); },
sub { __PACKAGE__->write_config(@_); });
+my %prune_option = (
+ optional => 1,
+ type => 'integer', minimum => '0',
+ format_description => 'N',
+);
+
+my $prune_backups_format = {
+ 'keep-last' => {
+ %prune_option,
+ description => 'Keep the last <N> backups.',
+ },
+ 'keep-hourly' => {
+ %prune_option,
+ description => 'Keep backups for the last <N> different hours. If there is more' .
+ 'than one backup for a single hour, only the latest one is kept.'
+ },
+ 'keep-daily' => {
+ %prune_option,
+ description => 'Keep backups for the last <N> different days. If there is more' .
+ 'than one backup for a single day, only the latest one is kept.'
+ },
+ 'keep-weekly' => {
+ %prune_option,
+ description => 'Keep backups for the last <N> different weeks. If there is more' .
+ 'than one backup for a single week, only the latest one is kept.'
+ },
+ 'keep-monthly' => {
+ %prune_option,
+ description => 'Keep backups for the last <N> different months. If there is more' .
+ 'than one backup for a single month, only the latest one is kept.'
+ },
+ 'keep-yearly' => {
+ %prune_option,
+ description => 'Keep backups for the last <N> different years. If there is more' .
+ 'than one backup for a single year, only the latest one is kept.'
+ },
+};
+PVE::JSONSchema::register_format('prune-backups', $prune_backups_format, \&validate_prune_backups);
+sub validate_prune_backups {
+ my ($keep) = @_;
+
+ die "at least one keep-option must be set and positive\n"
+ if !grep { $_ } values %{$keep};
+
+ return $keep;
+}
+register_standard_option('prune-backups', {
+ description => "The retention options with shorter intervals are processed first " .
+ "with --keep-last being the very first one. Each option covers a " .
+ "specific period of time. We say that backups within this period " .
+ "are covered by this option. The next option does not take care " .
+ "of already covered backups and only considers older backups.",
+ optional => 1,
+ type => 'string',
+ format => 'prune-backups',
+});
my $defaultData = {
propertyList => {
@@ -68,6 +124,7 @@ my $defaultData = {
minimum => 0,
optional => 1,
},
+ 'prune-backups' => get_standard_option('prune-backups'),
shared => {
description => "Mark storage as shared.",
type => 'boolean',
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pve-devel] [PATCH v4 storage 2/7] Add prune_backups to storage API
2020-07-09 12:45 [pve-devel] [PATCH-SERIES v4] fix #2649: introduce prune-backups property for storages supporting backups Fabian Ebner
2020-07-09 12:45 ` [pve-devel] [PATCH v4 storage 1/7] Introduce prune-backups property for directory-based storages Fabian Ebner
@ 2020-07-09 12:45 ` Fabian Ebner
2020-07-24 17:07 ` [pve-devel] applied: " Thomas Lamprecht
2020-07-09 12:45 ` [pve-devel] [PATCH v4 storage 3/7] Add API and pvesm call for prune_backups Fabian Ebner
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Fabian Ebner @ 2020-07-09 12:45 UTC (permalink / raw)
To: pve-devel
Implement it for generic storages supporting backups
(i.e. directory-based storages) and add a wrapper for PBS.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
Changes from v3:
* prune_mark: merge loops
* allow filtering by type
* more error handling
* allow passing along a log function
* add more tests
PVE/Storage.pm | 91 +++++++++-
PVE/Storage/PBSPlugin.pm | 68 ++++++++
PVE/Storage/Plugin.pm | 65 +++++++
test/prune_backups_test.pm | 342 +++++++++++++++++++++++++++++++++++++
test/run_plugin_tests.pl | 1 +
5 files changed, 565 insertions(+), 2 deletions(-)
create mode 100644 test/prune_backups_test.pm
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index edf9a2e..5c44386 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -40,11 +40,11 @@ use PVE::Storage::DRBDPlugin;
use PVE::Storage::PBSPlugin;
# Storage API version. Icrement it on changes in storage API interface.
-use constant APIVER => 5;
+use constant APIVER => 6;
# Age is the number of versions we're backward compatible with.
# This is like having 'current=APIVER' and age='APIAGE' in libtool,
# see https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
-use constant APIAGE => 4;
+use constant APIAGE => 5;
# load standard plugins
PVE::Storage::DirPlugin->register();
@@ -1540,6 +1540,93 @@ sub extract_vzdump_config {
}
}
+sub prune_backups {
+ my ($cfg, $storeid, $keep, $vmid, $type, $dryrun, $logfunc) = @_;
+
+ my $scfg = storage_config($cfg, $storeid);
+ die "storage '$storeid' does not support backups\n" if !$scfg->{content}->{backup};
+
+ if (!defined($keep)) {
+ die "no prune-backups options configured for storage '$storeid'\n"
+ if !defined($scfg->{'prune-backups'});
+ $keep = PVE::JSONSchema::parse_property_string('prune-backups', $scfg->{'prune-backups'});
+ }
+
+ my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+ return $plugin->prune_backups($scfg, $storeid, $keep, $vmid, $type, $dryrun, $logfunc);
+}
+
+my $prune_mark = sub {
+ my ($prune_entries, $keep_count, $id_func) = @_;
+
+ return if !$keep_count;
+
+ my $already_included = {};
+ my $newly_included = {};
+
+ foreach my $prune_entry (@{$prune_entries}) {
+ my $mark = $prune_entry->{mark};
+ my $id = $id_func->($prune_entry->{ctime});
+
+ next if $already_included->{$id};
+
+ if (defined($mark)) {
+ $already_included->{$id} = 1 if $mark eq 'keep';
+ next;
+ }
+
+ if (!$newly_included->{$id}) {
+ last if scalar(keys %{$newly_included}) >= $keep_count;
+ $newly_included->{$id} = 1;
+ $prune_entry->{mark} = 'keep';
+ } else {
+ $prune_entry->{mark} = 'remove';
+ }
+ }
+};
+
+sub prune_mark_backup_group {
+ my ($backup_group, $keep) = @_;
+
+ my $prune_list = [ sort { $b->{ctime} <=> $a->{ctime} } @{$backup_group} ];
+
+ $prune_mark->($prune_list, $keep->{'keep-last'}, sub {
+ my ($ctime) = @_;
+ return $ctime;
+ });
+ $prune_mark->($prune_list, $keep->{'keep-hourly'}, sub {
+ my ($ctime) = @_;
+ my (undef, undef, $hour, $day, $month, $year) = localtime($ctime);
+ return "$hour/$day/$month/$year";
+ });
+ $prune_mark->($prune_list, $keep->{'keep-daily'}, sub {
+ my ($ctime) = @_;
+ my (undef, undef, undef, $day, $month, $year) = localtime($ctime);
+ return "$day/$month/$year";
+ });
+ $prune_mark->($prune_list, $keep->{'keep-weekly'}, sub {
+ my ($ctime) = @_;
+ my ($sec, $min, $hour, $day, $month, $year) = localtime($ctime);
+ my $iso_week = int(strftime("%V", $sec, $min, $hour, $day, $month - 1, $year - 1900));
+ my $iso_week_year = int(strftime("%G", $sec, $min, $hour, $day, $month - 1, $year - 1900));
+ return "$iso_week/$iso_week_year";
+ });
+ $prune_mark->($prune_list, $keep->{'keep-monthly'}, sub {
+ my ($ctime) = @_;
+ my (undef, undef, undef, undef, $month, $year) = localtime($ctime);
+ return "$month/$year";
+ });
+ $prune_mark->($prune_list, $keep->{'keep-yearly'}, sub {
+ my ($ctime) = @_;
+ my $year = (localtime($ctime))[5];
+ return "$year";
+ });
+
+ foreach my $prune_entry (@{$prune_list}) {
+ $prune_entry->{mark} //= 'remove';
+ }
+}
+
sub volume_export {
my ($cfg, $fh, $volid, $format, $snapshot, $base_snapshot, $with_snapshots) = @_;
diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index 87b09f2..12f56a9 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -258,6 +258,74 @@ sub extract_vzdump_config {
return $config;
}
+sub prune_backups {
+ my ($class, $scfg, $storeid, $keep, $vmid, $type, $dryrun, $logfunc) = @_;
+
+ $logfunc //= sub { print "$_[1]\n" };
+
+ my $backups = $class->list_volumes($storeid, $scfg, $vmid, ['backup']);
+
+ $type = 'vm' if defined($type) && $type eq 'qemu';
+ $type = 'ct' if defined($type) && $type eq 'lxc';
+
+ my $backup_groups = {};
+ foreach my $backup (@{$backups}) {
+ (my $backup_type = $backup->{format}) =~ s/^pbs-//;
+
+ next if defined($type) && $backup_type ne $type;
+
+ my $backup_group = "$backup_type/$backup->{vmid}";
+ $backup_groups->{$backup_group} = 1;
+ }
+
+ my @param;
+ foreach my $opt (keys %{$keep}) {
+ push @param, "--$opt";
+ push @param, "$keep->{$opt}";
+ }
+
+ push @param, '--dry-run' if $dryrun;
+
+ my $prune_list = [];
+ my $failed;
+
+ foreach my $backup_group (keys %{$backup_groups}) {
+ $logfunc->('info', "running 'proxmox-backup-client prune' for '$backup_group'")
+ if !$dryrun;
+ eval {
+ my $res = run_client_cmd($scfg, $storeid, 'prune', [ $backup_group, @param ]);
+
+ foreach my $backup (@{$res}) {
+ die "result from proxmox-backup-client is not as expected\n"
+ if !defined($backup->{'backup-time'})
+ || !defined($backup->{'backup-type'})
+ || !defined($backup->{'backup-id'})
+ || !defined($backup->{'keep'});
+
+ my $ctime = $backup->{'backup-time'};
+ my $type = $backup->{'backup-type'};
+ my $vmid = $backup->{'backup-id'};
+ my $volid = print_volid($storeid, $type, $vmid, $ctime);
+
+ push @{$prune_list}, {
+ ctime => $ctime,
+ mark => $backup->{keep} ? 'keep' : 'remove',
+ type => $type eq 'vm' ? 'qemu' : 'lxc',
+ vmid => $vmid,
+ volid => $volid,
+ };
+ }
+ };
+ if (my $err = $@) {
+ $logfunc->('err', "prune '$backup_group': $err\n");
+ $failed = 1;
+ }
+ }
+ die "error pruning backups - check log\n" if $failed;
+
+ return $prune_list;
+}
+
my $autogen_encryption_key = sub {
my ($scfg, $storeid) = @_;
my $encfile = pbs_encryption_key_file_name($scfg, $storeid);
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 6e321ae..8a58ff4 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -1174,6 +1174,71 @@ sub check_connection {
return 1;
}
+sub prune_backups {
+ my ($class, $scfg, $storeid, $keep, $vmid, $type, $dryrun, $logfunc) = @_;
+
+ $logfunc //= sub { print "$_[1]\n" };
+
+ my $backups = $class->list_volumes($storeid, $scfg, $vmid, ['backup']);
+
+ my $backup_groups = {};
+ my $prune_list = [];
+
+ foreach my $backup (@{$backups}) {
+ my $volid = $backup->{volid};
+ my $backup_vmid = $backup->{vmid};
+ my $archive_info = eval { PVE::Storage::archive_info($volid) } // {};
+ my $backup_type = $archive_info->{type} // 'unknown';
+
+ next if defined($type) && $type ne $backup_type;
+
+ my $prune_entry = {
+ ctime => $backup->{ctime},
+ type => $backup_type,
+ volid => $volid,
+ };
+
+ $prune_entry->{vmid} = $backup_vmid if defined($backup_vmid);
+
+ if ($archive_info->{is_std_name}) {
+ $prune_entry->{ctime} = $archive_info->{ctime};
+ my $group = "$backup_type/$backup_vmid";
+ push @{$backup_groups->{$group}}, $prune_entry;
+ } else {
+ # ignore backups that don't use the standard naming scheme
+ $prune_entry->{mark} = 'protected';
+ }
+
+ push @{$prune_list}, $prune_entry;
+ }
+
+ foreach my $backup_group (values %{$backup_groups}) {
+ PVE::Storage::prune_mark_backup_group($backup_group, $keep);
+ }
+
+ my $failed;
+ if (!$dryrun) {
+ foreach my $prune_entry (@{$prune_list}) {
+ next if $prune_entry->{mark} ne 'remove';
+
+ my $volid = $prune_entry->{volid};
+ $logfunc->('info', "removing backup '$volid'");
+ eval {
+ my (undef, $volname) = parse_volume_id($volid);
+ my $archive_path = $class->filesystem_path($scfg, $volname);
+ PVE::Storage::archive_remove($archive_path);
+ };
+ if (my $err = $@) {
+ $logfunc->('err', "error when removing backup '$volid' - $err\n");
+ $failed = 1;
+ }
+ }
+ }
+ die "error pruning backups - check log\n" if $failed;
+
+ return $prune_list;
+}
+
# Import/Export interface:
# Any path based storage is assumed to support 'raw' and 'tar' streams, so
# the default implementations will return this if $scfg->{path} is set,
diff --git a/test/prune_backups_test.pm b/test/prune_backups_test.pm
new file mode 100644
index 0000000..1e6a3d1
--- /dev/null
+++ b/test/prune_backups_test.pm
@@ -0,0 +1,342 @@
+package PVE::Storage::TestPruneBackups;
+
+use strict;
+use warnings;
+
+use lib qw(..);
+
+use PVE::Storage;
+use Test::More;
+use Test::MockModule;
+
+my $storeid = 'BackTest123';
+my @vmids = (1234, 9001);
+
+# only includes the information needed for prune_backups
+my $mocked_backups_lists = {};
+
+my $basetime = 1577881101; # 2020_01_01-12_18_21 UTC
+
+foreach my $vmid (@vmids) {
+ push @{$mocked_backups_lists->{default}}, (
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2018_05_26-11_18_21.tar.zst",
+ 'ctime' => $basetime - 585*24*60*60 - 60*60,
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2019_12_31-11_18_21.tar.zst",
+ 'ctime' => $basetime - 24*60*60 - 60*60,
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2019_12_31-11_19_21.tar.zst",
+ 'ctime' => $basetime - 24*60*60 - 60*60 + 60,
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2020_01_01-11_18_21.tar.zst",
+ 'ctime' => $basetime - 60*60,
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2020_01_01-12_18_21.tar.zst",
+ 'ctime' => $basetime,
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-lxc-$vmid-2020_01_01-12_18_21.tar.zst",
+ 'ctime' => $basetime,
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-$vmid-renamed.tar.zst",
+ 'ctime' => 1234,
+ 'vmid' => $vmid,
+ },
+ );
+}
+push @{$mocked_backups_lists->{year1970}}, (
+ {
+ 'volid' => "$storeid:backup/vzdump-lxc-321-1970_01_01-00_01_23.tar.zst",
+ 'ctime' => 83,
+ 'vmid' => 321,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-lxc-321-2070_01_01-00_01_00.tar.zst",
+ 'ctime' => 60*60*24 * (365*100 + 25) + 60,
+ 'vmid' => 321,
+ },
+);
+push @{$mocked_backups_lists->{novmid}}, (
+ {
+ 'volid' => "$storeid:backup/vzdump-lxc-novmid.tar.gz",
+ 'ctime' => 1234,
+ },
+);
+my $current_list;
+my $mock_plugin = Test::MockModule->new('PVE::Storage::Plugin');
+$mock_plugin->redefine(list_volumes => sub {
+ my ($class, $storeid, $scfg, $vmid, $content_types) = @_;
+
+ my $list = $mocked_backups_lists->{$current_list};
+
+ return $list if !defined($vmid);
+
+ return [ grep { $_->{vmid} eq $vmid } @{$list} ];
+});
+
+sub generate_expected {
+ my ($vmids, $type, $marks) = @_;
+
+ my @expected;
+ foreach my $vmid (@{$vmids}) {
+ push @expected, (
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2018_05_26-11_18_21.tar.zst",
+ 'type' => 'qemu',
+ 'ctime' => $basetime - 585*24*60*60 - 60*60,
+ 'mark' => $marks->[0],
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2019_12_31-11_18_21.tar.zst",
+ 'type' => 'qemu',
+ 'ctime' => $basetime - 24*60*60 - 60*60,
+ 'mark' => $marks->[1],
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2019_12_31-11_19_21.tar.zst",
+ 'type' => 'qemu',
+ 'ctime' => $basetime - 24*60*60 - 60*60 + 60,
+ 'mark' => $marks->[2],
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2020_01_01-11_18_21.tar.zst",
+ 'type' => 'qemu',
+ 'ctime' => $basetime - 60*60,
+ 'mark' => $marks->[3],
+ 'vmid' => $vmid,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2020_01_01-12_18_21.tar.zst",
+ 'type' => 'qemu',
+ 'ctime' => $basetime,
+ 'mark' => $marks->[4],
+ 'vmid' => $vmid,
+ },
+ ) if !defined($type) || $type eq 'qemu';
+ push @expected, (
+ {
+ 'volid' => "$storeid:backup/vzdump-lxc-$vmid-2020_01_01-12_18_21.tar.zst",
+ 'type' => 'lxc',
+ 'ctime' => $basetime,
+ 'mark' => $marks->[5],
+ 'vmid' => $vmid,
+ },
+ ) if !defined($type) || $type eq 'lxc';
+ push @expected, (
+ {
+ 'volid' => "$storeid:backup/vzdump-$vmid-renamed.tar.zst",
+ 'type' => 'unknown',
+ 'ctime' => 1234,
+ 'mark' => 'protected',
+ 'vmid' => $vmid,
+ },
+ ) if !defined($type);
+ }
+ return [ sort { $a->{volid} cmp $b->{volid} } @expected ];
+}
+
+# an array of test cases, each test is comprised of the following keys:
+# description => to identify a single test
+# vmid => VMID or undef for all
+# type => 'qemu' or 'lxc' or undef for all
+# keep => options describing what to keep
+# list => backups list to use. defaults to 'default'
+# expected => what prune_backups should return
+#
+# most of them are created further below
+my $tests = [
+ {
+ description => 'last=3, multiple IDs',
+ keep => {
+ 'keep-last' => 3,
+ },
+ expected => generate_expected(\@vmids, undef, ['remove', 'remove', 'keep', 'keep', 'keep', 'keep']),
+ },
+ {
+ description => 'weekly=2, one ID',
+ vmid => $vmids[0],
+ keep => {
+ 'keep-weekly' => 2,
+ },
+ expected => generate_expected([$vmids[0]], undef, ['keep', 'remove', 'remove', 'remove', 'keep', 'keep']),
+ },
+ {
+ description => 'daily=weekly=monthly=1, multiple IDs',
+ keep => {
+ 'keep-hourly' => 0,
+ 'keep-daily' => 1,
+ 'keep-weekly' => 1,
+ 'keep-monthly' => 1,
+ },
+ expected => generate_expected(\@vmids, undef, ['keep', 'remove', 'keep', 'remove', 'keep', 'keep']),
+ },
+ {
+ description => 'hourly=4, one ID',
+ vmid => $vmids[0],
+ keep => {
+ 'keep-hourly' => 4,
+ 'keep-daily' => 0,
+ },
+ expected => generate_expected([$vmids[0]], undef, ['keep', 'remove', 'keep', 'keep', 'keep', 'keep']),
+ },
+ {
+ description => 'yearly=2, multiple IDs',
+ keep => {
+ 'keep-hourly' => 0,
+ 'keep-daily' => 0,
+ 'keep-weekly' => 0,
+ 'keep-monthly' => 0,
+ 'keep-yearly' => 2,
+ },
+ expected => generate_expected(\@vmids, undef, ['remove', 'remove', 'keep', 'remove', 'keep', 'keep']),
+ },
+ {
+ description => 'last=2,hourly=2 one ID',
+ vmid => $vmids[0],
+ keep => {
+ 'keep-last' => 2,
+ 'keep-hourly' => 2,
+ },
+ expected => generate_expected([$vmids[0]], undef, ['keep', 'remove', 'keep', 'keep', 'keep', 'keep']),
+ },
+ {
+ description => 'last=1,monthly=2, multiple IDs',
+ keep => {
+ 'keep-last' => 1,
+ 'keep-monthly' => 2,
+ },
+ expected => generate_expected(\@vmids, undef, ['keep', 'remove', 'keep', 'remove', 'keep', 'keep']),
+ },
+ {
+ description => 'monthly=3, one ID',
+ vmid => $vmids[0],
+ keep => {
+ 'keep-monthly' => 3,
+ },
+ expected => generate_expected([$vmids[0]], undef, ['keep', 'remove', 'keep', 'remove', 'keep', 'keep']),
+ },
+ {
+ description => 'last=daily=weekly=1, multiple IDs',
+ keep => {
+ 'keep-last' => 1,
+ 'keep-daily' => 1,
+ 'keep-weekly' => 1,
+ },
+ expected => generate_expected(\@vmids, undef, ['keep', 'remove', 'keep', 'remove', 'keep', 'keep']),
+ },
+ {
+ description => 'daily=2, one ID',
+ vmid => $vmids[0],
+ keep => {
+ 'keep-daily' => 2,
+ },
+ expected => generate_expected([$vmids[0]], undef, ['remove', 'remove', 'keep', 'remove', 'keep', 'keep']),
+ },
+ {
+ description => 'weekly=monthly=1, multiple IDs',
+ keep => {
+ 'keep-weekly' => 1,
+ 'keep-monthly' => 1,
+ },
+ expected => generate_expected(\@vmids, undef, ['keep', 'remove', 'remove', 'remove', 'keep', 'keep']),
+ },
+ {
+ description => 'weekly=yearly=1, one ID',
+ vmid => $vmids[0],
+ keep => {
+ 'keep-weekly' => 1,
+ 'keep-yearly' => 1,
+ },
+ expected => generate_expected([$vmids[0]], undef, ['keep', 'remove', 'remove', 'remove', 'keep', 'keep']),
+ },
+ {
+ description => 'weekly=yearly=1, one ID, type qemu',
+ vmid => $vmids[0],
+ type => 'qemu',
+ keep => {
+ 'keep-weekly' => 1,
+ 'keep-yearly' => 1,
+ },
+ expected => generate_expected([$vmids[0]], 'qemu', ['keep', 'remove', 'remove', 'remove', 'keep', '']),
+ },
+ {
+ description => 'week=yearly=1, one ID, type lxc',
+ vmid => $vmids[0],
+ type => 'lxc',
+ keep => {
+ 'keep-last' => 1,
+ },
+ expected => generate_expected([$vmids[0]], 'lxc', ['', '', '', '', '', 'keep']),
+ },
+ {
+ description => 'yearly=1, year before 2000',
+ keep => {
+ 'keep-yearly' => 1,
+ },
+ list => 'year1970',
+ expected => [
+ {
+ 'volid' => "$storeid:backup/vzdump-lxc-321-1970_01_01-00_01_23.tar.zst",
+ 'ctime' => 83,
+ 'mark' => 'remove',
+ 'type' => 'lxc',
+ 'vmid' => 321,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-lxc-321-2070_01_01-00_01_00.tar.zst",
+ 'ctime' => 60*60*24 * (365*100 + 25) + 60,
+ 'mark' => 'keep',
+ 'type' => 'lxc',
+ 'vmid' => 321,
+ },
+ ],
+ },
+ {
+ description => 'last=1, ne ID, year before 2000',
+ keep => {
+ 'keep-last' => 1,
+ },
+ list => 'novmid',
+ expected => [
+ {
+ 'volid' => "$storeid:backup/vzdump-lxc-novmid.tar.gz",
+ 'ctime' => 1234,
+ 'mark' => 'protected',
+ 'type' => 'lxc',
+ },
+ ],
+ },
+];
+
+plan tests => scalar @$tests;
+
+for my $tt (@$tests) {
+
+ my $got = eval {
+ $current_list = $tt->{list} // 'default';
+ my $res = PVE::Storage::Plugin->prune_backups($tt->{scfg}, $storeid, $tt->{keep}, $tt->{vmid}, $tt->{type}, 1);
+ return [ sort { $a->{volid} cmp $b->{volid} } @{$res} ];
+ };
+ $got = $@ if $@;
+
+ is_deeply($got, $tt->{expected}, $tt->{description}) || diag(explain($got));
+}
+
+done_testing();
+
+1;
diff --git a/test/run_plugin_tests.pl b/test/run_plugin_tests.pl
index 54322bb..d33429a 100755
--- a/test/run_plugin_tests.pl
+++ b/test/run_plugin_tests.pl
@@ -16,6 +16,7 @@ my $res = $harness->runtests(
"path_to_volume_id_test.pm",
"get_subdir_test.pm",
"filesystem_path_test.pm",
+ "prune_backups_test.pm",
);
exit -1 if !$res || $res->{failed} || $res->{parse_errors};
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pve-devel] applied: [PATCH v4 storage 2/7] Add prune_backups to storage API
2020-07-09 12:45 ` [pve-devel] [PATCH v4 storage 2/7] Add prune_backups to storage API Fabian Ebner
@ 2020-07-24 17:07 ` Thomas Lamprecht
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Lamprecht @ 2020-07-24 17:07 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Ebner
Am 7/9/20 um 2:45 PM schrieb Fabian Ebner:
> Implement it for generic storages supporting backups
> (i.e. directory-based storages) and add a wrapper for PBS.
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>
> Changes from v3:
> * prune_mark: merge loops
> * allow filtering by type
> * more error handling
> * allow passing along a log function
> * add more tests
>
> PVE/Storage.pm | 91 +++++++++-
> PVE/Storage/PBSPlugin.pm | 68 ++++++++
> PVE/Storage/Plugin.pm | 65 +++++++
> test/prune_backups_test.pm | 342 +++++++++++++++++++++++++++++++++++++
> test/run_plugin_tests.pl | 1 +
> 5 files changed, 565 insertions(+), 2 deletions(-)
> create mode 100644 test/prune_backups_test.pm
>
applied, thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread
* [pve-devel] [PATCH v4 storage 3/7] Add API and pvesm call for prune_backups
2020-07-09 12:45 [pve-devel] [PATCH-SERIES v4] fix #2649: introduce prune-backups property for storages supporting backups Fabian Ebner
2020-07-09 12:45 ` [pve-devel] [PATCH v4 storage 1/7] Introduce prune-backups property for directory-based storages Fabian Ebner
2020-07-09 12:45 ` [pve-devel] [PATCH v4 storage 2/7] Add prune_backups to storage API Fabian Ebner
@ 2020-07-09 12:45 ` Fabian Ebner
2020-07-13 7:04 ` Fabian Ebner
2020-07-24 17:08 ` Thomas Lamprecht
2020-07-09 12:45 ` [pve-devel] [PATCH v4 guest-common 4/7] Add prune-backups option to vzdump parameters Fabian Ebner
` (3 subsequent siblings)
6 siblings, 2 replies; 16+ messages in thread
From: Fabian Ebner @ 2020-07-09 12:45 UTC (permalink / raw)
To: pve-devel
For the pvesm call use a wrapper and a --dry-run option to redirect
to the correct API call.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
Changes from v3:
* allow filtering by type
* pvesm: redirect to correct API call according to --dry-run
PVE/API2/Storage/Makefile | 2 +-
PVE/API2/Storage/PruneBackups.pm | 164 +++++++++++++++++++++++++++++++
PVE/API2/Storage/Status.pm | 7 ++
PVE/CLI/pvesm.pm | 126 ++++++++++++++++++++++++
4 files changed, 298 insertions(+), 1 deletion(-)
create mode 100644 PVE/API2/Storage/PruneBackups.pm
diff --git a/PVE/API2/Storage/Makefile b/PVE/API2/Storage/Makefile
index a33525b..3f667e8 100644
--- a/PVE/API2/Storage/Makefile
+++ b/PVE/API2/Storage/Makefile
@@ -1,5 +1,5 @@
-SOURCES= Content.pm Status.pm Config.pm
+SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm
.PHONY: install
install:
diff --git a/PVE/API2/Storage/PruneBackups.pm b/PVE/API2/Storage/PruneBackups.pm
new file mode 100644
index 0000000..a84d1c8
--- /dev/null
+++ b/PVE/API2/Storage/PruneBackups.pm
@@ -0,0 +1,164 @@
+package PVE::API2::Storage::PruneBackups;
+
+use strict;
+use warnings;
+
+use PVE::Cluster;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RESTHandler;
+use PVE::RPCEnvironment;
+use PVE::Storage;
+use PVE::Tools qw(extract_param);
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+ name => 'dryrun',
+ path => '',
+ method => 'GET',
+ description => "Get prune information for backups. NOTE: this is only a preview and might not be exactly " .
+ "what a subsequent prune call does, if the hour changes or if backups are removed/added " .
+ "in the meantime.",
+ permissions => {
+ check => ['perm', '/storage/{storage}', ['Datastore.Audit', 'Datastore.AllocateSpace'], any => 1],
+ },
+ protected => 1,
+ proxyto => 'node',
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ storage => get_standard_option('pve-storage-id', {
+ completion => \&PVE::Storage::complete_storage_enabled,
+ }),
+ 'prune-backups' => get_standard_option('prune-backups', {
+ description => "Use these retention options instead of those from the storage configuration.",
+ optional => 1,
+ }),
+ type => {
+ description => "Either 'qemu' or 'lxc'. Only consider backups for guests of this type.",
+ type => 'string',
+ optional => 1,
+ enum => ['qemu', 'lxc'],
+ },
+ vmid => get_standard_option('pve-vmid', {
+ description => "Only consider backups for this guest.",
+ optional => 1,
+ completion => \&PVE::Cluster::complete_vmid,
+ }),
+ },
+ },
+ returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ volid => {
+ description => "Backup volume ID.",
+ type => 'string',
+ },
+ 'ctime' => {
+ description => "Creation time of the backup (seconds since the UNIX epoch).",
+ type => 'integer',
+ },
+ 'mark' => {
+ description => "Whether the backup would be kept or removed. For backups that don't " .
+ "use the standard naming scheme, it's 'protected'.",
+ type => 'string',
+ },
+ type => {
+ description => "One of 'qemu', 'lxc', 'openvz' or 'unknown'.",
+ type => 'string',
+ },
+ 'vmid' => {
+ description => "The VM the backup belongs to.",
+ type => 'integer',
+ optional => 1,
+ },
+ },
+ },
+ },
+ code => sub {
+ my ($param) = @_;
+
+ my $cfg = PVE::Storage::config();
+
+ my $vmid = extract_param($param, 'vmid');
+ my $type = extract_param($param, 'type');
+ my $storeid = extract_param($param, 'storage');
+
+ my $prune_backups = extract_param($param, 'prune-backups');
+ $prune_backups = PVE::JSONSchema::parse_property_string('prune-backups', $prune_backups)
+ if defined($prune_backups);
+
+ return PVE::Storage::prune_backups($cfg, $storeid, $prune_backups, $vmid, $type, 1);
+ }});
+
+__PACKAGE__->register_method ({
+ name => 'delete',
+ path => '',
+ method => 'DELETE',
+ description => "Prune backups. Only those using the standard naming scheme are considered.",
+ permissions => {
+ description => "You need the 'Datastore.Allocate' privilege on the storage " .
+ "(or if a VM ID is specified, 'Datastore.AllocateSpace' and 'VM.Backup' for the VM).",
+ user => 'all',
+ },
+ protected => 1,
+ proxyto => 'node',
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ storage => get_standard_option('pve-storage-id', {
+ completion => \&PVE::Storage::complete_storage,
+ }),
+ 'prune-backups' => get_standard_option('prune-backups', {
+ description => "Use these retention options instead of those from the storage configuration.",
+ }),
+ type => {
+ description => "Either 'qemu' or 'lxc'. Only consider backups for guests of this type.",
+ type => 'string',
+ optional => 1,
+ enum => ['qemu', 'lxc'],
+ },
+ vmid => get_standard_option('pve-vmid', {
+ description => "Only prune backups for this VM.",
+ completion => \&PVE::Cluster::complete_vmid,
+ optional => 1,
+ }),
+ },
+ },
+ returns => { type => 'string' },
+ code => sub {
+ my ($param) = @_;
+
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $authuser = $rpcenv->get_user();
+
+ my $cfg = PVE::Storage::config();
+
+ my $vmid = extract_param($param, 'vmid');
+ my $type = extract_param($param, 'type');
+ my $storeid = extract_param($param, 'storage');
+
+ my $prune_backups = extract_param($param, 'prune-backups');
+ $prune_backups = PVE::JSONSchema::parse_property_string('prune-backups', $prune_backups)
+ if defined($prune_backups);
+
+ if (defined($vmid)) {
+ $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']);
+ $rpcenv->check($authuser, "/vms/$vmid", ['VM.Backup']);
+ } else {
+ $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate']);
+ }
+
+ my $id = (defined($vmid) ? "$vmid@" : '') . $storeid;
+ my $worker = sub {
+ PVE::Storage::prune_backups($cfg, $storeid, $prune_backups, $vmid, $type, 0);
+ };
+
+ return $rpcenv->fork_worker('prunebackups', $id, $authuser, $worker);
+ }});
+
+1;
diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index d9d9b36..d12643f 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -11,6 +11,7 @@ use PVE::Cluster;
use PVE::RRD;
use PVE::Storage;
use PVE::API2::Storage::Content;
+use PVE::API2::Storage::PruneBackups;
use PVE::RESTHandler;
use PVE::RPCEnvironment;
use PVE::JSONSchema qw(get_standard_option);
@@ -18,6 +19,11 @@ use PVE::Exception qw(raise_param_exc);
use base qw(PVE::RESTHandler);
+__PACKAGE__->register_method ({
+ subclass => "PVE::API2::Storage::PruneBackups",
+ path => '{storage}/prunebackups',
+});
+
__PACKAGE__->register_method ({
subclass => "PVE::API2::Storage::Content",
# set fragment delimiter (no subdirs) - we need that, because volume
@@ -214,6 +220,7 @@ __PACKAGE__->register_method ({
{ subdir => 'upload' },
{ subdir => 'rrd' },
{ subdir => 'rrddata' },
+ { subdir => 'prunebackups' },
];
return $res;
diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index f223c92..405224c 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -12,8 +12,10 @@ use PVE::Cluster;
use PVE::INotify;
use PVE::RPCEnvironment;
use PVE::Storage;
+use PVE::Tools qw(extract_param);
use PVE::API2::Storage::Config;
use PVE::API2::Storage::Content;
+use PVE::API2::Storage::PruneBackups;
use PVE::API2::Storage::Status;
use PVE::JSONSchema qw(get_standard_option);
use PVE::PTY;
@@ -720,6 +722,99 @@ __PACKAGE__->register_method ({
return PVE::Storage::scan_zfs();
}});
+__PACKAGE__->register_method ({
+ name => 'prunebackups',
+ path => 'prunebackups',
+ method => 'GET',
+ description => "Prune backups. This is only a wrapper for the proper API endpoints.",
+ protected => 1,
+ proxyto => 'node',
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ 'dry-run' => {
+ description => "Only show what would be pruned, don't delete anything.",
+ type => 'boolean',
+ optional => 1,
+ },
+ node => get_standard_option('pve-node'),
+ storage => get_standard_option('pve-storage-id', {
+ completion => \&PVE::Storage::complete_storage_enabled,
+ }),
+ 'prune-backups' => get_standard_option('prune-backups', {
+ description => "Use these retention options instead of those from the storage configuration.",
+ optional => 1,
+ }),
+ type => {
+ description => "Either 'qemu' or 'lxc'. Only consider backups for guests of this type.",
+ type => 'string',
+ optional => 1,
+ enum => ['qemu', 'lxc'],
+ },
+ vmid => get_standard_option('pve-vmid', {
+ description => "Only consider backups for this guest.",
+ optional => 1,
+ completion => \&PVE::Cluster::complete_vmid,
+ }),
+ },
+ },
+ returns => {
+ type => 'object',
+ properties => {
+ dryrun => {
+ description => 'If it was a dry run or not. The list will only be defined in that case.',
+ type => 'boolean',
+ },
+ list => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ volid => {
+ description => "Backup volume ID.",
+ type => 'string',
+ },
+ 'ctime' => {
+ description => "Creation time of the backup (seconds since the UNIX epoch).",
+ type => 'integer',
+ },
+ 'mark' => {
+ description => "Whether the backup would be kept or removed. For backups that don't " .
+ "use the standard naming scheme, it's 'protected'.",
+ type => 'string',
+ },
+ type => {
+ description => "One of 'qemu', 'lxc', 'openvz' or 'unknown'.",
+ type => 'string',
+ },
+ 'vmid' => {
+ description => "The VM the backup belongs to.",
+ type => 'integer',
+ optional => 1,
+ },
+ },
+ },
+ },
+ },
+ },
+ code => sub {
+ my ($param) = @_;
+
+ my $dryrun = extract_param($param, 'dry-run') ? 1 : 0;
+
+ my $list = [];
+ if ($dryrun) {
+ $list = PVE::API2::Storage::PruneBackups->dryrun($param);
+ } else {
+ PVE::API2::Storage::PruneBackups->delete($param);
+ }
+
+ return {
+ dryrun => $dryrun,
+ list => $list,
+ };
+ }});
+
our $cmddef = {
add => [ "PVE::API2::Storage::Config", 'create', ['type', 'storage'] ],
set => [ "PVE::API2::Storage::Config", 'update', ['storage'] ],
@@ -819,6 +914,37 @@ our $cmddef = {
print "APIVER $res->{apiver}\n";
print "APIAGE $res->{apiage}\n";
}],
+ 'prune-backups' => [ __PACKAGE__, 'prunebackups', ['storage'], { node => $nodename }, sub {
+ my $res = shift;
+
+ my ($dryrun, $list) = ($res->{dryrun}, $res->{list});
+
+ return if !$dryrun;
+
+ print "NOTE: this is only a preview and might not be exactly what a subsequent prune call does,\n" .
+ "if the hour changes or if backups are removed/added in the meantime.\n\n";
+
+ my @sorted = sort {
+ my $vmcmp = PVE::Tools::safe_compare($a->{vmid}, $b->{vmid}, sub { $_[0] <=> $_[1] });
+ return $vmcmp if $vmcmp ne 0;
+ return $a->{ctime} <=> $b->{ctime};
+ } @{$list};
+
+ my $maxlen = 0;
+ foreach my $backup (@sorted) {
+ my $volid = $backup->{volid};
+ $maxlen = length($volid) if length($volid) > $maxlen;
+ }
+ $maxlen+=1;
+
+ printf("%-${maxlen}s %15s %10s\n", 'Backup', 'Backup-ID', 'Prune-Mark');
+ foreach my $backup (@sorted) {
+ my $type = $backup->{type};
+ my $vmid = $backup->{vmid};
+ my $backup_id = defined($vmid) ? "$type/$vmid" : "$type";
+ printf("%-${maxlen}s %15s %10s\n", $backup->{volid}, $backup_id, $backup->{mark});
+ }
+ }],
};
1;
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [pve-devel] [PATCH v4 storage 3/7] Add API and pvesm call for prune_backups
2020-07-09 12:45 ` [pve-devel] [PATCH v4 storage 3/7] Add API and pvesm call for prune_backups Fabian Ebner
@ 2020-07-13 7:04 ` Fabian Ebner
2020-07-24 17:08 ` Thomas Lamprecht
1 sibling, 0 replies; 16+ messages in thread
From: Fabian Ebner @ 2020-07-13 7:04 UTC (permalink / raw)
To: pve-devel
Am 09.07.20 um 14:45 schrieb Fabian Ebner:
> For the pvesm call use a wrapper and a --dry-run option to redirect
> to the correct API call.
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>
> Changes from v3:
> * allow filtering by type
> * pvesm: redirect to correct API call according to --dry-run
>
> PVE/API2/Storage/Makefile | 2 +-
> PVE/API2/Storage/PruneBackups.pm | 164 +++++++++++++++++++++++++++++++
> PVE/API2/Storage/Status.pm | 7 ++
> PVE/CLI/pvesm.pm | 126 ++++++++++++++++++++++++
> 4 files changed, 298 insertions(+), 1 deletion(-)
> create mode 100644 PVE/API2/Storage/PruneBackups.pm
>
> diff --git a/PVE/API2/Storage/Makefile b/PVE/API2/Storage/Makefile
> index a33525b..3f667e8 100644
> --- a/PVE/API2/Storage/Makefile
> +++ b/PVE/API2/Storage/Makefile
> @@ -1,5 +1,5 @@
>
> -SOURCES= Content.pm Status.pm Config.pm
> +SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm
>
> .PHONY: install
> install:
> diff --git a/PVE/API2/Storage/PruneBackups.pm b/PVE/API2/Storage/PruneBackups.pm
> new file mode 100644
> index 0000000..a84d1c8
> --- /dev/null
> +++ b/PVE/API2/Storage/PruneBackups.pm
> @@ -0,0 +1,164 @@
> +package PVE::API2::Storage::PruneBackups;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Cluster;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::RESTHandler;
> +use PVE::RPCEnvironment;
> +use PVE::Storage;
> +use PVE::Tools qw(extract_param);
> +
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> + name => 'dryrun',
> + path => '',
> + method => 'GET',
> + description => "Get prune information for backups. NOTE: this is only a preview and might not be exactly " .
> + "what a subsequent prune call does, if the hour changes or if backups are removed/added " .
The part "if the hour changes or" should be removed here. It's true that
a new backup can change the pruning selection in different ways
depending on when it is created, but as long as there's the same backups
on the storage, the pruning selection does *not* change.
The same applies to the text further below...
> + "in the meantime.",
> + permissions => {
> + check => ['perm', '/storage/{storage}', ['Datastore.Audit', 'Datastore.AllocateSpace'], any => 1],
> + },
> + protected => 1,
> + proxyto => 'node',
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + storage => get_standard_option('pve-storage-id', {
> + completion => \&PVE::Storage::complete_storage_enabled,
> + }),
> + 'prune-backups' => get_standard_option('prune-backups', {
> + description => "Use these retention options instead of those from the storage configuration.",
> + optional => 1,
> + }),
> + type => {
> + description => "Either 'qemu' or 'lxc'. Only consider backups for guests of this type.",
> + type => 'string',
> + optional => 1,
> + enum => ['qemu', 'lxc'],
> + },
> + vmid => get_standard_option('pve-vmid', {
> + description => "Only consider backups for this guest.",
> + optional => 1,
> + completion => \&PVE::Cluster::complete_vmid,
> + }),
> + },
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => 'object',
> + properties => {
> + volid => {
> + description => "Backup volume ID.",
> + type => 'string',
> + },
> + 'ctime' => {
> + description => "Creation time of the backup (seconds since the UNIX epoch).",
> + type => 'integer',
> + },
> + 'mark' => {
> + description => "Whether the backup would be kept or removed. For backups that don't " .
> + "use the standard naming scheme, it's 'protected'.",
> + type => 'string',
> + },
> + type => {
> + description => "One of 'qemu', 'lxc', 'openvz' or 'unknown'.",
> + type => 'string',
> + },
> + 'vmid' => {
> + description => "The VM the backup belongs to.",
> + type => 'integer',
> + optional => 1,
> + },
> + },
> + },
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $cfg = PVE::Storage::config();
> +
> + my $vmid = extract_param($param, 'vmid');
> + my $type = extract_param($param, 'type');
> + my $storeid = extract_param($param, 'storage');
> +
> + my $prune_backups = extract_param($param, 'prune-backups');
> + $prune_backups = PVE::JSONSchema::parse_property_string('prune-backups', $prune_backups)
> + if defined($prune_backups);
> +
> + return PVE::Storage::prune_backups($cfg, $storeid, $prune_backups, $vmid, $type, 1);
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'delete',
> + path => '',
> + method => 'DELETE',
> + description => "Prune backups. Only those using the standard naming scheme are considered.",
> + permissions => {
> + description => "You need the 'Datastore.Allocate' privilege on the storage " .
> + "(or if a VM ID is specified, 'Datastore.AllocateSpace' and 'VM.Backup' for the VM).",
> + user => 'all',
> + },
> + protected => 1,
> + proxyto => 'node',
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + storage => get_standard_option('pve-storage-id', {
> + completion => \&PVE::Storage::complete_storage,
> + }),
> + 'prune-backups' => get_standard_option('prune-backups', {
> + description => "Use these retention options instead of those from the storage configuration.",
> + }),
> + type => {
> + description => "Either 'qemu' or 'lxc'. Only consider backups for guests of this type.",
> + type => 'string',
> + optional => 1,
> + enum => ['qemu', 'lxc'],
> + },
> + vmid => get_standard_option('pve-vmid', {
> + description => "Only prune backups for this VM.",
> + completion => \&PVE::Cluster::complete_vmid,
> + optional => 1,
> + }),
> + },
> + },
> + returns => { type => 'string' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $authuser = $rpcenv->get_user();
> +
> + my $cfg = PVE::Storage::config();
> +
> + my $vmid = extract_param($param, 'vmid');
> + my $type = extract_param($param, 'type');
> + my $storeid = extract_param($param, 'storage');
> +
> + my $prune_backups = extract_param($param, 'prune-backups');
> + $prune_backups = PVE::JSONSchema::parse_property_string('prune-backups', $prune_backups)
> + if defined($prune_backups);
> +
> + if (defined($vmid)) {
> + $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']);
> + $rpcenv->check($authuser, "/vms/$vmid", ['VM.Backup']);
> + } else {
> + $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate']);
> + }
> +
> + my $id = (defined($vmid) ? "$vmid@" : '') . $storeid;
> + my $worker = sub {
> + PVE::Storage::prune_backups($cfg, $storeid, $prune_backups, $vmid, $type, 0);
> + };
> +
> + return $rpcenv->fork_worker('prunebackups', $id, $authuser, $worker);
> + }});
> +
> +1;
> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
> index d9d9b36..d12643f 100644
> --- a/PVE/API2/Storage/Status.pm
> +++ b/PVE/API2/Storage/Status.pm
> @@ -11,6 +11,7 @@ use PVE::Cluster;
> use PVE::RRD;
> use PVE::Storage;
> use PVE::API2::Storage::Content;
> +use PVE::API2::Storage::PruneBackups;
> use PVE::RESTHandler;
> use PVE::RPCEnvironment;
> use PVE::JSONSchema qw(get_standard_option);
> @@ -18,6 +19,11 @@ use PVE::Exception qw(raise_param_exc);
>
> use base qw(PVE::RESTHandler);
>
> +__PACKAGE__->register_method ({
> + subclass => "PVE::API2::Storage::PruneBackups",
> + path => '{storage}/prunebackups',
> +});
> +
> __PACKAGE__->register_method ({
> subclass => "PVE::API2::Storage::Content",
> # set fragment delimiter (no subdirs) - we need that, because volume
> @@ -214,6 +220,7 @@ __PACKAGE__->register_method ({
> { subdir => 'upload' },
> { subdir => 'rrd' },
> { subdir => 'rrddata' },
> + { subdir => 'prunebackups' },
> ];
>
> return $res;
> diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
> index f223c92..405224c 100755
> --- a/PVE/CLI/pvesm.pm
> +++ b/PVE/CLI/pvesm.pm
> @@ -12,8 +12,10 @@ use PVE::Cluster;
> use PVE::INotify;
> use PVE::RPCEnvironment;
> use PVE::Storage;
> +use PVE::Tools qw(extract_param);
> use PVE::API2::Storage::Config;
> use PVE::API2::Storage::Content;
> +use PVE::API2::Storage::PruneBackups;
> use PVE::API2::Storage::Status;
> use PVE::JSONSchema qw(get_standard_option);
> use PVE::PTY;
> @@ -720,6 +722,99 @@ __PACKAGE__->register_method ({
> return PVE::Storage::scan_zfs();
> }});
>
> +__PACKAGE__->register_method ({
> + name => 'prunebackups',
> + path => 'prunebackups',
> + method => 'GET',
> + description => "Prune backups. This is only a wrapper for the proper API endpoints.",
> + protected => 1,
> + proxyto => 'node',
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + 'dry-run' => {
> + description => "Only show what would be pruned, don't delete anything.",
> + type => 'boolean',
> + optional => 1,
> + },
> + node => get_standard_option('pve-node'),
> + storage => get_standard_option('pve-storage-id', {
> + completion => \&PVE::Storage::complete_storage_enabled,
> + }),
> + 'prune-backups' => get_standard_option('prune-backups', {
> + description => "Use these retention options instead of those from the storage configuration.",
> + optional => 1,
> + }),
> + type => {
> + description => "Either 'qemu' or 'lxc'. Only consider backups for guests of this type.",
> + type => 'string',
> + optional => 1,
> + enum => ['qemu', 'lxc'],
> + },
> + vmid => get_standard_option('pve-vmid', {
> + description => "Only consider backups for this guest.",
> + optional => 1,
> + completion => \&PVE::Cluster::complete_vmid,
> + }),
> + },
> + },
> + returns => {
> + type => 'object',
> + properties => {
> + dryrun => {
> + description => 'If it was a dry run or not. The list will only be defined in that case.',
> + type => 'boolean',
> + },
> + list => {
> + type => 'array',
> + items => {
> + type => 'object',
> + properties => {
> + volid => {
> + description => "Backup volume ID.",
> + type => 'string',
> + },
> + 'ctime' => {
> + description => "Creation time of the backup (seconds since the UNIX epoch).",
> + type => 'integer',
> + },
> + 'mark' => {
> + description => "Whether the backup would be kept or removed. For backups that don't " .
> + "use the standard naming scheme, it's 'protected'.",
> + type => 'string',
> + },
> + type => {
> + description => "One of 'qemu', 'lxc', 'openvz' or 'unknown'.",
> + type => 'string',
> + },
> + 'vmid' => {
> + description => "The VM the backup belongs to.",
> + type => 'integer',
> + optional => 1,
> + },
> + },
> + },
> + },
> + },
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $dryrun = extract_param($param, 'dry-run') ? 1 : 0;
> +
> + my $list = [];
> + if ($dryrun) {
> + $list = PVE::API2::Storage::PruneBackups->dryrun($param);
> + } else {
> + PVE::API2::Storage::PruneBackups->delete($param);
> + }
> +
> + return {
> + dryrun => $dryrun,
> + list => $list,
> + };
> + }});
> +
> our $cmddef = {
> add => [ "PVE::API2::Storage::Config", 'create', ['type', 'storage'] ],
> set => [ "PVE::API2::Storage::Config", 'update', ['storage'] ],
> @@ -819,6 +914,37 @@ our $cmddef = {
> print "APIVER $res->{apiver}\n";
> print "APIAGE $res->{apiage}\n";
> }],
> + 'prune-backups' => [ __PACKAGE__, 'prunebackups', ['storage'], { node => $nodename }, sub {
> + my $res = shift;
> +
> + my ($dryrun, $list) = ($res->{dryrun}, $res->{list});
> +
> + return if !$dryrun;
> +
> + print "NOTE: this is only a preview and might not be exactly what a subsequent prune call does,\n" .
> + "if the hour changes or if backups are removed/added in the meantime.\n\n";
...here
> +
> + my @sorted = sort {
> + my $vmcmp = PVE::Tools::safe_compare($a->{vmid}, $b->{vmid}, sub { $_[0] <=> $_[1] });
> + return $vmcmp if $vmcmp ne 0;
> + return $a->{ctime} <=> $b->{ctime};
> + } @{$list};
> +
> + my $maxlen = 0;
> + foreach my $backup (@sorted) {
> + my $volid = $backup->{volid};
> + $maxlen = length($volid) if length($volid) > $maxlen;
> + }
> + $maxlen+=1;
> +
> + printf("%-${maxlen}s %15s %10s\n", 'Backup', 'Backup-ID', 'Prune-Mark');
> + foreach my $backup (@sorted) {
> + my $type = $backup->{type};
> + my $vmid = $backup->{vmid};
> + my $backup_id = defined($vmid) ? "$type/$vmid" : "$type";
> + printf("%-${maxlen}s %15s %10s\n", $backup->{volid}, $backup_id, $backup->{mark});
> + }
> + }],
> };
>
> 1;
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [pve-devel] [PATCH v4 storage 3/7] Add API and pvesm call for prune_backups
2020-07-09 12:45 ` [pve-devel] [PATCH v4 storage 3/7] Add API and pvesm call for prune_backups Fabian Ebner
2020-07-13 7:04 ` Fabian Ebner
@ 2020-07-24 17:08 ` Thomas Lamprecht
1 sibling, 0 replies; 16+ messages in thread
From: Thomas Lamprecht @ 2020-07-24 17:08 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Ebner
Am 7/9/20 um 2:45 PM schrieb Fabian Ebner:
> For the pvesm call use a wrapper and a --dry-run option to redirect
> to the correct API call.
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>
> Changes from v3:
> * allow filtering by type
> * pvesm: redirect to correct API call according to --dry-run
>
> PVE/API2/Storage/Makefile | 2 +-
> PVE/API2/Storage/PruneBackups.pm | 164 +++++++++++++++++++++++++++++++
> PVE/API2/Storage/Status.pm | 7 ++
> PVE/CLI/pvesm.pm | 126 ++++++++++++++++++++++++
> 4 files changed, 298 insertions(+), 1 deletion(-)
> create mode 100644 PVE/API2/Storage/PruneBackups.pm
>
applied, thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread
* [pve-devel] [PATCH v4 guest-common 4/7] Add prune-backups option to vzdump parameters
2020-07-09 12:45 [pve-devel] [PATCH-SERIES v4] fix #2649: introduce prune-backups property for storages supporting backups Fabian Ebner
` (2 preceding siblings ...)
2020-07-09 12:45 ` [pve-devel] [PATCH v4 storage 3/7] Add API and pvesm call for prune_backups Fabian Ebner
@ 2020-07-09 12:45 ` Fabian Ebner
2020-08-20 15:39 ` [pve-devel] applied: " Thomas Lamprecht
2020-07-09 12:45 ` [pve-devel] [PATCH v4 manager 5/7] make use of archive_info and archive_remove Fabian Ebner
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Fabian Ebner @ 2020-07-09 12:45 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
New in v4
PVE/VZDump/Common.pm | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/PVE/VZDump/Common.pm b/PVE/VZDump/Common.pm
index 909e3af..2f3c16e 100644
--- a/PVE/VZDump/Common.pm
+++ b/PVE/VZDump/Common.pm
@@ -210,6 +210,10 @@ my $confdesc = {
minimum => 1,
default => 1,
},
+ 'prune-backups' => get_standard_option('prune-backups', {
+ description => "Use these retention options instead of those from the storage configuration.",
+ optional => 1,
+ }),
remove => {
type => 'boolean',
description => "Remove old backup files if there are more than 'maxfiles' backup files.",
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pve-devel] [PATCH v4 manager 5/7] make use of archive_info and archive_remove
2020-07-09 12:45 [pve-devel] [PATCH-SERIES v4] fix #2649: introduce prune-backups property for storages supporting backups Fabian Ebner
` (3 preceding siblings ...)
2020-07-09 12:45 ` [pve-devel] [PATCH v4 guest-common 4/7] Add prune-backups option to vzdump parameters Fabian Ebner
@ 2020-07-09 12:45 ` Fabian Ebner
2020-08-21 11:31 ` [pve-devel] applied: " Thomas Lamprecht
2020-07-09 12:45 ` [pve-devel] [PATCH v4 manager 6/7] Allow prune-backups as an alternative to maxfiles Fabian Ebner
2020-07-09 12:45 ` [pve-devel] [PATCH v4 manager 7/7] Always use prune-backups instead of maxfiles internally Fabian Ebner
6 siblings, 1 reply; 16+ messages in thread
From: Fabian Ebner @ 2020-07-09 12:45 UTC (permalink / raw)
To: pve-devel
to avoid some code duplication.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
PVE/VZDump.pm | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 601cd56e..2f509555 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -631,10 +631,15 @@ sub get_backup_file_list {
my $bklist = [];
foreach my $fn (<$dir/${bkname}-*>) {
next if $exclude_fn && $fn eq $exclude_fn;
- if ($fn =~ m!/(${bkname}-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2})\.(tgz|((tar|vma)(\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)))$!) {
- $fn = "$dir/$1"; # untaint
- my $t = timelocal ($7, $6, $5, $4, $3 - 1, $2);
- push @$bklist, [$fn, $t];
+
+ my $archive_info = eval { PVE::Storage::archive_info($fn) } // {};
+ if ($archive_info->{is_std_name}) {
+ my $filename = $archive_info->{filename};
+ my $backup = {
+ 'path' => "$dir/$filename",
+ 'ctime' => $archive_info->{ctime},
+ };
+ push @{$bklist}, $backup;
}
}
@@ -927,15 +932,13 @@ sub exec_backup_task {
$opts->{scfg}, $opts->{storage}, 'prune', $args, logfunc => $logfunc);
} else {
my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname, $task->{target});
- $bklist = [ sort { $b->[1] <=> $a->[1] } @$bklist ];
+ $bklist = [ sort { $b->{ctime} <=> $a->{ctime} } @$bklist ];
while (scalar (@$bklist) >= $maxfiles) {
my $d = pop @$bklist;
- debugmsg ('info', "delete old backup '$d->[0]'", $logfd);
- unlink $d->[0];
- my $logfn = $d->[0];
- $logfn =~ s/\.(tgz|((tar|vma)(\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?))$/\.log/;
- unlink $logfn;
+ my $archive_path = $d->{path};
+ debugmsg ('info', "delete old backup '$archive_path'", $logfd);
+ PVE::Storage::archive_remove($archive_path);
}
}
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pve-devel] [PATCH v4 manager 6/7] Allow prune-backups as an alternative to maxfiles
2020-07-09 12:45 [pve-devel] [PATCH-SERIES v4] fix #2649: introduce prune-backups property for storages supporting backups Fabian Ebner
` (4 preceding siblings ...)
2020-07-09 12:45 ` [pve-devel] [PATCH v4 manager 5/7] make use of archive_info and archive_remove Fabian Ebner
@ 2020-07-09 12:45 ` Fabian Ebner
2020-07-09 12:45 ` [pve-devel] [PATCH v4 manager 7/7] Always use prune-backups instead of maxfiles internally Fabian Ebner
6 siblings, 0 replies; 16+ messages in thread
From: Fabian Ebner @ 2020-07-09 12:45 UTC (permalink / raw)
To: pve-devel
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>
---
Changes from v3:
* allow prune-backups as a vzdump parameter
* use log function for output when pruning
* only make maxfiles and prune-backups conflict when they
have the same preference level (e.g. both are passed as an option).
PVE/API2/VZDump.pm | 4 +--
PVE/VZDump.pm | 88 ++++++++++++++++++++++++++++++++--------------
2 files changed, 64 insertions(+), 28 deletions(-)
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 2eda973e..19fa1e3b 100644
--- 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 2f509555..17153fe4 100644
--- 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"
+ 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;
@@ -651,6 +665,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 +718,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 +738,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}) {
@@ -923,23 +948,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);
}
}
@@ -1126,6 +1156,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 " .
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pve-devel] [PATCH v4 manager 7/7] Always use prune-backups instead of maxfiles internally
2020-07-09 12:45 [pve-devel] [PATCH-SERIES v4] fix #2649: introduce prune-backups property for storages supporting backups Fabian Ebner
` (5 preceding siblings ...)
2020-07-09 12:45 ` [pve-devel] [PATCH v4 manager 6/7] Allow prune-backups as an alternative to maxfiles Fabian Ebner
@ 2020-07-09 12:45 ` Fabian Ebner
2020-08-21 11:33 ` Thomas Lamprecht
6 siblings, 1 reply; 16+ messages in thread
From: Fabian Ebner @ 2020-07-09 12:45 UTC (permalink / raw)
To: pve-devel
For the use case with '--dumpdir', it's not possible to call prune_backups
directly, so a little bit of special handling is required there.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
PVE/VZDump.pm | 42 ++++++++++++++++--------------------------
1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 17153fe4..d87ef857 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -484,8 +484,10 @@ sub new {
die "internal error";
}
- if (!defined($opts->{'prune-backups'}) && !defined($opts->{maxfiles})) {
- $opts->{maxfiles} = $defaults->{maxfiles};
+ if (!defined($opts->{'prune-backups'})) {
+ $opts->{maxfiles} //= $defaults->{maxfiles};
+ $opts->{'prune_backups'} = { 'keep-last' => $opts->{maxfiles} };
+ delete $opts->{maxfiles};
}
if ($opts->{tmpdir} && ! -d $opts->{tmpdir}) {
@@ -717,16 +719,11 @@ sub exec_backup_task {
my $bkname = "vzdump-$vmtype-$vmid";
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;
- }
+ foreach my $keep (values %{$prune_options}) {
+ $backup_limit += $keep;
}
if ($backup_limit && !$opts->{remove}) {
@@ -949,25 +946,18 @@ sub exec_backup_task {
# purge older backup
if ($opts->{remove}) {
- if ($maxfiles) {
+ if (!defined($opts->{storage})) {
+ my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname, $task->{target});
+ PVE::Storage::prune_mark_backup_group($bklist, $prune_options);
- 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);
- }
+ foreach my $prune_entry (@{$bklist}) {
+ next if $prune_entry->{mark} ne 'remove';
+
+ my $archive_path = $prune_entry->{path};
+ debugmsg ('info', "delete old backup '$archive_path'", $logfd);
+ PVE::Storage::archive_remove($archive_path);
}
- } elsif (defined($prune_options)) {
+ } else {
my $logfunc = sub { debugmsg($_[0], $_[1], $logfd) };
PVE::Storage::prune_backups($cfg, $opts->{storage}, $prune_options, $vmid, $vmtype, 0, $logfunc);
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [pve-devel] [PATCH v4 manager 7/7] Always use prune-backups instead of maxfiles internally
2020-07-09 12:45 ` [pve-devel] [PATCH v4 manager 7/7] Always use prune-backups instead of maxfiles internally Fabian Ebner
@ 2020-08-21 11:33 ` Thomas Lamprecht
2020-08-24 7:25 ` Fabian Ebner
0 siblings, 1 reply; 16+ messages in thread
From: Thomas Lamprecht @ 2020-08-21 11:33 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Ebner
On 09.07.20 14:45, Fabian Ebner wrote:
> For the use case with '--dumpdir', it's not possible to call prune_backups
> directly, so a little bit of special handling is required there.
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> PVE/VZDump.pm | 42 ++++++++++++++++--------------------------
> 1 file changed, 16 insertions(+), 26 deletions(-)
>
breaks having a max backup set on a storage, before this it complains, after
this I can make as many backups I want..
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [pve-devel] [PATCH v4 manager 7/7] Always use prune-backups instead of maxfiles internally
2020-08-21 11:33 ` Thomas Lamprecht
@ 2020-08-24 7:25 ` Fabian Ebner
0 siblings, 0 replies; 16+ messages in thread
From: Fabian Ebner @ 2020-08-24 7:25 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: Thomas Lamprecht
Am 21.08.20 um 13:33 schrieb Thomas Lamprecht:
> On 09.07.20 14:45, Fabian Ebner wrote:
>> For the use case with '--dumpdir', it's not possible to call prune_backups
>> directly, so a little bit of special handling is required there.
>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>> PVE/VZDump.pm | 42 ++++++++++++++++--------------------------
>> 1 file changed, 16 insertions(+), 26 deletions(-)
>>
>
> breaks having a max backup set on a storage, before this it complains, after
> this I can make as many backups I want..
>
Sorry, turns out it was a typo:
Am 09.07.20 um 14:45 schrieb Fabian Ebner:
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 17153fe4..d87ef857 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -484,8 +484,10 @@ sub new {
> die "internal error";
> }
>
> - if (!defined($opts->{'prune-backups'}) &&
!defined($opts->{maxfiles})) {
> - $opts->{maxfiles} = $defaults->{maxfiles};
> + if (!defined($opts->{'prune-backups'})) {
> + $opts->{maxfiles} //= $defaults->{maxfiles};
> + $opts->{'prune_backups'} = { 'keep-last' => $opts->{maxfiles} };
With 'prune-backups' here, it should work. Should I send a v5?
> + delete $opts->{maxfiles};
> }
^ permalink raw reply [flat|nested] 16+ messages in thread