public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups
@ 2022-03-29 12:53 Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 storage 1/1] plugins: allow limiting the number of protected backups per guest Fabian Ebner
                   ` (12 more replies)
  0 siblings, 13 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

The series consists of two parts:

First part (up to docs 2/2) is for introducing a storage property
to limit the number of protected backups and only count unprotected
backups for the limit check in vzdump. This is useful to still be able
to limit the amount of backups a user can make, because protected
backups are not considered when pruning, and a user with backup
privilege could mark their new backups as protected each time.

Second part introduces 'protected' and a 'notes-template' option to
generate notes from a template string with certain variables for
vzdump, and adds them for manual backup and backup jobs in the UI.


Changes from v1:
    * Add some rationale to the cover letter.
    * Drop already applied patch.
    * Default to unlimited for privileged users. I also dropped the
      patch to dynamically set the property upon storage creation in
      the UI, because the default itself is more dynamic now.
    * Switch to a template string for notes, supporting certain
      variables.


Previous discussion here:
https://lists.proxmox.com/pipermail/pve-devel/2021-December/051214.html


storage:

Fabian Ebner (1):
  plugins: allow limiting the number of protected backups per guest

 PVE/Storage.pm                 | 35 ++++++++++++++++++++++++++++++++++
 PVE/Storage/BTRFSPlugin.pm     |  3 ++-
 PVE/Storage/CIFSPlugin.pm      |  1 +
 PVE/Storage/CephFSPlugin.pm    |  1 +
 PVE/Storage/DirPlugin.pm       |  1 +
 PVE/Storage/GlusterfsPlugin.pm |  1 +
 PVE/Storage/NFSPlugin.pm       |  1 +
 PVE/Storage/PBSPlugin.pm       |  1 +
 PVE/Storage/Plugin.pm          |  7 +++++++
 9 files changed, 50 insertions(+), 1 deletion(-)


manager:

Fabian Ebner (8):
  vzdump: backup file list: drop unused parameter
  vzdump: backup limit: only count unprotected backups
  ui: storage edit: retention: add max-protected-backups setting
  vzdump: support setting protected status
  partially close #438: vzdump: support setting notes-template
  ui: backup: allow setting protected and notes-template for manual
    backup
  close #438: ui: backup job: allow setting a notes-template for a job
  ui: backup job: set guest name as default notes-template

 PVE/VZDump.pm                        | 89 +++++++++++++++++++++-------
 www/manager6/dc/Backup.js            | 18 ++++++
 www/manager6/panel/BackupJobPrune.js | 47 ++++++++++++---
 www/manager6/storage/Base.js         |  1 +
 www/manager6/window/Backup.js        | 25 +++++++-
 5 files changed, 147 insertions(+), 33 deletions(-)


docs:

Fabian Ebner (2):
  storage: switch to prune-backups in examples
  vzdump/storage: mention protected backups limit and give an example

 pve-storage-dir.adoc | 9 +++++----
 pve-storage-pbs.adoc | 2 +-
 vzdump.adoc          | 5 +++++
 3 files changed, 11 insertions(+), 5 deletions(-)


guest-common:

Fabian Ebner (1):
  vzdump: schema: add 'notes-template' and 'protected' properties

 src/PVE/VZDump/Common.pm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 storage 1/1] plugins: allow limiting the number of protected backups per guest
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-04-06 10:42   ` [pve-devel] applied: " Fabian Grünbichler
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 1/8] vzdump: backup file list: drop unused parameter Fabian Ebner
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

The ability to mark backups as protected broke the implicit assumption
in vzdump that remove=1 and current number of backups being the limit
(i.e. sum of all keep options) will result in a backup being removed.

Introduce a new storage property 'max-protected-backups' to limit the
number of protected backups per guest. Use 5 as a default value, as it
should cover most use cases, while still not having too big of a
potential overhead in many scenarios.

For external plugins that do not return the backup subtype in
list_volumes, all protected backups with the same ID will count
towards the limit.

An alternative would be to count the protected backups when pruning.
While that would avoid the need for a new property, it would break the
current semantics of protected backups being ignored for pruning. It
also would be less flexible, e.g. for PBS, it can make sense to have
both keep-all=1 and a limit for the number of protected snapshots on
the PVE side.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * Use different default depending on privilege level.

 PVE/Storage.pm                 | 35 ++++++++++++++++++++++++++++++++++
 PVE/Storage/BTRFSPlugin.pm     |  3 ++-
 PVE/Storage/CIFSPlugin.pm      |  1 +
 PVE/Storage/CephFSPlugin.pm    |  1 +
 PVE/Storage/DirPlugin.pm       |  1 +
 PVE/Storage/GlusterfsPlugin.pm |  1 +
 PVE/Storage/NFSPlugin.pm       |  1 +
 PVE/Storage/PBSPlugin.pm       |  1 +
 PVE/Storage/Plugin.pm          |  7 +++++++
 9 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 6112991..1a4454c 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -211,6 +211,17 @@ sub storage_can_replicate {
     return $plugin->storage_can_replicate($scfg, $storeid, $format);
 }
 
