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 C4BDA70C38 for ; Thu, 30 Sep 2021 13:42:57 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3D3A21EF15 for ; Thu, 30 Sep 2021 13:42:27 +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 935591ED4C for ; Thu, 30 Sep 2021 13:42:20 +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 6D15044C3C for ; Thu, 30 Sep 2021 13:42:20 +0200 (CEST) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Thu, 30 Sep 2021 13:42:08 +0200 Message-Id: <20210930114215.240095-6-f.ebner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210930114215.240095-1-f.ebner@proxmox.com> References: <20210930114215.240095-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.298 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. [plugin.pm, storage.pm, content.pm, dirplugin.pm] Subject: [pve-devel] [PATCH v2 storage 5/7] 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: Thu, 30 Sep 2021 11:42:57 -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. For pruning, renamed backups already behaved similiar to how protected backups will, but there are a few reasons to not just use that for implementing the new feature: 1. It wouldn't protect against removal. 2. It would make it necessary to rename notes and log files too. 3. It wouldn't naturally extend to other volumes if that's needed. Signed-off-by: Fabian Ebner --- Changes from v1: * Rebase, because of newly introduced fall-back to {get, update}_volume_notes. * Don't die if unlink failed with ENOENT. * Add rationale about not re-using "renamed is protected" behavior to the commit message. PVE/API2/Storage/Content.pm | 37 ++++++++++++++++++++++++++++--------- PVE/Storage.pm | 9 +++++++++ PVE/Storage/DirPlugin.pm | 30 ++++++++++++++++++++++++++++++ PVE/Storage/Plugin.pm | 7 +++++++ test/prune_backups_test.pm | 13 +++++++++++++ 5 files changed, 87 insertions(+), 9 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 b496d06..0d9bf51 100644 --- a/PVE/Storage/DirPlugin.pm +++ b/PVE/Storage/DirPlugin.pm @@ -5,6 +5,7 @@ use warnings; use Cwd; use File::Path; +use IO::File; use POSIX; use PVE::Storage::Plugin; @@ -132,6 +133,14 @@ sub get_volume_attribute { return $class->get_volume_notes($scfg, $storeid, $volname); } + my ($vtype) = $class->parse_volname($volname); + return if $vtype ne 'backup'; + + if ($attribute eq 'protected') { + my $path = $class->filesystem_path($scfg, $volname); + return -e PVE::Storage::protection_file_path($path) ? 1 : 0; + } + return; } @@ -142,6 +151,27 @@ sub update_volume_attribute { return $class->update_volume_notes($scfg, $storeid, $volname, $value); } + my ($vtype) = $class->parse_volname($volname); + die "only backups support attribute '$attribute'\n" if $vtype ne 'backup'; + + if ($attribute eq 'protected') { + my $path = $class->filesystem_path($scfg, $volname); + 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 $! == ENOENT + or die "could not delete protection file '$protection_path' - $!\n"; + } + + return; + } + die "attribute '$attribute' is not supported for storage type '$scfg->{type}'\n"; } 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