[pve-devel] [PATCH v2 manager 1/2] close #4513: ui: backup job: add performance tab

[pve-devel] [PATCH v2 manager 1/2] close #4513: ui: backup job: add performance tab

[Fiona Ebner]

pigz is not exposed, because it only works after manually installing
the pigz package.

ionice is not exposed, because it only works in combination with the
BFQ scheduler and even then not in all cases (only affects the
compressor when doing snapshot/suspend mode backup of a VM).

These can still be added with appropriate notes if there is enough
user demand.

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

Changes in v2:
  * drop mistakenly copy-pased hint name 'pbs-hint'
  * use second column for description
  * align empty text right to match what bwlimit field does
  * use title case for all labels
  * mention that zstd thread count setting does not apply to PBS
  * move bwlimit to the top as it always applies and is most relevant
    in many cases

 www/manager6/Makefile                   |   1 +
 www/manager6/dc/Backup.js               |  12 +++
 www/manager6/panel/BackupPerformance.js | 123 ++++++++++++++++++++++++
 3 files changed, 136 insertions(+)
 create mode 100644 www/manager6/panel/BackupPerformance.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 336355ab..4c30e218 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -88,6 +88,7 @@ JSSRC= 							\
 	grid/ResourceGrid.js				\
 	panel/ConfigPanel.js				\
 	panel/BackupJobPrune.js				\
+	panel/BackupPerformance.js			\
 	panel/HealthWidget.js				\
 	panel/IPSet.js					\
 	panel/RunningChart.js				\
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 03a02651..d4597c70 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -178,6 +178,11 @@ Ext.define('PVE.dc.BackupEdit', {
 				PVE.Utils.unEscapeNotesTemplate(data['notes-template']);
 			}
 
+			if (data.performance) {
+			    Object.assign(data, data.performance);
+			    delete data.performance;
+			}
+
 			view.setValues(data);
 		    },
 		});
@@ -417,6 +422,13 @@ Ext.define('PVE.dc.BackupEdit', {
 			},
 		    ],
 		},
+		{
+		    xtype: 'pveBackupPerformancePanel',
+		    title: gettext('Performance'),
+		    cbind: {
+			isCreate: '{isCreate}',
+		    },
+		},
 	    ],
 	},
     ],
