On 21.04.22 13:26, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
> ---
> 
> New in v2.
> 
> Dependency bump for qemu-server needed
> 
>  www/manager6/window/Restore.js | 77 +++++++++++++++++++++++++++++++++-
>  1 file changed, 75 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
> index 25babf89..23c244f3 100644
> --- a/www/manager6/window/Restore.js
> +++ b/www/manager6/window/Restore.js
> @@ -21,6 +21,7 @@ Ext.define('PVE.window.Restore', {
>  
>           Proxmox.Utils.API2Request({
>               url: `/nodes/${view.nodename}/vzdump/extractconfig`,
> +             waitMsgTarget: view,
>               method: 'GET',
>               params: {
>                   volume: view.volid,
> @@ -38,6 +39,28 @@ Ext.define('PVE.window.Restore', {
>                               `storage/${view.nodename}/${match[3]}`,
>                           );
>                           storagesAvailable = storagesAvailable && 
> currentAvailable;
> +                     } else {

hmm, ok you reuse the forEach, so the every of the previous patch review may 
not work, but I'd 
still do some changes, see below.

> +                         match = line.match(/^([^:]+):\s*(\S+)\s*$/);

can be: 

let [_, key, value] = line.match(/^([^:]+):\s*(\S+)\s*$/) ?? [];

if (!key) {
    return;
}

...

And then this could be move out of this if and handled like:

if (key === #qmdump#map) {
     // ...
} else if (key === 'name' || 'hostname') {
     view.lookupReference('nameField').setEmptyText(value);
) else if (key === 'memory') {
   // ...
} ...

Alternatively a single else and an object to map from key to xField would be 
also an option, but the
switch is almost never too verbose and often, and that's probably just my 
opinion, slightly consufing
to parse when skimming over code quickly.

> +                         if (match) {
> +                             let [_, key, value] = match;
> +                             switch (key) {
> +                                 case 'name':
> +                                 case 'hostname':
> +                                     
> view.lookupReference('nameField').setEmptyText(value);
> +                                     break;
> +                                 case 'memory':
> +                                     
> view.lookupReference('memoryField').setEmptyText(value);
> +                                     break;
> +                                 case 'cores':
> +                                     
> view.lookupReference('coresField').setEmptyText(value);
> +                                     break;
> +                                 case 'sockets':
> +                                     
> view.lookupReference('socketsField').setEmptyText(value);
> +                                     break;
> +                                 default:
> +                                     break;
> +                             }
> +                         }
>                       }
>                   });
>  
> @@ -207,6 +230,49 @@ Ext.define('PVE.window.Restore', {
>           });
>       }
>  
> +     items.push(
> +         {
> +             xtype: 'displayfield',
> +             value: `${gettext('Override Settings')}:`,

I'd maybe even use a 'fieldset' widget here for grouping this nicely.

> +         },
> +         {
> +             xtype: 'textfield',
> +             fieldLabel: gettext('Name'),
> +             name: 'name',
> +             reference: 'nameField',
> +             allowBlank: true,
> +         },
> +         {
> +             xtype: 'pveMemoryField',
> +             fieldLabel: gettext('Memory'),
> +             name: 'memory',
> +             reference: 'memoryField',
> +             value: '',
> +             allowBlank: true,
> +         },
> +         {
> +             xtype: 'proxmoxintegerfield',
> +             fieldLabel: gettext('Cores'),
> +             name: 'cores',
> +             reference: 'coresField',
> +             minValue: 1,
> +             maxValue: 128,
> +             allowBlank: true,
> +         },
> +     );
> +
> +     if (me.vmtype === 'qemu') {
> +         items.push({
> +             xtype: 'proxmoxintegerfield',
> +             fieldLabel: gettext('Sockets'),
> +             name: 'sockets',
> +             reference: 'socketsField',
> +             minValue: 1,
> +             maxValue: 4,
> +             allowBlank: true,
> +         });
> +     }
> +
>       me.formPanel = Ext.create('Ext.form.Panel', {
>           bodyPadding: 10,
>           border: false,
> @@ -254,8 +320,15 @@ Ext.define('PVE.window.Restore', {
>               if (values['live-restore']) { params['live-restore'] = 1; }
>               if (values.storage) { params.storage = values.storage; }
>  
> -             if (values.bwlimit !== undefined) {
> -                 params.bwlimit = values.bwlimit;
> +             ['bwlimit', 'cores', 'name', 'memory', 
> 'sockets'].forEach(function(opt) {
> +                 if (values[opt] !== undefined && values[opt] !== null && 
> values[opt] !== '') {

should be collapsible via:

if ((values[opt] ?? '') !== '') 

at which point we could move it into a filter combinator, like:

['bwlimit', 'cores', 'name', 'memory', 'sockets']
    .filter(opt => (values[opt] ?? '') !== '')
    .forEach(opt => param[opt] = values[opt]);

> +                     params[opt] = values[opt];
> +                 }
> +             });
> +
> +             if (params.name && me.vmtype === 'lxc') {
> +                 params.hostname = params.name;
> +                 delete params.name;
>               }
>  
>               var url;



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to