From 8009417d0dac475183a06f56dc8a27c0e223eb48 Mon Sep 17 00:00:00 2001 From: Fabian Ebner Date: Tue, 29 Mar 2022 14:53:13 +0200 Subject: [PATCH] plugins: allow limiting the number of protected backups per guest The ability to mark backups as protected broke the implicit assumption in vzdump that remove=1 and current number of backups being the limit (i.e. sum of all keep options) will result in a backup being removed. Introduce a new storage property 'max-protected-backups' to limit the number of protected backups per guest. Use 5 as a default value, as it should cover most use cases, while still not having too big of a potential overhead in many scenarios. For external plugins that do not return the backup subtype in list_volumes, all protected backups with the same ID will count towards the limit. An alternative would be to count the protected backups when pruning. While that would avoid the need for a new property, it would break the current semantics of protected backups being ignored for pruning. It also would be less flexible, e.g. for PBS, it can make sense to have both keep-all=1 and a limit for the number of protected snapshots on the PVE side. Signed-off-by: Fabian Ebner --- PVE/Storage.pm | 35 ++++++++++++++++++++++++++++++++++ PVE/Storage/BTRFSPlugin.pm | 3 ++- PVE/Storage/CIFSPlugin.pm | 1 + PVE/Storage/CephFSPlugin.pm | 1 + PVE/Storage/DirPlugin.pm | 1 + PVE/Storage/GlusterfsPlugin.pm | 1 + PVE/Storage/NFSPlugin.pm | 1 + PVE/Storage/PBSPlugin.pm | 1 + PVE/Storage/Plugin.pm | 7 +++++++ 9 files changed, 50 insertions(+), 1 deletion(-) diff --git a/PVE/Storage.pm b/PVE/Storage.pm index 3b86956..72458cf 100755 --- a/PVE/Storage.pm +++ b/PVE/Storage.pm @@ -211,6 +211,17 @@ sub storage_can_replicate { return $plugin->storage_can_replicate($scfg, $storeid, $format); } +sub get_max_protected_backups { + my ($scfg, $storeid) = @_; + + return $scfg->{'max-protected-backups'} if defined($scfg->{'max-protected-backups'}); + + my $rpcenv = PVE::RPCEnvironment::get(); + my $authuser = $rpcenv->get_user(); + + return $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate'], 1) ? -1 : 5; +} + sub storage_ids { my ($cfg) = @_; @@ -240,6 +251,30 @@ sub update_volume_attribute { my $scfg = storage_config($cfg, $storeid); my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); + my ($vtype, undef, $vmid) = $plugin->parse_volname($volname); + my $max_protected_backups = get_max_protected_backups($scfg, $storeid); + + if ( + $vtype eq 'backup' + && $vmid + && $attribute eq 'protected' + && $value + && !$plugin->get_volume_attribute($scfg, $storeid, $volname, 'protected') + && $max_protected_backups > -1 # -1 is unlimited + ) { + my $backups = $plugin->list_volumes($storeid, $scfg, $vmid, ['backup']); + my ($backup_type) = map { $_->{subtype} } grep { $_->{volid} eq $volid } $backups->@*; + + my $protected_count = grep { + $_->{protected} && (!$backup_type || ($_->{subtype} && $_->{subtype} eq $backup_type)) + } $backups->@*; + + if ($max_protected_backups <= $protected_count) { + die "The number of protected backups per guest is limited to $max_protected_backups ". + "on storage '$storeid'\n"; + } + } + return $plugin->update_volume_attribute($scfg, $storeid, $volname, $attribute, $value); } diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm index c8caa41..7dac34b 100644 --- a/PVE/Storage/BTRFSPlugin.pm +++ b/PVE/Storage/BTRFSPlugin.pm @@ -67,7 +67,8 @@ sub options { shared => { optional => 1 }, disable => { optional => 1 }, maxfiles => { optional => 1 }, - 'prune-backups'=> { optional => 1 }, + 'prune-backups' => { optional => 1 }, + 'max-protected-backups' => { optional => 1 }, content => { optional => 1 }, format => { optional => 1 }, is_mountpoint => { optional => 1 }, diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm index d5efa5f..982040a 100644 --- a/PVE/Storage/CIFSPlugin.pm +++ b/PVE/Storage/CIFSPlugin.pm @@ -134,6 +134,7 @@ sub options { disable => { optional => 1 }, maxfiles => { optional => 1 }, 'prune-backups' => { optional => 1 }, + 'max-protected-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 f75c1b8..4976747 100644 --- a/PVE/Storage/CephFSPlugin.pm +++ b/PVE/Storage/CephFSPlugin.pm @@ -155,6 +155,7 @@ sub options { maxfiles => { optional => 1 }, keyring => { optional => 1 }, 'prune-backups' => { optional => 1 }, + 'max-protected-backups' => { optional => 1 }, 'fs-name' => { optional => 1 }, }; } diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm index c60818b..1baad63 100644 --- a/PVE/Storage/DirPlugin.pm +++ b/PVE/Storage/DirPlugin.pm @@ -58,6 +58,7 @@ sub options { disable => { optional => 1 }, maxfiles => { optional => 1 }, 'prune-backups' => { optional => 1 }, + 'max-protected-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 d8d2b88..ad386d2 100644 --- a/PVE/Storage/GlusterfsPlugin.pm +++ b/PVE/Storage/GlusterfsPlugin.pm @@ -133,6 +133,7 @@ sub options { disable => { optional => 1 }, maxfiles => { optional => 1 }, 'prune-backups' => { optional => 1 }, + 'max-protected-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 0400c93..5bd7313 100644 --- a/PVE/Storage/NFSPlugin.pm +++ b/PVE/Storage/NFSPlugin.pm @@ -85,6 +85,7 @@ sub options { disable => { optional => 1 }, maxfiles => { optional => 1 }, 'prune-backups' => { optional => 1 }, + 'max-protected-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 4b3f349..687901f 100644 --- a/PVE/Storage/PBSPlugin.pm +++ b/PVE/Storage/PBSPlugin.pm @@ -73,6 +73,7 @@ sub options { 'master-pubkey' => { optional => 1 }, maxfiles => { optional => 1 }, 'prune-backups' => { optional => 1 }, + 'max-protected-backups' => { optional => 1 }, fingerprint => { optional => 1 }, }; } diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm index 6673409..521e3e9 100644 --- a/PVE/Storage/Plugin.pm +++ b/PVE/Storage/Plugin.pm @@ -155,6 +155,13 @@ my $defaultData = { optional => 1, }, 'prune-backups' => get_standard_option('prune-backups'), + 'max-protected-backups' => { + description => "Maximal number of protected backups per guest. Use '-1' for unlimited.", + type => 'integer', + minimum => -1, + optional => 1, + default => "Unlimited for users with Datastore.Allocate privilege, 5 for other users", + }, shared => { description => "Mark storage as shared.", type => 'boolean',