diff --git a/www/manager6/panel/BackupPerformance.js b/www/manager6/panel/BackupPerformance.js
new file mode 100644
index 00000000..04de5677
--- /dev/null
+++ b/www/manager6/panel/BackupPerformance.js
@@ -0,0 +1,123 @@
+/*
+ * Input panel for backup performance settings intended to be used as part of an edit/create window.
+ */
+Ext.define('PVE.panel.BackupPerformance', {
+    extend: 'Proxmox.panel.InputPanel',
+    xtype: 'pveBackupPerformancePanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    cbindData: function() {
+	let me = this;
+	me.isCreate = !!me.isCreate;
+	return {};
+    },
+
+    onGetValues: function(formValues) {
+	if (this.needMask) { // isMasked() may not yet be true if not rendered once
+	    return {};
+	}
+
+	let options = { 'delete': [] };
+
+	let performance = {};
+	let performanceOptions = ['max-workers'];
+
+	for (const [key, value] of Object.entries(formValues)) {
+	    if (performanceOptions.includes(key)) {
+		performance[key] = value;
+	    // deleteEmpty is not currently implemented for pveBandwidthField
+	    } else if (key === 'bwlimit' && value === '') {
+		options.delete.push('bwlimit');
+	    } else if (key === 'delete') {
+		if (Array.isArray(value)) {
+		    value.filter(opt => !performanceOptions.includes(opt)).forEach(
+			opt => options.delete.push(opt),
+		    );
+		} else if (!performanceOptions.includes(formValues.delete)) {
+		    options.delete.push(value);
+		}
+	    } else {
+		options[key] = value;
+	    }
+	}
+
+	if (Object.keys(performance).length > 0) {
+	    options.performance = PVE.Parser.printPropertyString(performance);
+	} else {
+	    options.delete.push('performance');
+	}
+
+	if (this.isCreate) {
+	    delete options.delete;
+	}
+
+	return options;
+    },
+
+    column1: [
+	{
+	    xtype: 'pveBandwidthField',
+	    name: 'bwlimit',
+	    fieldLabel: gettext('Bandwidth Limit'),
+	    emptyText: gettext('use fallback'),
+	    backendUnit: 'KiB',
+	    allowZero: true,
+	    emptyValue: '',
+	    autoEl: {
+		tag: 'div',
+		'data-qtip': Ext.String.format(gettext('Use {0} for unlimited'), 0),
+	    },
+	},
+	{
+	    xtype: 'proxmoxintegerfield',
+	    name: 'zstd',
+	    fieldLabel: Ext.String.format(gettext('{0} Threads'), 'Zstd'),
+	    fieldStyle: 'text-align: right',
+	    emptyText: gettext('use fallback'),
+	    minValue: 0,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	    autoEl: {
+		tag: 'div',
+		'data-qtip': gettext('With 0, half of the available cores are used'),
+	    },
+	},
+	{
+	    xtype: 'proxmoxintegerfield',
+	    name: 'max-workers',
+	    minValue: 1,
+	    maxValue: 256,
+	    fieldLabel: gettext('VM Workers'),
+	    fieldStyle: 'text-align: right',
+	    emptyText: gettext('use fallback'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
+    ],
+
+    column2: [
+	{
+	    xtype: 'displayfield',
+	    value: gettext('Limit I/O bandwidth'),
+	},
+	{
+	    xtype: 'displayfield',
+	    value: `${gettext('Threads used for zstd compression')} (${gettext('non-PBS')})`,
+	},
+	{
+	    xtype: 'displayfield',
+	    value: `${gettext('I/O workers in the QEMU process')} (${gettext('VM only')})`,
+	},
+    ],
+
+    columnB: [
+	{
+	    xtype: 'component',
+	    userCls: 'pmx-hint',
+	    padding: '5 1',
+	    html: gettext("Note that vzdump.conf is used as a fallback"),
+	},
+    ],
+});
-- 
2.39.2

[pve-devel] [PATCH v2 manager 2/2] ui: backup job: disable zstd thread count field when zstd isn't used

[Fiona Ebner]

Also need to check for enable/disable of the compression selector,
because with PBS the value zstd is set, but the thread count setting
doesn't apply.

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

New in v2.

 www/manager6/dc/Backup.js               | 18 ++++++++++++++++++
 www/manager6/panel/BackupPerformance.js | 13 +++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index d4597c70..3aea9e66 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -137,6 +137,18 @@ Ext.define('PVE.dc.BackupEdit', {
 	    }
 	},
 
+	compressionChange: function(f, value, oldValue) {
+	    this.getView().lookup('backupPerformance').updateCompression(value, f.isDisabled());
+	},
+
+	compressionDisable: function(f) {
+	    this.getView().lookup('backupPerformance').updateCompression(f.getValue(), true);
+	},
+
+	compressionEnable: function(f) {
+	    this.getView().lookup('backupPerformance').updateCompression(f.getValue(), false);
+	},
+
 	init: function(view) {
 	    let me = this;
 	    if (view.isCreate) {
@@ -310,6 +322,11 @@ Ext.define('PVE.dc.BackupEdit', {
 					deleteEmpty: '{!isCreate}',
 				    },
 				    value: 'zstd',
+				    listeners: {
+					change: 'compressionChange',
+					disable: 'compressionDisable',
+					enable: 'compressionEnable',
+				    },
 				},
 				{
 				    xtype: 'pveBackupModeSelector',
@@ -424,6 +441,7 @@ Ext.define('PVE.dc.BackupEdit', {
 		},
 		{
 		    xtype: 'pveBackupPerformancePanel',
+		    reference: 'backupPerformance',
 		    title: gettext('Performance'),
 		    cbind: {
 			isCreate: '{isCreate}',
diff --git a/www/manager6/panel/BackupPerformance.js b/www/manager6/panel/BackupPerformance.js
index 04de5677..d807c4b0 100644
--- a/www/manager6/panel/BackupPerformance.js
+++ b/www/manager6/panel/BackupPerformance.js
@@ -12,6 +12,10 @@ Ext.define('PVE.panel.BackupPerformance', {
 	return {};
     },
 
+    controller: {
+	xclass: 'Ext.app.ViewController',
+    },
+
     onGetValues: function(formValues) {
 	if (this.needMask) { // isMasked() may not yet be true if not rendered once
 	    return {};
@@ -54,6 +58,14 @@ Ext.define('PVE.panel.BackupPerformance', {
 	return options;
     },
 
+    updateCompression: function(value, disabled) {
+	if (!disabled && value === 'zstd') {
+	    this.lookup('zstdThreadCount').setDisabled(false);
+	} else {
+	    this.lookup('zstdThreadCount').setDisabled(true);
+	}
+    },
+
     column1: [
 	{
 	    xtype: 'pveBandwidthField',
@@ -71,6 +83,7 @@ Ext.define('PVE.panel.BackupPerformance', {
 	{
 	    xtype: 'proxmoxintegerfield',
 	    name: 'zstd',
+	    reference: 'zstdThreadCount',
 	    fieldLabel: Ext.String.format(gettext('{0} Threads'), 'Zstd'),
 	    fieldStyle: 'text-align: right',
 	    emptyText: gettext('use fallback'),
-- 
2.39.2

[pve-devel] [PATCH v2 docs] backup: update information about performance settings

[Fiona Ebner]

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

No changes in v2.

 vzdump.adoc | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/vzdump.adoc b/vzdump.adoc
index f3eadcd..bd5bc58 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -214,11 +214,12 @@ the behaviour for catching up. By enabling the `Repeat missed` option
 (`repeat-missed` in the config), you can tell the scheduler that it should run
 missed jobs as soon as possible.
 
-There are a few settings for tuning backup performance not exposed in the UI.
-The most notable is `bwlimit` for limiting IO bandwidth. The amount of threads
-used for the compressor can be controlled with the `pigz` (replacing `gzip`),
-respectively, `zstd` setting. Furthermore, there are `ionice` and, as part of
-the `performance` setting, `max-workers` (affects VM backups only). See the
+There are a few settings for tuning backup performance (some not exposed in the
+UI). The most notable is `bwlimit` for limiting IO bandwidth. The amount of
+threads used for the compressor can be controlled with the `pigz` (replacing
+`gzip`), respectively, `zstd` setting. Furthermore, there are `ionice` (when the
+BFQ scheduler is used) and, as part of the `performance` setting, `max-workers`
+(affects VM backups only). See the
 xref:vzdump_configuration[configuration options] for details.
 
 [[vzdump_retention]]
-- 
2.39.2

[pve-devel] [PATCH v2 guest-common] vzdump: config: improve description of ionice setting

[Fiona Ebner]

The CFQ scheduler was removed with Linux 5.0 and ionice is now used
by the newer BFQ scheduler. Mention what the special value 8 does.
Also mention that for snapshot and suspend mode backups of VMs, the
setting only affects the compressor, because the kvm process is not a
child process of vzdump then and does not inherit the ionice priority.

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

No changes in v2.

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

diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index 64c4e4e..a6fe483 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -212,7 +212,10 @@ my $confdesc = {
     },
     ionice => {
 	type => 'integer',
-	description => "Set CFQ ionice priority.",
+	description => "Set IO priority when using the BFQ scheduler. For snapshot and suspend "
+	    ."mode backups of VMs, this only affects the compressor. A value of 8 means the idle "
+	    ."priority is used, otherwise the best-effort priority is used with the specified "
+	    ."value.",
 	optional => 1,
 	minimum => 0,
 	maximum => 8,
-- 
2.39.2

[pve-devel] applied: [PATCH v2 guest-common] vzdump: config: improve description of ionice setting

[Thomas Lamprecht]

Am 07/06/2023 um 16:54 schrieb Fiona Ebner:
> The CFQ scheduler was removed with Linux 5.0 and ionice is now used
> by the newer BFQ scheduler. Mention what the special value 8 does.
> Also mention that for snapshot and suspend mode backups of VMs, the
> setting only affects the compressor, because the kvm process is not a
> child process of vzdump then and does not inherit the ionice priority.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> No changes in v2.
> 
>  src/PVE/VZDump/Common.pm | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
>

applied, thanks!