From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.csapak@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) server-digest SHA256)
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id F188F60D4B
for <pve-devel@lists.proxmox.com>; Thu, 3 Sep 2020 16:29:33 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id E41711A2E9
for <pve-devel@lists.proxmox.com>; Thu, 3 Sep 2020 16:29:33 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[212.186.127.180])
(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 CD3FE1A2DF
for <pve-devel@lists.proxmox.com>; Thu, 3 Sep 2020 16:29:32 +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 95B67449E6
for <pve-devel@lists.proxmox.com>; Thu, 3 Sep 2020 16:29:32 +0200 (CEST)
To: pve-devel@lists.proxmox.com
References: <20200825092449.49410-1-d.jaeger@proxmox.com>
<20200825092449.49410-4-d.jaeger@proxmox.com>
From: Dominik Csapak <d.csapak@proxmox.com>
Message-ID: <aedd5d7e-b86e-83f5-9e15-c69571796dbb@proxmox.com>
Date: Thu, 3 Sep 2020 16:29:28 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:81.0) Gecko/20100101
Thunderbird/81.0
MIME-Version: 1.0
In-Reply-To: <20200825092449.49410-4-d.jaeger@proxmox.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.761 Adjusted score from AWL reputation of From: address
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
NICE_REPLY_A -0.324 Looks like a legit reply (A)
RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/,
medium trust
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [me.drive, caps.storage, params.storage]
Subject: Re: [pve-devel] [PATCH manager v3 3/3] Hardware View: Add GUI for
importdisk
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: Thu, 03 Sep 2020 14:29:34 -0000
On 8/25/20 11:24 AM, Dominic Jäger wrote:
> Make importing single disks easier.
> Required to import a whole VM via GUI.
>
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
> ---
> v2->v3: Use the new submitUrl parameter from widget-tookit
>
> Depends on both other patches
>
> www/manager6/qemu/HDEdit.js | 83 +++++++++++++++++++++++--------
> www/manager6/qemu/HardwareView.js | 20 ++++++++
> 2 files changed, 81 insertions(+), 22 deletions(-)
>
> diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
> index e2a5b914..edebbbc1 100644
> --- a/www/manager6/qemu/HDEdit.js
> +++ b/www/manager6/qemu/HDEdit.js
> @@ -76,23 +76,46 @@ Ext.define('PVE.qemu.HDInputPanel', {
> me.drive.format = values.diskformat;
> }
>
> - PVE.Utils.propertyStringSet(me.drive, !values.backup, 'backup', '0');
> - PVE.Utils.propertyStringSet(me.drive, values.noreplicate, 'replicate', 'no');
> - PVE.Utils.propertyStringSet(me.drive, values.discard, 'discard', 'on');
> - PVE.Utils.propertyStringSet(me.drive, values.ssd, 'ssd', 'on');
> - PVE.Utils.propertyStringSet(me.drive, values.iothread, 'iothread', 'on');
> - PVE.Utils.propertyStringSet(me.drive, values.cache, 'cache');
> -
> - var names = ['mbps_rd', 'mbps_wr', 'iops_rd', 'iops_wr'];
> - Ext.Array.each(names, function(name) {
> - var burst_name = name + '_max';
> - PVE.Utils.propertyStringSet(me.drive, values[name], name);
> - PVE.Utils.propertyStringSet(me.drive, values[burst_name], burst_name);
> - });
> -
> -
> - params[confid] = PVE.Parser.printQemuDrive(me.drive);
> + if (me.isImport) {
> + // These keys & values are accepted by the API as they are
> + let simple = ['backup', 'ssd', 'iothread', 'cache'];
> + let burst = ['mbps_rd', 'mbps_wr', 'iops_rd', 'iops_wr'];
> + burst = burst.concat(burst.map(x => `${x}_max`));
> + let available = simple.concat(burst);
> +
> + let addValues = key => `${key}=${values[key]}`;
> + let selectedKeys = x => values[x];
> + let options = available.filter(selectedKeys).map(addValues).join();
> +
> + // These need modification for the API
> + options += values.discard ? ',discard=on' : '';
> + options += values.noreplicate ? ',replicate=0' : '';
> +
> + params.device_options = options;
why do you do this?
you can simply reuse the propertyStringSet function again, just
use a different object
...drive code...
let drive_obj = import ? {} : me.drive;
PVE.Utils.propertyStringSet(drive_obj....);
and then use drive_obj instead of either your values or me.drive
(you can use PVE.Parser.printPropertyString to get the 'propertyString'
format)
on another note, here in onGetValues you can replace the url with the
correct submiturl if you want, no need to change widget toolkit
> + } else {
> + PVE.Utils.propertyStringSet(me.drive, !values.backup, 'backup', '0');
> + PVE.Utils.propertyStringSet(me.drive, values.noreplicate, 'replicate', 'no');
> + PVE.Utils.propertyStringSet(me.drive, values.discard, 'discard', 'on');
> + PVE.Utils.propertyStringSet(me.drive, values.ssd, 'ssd', 'on');
> + PVE.Utils.propertyStringSet(me.drive, values.iothread, 'iothread', 'on');
> + PVE.Utils.propertyStringSet(me.drive, values.cache, 'cache');
> +
> + var names = ['mbps_rd', 'mbps_wr', 'iops_rd', 'iops_wr'];
> + Ext.Array.each(names, function(name) {
> + var burst_name = name + '_max';
> + PVE.Utils.propertyStringSet(me.drive, values[name], name);
> + PVE.Utils.propertyStringSet(me.drive, values[burst_name], burst_name);
> + });
> + }
>
> + if (me.isImport) {
> + params.source = values.inputImage;
> + params.device = values.controller + values.deviceid;
> + params.storage = values.hdstorage;
> + if (values.diskformat) params.format = values.diskformat;
> + } else {
> + params[confid] = PVE.Parser.printQemuDrive(me.drive);
> + }
> return params;
> },
>
> @@ -199,14 +222,17 @@ Ext.define('PVE.qemu.HDInputPanel', {
> allowBlank: false
> });
> me.column1.push(me.unusedDisks);
> - } else if (me.isCreate) {
> - me.column1.push({
> + } else if (me.isCreate || me.isImport) {
> + let selector = {
> xtype: 'pveDiskStorageSelector',
> storageContent: 'images',
> name: 'disk',
> nodename: me.nodename,
> - autoSelect: me.insideWizard
> - });
> + hideSize: me.isImport,
> + autoSelect: me.insideWizard || me.isImport,
> + };
> + if (me.isImport) selector.storageLabel = gettext('Target storage');
> + me.column1.push(selector);
> } else {
> me.column1.push({
> xtype: 'textfield',
> @@ -231,6 +257,14 @@ Ext.define('PVE.qemu.HDInputPanel', {
> name: 'discard'
> }
> );
> + if (me.isImport) {
> + me.column2.push({
> + xtype: 'textfield',
> + fieldLabel: gettext('Source image'),
> + name: 'inputImage',
> + emptyText: '/home/user/disk.qcow2',
> + });
> + }
>
> me.advancedColumn1.push(
> {
> @@ -372,14 +406,19 @@ Ext.define('PVE.qemu.HDEdit', {
> confid: me.confid,
> nodename: nodename,
> unused: unused,
> - isCreate: me.isCreate
> + isCreate: me.isCreate,
> + isImport: me.isImport,
> });
>
> var subject;
> if (unused) {
> me.subject = gettext('Unused Disk');
> + } else if (me.isImport) {
> + me.subject = gettext('Import Disk');
> + me.submitText = 'Import';
> + me.backgroundDelay = undefined;
> } else if (me.isCreate) {
> - me.subject = gettext('Hard Disk');
> + me.subject = gettext('Hard Disk');
> } else {
> me.subject = gettext('Hard Disk') + ' (' + me.confid + ')';
> }
> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
> index 40b3fe86..5598214b 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -436,6 +436,25 @@ Ext.define('PVE.qemu.HardwareView', {
> handler: run_move
> });
>
> + var import_btn = new Proxmox.button.Button({
> + text: gettext('Import disk'),
> + hidden: !(caps.vms['VM.Allocate'] &&
> + caps.storage['Datastore.AllocateTemplate'] &&
> + caps.storage['Datastore.AllocateSpace']),
> + handler: function() {
> + let url = `/api2/extjs/${baseurl}`;
> + var win = Ext.create('PVE.qemu.HDEdit', {
> + method: 'POST',
> + url: url,
> + submitUrl: url.replace('config', 'importdisk'),
this is wrong, replace only replaces the first instance
if my node is named 'confignode1'
the resulting url is now
/api2/extjs/nodes/importdisknode1/qemu/$ID/config
(which probably does no harm but will not work)
if you do this here, i would construct 2 baseurls
and construct both urls independendtly
if you decide to replace config with importdisk
in HDEdit, i would use a regex replace
/\/config$/\/importdisk/
or use
url.lastIndexOf("/config")
to not run into such issues
> + pveSelNode: me.pveSelNode,
> + isImport: true,
> + });
> + win.on('destroy', me.reload, me);
> + win.show();
> + },
> + });
> +
> var remove_btn = new Proxmox.button.Button({
> text: gettext('Remove'),
> defaultText: gettext('Remove'),
> @@ -752,6 +771,7 @@ Ext.define('PVE.qemu.HardwareView', {
> edit_btn,
> resize_btn,
> move_btn,
> + import_btn,
> revert_btn
> ],
> rows: rows,
>