From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; Sat, 23 Apr 2022 12:07:48 +0200 (CEST)
Message-ID: <c4515f35-7be3-86cd-2001-85b5532eca02@proxmox.com>
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 <pve-devel@lists.proxmox.com>,
Fabian Ebner <f.ebner@proxmox.com>
References: <20220421112659.74011-1-f.ebner@proxmox.com>
<20220421112659.74011-13-f.ebner@proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
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 <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=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 <f.ebner@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;