From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 4B7DB1FF15E for ; Fri, 18 Oct 2024 09:41:56 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5F42A1C93F; Fri, 18 Oct 2024 09:42:29 +0200 (CEST) Message-ID: <8e603ada-fc40-4a54-8121-96c83c1c6511@proxmox.com> Date: Fri, 18 Oct 2024 09:42:24 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Lukas Wagner To: Proxmox VE development discussion , Daniel Kral References: <20241016164711.934544-1-d.kral@proxmox.com> <20241016164711.934544-5-d.kral@proxmox.com> Content-Language: de-AT, en-US In-Reply-To: <20241016164711.934544-5-d.kral@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.007 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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, result.data] Subject: Re: [pve-devel] [PATCH manager 4/5] ui: vm: make cloudinit drive editable 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: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Some suggestions inline. Skimmed over the code to spot style issues, correctness was not really checked. Same remarks regarding `var` vs `let` apply also to this patch. On 2024-10-16 18:47, Daniel Kral wrote: > Implements the functionality to allow subsequent changes to the > CloudInit drive under the VM "Hardware" tab. This is needed to implement > further changes in a future commit. > > Signed-off-by: Daniel Kral > --- > www/manager6/qemu/CIDriveEdit.js | 100 ++++++++++++++++++++++-------- > www/manager6/qemu/HardwareView.js | 4 +- > 2 files changed, 77 insertions(+), 27 deletions(-) > > diff --git a/www/manager6/qemu/CIDriveEdit.js b/www/manager6/qemu/CIDriveEdit.js > index a9ca8bf1..0494f9c5 100644 > --- a/www/manager6/qemu/CIDriveEdit.js > +++ b/www/manager6/qemu/CIDriveEdit.js > @@ -4,28 +4,55 @@ Ext.define('PVE.qemu.CIDriveInputPanel', { > > insideWizard: false, > > - vmconfig: {}, // used to select usused disks > + vmconfig: {}, // used to select unused disks Maybe split this typo fix into a separate commit? > > onGetValues: function(values) { > var me = this; > > - var drive = {}; > var params = {}; > - drive.file = values.hdstorage + ":cloudinit"; > - drive.format = values.diskformat; > - params[values.controller + values.deviceid] = PVE.Parser.printQemuDrive(drive); > + var confid = me.confid || values.controller + values.deviceid; > + > + // only set these when we create cloudinit files > + if (me.isCreate) { > + me.drive.file = values.hdstorage + ":cloudinit"; > + me.drive.format = values.diskformat; > + } > + > + params[confid] = PVE.Parser.printQemuDrive(me.drive); > + > return params; > }, > > setNodename: function(nodename) { > var me = this; > + Maybe do this in a separate commit if you want to do a change like this to e.g. improve readability > me.down('#hdstorage').setNodename(nodename); > me.down('#hdimage').setStorage(undefined, nodename); > }, > > setVMConfig: function(config) { > var me = this; > - me.down('#drive').setVMConfig(config, 'cdrom'); > + > + let bussel = me.down('#drive'); > + if (bussel) { > + bussel.setVMConfig(config, 'cdrom'); > + } > + }, bussel as a in ... bus selector? Maybe rather call it busSelector, has less of a 'huh?'-factor :) > + > + setDrive: function(drive) { > + var me = this; > + > + var values = {}; > + > + var match = drive.file.match(/^([^:]+):/); > + if (match) { > + values.hdstorage = match[1]; > + } > + values.hdimage = drive.file; > + > + me.drive = drive; > + > + me.setValues(values); > }, > > initComponent: function() { > @@ -33,22 +60,30 @@ Ext.define('PVE.qemu.CIDriveInputPanel', { > > me.drive = {}; > > - me.items = [ > - { > + let items = []; > + > + if (me.isCreate) { > + items.push({ > xtype: 'pveControllerSelector', > withVirtIO: false, > itemId: 'drive', > fieldLabel: gettext('CloudInit Drive'), > name: 'drive', > - }, > - { > - xtype: 'pveDiskStorageSelector', > - itemId: 'storselector', > - storageContent: 'images', > - nodename: me.nodename, > - hideSize: true, > - }, > - ]; > + }); > + } > + > + items.push({ > + xtype: 'pveDiskStorageSelector', > + itemId: 'storselector', > + storageContent: 'images', > + nodename: me.nodename, > + disabled: !me.isCreate, > + hideFormat: !me.isCreate, > + hideSize: true, > + }); > + > + me.items = items; > + > me.callParent(); > }, > }); > @@ -57,9 +92,6 @@ Ext.define('PVE.qemu.CIDriveEdit', { > extend: 'Proxmox.window.Edit', > xtype: 'pveCIDriveEdit', > > - isCreate: true, > - subject: gettext('CloudInit Drive'), > - > initComponent: function() { > var me = this; > > @@ -68,17 +100,35 @@ Ext.define('PVE.qemu.CIDriveEdit', { > throw "no node name specified"; > } > > - me.items = [{ > - xtype: 'pveCIDriveInputPanel', > - itemId: 'cipanel', > + me.isCreate = !me.confid; > + > + var ipanel = Ext.create('PVE.qemu.CIDriveInputPanel', { > + confid: me.confid, > nodename: nodename, > - }]; > + isCreate: me.isCreate, > + }); > + > + Ext.applyIf(me, { > + subject: gettext('CloudInit Drive'), > + items: [ipanel], > + }); > > me.callParent(); > > me.load({ > success: function(response, opts) { > - me.down('#cipanel').setVMConfig(response.result.data); > + ipanel.setVMConfig(response.result.data); > + > + if (me.confid) { > + var value = response.result.data[me.confid]; > + var drive = PVE.Parser.parseQemuDrive(me.confid, value); > + if (!drive) { > + Ext.Msg.alert('Error', 'Unable to parse drive options'); > + me.close(); > + return; > + } > + ipanel.setDrive(drive); > + } > }, > }); > }, > diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js > index 86d5f4cf..c7a77bd9 100644 > --- a/www/manager6/qemu/HardwareView.js > +++ b/www/manager6/qemu/HardwareView.js > @@ -350,7 +350,7 @@ Ext.define('PVE.qemu.HardwareView', { > if (rowdef.isOnStorageBus) { > let value = me.getObjectValue(rec.data.key, '', true); > if (isCloudInitKey(value)) { > - return; > + editor = 'PVE.qemu.CIDriveEdit'; > } else if (value.match(/media=cdrom/)) { > editor = 'PVE.qemu.CDEdit'; > } else if (!diskCap) { > @@ -629,7 +629,7 @@ Ext.define('PVE.qemu.HardwareView', { > remove_btn.RESTMethod = isUnusedDisk || (isDisk && isRunning) ? 'POST' : 'PUT'; > > edit_btn.setDisabled( > - deleted || !row.editor || isCloudInit || (isCDRom && !cdromCap) || (isDisk && !diskCap)); > + deleted || !row.editor || (isCDRom && !cdromCap) || (isDisk && !diskCap)); > > diskaction_btn.setDisabled( > pending || -- - Lukas _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel