[pve-devel] [RFC storage 4/6] fix #3307: make it possible to set protection for backups

[Fabian Ebner <f.ebner@proxmox.com>]

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 <f.ebner@proxmox.com>
---

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