From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7E8DDE664 for ; Sat, 23 Apr 2022 12:08:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6AFC52D81F for ; Sat, 23 Apr 2022 12:07:55 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id B2C592D813 for ; Sat, 23 Apr 2022 12:07:54 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 7F592427A6 for ; Sat, 23 Apr 2022 12:07:48 +0200 (CEST) Message-ID: Date: Sat, 23 Apr 2022 12:07:47 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:100.0) Gecko/20100101 Thunderbird/100.0 Content-Language: en-US To: Proxmox VE development discussion , Fabian Ebner References: <20220421112659.74011-1-f.ebner@proxmox.com> <20220421112659.74011-13-f.ebner@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20220421112659.74011-13-f.ebner@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.613 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.165 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH v2 manager 2/3] ui: restore: allow override of some settings X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 23 Apr 2022 10:08:25 -0000 On 21.04.22 13:26, Fabian Ebner wrote: > Signed-off-by: Fabian Ebner > --- > > 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;