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 <[email protected]>
> ---
>  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"),
> +     },
> +    ],
> +});



_______________________________________________
pve-devel mailing list
[email protected]
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to