+sub get_max_protected_backups {
+    my ($scfg, $storeid) = @_;
+
+    return $scfg->{'max-protected-backups'} if defined($scfg->{'max-protected-backups'});
+
+    my $rpcenv = PVE::RPCEnvironment::get();
+    my $authuser = $rpcenv->get_user();
+
+    return $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate'], 1) ? -1 : 5;
+}
+
 sub storage_ids {
     my ($cfg) = @_;
 
@@ -240,6 +251,30 @@ sub update_volume_attribute {
     my $scfg = storage_config($cfg, $storeid);
     my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
 
+    my ($vtype, undef, $vmid) = $plugin->parse_volname($volname);
+    my $max_protected_backups = get_max_protected_backups($scfg, $storeid);
+
+    if (
+	$vtype eq 'backup'
+	&& $vmid
+	&& $attribute eq 'protected'
+	&& $value
+	&& !$plugin->get_volume_attribute($scfg, $storeid, $volname, 'protected')
+	&& $max_protected_backups > -1 # -1 is unlimited
+    ) {
+	my $backups = $plugin->list_volumes($storeid, $scfg, $vmid, ['backup']);
+	my ($backup_type) = map { $_->{subtype} } grep { $_->{volid} eq $volid } $backups->@*;
+
+	my $protected_count = grep {
+	    $_->{protected} && (!$backup_type || ($_->{subtype} && $_->{subtype} eq $backup_type))
+	} $backups->@*;
+
+	if ($max_protected_backups <= $protected_count) {
+	    die "The number of protected backups per guest is limited to $max_protected_backups ".
+		"on storage '$storeid'\n";
+	}
+    }
+
     return $plugin->update_volume_attribute($scfg, $storeid, $volname, $attribute, $value);
 }
 
diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm
index c8caa41..7dac34b 100644
--- a/PVE/Storage/BTRFSPlugin.pm
+++ b/PVE/Storage/BTRFSPlugin.pm
@@ -67,7 +67,8 @@ sub options {
 	shared => { optional => 1 },
 	disable => { optional => 1 },
 	maxfiles => { optional => 1 },
-	'prune-backups'=> { optional => 1 },
+	'prune-backups' => { optional => 1 },
+	'max-protected-backups' => { optional => 1 },
 	content => { optional => 1 },
 	format => { optional => 1 },
 	is_mountpoint => { optional => 1 },
diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index d5efa5f..982040a 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -134,6 +134,7 @@ sub options {
 	disable => { optional => 1 },
 	maxfiles => { optional => 1 },
 	'prune-backups' => { optional => 1 },
+	'max-protected-backups' => { optional => 1 },
 	content => { optional => 1 },
 	format => { optional => 1 },
 	username => { optional => 1 },
diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
index f75c1b8..4976747 100644
--- a/PVE/Storage/CephFSPlugin.pm
+++ b/PVE/Storage/CephFSPlugin.pm
@@ -155,6 +155,7 @@ sub options {
 	maxfiles => { optional => 1 },
 	keyring => { optional => 1 },
 	'prune-backups' => { optional => 1 },
+	'max-protected-backups' => { optional => 1 },
 	'fs-name' => { optional => 1 },
     };
 }
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index c60818b..1baad63 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -58,6 +58,7 @@ sub options {
 	disable => { optional => 1 },
 	maxfiles => { optional => 1 },
 	'prune-backups' => { optional => 1 },
+	'max-protected-backups' => { optional => 1 },
 	content => { optional => 1 },
 	format => { optional => 1 },
 	mkdir => { optional => 1 },
diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
index d8d2b88..ad386d2 100644
--- a/PVE/Storage/GlusterfsPlugin.pm
+++ b/PVE/Storage/GlusterfsPlugin.pm
@@ -133,6 +133,7 @@ sub options {
 	disable => { optional => 1 },
 	maxfiles => { optional => 1 },
 	'prune-backups' => { optional => 1 },
+	'max-protected-backups' => { optional => 1 },
 	content => { optional => 1 },
 	format => { optional => 1 },
 	mkdir => { optional => 1 },
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index 0400c93..5bd7313 100644
--- a/PVE/Storage/NFSPlugin.pm
+++ b/PVE/Storage/NFSPlugin.pm
@@ -85,6 +85,7 @@ sub options {
 	disable => { optional => 1 },
 	maxfiles => { optional => 1 },
 	'prune-backups' => { optional => 1 },
+	'max-protected-backups' => { optional => 1 },
 	options => { optional => 1 },
 	content => { optional => 1 },
 	format => { optional => 1 },
diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index 4b3f349..687901f 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -73,6 +73,7 @@ sub options {
 	'master-pubkey' => { optional => 1 },
 	maxfiles => { optional => 1 },
 	'prune-backups' => { optional => 1 },
+	'max-protected-backups' => { optional => 1 },
 	fingerprint => { optional => 1 },
     };
 }
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 6673409..521e3e9 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -155,6 +155,13 @@ my $defaultData = {
 	    optional => 1,
 	},
 	'prune-backups' => get_standard_option('prune-backups'),
+	'max-protected-backups' => {
+	    description => "Maximal number of protected backups per guest. Use '-1' for unlimited.",
+	    type => 'integer',
+	    minimum => -1,
+	    optional => 1,
+	    default => "Unlimited for users with Datastore.Allocate privilege, 5 for other users",
+	},
 	shared => {
 	    description => "Mark storage as shared.",
 	    type => 'boolean',
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 manager 1/8] vzdump: backup file list: drop unused parameter
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 storage 1/1] plugins: allow limiting the number of protected backups per guest Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 2/8] vzdump: backup limit: only count unprotected backups Fabian Ebner
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

It's not used anymore since b5a3ab3d20f3b62f615ea09341a2c83fc3d243a2
aligned pruning in a dumpdir with pruning on a storage.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

 PVE/VZDump.pm | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 65867325..124d3843 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -692,12 +692,10 @@ sub compressor_info {
 }
 
 sub get_backup_file_list {
-    my ($dir, $bkname, $exclude_fn) = @_;
+    my ($dir, $bkname) = @_;
 
     my $bklist = [];
     foreach my $fn (<$dir/${bkname}-*>) {
-	next if $exclude_fn && $fn eq $exclude_fn;
-
 	my $archive_info = eval { PVE::Storage::archive_info($fn) } // {};
 	if ($archive_info->{is_std_name}) {
 	    my $filename = $archive_info->{filename};
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 manager 2/8] vzdump: backup limit: only count unprotected backups
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 storage 1/1] plugins: allow limiting the number of protected backups per guest Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 1/8] vzdump: backup file list: drop unused parameter Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [RFC v2 manager 3/8] ui: storage edit: retention: add max-protected-backups setting Fabian Ebner
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

since they are the ones relevant for pruning and protected backups
have their own separate limit.

Since get_backup_file_list is only used in places where the
unprotected backups are needed, adapt the helper accordingly.

If there is a storage, use PVE::Storage::volume_list to count the
unprotected backups. This avoids a direct invocation of the
proxmox-backup-client for PBS and the limit check can also work for
external storage plugins which might not be dir-based or name the
backups differently.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

Dependency bump for storage is needed for it to filter by backup type
and not just ID, and also since we'd like to have the limiting
behavior for protected there, before making the limit weaker here.

 PVE/VZDump.pm | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 124d3843..fe8a9b28 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -691,16 +691,17 @@ sub compressor_info {
     }
 }
 
-sub get_backup_file_list {
+sub get_unprotected_backup_file_list {
     my ($dir, $bkname) = @_;
 
     my $bklist = [];
     foreach my $fn (<$dir/${bkname}-*>) {
 	my $archive_info = eval { PVE::Storage::archive_info($fn) } // {};
 	if ($archive_info->{is_std_name}) {
-	    my $filename = $archive_info->{filename};
+	    my $path = "$dir/$archive_info->{filename}";
+	    next if -e PVE::Storage::protection_file_path($path);
 	    my $backup = {
-		'path' => "$dir/$filename",
+		'path' => $path,
 		'ctime' => $archive_info->{ctime},
 	    };
 	    push @{$bklist}, $backup;
@@ -780,13 +781,15 @@ sub exec_backup_task {
 
 	if ($backup_limit && !$opts->{remove}) {
 	    my $count;
-	    if ($self->{opts}->{pbs}) {
-		my $res = PVE::Storage::PBSPlugin::run_client_cmd($opts->{scfg}, $opts->{storage}, 'snapshots', $pbs_group_name);
-		$count = scalar(@$res);
+	    if (my $storeid = $opts->{storage}) {
+		my $backups = PVE::Storage::volume_list($cfg, $storeid, $vmid, 'backup');
+		$count = grep {
+		    !$_->{protected} && (!$_->{subtype} || $_->{subtype} eq $vmtype)
+		} $backups->@*;
 	    } else {
-		my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname);
-		$count = scalar(@$bklist);
+		$count = scalar(get_unprotected_backup_file_list($opts->{dumpdir}, $bkname)->@*);
 	    }
+
 	    die "There is a max backup limit of $backup_limit enforced by the".
 		" target storage or the vzdump parameters.".
 		" Either increase the limit or delete old backup(s).\n"
@@ -996,13 +999,7 @@ sub exec_backup_task {
 	    debugmsg ('info', "prune older backups with retention: $keepstr", $logfd);
 	    my $pruned = 0;
 	    if (!defined($opts->{storage})) {
-		my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname);
-
-		for my $prune_entry ($bklist->@*) {
-		    if (-e PVE::Storage::protection_file_path($prune_entry->{path})) {
-			$prune_entry->{mark} = 'protected';
-		    }
-		}
+		my $bklist = get_unprotected_backup_file_list($opts->{dumpdir}, $bkname);
 
 		PVE::Storage::prune_mark_backup_group($bklist, $prune_options);
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [RFC v2 manager 3/8] ui: storage edit: retention: add max-protected-backups setting
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (2 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 2/8] vzdump: backup limit: only count unprotected backups Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 docs 1/2] storage: switch to prune-backups in examples Fabian Ebner
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

I feel like this too might not be needed anymore, now that the default
is more dynamic.

Dependency bump for storage needed.

Changes from v1:
    * Adapt empty text to reflect new default.

 www/manager6/panel/BackupJobPrune.js | 47 ++++++++++++++++++++++------
 www/manager6/storage/Base.js         |  1 +
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/www/manager6/panel/BackupJobPrune.js b/www/manager6/panel/BackupJobPrune.js
index a58fe1fe..2bc3e633 100644
--- a/www/manager6/panel/BackupJobPrune.js
+++ b/www/manager6/panel/BackupJobPrune.js
@@ -15,21 +15,32 @@ Ext.define('PVE.panel.BackupJobPrune', {
 	} else if (this.isCreate && !this.rendered) {
 	    return this.keepAllDefaultForCreate ? { 'prune-backups': 'keep-all=1' } : {};
 	}
+
+	let options = { 'delete': [] };
+
+	if ('max-protected-backups' in formValues) {
+	    options['max-protected-backups'] = formValues['max-protected-backups'];
+	} else if (this.hasMaxProtected) {
+	    options.delete.push('max-protected-backups');
+	}
+
+	delete formValues['max-protected-backups'];
 	delete formValues.delete;
+
 	let retention = PVE.Parser.printPropertyString(formValues);
 	if (retention === '') {
-	    if (this.isCreate) {
-		return {};
-	    }
-	    // always delete old 'maxfiles' on edit, we map it to keep-last on window load
-	    return {
-		'delete': ['prune-backups', 'maxfiles'],
-	    };
+	    options.delete.push('prune-backups');
+	} else {
+	    options['prune-backups'] = retention;
 	}
-	let options = { 'prune-backups': retention };
+
 	if (!this.isCreate) {
-	    options.delete = 'maxfiles';
+	    // always delete old 'maxfiles' on edit, we map it to keep-last on window load
+	    options.delete.push('maxfiles');
+	} else {
+	    delete options.delete;
 	}
+
 	return options;
     },
 
@@ -57,6 +68,10 @@ Ext.define('PVE.panel.BackupJobPrune', {
 	    }
 	    panel.down('component[name=pbs-hint]').setHidden(!panel.showPBSHint);
 
+	    let maxProtected = panel.down('proxmoxintegerfield[name=max-protected-backups]');
+	    maxProtected.setDisabled(!panel.hasMaxProtected);
+	    maxProtected.setHidden(!panel.hasMaxProtected);
+
 	    panel.query('pmxPruneKeepField').forEach(field => {
 		field.on('change', panel.updateComponents, panel);
 	    });
@@ -95,5 +110,19 @@ Ext.define('PVE.panel.BackupJobPrune', {
 	    padding: '5 1',
 	    html: gettext("It's preferred to configure backup retention directly on the Proxmox Backup Server."),
 	},
+	{
+	    xtype: 'proxmoxintegerfield',
+	    name: 'max-protected-backups',
+	    fieldLabel: gettext('Maximum Protected'),
+	    minValue: -1,
+	    hidden: true,
+	    disabled: true,
+	    emptyText: 'unlimited with Datastore.Allocate privilege, 5 otherwise',
+	    deleteEmpty: true,
+	    autoEl: {
+		tag: 'div',
+		'data-qtip': Ext.String.format(gettext('Use {0} for unlimited'), -1),
+	    },
+	},
     ],
 });
diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js
index 89dc2eff..7f6d7a09 100644
--- a/www/manager6/storage/Base.js
+++ b/www/manager6/storage/Base.js
@@ -120,6 +120,7 @@ Ext.define('PVE.storage.BaseEdit', {
 		    {
 			xtype: 'pveBackupJobPrunePanel',
 			title: gettext('Backup Retention'),
+			hasMaxProtected: true,
 			isCreate: me.isCreate,
 			keepAllDefaultForCreate: true,
 			showPBSHint: me.ipanel.isPBS,
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 docs 1/2] storage: switch to prune-backups in examples
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (3 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [RFC v2 manager 3/8] ui: storage edit: retention: add max-protected-backups setting Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 docs 2/2] vzdump/storage: mention protected backups limit and give an example Fabian Ebner
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

because maxfiles is deprecated.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

 pve-storage-dir.adoc | 8 ++++----
 pve-storage-pbs.adoc | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/pve-storage-dir.adoc b/pve-storage-dir.adoc
index 090a44b..d1ca3eb 100644
--- a/pve-storage-dir.adoc
+++ b/pve-storage-dir.adoc
@@ -55,12 +55,12 @@ needs to be an absolute file system path.
 dir: backup
         path /mnt/backup
         content backup
-        maxfiles 7
+        prune-backups keep-last=7
 ----
 
-Above configuration defines a storage pool called `backup`. That pool
-can be used to store up to 7 backups (`maxfiles 7`) per VM. The real
-path for the backup files is `/mnt/backup/dump/...`.
+The above configuration defines a storage pool called `backup`. That pool can be
+used to store up to 7 backups (`keep-last=7`) per VM. The real path for the
+backup files is `/mnt/backup/dump/...`.
 
 
 File naming conventions
diff --git a/pve-storage-pbs.adoc b/pve-storage-pbs.adoc
index a3d7da1..0ec93de 100644
--- a/pve-storage-pbs.adoc
+++ b/pve-storage-pbs.adoc
@@ -71,7 +71,7 @@ pbs: backup
         server enya.proxmox.com
         content backup
         fingerprint 09:54:ef:..snip..:88:af:47:fe:4c:3b:cf:8b:26:88:0b:4e:3c:b2
-        maxfiles 0
+        prune-backups keep-all=1
         username archiver@pbs
 ----
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 docs 2/2] vzdump/storage: mention protected backups limit and give an example
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (4 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 docs 1/2] storage: switch to prune-backups in examples Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 guest-common 1/1] vzdump: schema: add 'notes-template' and 'protected' properties Fabian Ebner
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * Explicitly mention the (now more dynamic) default rather than
      just having it documented in the schema only.

 pve-storage-dir.adoc | 5 +++--
 vzdump.adoc          | 5 +++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/pve-storage-dir.adoc b/pve-storage-dir.adoc
index d1ca3eb..4116fe5 100644
--- a/pve-storage-dir.adoc
+++ b/pve-storage-dir.adoc
@@ -56,11 +56,12 @@ dir: backup
         path /mnt/backup
         content backup
         prune-backups keep-last=7
+        max-protected-backups 3
 ----
 
 The above configuration defines a storage pool called `backup`. That pool can be
-used to store up to 7 backups (`keep-last=7`) per VM. The real path for the
-backup files is `/mnt/backup/dump/...`.
+used to store up to 7 regular backups (`keep-last=7`) and 3 protected backups
+per VM. The real path for the backup files is `/mnt/backup/dump/...`.
 
 
 File naming conventions
diff --git a/vzdump.adoc b/vzdump.adoc
index b9dca2a..093f639 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -301,6 +301,11 @@ For filesystem-based storages, the protection is implemented via a sentinel file
 `<backup-name>.protected`. For Proxmox Backup Server, it is handled on the
 server side (available since Proxmox Backup Server version 2.1).
 
+Use the storage option `max-protected-backups` to control how many protected
+backups per guest are allowed on the storage. Use `-1` for unlimited. The
+default is unlimited for users with `Datastore.Allocate` privilege and `5` for
+other users.
+
 [[vzdump_restore]]
 Restore
 -------
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 guest-common 1/1] vzdump: schema: add 'notes-template' and 'protected' properties
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (5 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 docs 2/2] vzdump/storage: mention protected backups limit and give an example Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 4/8] vzdump: support setting protected status Fabian Ebner
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

In command_line(), the notes-template string is printed, quoted, but
otherwise as is, which is a bit ugly for multi-line notes. But it is
part of the commandline, so print it.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * Rename parameter to notes-template and adapt description for new
      functionality.

 src/PVE/VZDump/Common.pm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index 83d7413..84c97ba 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -233,7 +233,21 @@ my $confdesc = {
 	type => 'string',
 	description => 'Backup all known guest systems included in the specified pool.',
 	optional => 1,
-    }
+    },
+    'notes-template' => {
+	type => 'string',
+	description => "Template string for generating notes for the backup(s). It can contain ".
+	    "variables which will be replaced by their values. Currently supported are \$CLUSTER, ".
+	    "\$GUESTNAME, \$NODE, and \$VMID, but more might be added in the future.",
+	requires => 'storage',
+	optional => 1,
+    },
+    protected => {
+	type => 'boolean',
+	description => "If true, mark backup(s) as protected.",
+	requires => 'storage',
+	optional => 1,
+    },
 };
 
 sub get_confdesc {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 manager 4/8] vzdump: support setting protected status
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (6 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 guest-common 1/1] vzdump: schema: add 'notes-template' and 'protected' properties Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 5/8] partially close #438: vzdump: support setting notes-template Fabian Ebner
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

Check the number of protected backups early if the protected flag
is set.

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * Split setting notes (now notes-template) into its own patch.
    * Use new helper from storage to determine max-protected-backups.

 PVE/VZDump.pm | 43 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index fe8a9b28..eab4f240 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use Fcntl ':flock';
+use File::Basename;
 use File::Path;
 use IO::File;
 use IO::Select;
@@ -779,21 +780,33 @@ sub exec_backup_task {
 	    }
 	}
 
