[pve-devel] [PATCH manager] close #4513: ui: backup job: add performance tab
Thomas Lamprecht
t.lamprecht at proxmox.com
Tue Jun 6 17:23:29 CEST 2023
stumbled upon this again when checking out old(er) patches without comments,
some higher level comments inline.
Am 15/03/2023 um 14:24 schrieb 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 at proxmox.com>
> ---
> www/manager6/Makefile | 1 +
> www/manager6/dc/Backup.js | 12 +++
> www/manager6/panel/BackupPerformance.js | 110 ++++++++++++++++++++++++
> 3 files changed, 123 insertions(+)
> create mode 100644 www/manager6/panel/BackupPerformance.js
>
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index b73b729a..0c92b984 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 d0046177..8da6e7d7 100644
> --- a/www/manager6/dc/Backup.js
> +++ b/www/manager6/dc/Backup.js
> @@ -172,6 +172,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);
> },
> });
> @@ -411,6 +416,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..17e0f34c
> --- /dev/null
> +++ b/www/manager6/panel/BackupPerformance.js
> @@ -0,0 +1,110 @@
> +/*
> + * 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;
> + },
> +
hmm, maybe we should go for a layout with one field per row, but besides that a
short description about what it controls, like:
Label [ Field ] Description
Where the descriptions are sorta in the style of the CPU flag ones for VM CPU
edit, i.e., provide some context and hint of what this does.
Because max-workers "VM Worker Count" is not easily understandable IMO, i.e.,
one could think that's parallel backups done by separate workers (like e.g.,
max worker is used for migrate-all or HA LRM parallel workers)
> + column1: [
> + {
> + xtype: 'proxmoxintegerfield',
> + name: 'zstd',
> + fieldLabel: Ext.String.format('{0} ({1})', gettext('Thread count'), 'zstd'),
> + emptyText: gettext('use fallback'),
disabling that when ZSTD is not selected might slightly improve UX too.
> + 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: Ext.String.format('{0} ({1})', gettext('Worker count'), 'VM'),
> + emptyText: gettext('use fallback'),
> + cbind: {
> + deleteEmpty: '{!isCreate}',
> + },
> + },
> + ],
> +
> + column2: [
> + {
> + 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),
> + },
> + },
> + ],
> +
> + columnB: [
> + {
> + xtype: 'component',
> + userCls: 'pmx-hint',
> + name: 'pbs-hint',
> + padding: '5 1',
> + html: gettext("Note that vzdump.conf is used as a fallback"),
> + },
> + ],
> +});
More information about the pve-devel
mailing list