[pve-devel] [PATCH v2 storage 5/7] fix #3307: make it possible to set protection for backups
Fabian Ebner
f.ebner at proxmox.com
Thu Sep 30 13:42:08 CEST 2021
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 <f.ebner at proxmox.com>
---
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
More information about the pve-devel
mailing list