-	if ($backup_limit && !$opts->{remove}) {
+	if (($backup_limit && !$opts->{remove}) || $opts->{protected}) {
 	    my $count;
+	    my $protected_count;
 	    if (my $storeid = $opts->{storage}) {
-		my $backups = PVE::Storage::volume_list($cfg, $storeid, $vmid, 'backup');
-		$count = grep {
-		    !$_->{protected} && (!$_->{subtype} || $_->{subtype} eq $vmtype)
-		} $backups->@*;
+		my @backups = grep {
+		    !$_->{subtype} || $_->{subtype} eq $vmtype
+		} PVE::Storage::volume_list($cfg, $storeid, $vmid, 'backup')->@*;
+
+		$count = grep { !$_->{protected} } @backups;
+		$protected_count = scalar(@backups) - $count;
 	    } else {
 		$count = scalar(get_unprotected_backup_file_list($opts->{dumpdir}, $bkname)->@*);
 	    }
 
-	    die "There is a max backup limit of $backup_limit enforced by the".
-		" target storage or the vzdump parameters.".
-		" Either increase the limit or delete old backup(s).\n"
-		if $count >= $backup_limit;
+	    if ($opts->{protected}) {
+		my $max_protected = PVE::Storage::get_max_protected_backups(
+		    $opts->{scfg},
+		    $opts->{storage},
+		);
+		if ($max_protected > -1 && $protected_count >= $max_protected) {
+		    die "The number of protected backups per guest is limited to $max_protected ".
+			"on storage '$opts->{storage}'\n";
+		}
+	    } elsif ($count >= $backup_limit) {
+		die "There is a max backup limit of $backup_limit enforced by the target storage ".
+		    "or the vzdump parameters. Either increase the limit or delete old backups.\n";
+	    }
 	}
 
 	if (!$self->{opts}->{pbs}) {
@@ -994,6 +1007,18 @@ sub exec_backup_task {
 	    debugmsg ('info', "archive file size: $cs", $logfd);
 	}
 
+	# Mark as protected before pruning.
+	if (my $storeid = $opts->{storage}) {
+	    my $volname = $opts->{pbs} ? $task->{target} : basename($task->{target});
+	    my $volid = "${storeid}:backup/${volname}";
+
+	    if ($opts->{protected}) {
+		debugmsg('info', "marking backup as protected", $logfd);
+		eval { PVE::Storage::update_volume_attribute($cfg, $volid, 'protected', 1) };
+		die "unable to set protected flag - $@\n" if $@;
+	    }
+	}
+
 	if ($opts->{remove}) {
 	    my $keepstr = join(', ', map { "$_=$prune_options->{$_}" } sort keys %$prune_options);
 	    debugmsg ('info', "prune older backups with retention: $keepstr", $logfd);
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 manager 5/8] partially close #438: vzdump: support setting notes-template
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (7 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 4/8] vzdump: support setting protected status Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 6/8] ui: backup: allow setting protected and notes-template for manual backup Fabian Ebner
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * Split into its own patch.
    * Expand certain variables in the text.

Arguably, a warning from failing to set notes is not very visible, but
I didn't want to make it a full-blown error.

 PVE/VZDump.pm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index eab4f240..ec0f638d 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -70,6 +70,22 @@ sub run_command {
     PVE::Tools::run_command($cmdstr, %param, logfunc => $logfunc);
 }
 
+my $generate_notes = sub {
+    my ($notes_template, $task) = @_;
+
+    my $info = {
+	CLUSTER => PVE::Cluster::get_clinfo()->{cluster}->{name},
+	GUESTNAME => $task->{hostname},
+	NODE => PVE::INotify::nodename(),
+	VMID => $task->{vmid},
+    };
+
+    my $vars = join('|', keys $info->%*);
+    $notes_template =~ s/\$($vars)([^A-Za-z0-9_]|$)/\Q$info->{$1}\E$2/g;
+
+    return $notes_template;
+};
+
 my $parse_prune_backups_maxfiles = sub {
     my ($param, $kind) = @_;
 
@@ -1012,6 +1028,13 @@ sub exec_backup_task {
 	    my $volname = $opts->{pbs} ? $task->{target} : basename($task->{target});
 	    my $volid = "${storeid}:backup/${volname}";
 
+	    if ($opts->{'notes-template'} && $opts->{'notes-template'} ne '') {
+		debugmsg('info', "adding notes to backup", $logfd);
+		my $notes = $generate_notes->($opts->{'notes-template'}, $task);
+		eval { PVE::Storage::update_volume_attribute($cfg, $volid, 'notes', $notes) };
+		debugmsg('warn', "unable to add notes - $@", $logfd) if $@;
+	    }
+
 	    if ($opts->{protected}) {
 		debugmsg('info', "marking backup as protected", $logfd);
 		eval { PVE::Storage::update_volume_attribute($cfg, $volid, 'protected', 1) };
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 manager 6/8] ui: backup: allow setting protected and notes-template for manual backup
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (8 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 5/8] partially close #438: vzdump: support setting notes-template Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 7/8] close #438: ui: backup job: allow setting a notes-template for a job Fabian Ebner
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

Setting a width, so the text area can fill the horizontal space.

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

 www/manager6/window/Backup.js | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/www/manager6/window/Backup.js b/www/manager6/window/Backup.js
index 726122c8..60db5a45 100644
--- a/www/manager6/window/Backup.js
+++ b/www/manager6/window/Backup.js
@@ -154,19 +154,33 @@ Ext.define('PVE.window.Backup', {
 	    },
 	});
 
+	let protectedCheckbox = Ext.create('Proxmox.form.Checkbox', {
+	    name: 'protected',
+	    checked: false,
+	    uncheckedValue: 0,
+	    fieldLabel: gettext('Protected'),
+	});
+
 	me.formPanel = Ext.create('Proxmox.panel.InputPanel', {
 	    bodyPadding: 10,
 	    border: false,
 	    column1: [
 		storagesel,
 		modeSelector,
-		removeCheckbox,
+		protectedCheckbox,
 	    ],
 	    column2: [
 		compressionSelector,
 		mailtoField,
+		removeCheckbox,
 	    ],
 	    columnB: [
+		{
+		    xtype: 'textareafield',
+		    name: 'notes-template',
+		    fieldLabel: gettext('Notes'),
+		    anchor: '100%',
+		},
 		{
 		    xtype: 'label',
 		    name: 'pruneLabel',
@@ -229,6 +243,14 @@ Ext.define('PVE.window.Backup', {
 		    params.compress = values.compress;
 		}
 
+		if (values.protected) {
+		    params.protected = values.protected;
+		}
+
+		if (values['notes-template']) {
+		    params['notes-template'] = values['notes-template'];
+		}
+
 		Proxmox.Utils.API2Request({
 		    url: '/nodes/' + me.nodename + '/vzdump',
 		    params: params,
@@ -272,6 +294,7 @@ Ext.define('PVE.window.Backup', {
 	    modal: true,
 	    layout: 'auto',
 	    border: false,
+	    width: 600,
 	    items: [me.formPanel],
 	    buttons: [helpBtn, '->', submitBtn],
 	    listeners: {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 manager 7/8] close #438: ui: backup job: allow setting a notes-template for a job
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (9 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 6/8] ui: backup: allow setting protected and notes-template for manual backup Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 8/8] ui: backup job: set guest name as default notes-template Fabian Ebner
  2022-04-06 12:10 ` [pve-devel] partially-applied: [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Grünbichler
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

Add a tooltip to the comment field, to better distinguish it from the
notes-template.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v2.

 www/manager6/dc/Backup.js | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 2b892c6f..02a2bf4c 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -231,6 +231,23 @@ Ext.define('PVE.dc.BackupEdit', {
 		    name: 'comment',
 		    fieldLabel: gettext('Comment'),
 		    deleteEmpty: !me.isCreate,
+		    autoEl: {
+			tag: 'div',
+			'data-qtip': gettext('Description of the job'),
+		    },
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'notes-template',
+		    fieldLabel: gettext('Notes'),
+		    deleteEmpty: !me.isCreate,
+		    autoEl: {
+			tag: 'div',
+			'data-qtip': Ext.String.format(
+			    gettext('Notes added to the backups. Possible variables: {0}'),
+			    '$CLUSTER, $GUESTNAME, $NODE, $VMID',
+			),
+		    },
 		},
 	    ],
 	    onGetValues: function(values) {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH v2 manager 8/8] ui: backup job: set guest name as default notes-template
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (10 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 7/8] close #438: ui: backup job: allow setting a notes-template for a job Fabian Ebner
@ 2022-03-29 12:53 ` Fabian Ebner
  2022-04-06 12:10 ` [pve-devel] partially-applied: [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Grünbichler
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Ebner @ 2022-03-29 12:53 UTC (permalink / raw)
  To: pve-devel

for newly created jobs.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v2.

 www/manager6/dc/Backup.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 02a2bf4c..830888c2 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -241,6 +241,7 @@ Ext.define('PVE.dc.BackupEdit', {
 		    name: 'notes-template',
 		    fieldLabel: gettext('Notes'),
 		    deleteEmpty: !me.isCreate,
+		    value: me.isCreate ? '$GUESTNAME' : undefined,
 		    autoEl: {
 			tag: 'div',
 			'data-qtip': Ext.String.format(
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] applied: [PATCH v2 storage 1/1] plugins: allow limiting the number of protected backups per guest
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 storage 1/1] plugins: allow limiting the number of protected backups per guest Fabian Ebner
@ 2022-04-06 10:42   ` Fabian Grünbichler
  0 siblings, 0 replies; 15+ messages in thread
From: Fabian Grünbichler @ 2022-04-06 10:42 UTC (permalink / raw)
  To: Proxmox VE development discussion

On March 29, 2022 2:53 pm, Fabian Ebner wrote:
> The ability to mark backups as protected broke the implicit assumption
> in vzdump that remove=1 and current number of backups being the limit
> (i.e. sum of all keep options) will result in a backup being removed.
> 
> Introduce a new storage property 'max-protected-backups' to limit the
> number of protected backups per guest. Use 5 as a default value, as it
> should cover most use cases, while still not having too big of a
> potential overhead in many scenarios.
> 
> For external plugins that do not return the backup subtype in
> list_volumes, all protected backups with the same ID will count
> towards the limit.
> 
> An alternative would be to count the protected backups when pruning.
> While that would avoid the need for a new property, it would break the
> current semantics of protected backups being ignored for pruning. It
> also would be less flexible, e.g. for PBS, it can make sense to have
> both keep-all=1 and a limit for the number of protected snapshots on
> the PVE side.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v1:
>     * Use different default depending on privilege level.
> 
>  PVE/Storage.pm                 | 35 ++++++++++++++++++++++++++++++++++
>  PVE/Storage/BTRFSPlugin.pm     |  3 ++-
>  PVE/Storage/CIFSPlugin.pm      |  1 +
>  PVE/Storage/CephFSPlugin.pm    |  1 +
>  PVE/Storage/DirPlugin.pm       |  1 +
>  PVE/Storage/GlusterfsPlugin.pm |  1 +
>  PVE/Storage/NFSPlugin.pm       |  1 +
>  PVE/Storage/PBSPlugin.pm       |  1 +
>  PVE/Storage/Plugin.pm          |  7 +++++++
>  9 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 6112991..1a4454c 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -211,6 +211,17 @@ sub storage_can_replicate {
>      return $plugin->storage_can_replicate($scfg, $storeid, $format);
>  }
>  
> +sub get_max_protected_backups {
> +    my ($scfg, $storeid) = @_;
> +
> +    return $scfg->{'max-protected-backups'} if defined($scfg->{'max-protected-backups'});
> +
> +    my $rpcenv = PVE::RPCEnvironment::get();
> +    my $authuser = $rpcenv->get_user();
> +
> +    return $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate'], 1) ? -1 : 5;
> +}
> +
>  sub storage_ids {
>      my ($cfg) = @_;
>  
> @@ -240,6 +251,30 @@ sub update_volume_attribute {
>      my $scfg = storage_config($cfg, $storeid);
>      my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>  
> +    my ($vtype, undef, $vmid) = $plugin->parse_volname($volname);
> +    my $max_protected_backups = get_max_protected_backups($scfg, $storeid);
> +
> +    if (
> +	$vtype eq 'backup'
> +	&& $vmid
> +	&& $attribute eq 'protected'
> +	&& $value
> +	&& !$plugin->get_volume_attribute($scfg, $storeid, $volname, 'protected')
> +	&& $max_protected_backups > -1 # -1 is unlimited
> +    ) {
> +	my $backups = $plugin->list_volumes($storeid, $scfg, $vmid, ['backup']);
> +	my ($backup_type) = map { $_->{subtype} } grep { $_->{volid} eq $volid } $backups->@*;
> +
> +	my $protected_count = grep {
> +	    $_->{protected} && (!$backup_type || ($_->{subtype} && $_->{subtype} eq $backup_type))
> +	} $backups->@*;
> +
> +	if ($max_protected_backups <= $protected_count) {
> +	    die "The number of protected backups per guest is limited to $max_protected_backups ".
> +		"on storage '$storeid'\n";
> +	}
> +    }
> +
>      return $plugin->update_volume_attribute($scfg, $storeid, $volname, $attribute, $value);
>  }
>  
> diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm
> index c8caa41..7dac34b 100644
> --- a/PVE/Storage/BTRFSPlugin.pm
> +++ b/PVE/Storage/BTRFSPlugin.pm
> @@ -67,7 +67,8 @@ sub options {
>  	shared => { optional => 1 },
>  	disable => { optional => 1 },
>  	maxfiles => { optional => 1 },
> -	'prune-backups'=> { optional => 1 },
> +	'prune-backups' => { optional => 1 },
> +	'max-protected-backups' => { optional => 1 },
>  	content => { optional => 1 },
>  	format => { optional => 1 },
>  	is_mountpoint => { optional => 1 },
> diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
> index d5efa5f..982040a 100644
> --- a/PVE/Storage/CIFSPlugin.pm
> +++ b/PVE/Storage/CIFSPlugin.pm
> @@ -134,6 +134,7 @@ sub options {
>  	disable => { optional => 1 },
>  	maxfiles => { optional => 1 },
>  	'prune-backups' => { optional => 1 },
> +	'max-protected-backups' => { optional => 1 },
>  	content => { optional => 1 },
>  	format => { optional => 1 },
>  	username => { optional => 1 },
> diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
> index f75c1b8..4976747 100644
> --- a/PVE/Storage/CephFSPlugin.pm
> +++ b/PVE/Storage/CephFSPlugin.pm
> @@ -155,6 +155,7 @@ sub options {
>  	maxfiles => { optional => 1 },
>  	keyring => { optional => 1 },
>  	'prune-backups' => { optional => 1 },
> +	'max-protected-backups' => { optional => 1 },
>  	'fs-name' => { optional => 1 },
>      };
>  }
> diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
> index c60818b..1baad63 100644
> --- a/PVE/Storage/DirPlugin.pm
> +++ b/PVE/Storage/DirPlugin.pm
> @@ -58,6 +58,7 @@ sub options {
>  	disable => { optional => 1 },
>  	maxfiles => { optional => 1 },
>  	'prune-backups' => { optional => 1 },
> +	'max-protected-backups' => { optional => 1 },
>  	content => { optional => 1 },
>  	format => { optional => 1 },
>  	mkdir => { optional => 1 },
> diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
> index d8d2b88..ad386d2 100644
> --- a/PVE/Storage/GlusterfsPlugin.pm
> +++ b/PVE/Storage/GlusterfsPlugin.pm
> @@ -133,6 +133,7 @@ sub options {
>  	disable => { optional => 1 },
>  	maxfiles => { optional => 1 },
>  	'prune-backups' => { optional => 1 },
> +	'max-protected-backups' => { optional => 1 },
>  	content => { optional => 1 },
>  	format => { optional => 1 },
>  	mkdir => { optional => 1 },
> diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
> index 0400c93..5bd7313 100644
> --- a/PVE/Storage/NFSPlugin.pm
> +++ b/PVE/Storage/NFSPlugin.pm
> @@ -85,6 +85,7 @@ sub options {
>  	disable => { optional => 1 },
>  	maxfiles => { optional => 1 },
>  	'prune-backups' => { optional => 1 },
> +	'max-protected-backups' => { optional => 1 },
>  	options => { optional => 1 },
>  	content => { optional => 1 },
>  	format => { optional => 1 },
> diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
> index 4b3f349..687901f 100644
> --- a/PVE/Storage/PBSPlugin.pm
> +++ b/PVE/Storage/PBSPlugin.pm
> @@ -73,6 +73,7 @@ sub options {
>  	'master-pubkey' => { optional => 1 },
>  	maxfiles => { optional => 1 },
>  	'prune-backups' => { optional => 1 },
> +	'max-protected-backups' => { optional => 1 },
>  	fingerprint => { optional => 1 },
>      };
>  }
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 6673409..521e3e9 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -155,6 +155,13 @@ my $defaultData = {
>  	    optional => 1,
>  	},
>  	'prune-backups' => get_standard_option('prune-backups'),
> +	'max-protected-backups' => {
> +	    description => "Maximal number of protected backups per guest. Use '-1' for unlimited.",
> +	    type => 'integer',
> +	    minimum => -1,
> +	    optional => 1,
> +	    default => "Unlimited for users with Datastore.Allocate privilege, 5 for other users",
> +	},
>  	shared => {
>  	    description => "Mark storage as shared.",
>  	    type => 'boolean',
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] partially-applied: [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups
  2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
                   ` (11 preceding siblings ...)
  2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 8/8] ui: backup job: set guest name as default notes-template Fabian Ebner
@ 2022-04-06 12:10 ` Fabian Grünbichler
  12 siblings, 0 replies; 15+ messages in thread
From: Fabian Grünbichler @ 2022-04-06 12:10 UTC (permalink / raw)
  To: Proxmox VE development discussion

pve-storage (+bump)
first part of series regarding counting protected + small follow-up 
discussed off-list

second part with note templates requires a re-send for {{VAR}} syntax.

On March 29, 2022 2:53 pm, Fabian Ebner wrote:
> The series consists of two parts:
> 
> First part (up to docs 2/2) is for introducing a storage property
> to limit the number of protected backups and only count unprotected
> backups for the limit check in vzdump. This is useful to still be able
> to limit the amount of backups a user can make, because protected
> backups are not considered when pruning, and a user with backup
> privilege could mark their new backups as protected each time.
> 
> Second part introduces 'protected' and a 'notes-template' option to
> generate notes from a template string with certain variables for
> vzdump, and adds them for manual backup and backup jobs in the UI.
> 
> 
> Changes from v1:
>     * Add some rationale to the cover letter.
>     * Drop already applied patch.
>     * Default to unlimited for privileged users. I also dropped the
>       patch to dynamically set the property upon storage creation in
>       the UI, because the default itself is more dynamic now.
>     * Switch to a template string for notes, supporting certain
>       variables.
> 
> 
> Previous discussion here:
> https://lists.proxmox.com/pipermail/pve-devel/2021-December/051214.html
> 
> 
> storage:
> 
> Fabian Ebner (1):
>   plugins: allow limiting the number of protected backups per guest
> 
>  PVE/Storage.pm                 | 35 ++++++++++++++++++++++++++++++++++
>  PVE/Storage/BTRFSPlugin.pm     |  3 ++-
>  PVE/Storage/CIFSPlugin.pm      |  1 +
>  PVE/Storage/CephFSPlugin.pm    |  1 +
>  PVE/Storage/DirPlugin.pm       |  1 +
>  PVE/Storage/GlusterfsPlugin.pm |  1 +
>  PVE/Storage/NFSPlugin.pm       |  1 +
>  PVE/Storage/PBSPlugin.pm       |  1 +
>  PVE/Storage/Plugin.pm          |  7 +++++++
>  9 files changed, 50 insertions(+), 1 deletion(-)
> 
> 
> manager:
> 
> Fabian Ebner (8):
>   vzdump: backup file list: drop unused parameter
>   vzdump: backup limit: only count unprotected backups
>   ui: storage edit: retention: add max-protected-backups setting
>   vzdump: support setting protected status
>   partially close #438: vzdump: support setting notes-template
>   ui: backup: allow setting protected and notes-template for manual
>     backup
>   close #438: ui: backup job: allow setting a notes-template for a job
>   ui: backup job: set guest name as default notes-template
> 
>  PVE/VZDump.pm                        | 89 +++++++++++++++++++++-------
>  www/manager6/dc/Backup.js            | 18 ++++++
>  www/manager6/panel/BackupJobPrune.js | 47 ++++++++++++---
>  www/manager6/storage/Base.js         |  1 +
>  www/manager6/window/Backup.js        | 25 +++++++-
>  5 files changed, 147 insertions(+), 33 deletions(-)
> 
> 
> docs:
> 
> Fabian Ebner (2):
>   storage: switch to prune-backups in examples
>   vzdump/storage: mention protected backups limit and give an example
> 
>  pve-storage-dir.adoc | 9 +++++----
>  pve-storage-pbs.adoc | 2 +-
>  vzdump.adoc          | 5 +++++
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> 
> guest-common:
> 
> Fabian Ebner (1):
>   vzdump: schema: add 'notes-template' and 'protected' properties
> 
>  src/PVE/VZDump/Common.pm | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-04-06 12:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-29 12:53 [pve-devel] [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 storage 1/1] plugins: allow limiting the number of protected backups per guest Fabian Ebner
2022-04-06 10:42   ` [pve-devel] applied: " Fabian Grünbichler
2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 1/8] vzdump: backup file list: drop unused parameter Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 2/8] vzdump: backup limit: only count unprotected backups Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [RFC v2 manager 3/8] ui: storage edit: retention: add max-protected-backups setting Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 docs 1/2] storage: switch to prune-backups in examples Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 docs 2/2] vzdump/storage: mention protected backups limit and give an example Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 guest-common 1/1] vzdump: schema: add 'notes-template' and 'protected' properties Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 4/8] vzdump: support setting protected status Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 5/8] partially close #438: vzdump: support setting notes-template Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 6/8] ui: backup: allow setting protected and notes-template for manual backup Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 7/8] close #438: ui: backup job: allow setting a notes-template for a job Fabian Ebner
2022-03-29 12:53 ` [pve-devel] [PATCH v2 manager 8/8] ui: backup job: set guest name as default notes-template Fabian Ebner
2022-04-06 12:10 ` [pve-devel] partially-applied: [PATCH-SERIES v2 storage/manager/guest-common/docs] improvements for protected backups Fabian Grünbichler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal