From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id F2AA984F23 for ; Thu, 16 Dec 2021 13:12:43 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C1A141E570 for ; Thu, 16 Dec 2021 13:12:43 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id C535E1E46E for ; Thu, 16 Dec 2021 13:12:38 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 8F4DF45310 for ; Thu, 16 Dec 2021 13:12:38 +0100 (CET) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Thu, 16 Dec 2021 13:12:24 +0100 Message-Id: <20211216121233.162288-3-f.ebner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211216121233.162288-1-f.ebner@proxmox.com> References: <20211216121233.162288-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.096 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [storage.pm, pbsplugin.pm, btrfsplugin.pm, cifsplugin.pm, cephfsplugin.pm, dirplugin.pm, nfsplugin.pm, glusterfsplugin.pm, plugin.pm] URI_NOVOWEL 0.5 URI hostname has long non-vowel sequence Subject: [pve-devel] [PATCH storage 2/2] plugins: allow limiting the number of protected backups per guest X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Dec 2021 12:12:44 -0000 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 | 24 ++++++++++++++++++++++++ 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, 39 insertions(+), 1 deletion(-) diff --git a/PVE/Storage.pm b/PVE/Storage.pm index d64019f..0643fad 100755 --- a/PVE/Storage.pm +++ b/PVE/Storage.pm @@ -232,6 +232,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 = $scfg->{'max-protected-backups'} // 5; + + 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 a3f9ebe..ac36d14 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 0a6c619..bbe0af4 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 => 5, + }, shared => { description => "Mark storage as shared.", type => 'boolean', -- 2.30.2