From: Fabian Ebner <f.ebner@proxmox.com>
Date: Thu, 30 Sep 2021 11:42:08 +0000 (+0200)
Subject: fix #3307: make it possible to set protection for backups
X-Git-Url: https://git.proxmox.com/?p=pve-storage.git;a=commitdiff_plain;h=56897a920350d4429485e27b2b165860c6c543cf

fix #3307: make it possible to set protection for backups

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

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 223fbb5..fdc21d9 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1456,6 +1456,12 @@ sub decompressor_info {
     return $info;
 }
 
+sub protection_file_path {
+    my ($path) = @_;
+
+    return "${path}.protected";
+}
+
 sub archive_info {
     my ($archive) = shift;
     my $info;
@@ -1487,6 +1493,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 2cb55b8..c60818b 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;
@@ -133,6 +134,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;
 }
 
@@ -143,6 +152,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 84de785..409a05c 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -828,6 +828,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) {
@@ -917,6 +920,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) = @_;
 
@@ -1164,6 +1168,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 = {
@@ -1370,6 +1375,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',