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 55EE36AC52 for ; Fri, 17 Sep 2021 15:03:07 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AE8DE8882 for ; Fri, 17 Sep 2021 15:02:36 +0200 (CEST) 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 198518672 for ; Fri, 17 Sep 2021 15:02:32 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E6B4A4427F for ; Fri, 17 Sep 2021 15:02:31 +0200 (CEST) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Fri, 17 Sep 2021 15:02:24 +0200 Message-Id: <20210917130227.248852-5-f.ebner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210917130227.248852-1-f.ebner@proxmox.com> References: <20210917130227.248852-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.323 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, plugin.pm, dirplugin.pm, content.pm] Subject: [pve-devel] [RFC storage 4/6] fix #3307: make it possible to set protection for backups 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: Fri, 17 Sep 2021 13:03:07 -0000 A protected backup is not removed by free_image and ignored when pruning. The protection_file_path function is introduced in Storage.pm, so that it can also be used by vzdump itself and in archive_remove. Signed-off-by: Fabian Ebner --- Needs an APIAGE+APIVER bump (can be covered by the one for patch #2), because of the new attribute. I decided to not just re-use the "renamed is protected" behavior when pruning, because: 1. it wouldn't protect against removal 2. it would make it necessary to rename notes and log files too Should the protection files rather be hidden files? Should we also bother to set the immutable flag on filesystems that support it? PVE/API2/Storage/Content.pm | 37 ++++++++++++++++++++++++++++--------- PVE/Storage.pm | 9 +++++++++ PVE/Storage/DirPlugin.pm | 26 ++++++++++++++++++++++++-- PVE/Storage/Plugin.pm | 7 +++++++ test/prune_backups_test.pm | 13 +++++++++++++ 5 files changed, 81 insertions(+), 11 deletions(-) diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm index b3dc593..45b8de8 100644 --- a/PVE/API2/Storage/Content.pm +++ b/PVE/API2/Storage/Content.pm @@ -113,6 +113,11 @@ __PACKAGE__->register_method ({ }, optional => 1, }, + protected => { + description => "Protection status. Currently only supported for backups.", + type => 'boolean', + optional => 1, + }, }, }, links => [ { rel => 'child', href => "{volid}" } ], @@ -295,7 +300,12 @@ __PACKAGE__->register_method ({ description => "Optional notes.", optional => 1, type => 'string', - } + }, + protected => { + description => "Protection status. Currently only supported for backups.", + type => 'boolean', + optional => 1, + }, }, }, code => sub { @@ -321,12 +331,14 @@ __PACKAGE__->register_method ({ format => $format, }; - # keep going if fetching an optional attribute fails - eval { - my $notes = PVE::Storage::get_volume_attribute($cfg, $volid, 'notes'); - $entry->{notes} = $notes if defined($notes); - }; - warn $@ if $@; + for my $attribute (qw(notes protected)) { + # keep going if fetching an optional attribute fails + eval { + my $value = PVE::Storage::get_volume_attribute($cfg, $volid, $attribute); + $entry->{$attribute} = $value if defined($value); + }; + warn $@ if $@; + } return $entry; }}); @@ -356,6 +368,11 @@ __PACKAGE__->register_method ({ type => 'string', optional => 1, }, + protected => { + description => "Protection status. Currently only supported for backups.", + type => 'boolean', + optional => 1, + }, }, }, returns => { type => 'null' }, @@ -371,8 +388,10 @@ __PACKAGE__->register_method ({ PVE::Storage::check_volume_access($rpcenv, $authuser, $cfg, undef, $volid); - if (exists $param->{notes}) { - PVE::Storage::update_volume_attribute($cfg, $volid, 'notes', $param->{notes}); + for my $attr (qw(notes protected)) { + if (exists $param->{$attr}) { + PVE::Storage::update_volume_attribute($cfg, $volid, $attr, $param->{$attr}); + } } return undef; diff --git a/PVE/Storage.pm b/PVE/Storage.pm index e7d8e62..3beb645 100755 --- a/PVE/Storage.pm +++ b/PVE/Storage.pm @@ -1463,6 +1463,12 @@ sub decompressor_info { return $info; } +sub protection_file_path { + my ($path) = @_; + + return "${path}.protected"; +} + sub archive_info { my ($archive) = shift; my $info; @@ -1494,6 +1500,9 @@ sub archive_info { sub archive_remove { my ($archive_path) = @_; + die "cannot remove protected archive '$archive_path'\n" + if -e protection_file_path($archive_path); + my $dirname = dirname($archive_path); my $archive_info = eval { archive_info($archive_path) } // {}; my $logfn = $archive_info->{logfilename}; diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm index 6ab9ef2..99a6eaf 100644 --- a/PVE/Storage/DirPlugin.pm +++ b/PVE/Storage/DirPlugin.pm @@ -2,8 +2,11 @@ package PVE::Storage::DirPlugin; use strict; use warnings; + use Cwd; use File::Path; +use IO::File; + use PVE::Storage::Plugin; use PVE::JSONSchema qw(get_standard_option); @@ -93,13 +96,16 @@ sub get_volume_attribute { my ($vtype) = $class->parse_volname($volname); return if $vtype ne 'backup'; + my $path = $class->filesystem_path($scfg, $volname); + if ($attribute eq 'notes') { - my $path = $class->filesystem_path($scfg, $volname); $path .= $class->SUPER::NOTES_EXT; return PVE::Tools::file_get_contents($path) if -f $path; return ''; + } elsif ($attribute eq 'protected') { + return -e PVE::Storage::protection_file_path($path) ? 1 : 0; } return; @@ -111,8 +117,9 @@ sub update_volume_attribute { my ($vtype) = $class->parse_volname($volname); die "only backups support attribute '$attribute'\n" if $vtype ne 'backup'; + my $path = $class->filesystem_path($scfg, $volname); + if ($attribute eq 'notes') { - my $path = $class->filesystem_path($scfg, $volname); $path .= $class->SUPER::NOTES_EXT; if (defined($value) && $value ne '') { @@ -120,6 +127,21 @@ sub update_volume_attribute { } elsif (-e $path) { unlink $path or die "could not delete notes - $!\n"; } + return; + } elsif ($attribute eq 'protected') { + my $protection_path = PVE::Storage::protection_file_path($path); + + return if !((-e $protection_path) xor $value); # protection status already correct + + if ($value) { + my $fh = IO::File->new($protection_path, O_CREAT, 0644) + or die "unable to create protection file '$protection_path' - $!\n"; + close($fh); + } else { + unlink $protection_path + or die "unable to remove protection file '$protection_path' - $!\n"; + } + return; } diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm index 1182008..1ebd705 100644 --- a/PVE/Storage/Plugin.pm +++ b/PVE/Storage/Plugin.pm @@ -782,6 +782,9 @@ sub alloc_image { sub free_image { my ($class, $storeid, $scfg, $volname, $isBase, $format) = @_; + die "cannot remove protected volume '$volname' on '$storeid'\n" + if $class->get_volume_attribute($scfg, $storeid, $volname, 'protected'); + my $path = $class->filesystem_path($scfg, $volname); if ($isBase) { @@ -871,6 +874,7 @@ sub update_volume_notes { # Should die if there is an error fetching the attribute. # Possible attributes: # notes - user-provided comments/notes. +# protected - not to be removed by free_image, and for backups, ignored when pruning. sub get_volume_attribute { my ($class, $scfg, $storeid, $volname, $attribute) = @_; @@ -1115,6 +1119,7 @@ my $get_subdir_files = sub { $info->{notes} = $notes if defined($notes); } + $info->{protected} = 1 if -e PVE::Storage::protection_file_path($original); } elsif ($tt eq 'snippets') { $info = { @@ -1320,6 +1325,8 @@ sub prune_backups { $prune_entry->{mark} = 'protected'; } + $prune_entry->{mark} = 'protected' if $backup->{protected}; + push @{$prune_list}, $prune_entry; } diff --git a/test/prune_backups_test.pm b/test/prune_backups_test.pm index a87c4b3..8ad6144 100644 --- a/test/prune_backups_test.pm +++ b/test/prune_backups_test.pm @@ -29,6 +29,12 @@ foreach my $vmid (@vmids) { 'ctime' => $basetime - 24*60*60 - 60*60, 'vmid' => $vmid, }, + { + 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2019_12_31-11_18_51.tar.zst", + 'ctime' => $basetime - 24*60*60 - 60*60 + 30, + 'vmid' => $vmid, + 'protected' => 1, + }, { 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2019_12_31-11_19_21.tar.zst", 'ctime' => $basetime - 24*60*60 - 60*60 + 60, @@ -140,6 +146,13 @@ sub generate_expected { 'mark' => $marks->[1], 'vmid' => $vmid, }, + { + 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2019_12_31-11_18_51.tar.zst", + 'type' => 'qemu', + 'ctime' => $basetime - 24*60*60 - 60*60 + 30, + 'mark' => 'protected', + 'vmid' => $vmid, + }, { 'volid' => "$storeid:backup/vzdump-qemu-$vmid-2019_12_31-11_19_21.tar.zst", 'type' => 'qemu', -- 2.30.2