[pve-devel] [PATCH-SEREIES v3 common/guest-common/manager] add protected and notes-template parameters for vzdump

[pve-devel] [PATCH-SEREIES v3 common/guest-common/manager] add protected and notes-template parameters for vzdump

[Fabian Ebner]

Introduce 'protected' to automatically mark a backup as protected
upon completion, and 'notes-template' to generate notes from a
template string with certain variables.

Add both to the UI for manual backups and add 'notes-template' to the
UI for backup jobs.


Changes from v2:
    * Dropped already applied half of the series.
    * Switch to {{var}} syntax instead of $VAR.
    * Add a patch to have doc generation handle the braces.


common:

Fabian Ebner (1):
  REST handler: get property description: escape curly braces for
    asciidoc

 src/PVE/RESTHandler.pm | 3 +++
 1 file changed, 3 insertions(+)


guest-common:

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

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


manager:

Fabian Ebner (4):
  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

 PVE/VZDump.pm                 | 66 ++++++++++++++++++++++++++++++-----
 www/manager6/dc/Backup.js     | 18 ++++++++++
 www/manager6/window/Backup.js | 33 +++++++++++++++++-
 3 files changed, 107 insertions(+), 10 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH v3 common 1/1] REST handler: get property description: escape curly braces for asciidoc

[Fabian Ebner]

Text enclosed in unescaped curly braces will be interpreted as an
attribute reference breaking and e.g. lead to the description not
showing up at all a generated man page further down the line.

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

New in v3.

Tried out various uses of curly braces and seems to work for man
pages, HTML doc and API-viewer.

 src/PVE/RESTHandler.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm
index 4f1458b..c746181 100644
--- a/src/PVE/RESTHandler.pm
+++ b/src/PVE/RESTHandler.pm
@@ -517,6 +517,9 @@ my $get_property_description = sub {
 	chomp $wdescr;
 	$wdescr =~ s/^$/+/mg;
 
+	$wdescr =~ s/{/\\{/g;
+	$wdescr =~ s/}/\\}/g;
+
 	$res .= $wdescr . "\n";
 
 	if (my $req = $phash->{requires}) {
-- 
2.30.2

[pve-devel] [PATCH v3 guest-common 1/1] vzdump: schema: add 'notes-template' and 'protected' properties

[Fabian Ebner]

In command_line(), notes are 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 v2:
    * Switch to {{var}} syntax.

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

diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index 83d7413..d8c78c4 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -233,7 +233,22 @@ 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

[pve-devel] [PATCH v3 manager 1/4] vzdump: support setting protected status

[Fabian Ebner]

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>
---

No changes from v2.

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

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index f34a5969..f0a28a76 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;
@@ -780,21 +781,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 = grep { !$_->{mark} || $_->{mark} ne "protected" } get_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}) {
@@ -995,6 +1008,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

[pve-devel] [PATCH v3 manager 2/4] partially close #438: vzdump: support setting notes-template

[Fabian Ebner]

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

Changes from v2:
    * Switch to {{var}} syntax.

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 f0a28a76..ceecdbe3 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)\}\}/\Q$info->{$1}\E/g;
+
+    return $notes_template;
+};
+
 my $parse_prune_backups_maxfiles = sub {
     my ($param, $kind) = @_;
 
@@ -1013,6 +1029,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

[pve-devel] [PATCH v3 manager 3/4] ui: backup: allow setting protected and notes-template for manual backup

[Fabian Ebner]

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>
---

Changes from v2:
    * Add default value and tooltip for notes-tempalte.

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

diff --git a/www/manager6/window/Backup.js b/www/manager6/window/Backup.js
index 726122c8..c4aafec1 100644
--- a/www/manager6/window/Backup.js
+++ b/www/manager6/window/Backup.js
@@ -154,19 +154,41 @@ 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%',
+		    value: '{{guestname}}',
+		    autoEl: {
+			tag: 'div',
+			'data-qtip': Ext.String.format(
+			    gettext('Notes added to the backup. Possible variables are {0}'),
+			    '{{cluster}}, {{guestname}}, {{node}}, {{vmid}}',
+			),
+		    },
+		},
 		{
 		    xtype: 'label',
 		    name: 'pruneLabel',
@@ -229,6 +251,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 +302,7 @@ Ext.define('PVE.window.Backup', {
 	    modal: true,
 	    layout: 'auto',
 	    border: false,
+	    width: 600,
 	    items: [me.formPanel],
 	    buttons: [helpBtn, '->', submitBtn],
 	    listeners: {
-- 
2.30.2

[pve-devel] [PATCH v3 manager 4/4] close #438: ui: backup job: allow setting a notes-template for a job

[Fabian Ebner]

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

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

Changes from v2:
    * Squash in patch setting default value.
    * Switch to {{var}} syntax.
    * Use singular "backup" in gettext to be able to reuse same
      message as for manual backup.

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

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 2b892c6f..9b129266 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -231,6 +231,24 @@ 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,
+		    value: me.isCreate ? '{{guestname}}' : undefined,
+		    autoEl: {
+			tag: 'div',
+			'data-qtip': Ext.String.format(
+			    gettext('Notes added to the backup. Possible variables are {0}'),
+			    '{{cluster}}, {{guestname}}, {{node}}, {{vmid}}',
+			),
+		    },
 		},
 	    ],
 	    onGetValues: function(values) {
-- 
2.30.2

[pve-devel] applied: [PATCH v3 common 1/1] REST handler: get property description: escape curly braces for asciidoc

[Thomas Lamprecht]

On 07.04.22 12:05, Fabian Ebner wrote:
> Text enclosed in unescaped curly braces will be interpreted as an
> attribute reference breaking and e.g. lead to the description not
> showing up at all a generated man page further down the line.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> New in v3.
> 
> Tried out various uses of curly braces and seems to work for man
> pages, HTML doc and API-viewer.
> 
>  src/PVE/RESTHandler.pm | 3 +++
>  1 file changed, 3 insertions(+)
> 
>

applied, thanks!