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 UTF8SMTPS id 71337738C2 for ; Fri, 18 Jun 2021 14:56:42 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id 6675E27CA7 for ; Fri, 18 Jun 2021 14:56:42 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with UTF8SMTPS id 963F927C99 for ; Fri, 18 Jun 2021 14:56:38 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id 6189F4420D; Fri, 18 Jun 2021 14:56:38 +0200 (CEST) Message-ID: <02c7eae6-b56d-8be7-dcff-2dc52a86f7cc@proxmox.com> Date: Fri, 18 Jun 2021 14:56:33 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:90.0) Gecko/20100101 Thunderbird/90.0 Content-Language: en-US To: Proxmox VE development discussion , =?UTF-8?Q?Dominic_J=c3=a4ger?= References: <20210616113451.97856-1-d.jaeger@proxmox.com> From: Dominik Csapak In-Reply-To: <20210616113451.97856-1-d.jaeger@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.449 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% CTE_8BIT_MISMATCH 0.998 Header says 7bits but body disagrees KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.254 Looks like a legit reply (A) PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far 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, sencha.com, result.data] Subject: Re: [pve-devel] [PATCH manager] ui: HD edit: Add multiple disks & tabs 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: Fri, 18 Jun 2021 12:56:42 -0000 a few high level comments: this patch could probably be split into more smaller (more readable) patches.. also there seem to be some things leftover from the import feature, but no hard feelings there. i encountered a few bugs(some more sever, some not so), namely: * if i remove the second to last disk while i selected it, the last disk does not open the panel on the right side if i select it * if i add all disks (so clicking until i have scsi0-scsi31, virtio..) it adds ide0 multiple times at the end and i now get a popup that i cannot have duplicate ids. i'd rather expected that the add button gets disabled * adding many disks is *very* slow (maybe because of the many storage api calls?), not sure if we can improve that some comments to the overall design choices: while it feels good, i am not so sure about the 'plain' tab style as well as the bottom mounted buttons for the hd edit, it should probably look more like the storage edit window, where we have the normal tab style, and bodyPadding set to 0 also why do you split the 'options' and the 'bandwidth' options there, but not in the wizard? are not 'advanced' anymore when the vm exists? for the wizard, i'd probably move the tabs up (and leave the default style) and maybe put the add/remove button in tabs? though i am not sure honestly... as for add button, i think it would be better to have also text there, just for clarity (so '+ Add') what i kinda miss is the info which disk is not valid. if i add many disks, and one is not valid (for whatever reason), i cannot see that immediatly and have to click through all disks and all tabs (though i think it is not possible to have anything in the option tab to be invalid) otherwise i like it :) comments to the code inline: On 6/16/21 13:34, Dominic Jäger wrote: > Enable adding multiple disks in VM create wizard. > This is a first step for future import features. > > Split disk edit panel into multiple tabbed panels to make it less cluttered. > This affects the create wizard & the HD edit windows in the VM hardware view. > > Signed-off-by: Dominic Jäger > --- > www/manager6/Makefile | 6 +- > www/manager6/form/ControllerSelector.js | 20 + > www/manager6/qemu/CDEdit.js | 3 - > www/manager6/qemu/CreateWizard.js | 16 +- > www/manager6/qemu/HDEdit.js | 409 ------------------ > www/manager6/qemu/HardwareView.js | 9 +- > www/manager6/qemu/OSTypeEdit.js | 12 +- > .../qemu/disk/DiskBandwidthOptions.js | 192 ++++++++ > www/manager6/qemu/disk/DiskBasicOptions.js | 153 +++++++ > www/manager6/qemu/disk/DiskCollection.js | 282 ++++++++++++ > www/manager6/qemu/disk/DiskData.js | 241 +++++++++++ > www/manager6/qemu/disk/HardDisk.js | 215 +++++++++ > www/manager6/window/Wizard.js | 2 + > 13 files changed, 1139 insertions(+), 421 deletions(-) > delete mode 100644 www/manager6/qemu/HDEdit.js > create mode 100644 www/manager6/qemu/disk/DiskBandwidthOptions.js > create mode 100644 www/manager6/qemu/disk/DiskBasicOptions.js > create mode 100644 www/manager6/qemu/disk/DiskCollection.js > create mode 100644 www/manager6/qemu/disk/DiskData.js > create mode 100644 www/manager6/qemu/disk/HardDisk.js > > diff --git a/www/manager6/Makefile b/www/manager6/Makefile > index 6776d4ce..95f03d88 100644 > --- a/www/manager6/Makefile > +++ b/www/manager6/Makefile > @@ -198,7 +198,11 @@ JSSRC= \ > qemu/Config.js \ > qemu/CreateWizard.js \ > qemu/DisplayEdit.js \ > - qemu/HDEdit.js \ > + qemu/disk/DiskCollection.js \ > + qemu/disk/HardDisk.js \ > + qemu/disk/DiskData.js \ > + qemu/disk/DiskBasicOptions.js \ > + qemu/disk/DiskBandwidthOptions.js \ > qemu/HDEfi.js \ > qemu/HDMove.js \ > qemu/HDResize.js \ > diff --git a/www/manager6/form/ControllerSelector.js b/www/manager6/form/ControllerSelector.js > index daca2432..85f66956 100644 > --- a/www/manager6/form/ControllerSelector.js > +++ b/www/manager6/form/ControllerSelector.js > @@ -72,6 +72,26 @@ Ext.define('PVE.form.ControllerSelector', { > deviceid.validate(); > }, > > + deleteFromVMConfig: function(key) { > + const me = this; > + delete me.vmconfig[key]; > + }, > + > + getValues: function() { > + return this.query('field').map(x => x.getValue()); > + }, > + > + getValuesAsString: function() { > + return this.getValues().join(''); > + }, nit: i am not sure this warrants it's own function? or at least give it a better name, like 'getDiskId' ? (also, are we sure 'this.query' has a stable order? i guess so, but would be good to verify, or make the order explicit) > + > + setValue: function(value) { > + const regex = /([a-z]+)(\d+)/; > + const [_, controller, deviceid] = regex.exec(value); > + this.down('field[name=controller]').setValue(controller); > + this.down('field[name=deviceid]').setValue(deviceid); > + }, > + > initComponent: function() { > var me = this; > > diff --git a/www/manager6/qemu/CDEdit.js b/www/manager6/qemu/CDEdit.js > index 72c01037..27092d32 100644 > --- a/www/manager6/qemu/CDEdit.js > +++ b/www/manager6/qemu/CDEdit.js > @@ -84,9 +84,6 @@ Ext.define('PVE.qemu.CDInputPanel', { > checked: true, > listeners: { > change: function(f, value) { > - if (!me.rendered) { > - return; > - } nit: seems like an unrelated change.. > me.down('field[name=cdstorage]').setDisabled(!value); > var cdImageField = me.down('field[name=cdimage]'); > cdImageField.setDisabled(!value); > diff --git a/www/manager6/qemu/CreateWizard.js b/www/manager6/qemu/CreateWizard.js > index d4535c9d..2405b6f7 100644 > --- a/www/manager6/qemu/CreateWizard.js > +++ b/www/manager6/qemu/CreateWizard.js > @@ -154,7 +154,7 @@ Ext.define('PVE.qemu.CreateWizard', { > insideWizard: true, > }, > { > - xtype: 'pveQemuHDInputPanel', > + xtype: 'pveQemuDiskCollection', > bind: { > nodename: '{nodename}', > }, > @@ -251,6 +251,20 @@ Ext.define('PVE.qemu.CreateWizard', { > }, > }, > ], > + > + getValues: function() { > + let values = this.callParent(); > + for (const [key, value] of Object.entries(values)) { > + const re = /ide\d+|sata\d+|virtio\d+|scsi\d+|import_sources/; > + if (key.match(re) && Array.isArray(value)) { > + // Collected from different panels => array > + // But API & some GUI functions expect not array > + const sep = key === 'import_sources' ? '\0' : ','; > + values[key] = value.join(sep); > + } > + } > + return values; > + }, > }); > > > diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js > deleted file mode 100644 > index 95a98b0b..00000000 > --- a/www/manager6/qemu/HDEdit.js > +++ /dev/null > @@ -1,409 +0,0 @@ > -/* 'change' property is assigned a string and then a function */ > -Ext.define('PVE.qemu.HDInputPanel', { > - extend: 'Proxmox.panel.InputPanel', > - alias: 'widget.pveQemuHDInputPanel', > - onlineHelp: 'qm_hard_disk', > - > - insideWizard: false, > - > - unused: false, // ADD usused disk imaged > - > - vmconfig: {}, // used to select usused disks > - > - viewModel: {}, > - > - controller: { > - > - xclass: 'Ext.app.ViewController', > - > - onControllerChange: function(field) { > - var value = field.getValue(); > - > - var allowIOthread = value.match(/^(virtio|scsi)/); > - this.lookup('iothread').setDisabled(!allowIOthread); > - if (!allowIOthread) { > - this.lookup('iothread').setValue(false); > - } > - > - var virtio = value.match(/^virtio/); > - this.lookup('ssd').setDisabled(virtio); > - if (virtio) { > - this.lookup('ssd').setValue(false); > - } > - > - this.lookup('scsiController').setVisible(value.match(/^scsi/)); > - }, > - > - control: { > - 'field[name=controller]': { > - change: 'onControllerChange', > - afterrender: 'onControllerChange', > - }, > - 'field[name=iothread]': { > - change: function(f, value) { > - if (!this.getView().insideWizard) { > - return; > - } > - var vmScsiType = value ? 'virtio-scsi-single': 'virtio-scsi-pci'; > - this.lookupReference('scsiController').setValue(vmScsiType); > - }, > - }, > - }, > - > - init: function(view) { > - var vm = this.getViewModel(); > - if (view.isCreate) { > - vm.set('isIncludedInBackup', true); > - } > - }, > - }, > - > - onGetValues: function(values) { > - var me = this; > - > - var params = {}; > - var confid = me.confid || values.controller + values.deviceid; > - > - if (me.unused) { > - me.drive.file = me.vmconfig[values.unusedId]; > - confid = values.controller + values.deviceid; > - } else if (me.isCreate) { > - if (values.hdimage) { > - me.drive.file = values.hdimage; > - } else { > - me.drive.file = values.hdstorage + ":" + values.disksize; > - } > - 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); > - > - return params; > - }, > - > - setVMConfig: function(vmconfig) { > - var me = this; > - > - me.vmconfig = vmconfig; > - > - if (me.bussel) { > - me.bussel.setVMConfig(vmconfig); > - me.scsiController.setValue(vmconfig.scsihw); > - } > - if (me.unusedDisks) { > - var disklist = []; > - Ext.Object.each(vmconfig, function(key, value) { > - if (key.match(/^unused\d+$/)) { > - disklist.push([key, value]); > - } > - }); > - me.unusedDisks.store.loadData(disklist); > - me.unusedDisks.setValue(me.confid); > - } > - }, > - > - setDrive: function(drive) { > - var me = this; > - > - me.drive = drive; > - > - var values = {}; > - var match = drive.file.match(/^([^:]+):/); > - if (match) { > - values.hdstorage = match[1]; > - } > - > - values.hdimage = drive.file; > - values.backup = PVE.Parser.parseBoolean(drive.backup, 1); > - values.noreplicate = !PVE.Parser.parseBoolean(drive.replicate, 1); > - values.diskformat = drive.format || 'raw'; > - values.cache = drive.cache || '__default__'; > - values.discard = drive.discard === 'on'; > - values.ssd = PVE.Parser.parseBoolean(drive.ssd); > - values.iothread = PVE.Parser.parseBoolean(drive.iothread); > - > - values.mbps_rd = drive.mbps_rd; > - values.mbps_wr = drive.mbps_wr; > - values.iops_rd = drive.iops_rd; > - values.iops_wr = drive.iops_wr; > - values.mbps_rd_max = drive.mbps_rd_max; > - values.mbps_wr_max = drive.mbps_wr_max; > - values.iops_rd_max = drive.iops_rd_max; > - values.iops_wr_max = drive.iops_wr_max; > - > - me.setValues(values); > - }, > - > - setNodename: function(nodename) { > - var me = this; > - me.down('#hdstorage').setNodename(nodename); > - me.down('#hdimage').setStorage(undefined, nodename); > - }, > - > - initComponent: function() { > - var me = this; > - > - var labelWidth = 140; > - > - me.drive = {}; > - > - me.column1 = []; > - me.column2 = []; > - > - me.advancedColumn1 = []; > - me.advancedColumn2 = []; > - > - if (!me.confid || me.unused) { > - me.bussel = Ext.create('PVE.form.ControllerSelector', { > - vmconfig: me.insideWizard ? { ide2: 'cdrom' } : {}, > - }); > - me.column1.push(me.bussel); > - > - me.scsiController = Ext.create('Ext.form.field.Display', { > - fieldLabel: gettext('SCSI Controller'), > - reference: 'scsiController', > - bind: me.insideWizard ? { > - value: '{current.scsihw}', > - } : undefined, > - renderer: PVE.Utils.render_scsihw, > - submitValue: false, > - hidden: true, > - }); > - me.column1.push(me.scsiController); > - } > - > - if (me.unused) { > - me.unusedDisks = Ext.create('Proxmox.form.KVComboBox', { > - name: 'unusedId', > - fieldLabel: gettext('Disk image'), > - matchFieldWidth: false, > - listConfig: { > - width: 350, > - }, > - data: [], > - allowBlank: false, > - }); > - me.column1.push(me.unusedDisks); > - } else if (me.isCreate) { > - me.column1.push({ > - xtype: 'pveDiskStorageSelector', > - storageContent: 'images', > - name: 'disk', > - nodename: me.nodename, > - autoSelect: me.insideWizard, > - }); > - } else { > - me.column1.push({ > - xtype: 'textfield', > - disabled: true, > - submitValue: false, > - fieldLabel: gettext('Disk image'), > - name: 'hdimage', > - }); > - } > - > - me.column2.push( > - { > - xtype: 'CacheTypeSelector', > - name: 'cache', > - value: '__default__', > - fieldLabel: gettext('Cache'), > - }, > - { > - xtype: 'proxmoxcheckbox', > - fieldLabel: gettext('Discard'), > - reference: 'discard', > - name: 'discard', > - }, > - ); > - > - me.advancedColumn1.push( > - { > - xtype: 'proxmoxcheckbox', > - disabled: me.confid && me.confid.match(/^virtio/), > - fieldLabel: gettext('SSD emulation'), > - labelWidth: labelWidth, > - name: 'ssd', > - reference: 'ssd', > - }, > - { > - xtype: 'proxmoxcheckbox', > - disabled: me.confid && !me.confid.match(/^(virtio|scsi)/), > - fieldLabel: 'IO thread', > - labelWidth: labelWidth, > - reference: 'iothread', > - name: 'iothread', > - }, > - { > - xtype: 'numberfield', > - name: 'mbps_rd', > - minValue: 1, > - step: 1, > - fieldLabel: gettext('Read limit') + ' (MB/s)', > - labelWidth: labelWidth, > - emptyText: gettext('unlimited'), > - }, > - { > - xtype: 'numberfield', > - name: 'mbps_wr', > - minValue: 1, > - step: 1, > - fieldLabel: gettext('Write limit') + ' (MB/s)', > - labelWidth: labelWidth, > - emptyText: gettext('unlimited'), > - }, > - { > - xtype: 'proxmoxintegerfield', > - name: 'iops_rd', > - minValue: 10, > - step: 10, > - fieldLabel: gettext('Read limit') + ' (ops/s)', > - labelWidth: labelWidth, > - emptyText: gettext('unlimited'), > - }, > - { > - xtype: 'proxmoxintegerfield', > - name: 'iops_wr', > - minValue: 10, > - step: 10, > - fieldLabel: gettext('Write limit') + ' (ops/s)', > - labelWidth: labelWidth, > - emptyText: gettext('unlimited'), > - }, > - ); > - > - me.advancedColumn2.push( > - { > - xtype: 'proxmoxcheckbox', > - fieldLabel: gettext('Backup'), > - autoEl: { > - tag: 'div', > - 'data-qtip': gettext('Include volume in backup job'), > - }, > - labelWidth: labelWidth, > - name: 'backup', > - bind: { > - value: '{isIncludedInBackup}', > - }, > - }, > - { > - xtype: 'proxmoxcheckbox', > - fieldLabel: gettext('Skip replication'), > - labelWidth: labelWidth, > - name: 'noreplicate', > - }, > - { > - xtype: 'numberfield', > - name: 'mbps_rd_max', > - minValue: 1, > - step: 1, > - fieldLabel: gettext('Read max burst') + ' (MB)', > - labelWidth: labelWidth, > - emptyText: gettext('default'), > - }, > - { > - xtype: 'numberfield', > - name: 'mbps_wr_max', > - minValue: 1, > - step: 1, > - fieldLabel: gettext('Write max burst') + ' (MB)', > - labelWidth: labelWidth, > - emptyText: gettext('default'), > - }, > - { > - xtype: 'proxmoxintegerfield', > - name: 'iops_rd_max', > - minValue: 10, > - step: 10, > - fieldLabel: gettext('Read max burst') + ' (ops)', > - labelWidth: labelWidth, > - emptyText: gettext('default'), > - }, > - { > - xtype: 'proxmoxintegerfield', > - name: 'iops_wr_max', > - minValue: 10, > - step: 10, > - fieldLabel: gettext('Write max burst') + ' (ops)', > - labelWidth: labelWidth, > - emptyText: gettext('default'), > - }, > - ); > - > - me.callParent(); > - }, > -}); > - > -Ext.define('PVE.qemu.HDEdit', { > - extend: 'Proxmox.window.Edit', > - > - isAdd: true, > - > - backgroundDelay: 5, > - > - initComponent: function() { > - var me = this; > - > - var nodename = me.pveSelNode.data.node; > - if (!nodename) { > - throw "no node name specified"; > - } > - > - var unused = me.confid && me.confid.match(/^unused\d+$/); > - > - me.isCreate = me.confid ? unused : true; > - > - var ipanel = Ext.create('PVE.qemu.HDInputPanel', { > - confid: me.confid, > - nodename: nodename, > - unused: unused, > - isCreate: me.isCreate, > - }); > - > - if (unused) { > - me.subject = gettext('Unused Disk'); > - } else if (me.isCreate) { > - me.subject = gettext('Hard Disk'); > - } else { > - me.subject = gettext('Hard Disk') + ' (' + me.confid + ')'; > - } > - > - me.items = [ipanel]; > - > - me.callParent(); > - /* 'data' is assigned an empty array in same file, and here we > - * use it like an object > - */ > - me.load({ > - success: function(response, options) { > - 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(gettext('Error'), 'Unable to parse drive options'); > - me.close(); > - return; > - } > - ipanel.setDrive(drive); > - me.isValid(); // trigger validation > - } > - }, > - }); > - }, > -}); > diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js > index 200e3c28..5126fab8 100644 > --- a/www/manager6/qemu/HardwareView.js > +++ b/www/manager6/qemu/HardwareView.js > @@ -220,7 +220,7 @@ Ext.define('PVE.qemu.HardwareView', { > rows[confid] = { > group: 10, > iconCls: 'hdd-o', > - editor: 'PVE.qemu.HDEdit', > + editor: 'PVE.qemu.HardDiskWindow', > isOnStorageBus: true, > header: gettext('Hard Disk') + ' (' + confid +')', > cdheader: gettext('CD/DVD Drive') + ' (' + confid +')', > @@ -290,7 +290,7 @@ Ext.define('PVE.qemu.HardwareView', { > order: i, > iconCls: 'hdd-o', > del_extra_msg: gettext('This will permanently erase all data.'), > - editor: caps.vms['VM.Config.Disk'] ? 'PVE.qemu.HDEdit' : undefined, > + editor: caps.vms['VM.Config.Disk'] ? 'PVE.qemu.HardDiskWindow' : undefined, > header: gettext('Unused Disk') + ' ' + i.toString(), > }; > } > @@ -630,9 +630,10 @@ Ext.define('PVE.qemu.HardwareView', { > iconCls: 'fa fa-fw fa-hdd-o black', > disabled: !caps.vms['VM.Config.Disk'], > handler: function() { > - let win = Ext.create('PVE.qemu.HDEdit', { > + let win = Ext.create('PVE.qemu.HardDiskWindow', { > url: '/api2/extjs/' + baseurl, > - pveSelNode: me.pveSelNode, > + nodename: me.pveSelNode.data.node, > + isCreate: true, > }); > win.on('destroy', me.reload, me); > win.show(); > diff --git a/www/manager6/qemu/OSTypeEdit.js b/www/manager6/qemu/OSTypeEdit.js > index 438d7c6b..641d9394 100644 > --- a/www/manager6/qemu/OSTypeEdit.js > +++ b/www/manager6/qemu/OSTypeEdit.js > @@ -3,6 +3,7 @@ Ext.define('PVE.qemu.OSTypeInputPanel', { > alias: 'widget.pveQemuOSTypePanel', > onlineHelp: 'qm_os_settings', > insideWizard: false, > + ignoreDisks: false, is this also leftover from the import? this is not set anywhere to true and only read once (in the next hunk) > > controller: { > xclass: 'Ext.app.ViewController', > @@ -20,13 +21,18 @@ Ext.define('PVE.qemu.OSTypeInputPanel', { > }, > onOSTypeChange: function(field) { > var me = this, ostype = field.getValue(); > - if (!me.getView().insideWizard) { > + const view = me.getView(); > + if (!view.insideWizard) { > return; > } > var targetValues = PVE.qemu.OSDefaults.getDefaults(ostype); > - > - me.setWidget('pveBusSelector', targetValues.busType); > + if (!view.ignoreDisks) { > + const ids = Ext.ComponentQuery.query('pveBusSelector') > + .reduce((acc, cur) => acc.concat(cur.id), []); > + ids.forEach(i => me.setWidget(`#${i}`, targetValues.busType)); is that not simply a 'query('').map((rec) => rec.id)' ? no need to copy the whole array each time also we simply do one thing per id so the whole thing could be a 'forEach' simply? query('').forEach((rec) => me.setWidget(....rec.id...) ? > + } > me.setWidget('pveNetworkCardSelector', targetValues.networkCard); > + me.setWidget('pveQemuBiosSelector', targetValues.bios); nit: also unrelated change (also we do not have any 'bios' versions in the osdefaults yet) > var scsihw = targetValues.scsihw || '__default__'; > this.getViewModel().set('current.scsihw', scsihw); > }, > diff --git a/www/manager6/qemu/disk/DiskBandwidthOptions.js b/www/manager6/qemu/disk/DiskBandwidthOptions.js > new file mode 100644 > index 00000000..58e08f59 > --- /dev/null > +++ b/www/manager6/qemu/disk/DiskBandwidthOptions.js > @@ -0,0 +1,192 @@ > +/* 'change' property is assigned a string and then a function */ > +Ext.define('PVE.qemu.DiskBandwidthOptions', { > + extend: 'Proxmox.panel.InputPanel', > + alias: 'widget.pveQemuDiskBandwidthOptions', > + onlineHelp: 'qm_hard_disk', > + > + insideWizard: false, > + > + unused: false, // ADD usused disk imaged > + > + padding: '10 10 10 10', > + > + vmconfig: {}, // used to select usused disks > + > + viewModel: {}, why an empty viewmodel ? > + > + /** > + * All radiofields in pveQemuDiskCollection have the same scope > + * Make name of radiofields unique for each disk panel > + */ > + getRadioName() { > + return 'radio_' + this.id; > + }, leftover? i cannot see why this should be here? > + > + onGetValues: function(values) { > + const me = this; > + > + let params = {}; > + > + const confid = me.up('pveQemuHardDisk').down('pveQemuDiskData').getConfid(); i really do not like that this panel determines where it is used... could we do this differently? by bubbling the values up and combine them only at the top level ? or give the panels a reference to the drive, so they can set the properties on that also i am not really clear on how this works now, since there are 3 panels that all want to modify the same drive? > + > + const names = ['mbps_rd', 'mbps_wr', 'iops_rd', 'iops_wr']; > + Ext.Array.each(names, function(name) { > + let burst_name = name + '_max'; > + PVE.Utils.propertyStringSet(me.drive, values[name], name); > + PVE.Utils.propertyStringSet(me.drive, values[burst_name], burst_name); > + }); > + > + me.drive.file = 'dummy'; > + // no values => no comma > + params[confid] = PVE.Parser.printQemuDrive(me.drive).replace(/^dummy,?/, ""); this seems wrong? is this because somewhere else we simply concatenate the drive? thats another reason why i do not like this... > + > + return params; > + }, > + > + setVMConfig: function(vmconfig) { > + let me = this; > + > + me.vmconfig = vmconfig; > + > + if (me.bussel) { > + me.bussel.setVMConfig(vmconfig); > + me.scsiController.setValue(vmconfig.scsihw); > + } > + if (me.unusedDisks) { > + let disklist = []; > + Ext.Object.each(vmconfig, function(key, value) { > + if (key.match(/^unused\d+$/)) { > + disklist.push([key, value]); > + } > + }); > + me.unusedDisks.store.loadData(disklist); > + me.unusedDisks.setValue(me.confid); > + } > + }, > + > + setDrive: function(drive) { > + let me = this; > + > + me.drive = {}; > + [ > + 'interface', > + 'index', > + 'mbps_rd', > + 'mbps_wr', > + 'iops_rd', > + 'iops_wr', > + 'mbps_rd_max', > + 'mbps_wr_max', > + 'iops_rd_max', > + 'iops_wr_max', > + ].forEach(o => { me.drive[o] = drive[o]; }); > + > + let values = {}; > + values.mbps_rd = drive.mbps_rd; > + values.mbps_wr = drive.mbps_wr; > + values.iops_rd = drive.iops_rd; > + values.iops_wr = drive.iops_wr; > + values.mbps_rd_max = drive.mbps_rd_max; > + values.mbps_wr_max = drive.mbps_wr_max; > + values.iops_rd_max = drive.iops_rd_max; > + values.iops_wr_max = drive.iops_wr_max; > + > + me.setValues(values); would me.setValues(me.drive) not work? if a field does not exists for a property, it's simply not set anywhere right? > + }, > + > + > + setNodename: function(nodename) { > + // nothing > + }, if it does nothing, is it necessary to have? > + > + initComponent: function() { > + let me = this; > + > + let labelWidth = 140; > + > + me.drive = {}; > + > + me.column1 = []; > + me.column2 = []; > + > + me.column1.push( > + { > + xtype: 'numberfield', > + name: 'mbps_rd', > + minValue: 1, > + step: 1, > + fieldLabel: gettext('Read limit') + ' (MB/s)', > + labelWidth: labelWidth, > + emptyText: gettext('unlimited'), > + }, > + { > + xtype: 'numberfield', > + name: 'mbps_wr', > + minValue: 1, > + step: 1, > + fieldLabel: gettext('Write limit') + ' (MB/s)', > + labelWidth: labelWidth, > + emptyText: gettext('unlimited'), > + }, > + { > + xtype: 'proxmoxintegerfield', > + name: 'iops_rd', > + minValue: 10, > + step: 10, > + fieldLabel: gettext('Read limit') + ' (ops/s)', > + labelWidth: labelWidth, > + emptyText: gettext('unlimited'), > + }, > + { > + xtype: 'proxmoxintegerfield', > + name: 'iops_wr', > + minValue: 10, > + step: 10, > + fieldLabel: gettext('Write limit') + ' (ops/s)', > + labelWidth: labelWidth, > + emptyText: gettext('unlimited'), > + }, > + ); > + > + me.column2.push( > + { > + xtype: 'numberfield', > + name: 'mbps_rd_max', > + minValue: 1, > + step: 1, > + fieldLabel: gettext('Read max burst') + ' (MB)', > + labelWidth: labelWidth, > + emptyText: gettext('default'), > + }, > + { > + xtype: 'numberfield', > + name: 'mbps_wr_max', > + minValue: 1, > + step: 1, > + fieldLabel: gettext('Write max burst') + ' (MB)', > + labelWidth: labelWidth, > + emptyText: gettext('default'), > + }, > + { > + xtype: 'proxmoxintegerfield', > + name: 'iops_rd_max', > + minValue: 10, > + step: 10, > + fieldLabel: gettext('Read max burst') + ' (ops)', > + labelWidth: labelWidth, > + emptyText: gettext('default'), > + }, > + { > + xtype: 'proxmoxintegerfield', > + name: 'iops_wr_max', > + minValue: 10, > + step: 10, > + fieldLabel: gettext('Write max burst') + ' (ops)', > + labelWidth: labelWidth, > + emptyText: gettext('default'), > + }, > + ); i'd rather have those statically in the class if we can > + > + me.callParent(); > + }, > +}); > diff --git a/www/manager6/qemu/disk/DiskBasicOptions.js b/www/manager6/qemu/disk/DiskBasicOptions.js > new file mode 100644 > index 00000000..d582b1df > --- /dev/null > +++ b/www/manager6/qemu/disk/DiskBasicOptions.js > @@ -0,0 +1,153 @@ > +/* 'change' property is assigned a string and then a function */ > +Ext.define('PVE.qemu.DiskBasicOptions', { > + extend: 'Proxmox.panel.InputPanel', > + alias: 'widget.pveQemuDiskBasicOptions', > + onlineHelp: 'qm_hard_disk', > + > + insideWizard: false, > + > + unused: false, // ADD usused disk imaged > + > + padding: '10 10 10 10', > + > + vmconfig: {}, // used to select usused disks > + > + viewModel: {}, same as in BandwidthSelector > + > + /** > + * All radiofields in pveQemuDiskCollection have the same scope > + * Make name of radiofields unique for each disk panel > + */ > + getRadioName() { > + return 'radio_' + this.id; > + }, same as in BandwidthSelector > + > + onGetValues: function(values) { > + const me = this; > + > + let params = {}; > + > + const confid = me.up('pveQemuHardDisk').down('pveQemuDiskData').getConfid(); same as in BandwidthSelector > + > + PVE.Utils.propertyStringSet(me.drive, !values.backup, 'backup', '0'); > + PVE.Utils.propertyStringSet(me.drive, values.noreplicate, 'replicate', 'no'); > + PVE.Utils.propertyStringSet(me.drive, values.ssd, 'ssd', 'on'); > + PVE.Utils.propertyStringSet(me.drive, values.iothread, 'iothread', 'on'); > + > + me.drive.file = 'dummy'; > + // no values => no comma > + params[confid] = PVE.Parser.printQemuDrive(me.drive).replace(/^dummy,?/, ""); same as in BandwidthSelector > + > + return params; > + }, > + > + setVMConfig: function(vmconfig) { > + let me = this; > + > + me.vmconfig = vmconfig; > + > + if (me.bussel) { > + me.bussel.setVMConfig(vmconfig); > + me.scsiController.setValue(vmconfig.scsihw); > + } > + if (me.unusedDisks) { > + let disklist = []; > + Ext.Object.each(vmconfig, function(key, value) { > + if (key.match(/^unused\d+$/)) { > + disklist.push([key, value]); > + } > + }); > + me.unusedDisks.store.loadData(disklist); > + me.unusedDisks.setValue(me.confid); > + } > + }, > + > + setDrive: function(drive) { > + let me = this; > + > + me.drive = {}; > + > + [ > + 'interface', > + 'index', > + 'backup', > + 'replicate', > + 'ssd', > + 'iothread', > + ].forEach(o => { me.drive[o] = drive[o]; }); > + > + let values = {}; > + values.backup = PVE.Parser.parseBoolean(drive.backup, 1); > + values.noreplicate = !PVE.Parser.parseBoolean(drive.replicate, 1); > + values.ssd = PVE.Parser.parseBoolean(drive.ssd); > + values.iothread = PVE.Parser.parseBoolean(drive.iothread); > + > + me.setValues(values); same as in BandwidthSelector > + }, > + > + > + setNodename: function(nodename) { > + // nothing > + }, > + > + initComponent: function() { > + let me = this; > + > + let labelWidth = 140; > + > + me.drive = {}; > + > + me.column1 = []; > + me.column2 = []; > + > + me.column1.push( > + { > + xtype: 'proxmoxcheckbox', > + disabled: me.confid && me.confid.match(/^virtio/), > + fieldLabel: gettext('SSD emulation'), > + labelWidth: labelWidth, > + name: 'ssd', > + reference: 'ssd', > + }, > + { > + xtype: 'proxmoxcheckbox', > + disabled: me.confid && !me.confid.match(/^(virtio|scsi)/), > + fieldLabel: 'IO thread', > + labelWidth: labelWidth, > + reference: 'iothread', > + name: 'iothread', > + listeners: { > + change: function(f, value) { > + const disk = f.up('pveQemuHardDisk'); > + if (disk.insideWizard) { > + const vmScsiType = value ? 'virtio-scsi-single' : 'virtio-scsi-pci'; > + disk.down('field[name=scsiController]').setValue(vmScsiType); > + } > + }, > + }, > + }, > + ); > + > + me.column2.push( > + { > + xtype: 'proxmoxcheckbox', > + fieldLabel: gettext('Backup'), > + autoEl: { > + tag: 'div', > + 'data-qtip': gettext('Include volume in backup job'), > + }, > + labelWidth: labelWidth, > + name: 'backup', > + value: me.isCreate, > + }, > + { > + xtype: 'proxmoxcheckbox', > + fieldLabel: gettext('Skip replication'), > + labelWidth: labelWidth, > + name: 'noreplicate', > + }, > + ); same as in BandwidthSelector > + > + me.callParent(); > + }, > +}); > diff --git a/www/manager6/qemu/disk/DiskCollection.js b/www/manager6/qemu/disk/DiskCollection.js > new file mode 100644 > index 00000000..79815244 > --- /dev/null > +++ b/www/manager6/qemu/disk/DiskCollection.js > @@ -0,0 +1,282 @@ > +Ext.define('PVE.qemu.DiskCollection', { > + extend: 'Proxmox.panel.InputPanel', > + alias: 'widget.pveQemuDiskCollection', > + > + insideWizard: false, > + > + isCreate: false, > + > + hiddenDisks: [], > + > + leftColumnRatio: 0.25, > + > + column1: [ > + { > + // Adding to the panelContainer below automatically adds > + // items to the store > + xtype: 'gridpanel', > + scrollable: true, > + store: { > + xtype: 'store', > + storeId: 'diskstorage', > + // Use the panel as id > + // Panels have are objects and therefore unique > + // E.g. while adding new panels 'device' is ambiguous > + fields: ['device', 'panel'], i mean, technically it works to have the panel reference in a store, but why don't we want to store some id? let it autogenerate and reference it? > + removeByPanel: function(panel) { > + const recordIndex = this.findBy(record => record.data.panel === panel); > + this.removeAt(recordIndex); > + return recordIndex; > + } > + getLast: function() { > + const last = this.getCount() - 1; > + return this.getAt(last); > + }, stores already have a 'last' function? https://docs.sencha.com/extjs/6.0.1/classic/Ext.data.Store.html#method-last > + listeners: { > + remove: function(store, records, index, isMove, eOpts) { > + const view = Ext.ComponentQuery.query('pveQemuDiskCollection').shift(); shift seems unnecessary, since it shifts all elements one down but since we do not use the rest of the elements anyway you can do [0] ? > + records.forEach(r => { > + view.removePanel(r.get('panel')); > + view.deleteFromVMConfig(r.get('device')); > + }); > + }, > + }, > + }, > + enableColumnMove: false, > + enableColumnResize: false, > + enableColumnHide: false, > + columns: [ > + { > + text: gettext('Device'), > + dataIndex: 'device', > + flex: 4, > + menuDisabled: true, > + }, > + { > + flex: 1, > + xtype: 'actioncolumn', > + align: 'center', > + menuDisabled: true, > + items: [ > + { > + iconCls: 'x-fa fa-trash critical', > + tooltip: 'Delete', > + handler: function(tableview, rowIndex, colIndex, item, event, record) { > + Ext.getStore('diskstorage').remove(record); > + }, > + }, > + ], > + }, > + ], > + listeners: { > + select: function(_, record) { > + this.up('pveQemuDiskCollection') > + .down('#panelContainer') > + .setActiveItem(record.data.panel); > + }, > + }, > + anchor: '100% 90%', > + selectLast: function() { > + this.setSelection(this.store.getLast()); > + }, > + dockedItems: [ > + { > + xtype: 'toolbar', > + dock: 'bottom', > + ui: 'footer', > + style: { > + backgroundColor: 'transparent', > + }, > + layout: { > + pack: 'center', > + }, > + items: [ > + { > + iconCls: 'fa fa-plus-circle', > + itemId: 'addDisk', > + minWidth: '60', > + handler: function(button) { > + button.up('pveQemuDiskCollection').addDisk(); > + }, > + }, > + ], > + }, > + ], > + }, > + ], > + column2: [ > + { > + itemId: 'panelContainer', > + xtype: 'container', > + layout: 'card', > + items: [], > + listeners: { > + beforeRender: function() { > + // Initial disk if none have been added by manifest yet > + if (this.items.items.length === 0) { > + this.addDisk(); > + } i guess leftover from import? > + }, > + add: function(container, newPanel) { > + const store = Ext.getStore('diskstorage'); > + store.add({ device: newPanel.getDevice(), panel: newPanel }); > + container.setActiveItem(newPanel); > + }, > + }, > + defaultItem: { > + xtype: 'pveQemuHardDisk', > + bind: { > + nodename: '{nodename}', > + }, > + listeners: { > + // newPanel ... cloned + added defaultItem > + added: function(newPanel) { > + Ext.Array.each(newPanel.down('pveControllerSelector').query('field'), > + function(field) { > + //the fields don't exist earlier > + field.on('change', function() { > + const store = Ext.getStore('diskstorage'); > + > + // find by panel object because it is unique > + const recordIndex = store.findBy(record => > + record.data.panel === field.up('pveQemuHardDisk'), > + ); > + const controllerSelector = field.up('pveControllerSelector'); > + const newControllerAndId = controllerSelector.getValuesAsString(); > + store.getAt(recordIndex).set('device', newControllerAndId); > + }); > + }, > + ); > + const wizard = this.up('pveQemuCreateWizard'); > + Ext.Array.each(this.query('field'), function(field) { > + field.on('change', wizard.validcheck); > + field.on('validitychange', wizard.validcheck); > + }); any specific reason why this is an event of the added panel, and not a normal function that is called after adding a new panel? that way it would be way less confusing when it will called with what parameter also i do not like the whole down/up handling. it is ok if used sparingly, but if the lower components often require the upper elements, maybe it is better to give them some kind of reference or callbacks. this way the component does not have to know in which context it is used > + }, > + }, > + validator: function() { > + let valid = true; > + const fields = this.query('field, fieldcontainer'); > + Ext.Array.each(fields, function(field) { > + if (Ext.isFunction(field.isValid) && !field.isValid()) { > + field.isValid(); > + valid = false; i dont get it, we check if isValid is a function, call it if it is not, call it again? and set valid to false also indentation > + } > + }); > + return valid; > + }, > + }, > + > + // device ... device that the new disk should be assigned to, e.g. ide0, sata2 > + // path ... content of the textfield with source path > + addDisk(device, path) { > + const initialValues = this.up('window').getValues(); > + const item = Ext.clone(this.defaultItem); > + item.insideWizard = this.insideWizard; > + item.isCreate = this.isCreate; > + const added = this.add(item); would not easier to use 'Ext.apply' ? let foo = Ext.apply( { somedefaults: 'bar' }, this.defaultItem); ? > + // values in the storage will be updated by listeners > + if (path) { > + // Need to explicitly deactivate when not rendered > + added.down('radiofield[inputValue=empty]').setValue(false); > + added.down('radiofield[inputValue=path]').setValue(true); > + added.down('textfield[name=sourcePath]').setValue(path); > + } > + > + const selector = added.down('pveControllerSelector'); > + if (device) { > + selector.setValue(device); > + } else { > + selector.setVMConfig(initialValues); > + } should that not be handled by the creation of the 'defaultItem'? i'd rather like the 'defaultItem' to be its own class that gets device/path as configuration options and does the correct thing this coupling outside of the class is very brittle since what if we decide to remove a textfield or the controller selector? > + > + return added; > + }, > + removePanel: function(panelId) { > + this.remove(panelId, true); > + }, > + }, > + ], > + > + addDisk: function(device, path) { > + this.down('#panelContainer').addDisk(device, path); > + this.down('gridpanel').selectLast(); > + }, > + > + removePanel: function(panelId) { > + this.down('#panelContainer').removePanel(panelId); > + }, > + > + beforeRender: function() { > + const me = this; > + const leftColumnPanel = me.items.get(0).items.get(0); // not the gridpanel > + leftColumnPanel.setFlex(me.leftColumnRatio); > + // any other panel because this has no height yet > + const panelHeight = me.up('tabpanel').items.get(0).getHeight(); > + me.down('gridpanel').setHeight(panelHeight); > + }, > + > + setNodename: function(nodename) { > + this.nodename = nodename; > + this.query('pveQemuHardDisk').forEach(p => p.setNodename(nodename)); > + }, > + > + listeners: { > + afterrender: function() { > + const store = Ext.getStore('diskstorage'); > + const first = store.getAt(0); > + if (first) { > + this.down('gridpanel').setSelection(first); > + } > + }, > + }, > + > + // values ... is optional great that it's optional, but what does it ? ;) > + hasDuplicateDevices: function(values) { > + if (!values) { > + values = this.up('form').getValues(); > + } > + if (!Array.isArray(values.controller)) { > + return false; > + } > + for (let i = 0; i < values.controller.length - 1; i++) { > + for (let j = i+1; j < values.controller.length; j++) { > + if ( > + values.controller[i] === values.controller[j] && > + values.deviceid[i] === values.deviceid[j] > + ) { > + return true; > + } > + } > + } > + return false; > + }, > + > + onGetValues: function(values) { > + if (this.hasDuplicateDevices(values)) { > + Ext.Msg.alert(gettext('Error'), 'Equal target devices are forbidden. Make all unique!'); > + } > + // Each child panel has sufficient onGetValues() => Return nothing > + }, > + > + validator: function() { > + const me = this; > + const panels = me.down('#panelContainer').items.getRange(); > + return panels.every(panel => panel.validator()) && !me.hasDuplicateDevices(); > + }, > + > + initComponent: function() { > + this.callParent(); > + this.down('tableview').markDirty = false; why set this here and not statically in the class? (i think with 'viewConfig') > + this.down('#panelContainer').insideWizard = this.insideWizard; > + this.down('#panelContainer').isCreate = this.isCreate; at this point, i'd suggest using let me = this; etc.. > + }, > + > + deleteFromVMConfig: function(key) { > + this.query('pveQemuHardDisk').forEach(p => p.deleteFromVMConfig(key)); well that was a rollercoaster following this call remove action click store removal store event remove foreach record -> view.deleteFromVMConfig (this) foreach qemuHardDisk -> deleteFromVMConfig qemuHarddisk items each -> deletefromvmconfig optionspanel -> up / down -> get datapanel datapanel-> deleteFromVMConfig bussel.deleteFromVMConfig, which deletes the actual config from the vmconfig.... sorry but this is simply too contrived... why not having a (per wizard) vmconfig component/class/utility that you call directly? no up/down this could even emit events that the other components listen to, then you'd have something like: wizard.removeFromVMConfig('somekey') -> vmDataClass.remove('somekey') -> sendsEvent (e.g. 'keyremoved') all panels can subscribe to that... (there are 'globalevents' that can be fired/listened to, see the helpbutton for example) > + }, > + > + setVMConfig: function(vmconfig) { > + this.query('pveQemuHardDisk').forEach(p => p.setVMConfig(vmconfig)); > + }, > +}); > diff --git a/www/manager6/qemu/disk/DiskData.js b/www/manager6/qemu/disk/DiskData.js > new file mode 100644 > index 00000000..ff1bc163 > --- /dev/null > +++ b/www/manager6/qemu/disk/DiskData.js > @@ -0,0 +1,241 @@ > +/* 'change' property is assigned a string and then a function */ > +Ext.define('PVE.qemu.DiskData', { > + extend: 'Proxmox.panel.InputPanel', > + alias: 'widget.pveQemuDiskData', > + onlineHelp: 'qm_hard_disk', > + > + insideWizard: false, > + > + unused: false, > + > + padding: '10 10 10 10', > + > + vmconfig: {}, // used to select usused disks > + > + viewModel: {}, same as in BandwidthSelector > + > + /** > + * All radiofields in pveQemuDiskCollection have the same scope > + * Make name of radiofields unique for each disk > + */ > + getRadioName() { > + return 'radio_' + this.id; > + }, same as in BandwidthSelector > + > + getConfid() { > + const me = this; > + if (me.confid) { > + return me.confid; // When editing disks > + } > + // In wizard > + const pairs = Object.entries(me.getValues()); > + const confidArray = pairs.filter(([key, _]) => key !== "import_sources"); > + // confidArray contains 1 array of length 2, > + // e.g. confidArray = [["sata1", "local:-1,format=qcow2"]] > + const confid = confidArray.shift().shift(); > + return confid; > + }, > + > + onGetValues: function(values) { > + let me = this; > + > + let params = {}; > + let confid = me.confid || values.controller + values.deviceid; > + > + const isImport = values.sourceVolid || values.sourcePath; > + if (me.unused) { > + me.drive.file = me.vmconfig[values.unusedId]; > + confid = values.controller + values.deviceid; > + } else if (me.isCreate) { > + if (values.hdimage) { > + me.drive.file = values.hdimage; > + } else if (isImport) { > + me.drive.file = `${values.hdstorage}:-1`; > + } else { > + me.drive.file = values.hdstorage + ":" + values.disksize; > + } > + me.drive.format = values.diskformat; > + } > + > + PVE.Utils.propertyStringSet(me.drive, values.discard, 'discard', 'on'); > + PVE.Utils.propertyStringSet(me.drive, values.cache, 'cache'); > + > + if (isImport) { > + // exactly 1 of sourceVolid and sourcePath must be defined > + params.import_sources = `${confid}=${isImport}`; > + } > + > + params[confid] = PVE.Parser.printQemuDrive(me.drive); > + > + return params; > + }, > + > + setVMConfig: function(vmconfig) { > + let me = this; > + > + me.vmconfig = vmconfig; > + > + if (me.bussel) { > + me.bussel.setVMConfig(vmconfig); > + me.scsiController.setValue(vmconfig.scsihw); > + } > + if (me.unusedDisks) { > + let disklist = []; > + Ext.Object.each(vmconfig, function(key, value) { > + if (key.match(/^unused\d+$/)) { > + disklist.push([key, value]); > + } > + }); > + me.unusedDisks.store.loadData(disklist); > + me.unusedDisks.setValue(me.confid); > + } > + }, > + > + deleteFromVMConfig: function(key) { > + const me = this; > + if (me.bussel) { > + me.bussel.deleteFromVMConfig(key); > + } > + }, > + > + setDrive: function(drive) { > + let me = this; > + > + me.drive = {}; > + [ > + 'interface', > + 'index', > + 'file', > + 'format', > + 'cache', > + 'discard', > + ].forEach(o => { me.drive[o] = drive[o]; }); > + > + let values = {}; > + let match = drive.file.match(/^([^:]+):/); > + if (match) { > + values.hdstorage = match[1]; > + } > + > + values.hdimage = drive.file; > + values.diskformat = drive.format || 'raw'; > + values.cache = drive.cache || '__default__'; > + values.discard = drive.discard === 'on'; > + > + me.setValues(values); > + }, > + > + getDevice: function() { > + return this.bussel.getValuesAsString(); > + }, > + > + setNodename: function(nodename) { > + const me = this; > + const hdstorage = me.down('#hdstorage'); > + if (hdstorage) { // iff me.isCreate > + hdstorage.setNodename(nodename); > + } > + }, > + > + initComponent: function() { > + let me = this; > + > + > + me.drive = {}; > + > + me.column1 = []; > + me.column2 = []; > + > + const nodename = me.getViewModel().get('nodename'); > + > + if (!me.confid || me.unused) { > + const controllerColumn = me.column2; > + me.scsiController = Ext.create('Ext.form.field.Display', { > + fieldLabel: gettext('SCSI Controller'), > + reference: 'scsiController', > + name: 'scsiController', > + bind: me.insideWizard ? { > + value: '{current.scsihw}', > + } : undefined, > + renderer: PVE.Utils.render_scsihw, > + submitValue: false, > + hidden: true, > + }); > + > + me.bussel = Ext.create('PVE.form.ControllerSelector', { > + itemId: 'bussel', > + vmconfig: me.insideWizard ? { ide2: 'cdrom' } : {}, > + }); > + > + me.bussel.down('field[name=controller]').addListener('change', function(_, newValue) { > + const allowIOthread = newValue.match(/^(virtio|scsi)/); > + const iothreadField = me.up('pveQemuHardDisk').down('field[name=iothread]'); > + iothreadField.setDisabled(!allowIOthread); > + if (!allowIOthread) { > + iothreadField.setValue(false); > + } > + > + const virtio = newValue.match(/^virtio/); > + const ssdField = me.up('pveQemuHardDisk').down('field[name=ssd]'); > + ssdField.setDisabled(virtio); > + if (virtio) { > + ssdField.setValue(false); > + } > + > + me.scsiController.setVisible(newValue.match(/^scsi/)); > + }); > + > + controllerColumn.push(me.bussel); > + controllerColumn.push(me.scsiController); > + } > + > + if (me.unused) { > + me.unusedDisks = Ext.create('Proxmox.form.KVComboBox', { > + name: 'unusedId', > + fieldLabel: gettext('Disk image'), > + matchFieldWidth: false, > + listConfig: { > + width: 350, > + }, > + data: [], > + allowBlank: false, > + }); > + me.column1.push(me.unusedDisks); > + } else if (me.isCreate) { > + let selector = { > + xtype: 'pveDiskStorageSelector', > + storageContent: 'images', > + name: 'disk', > + nodename: nodename, > + autoSelect: me.insideWizard, > + }; > + selector.storageLabel = gettext('Storage'); > + me.column1.push(selector); > + } else { > + me.column1.push({ > + xtype: 'textfield', > + disabled: true, > + submitValue: false, > + fieldLabel: gettext('Disk image'), > + name: 'hdimage', > + }); > + } > + > + me.column2.push( > + { > + xtype: 'CacheTypeSelector', > + name: 'cache', > + value: '__default__', > + fieldLabel: gettext('Cache'), > + }, > + { > + xtype: 'proxmoxcheckbox', > + fieldLabel: gettext('Discard'), > + reference: 'discard', > + name: 'discard', > + }, > + ); > + > + me.callParent(); > + }, > +}); > diff --git a/www/manager6/qemu/disk/HardDisk.js b/www/manager6/qemu/disk/HardDisk.js > new file mode 100644 > index 00000000..3e352af8 > --- /dev/null > +++ b/www/manager6/qemu/disk/HardDisk.js > @@ -0,0 +1,215 @@ > +/* 'change' property is assigned a string and then a function */ > +Ext.define('PVE.qemu.HardDisk', { > + extend: 'Ext.tab.Panel', > + alias: 'widget.pveQemuHardDisk', > + onlineHelp: 'qm_hard_disk', > + > + plain: true, > + > + bind: { > + nodename: '{nodename}', > + }, should that not be defined where it is used? > + > + insideWizard: false, > + > + isCreate: false, > + > + setNodename: function(nodename) { > + this.nodename = nodename; > + this.items.each(panel => panel.setNodename(nodename)); > + }, > + > + setDrive: function(drive) { > + this.items.each(i => i.setDrive(drive)); > + }, > + > + getDevice: function() { > + return this.down('pveQemuDiskData').getDevice(); > + }, > + > + items: [ > + { > + title: gettext('Data'), > + xtype: 'pveQemuDiskData', > + bind: { > + nodename: '{nodename}', > + }, > + }, > + ], > + > + beforeRender: function() { > + const me = this; > + const tabPosition = me.insideWizard ? 'bottom' : 'top'; > + me.setTabPosition(tabPosition); why here and not in initComponent? > + // any other panel because this has no height yet > + if (me.insideWizard) { > + const panelHeight = me.up('tabpanel').items.get(0).getHeight(); > + me.setHeight(panelHeight); > + } > + }, > + > + initComponent: function() { > + const me = this; > + > + let diskData = me.items[0]; > + diskData.confid = me.confid; > + diskData.isCreate = me.isCreate; > + diskData.insideWizard = me.insideWizard > + > + const basicOptions = { > + xtype: 'pveQemuDiskBasicOptions', > + isCreate: me.isCreate, > + confid: me.confid, > + insideWizard: me.insideWizard, > + bind: { > + nodename: '{nodename}', > + }, > + }; > + const bandwidthOptions = { > + xtype: 'pveQemuDiskBandwidthOptions', > + isCreate: me.isCreate, > + insideWizard: me.insideWizard, > + bind: { > + nodename: '{nodename}', > + }, > + }; > + > + if (me.insideWizard) { > + me.items = me.items.concat([ > + { > + title: gettext('Options'), > + xtype: 'panel', > + layout: { > + type: 'vbox', > + }, > + defaults: { > + width: '100%', > + margin: '0 0 10 0', > + }, > + items: [ > + basicOptions, > + { > + xtype: 'box', > + autoEl: { tag: 'hr' }, > + }, > + bandwidthOptions, > + ], > + setNodename: function(nodename) { > + this.nodename = nodename; > + if (basicOptions.setNodename) { // added after initialization > + basicOptions.setNodename(nodename); > + bandwidthOptions.setNodename(nodename); > + } > + }, > + setVMConfig: function(vmconfig) { > + this.down('pveQemuDiskBasicOptions').setVMConfig(vmconfig); > + this.down('pveQemuDiskBandwidthOptions').setVMConfig(vmconfig); > + }, > + deleteFromVMConfig: function(key) { > + const panel = this.up('pveQemuHardDisk').down('pveQemuDiskData'); > + if (panel) { > + panel.deleteFromVMConfig(key); > + } > + }, > + setDrive: function(drive) { > + this.down('pveQemuDiskBasicOptions').setDrive(drive); > + this.down('pveQemuDiskBandwidthOptions').setDrive(drive); > + }, > + }, > + ]); > + } else { > + basicOptions.title = gettext('Options'); > + bandwidthOptions.title = gettext('Bandwidth'); > + me.items = me.items.concat([basicOptions, bandwidthOptions]); > + } > + > + me.callParent(); > + }, > + > + setVMConfig: function(vmconfig) { > + this.items.each(panel => panel.setVMConfig(vmconfig)); > + }, > + deleteFromVMConfig: function(key) { > + this.items.each(panel => panel.deleteFromVMConfig(key)); > + }, > +}); > + > +Ext.define('PVE.qemu.HardDiskWindow', { > + extend: 'Proxmox.window.Edit', > + > + isAdd: true, > + > + backgroundDelay: 5, > + > + setNodename: function(nodename) { > + this.nodename = nodename; > + this.down('pveQemuHDTabpanel').setNodename(nodename); > + }, > + > + initComponent: function() { > + let me = this; > + > + const selnode = me.pveSelNode && me.pveSelNode.data && me.pveSelNode.data.node; > + if (selnode && !me.nodename) { > + me.nodename = selnode; > + } > + if (!me.nodename) { > + throw "no node name specified"; > + } > + > + const unused = me.confid && me.confid.match(/^unused\d+$/); > + > + me.isCreate = me.confid ? unused : true; > + > + let ipanel = Ext.create('PVE.qemu.HardDisk', { > + confid: me.confid, > + unused: unused, > + isCreate: me.isCreate, > + }); > + ipanel.setNodename(me.nodename); > + > + if (unused) { > + me.subject = gettext('Unused Disk'); > + } else if (me.isCreate) { > + me.subject = gettext('Hard Disk'); > + } else { > + me.subject = gettext('Hard Disk') + ' (' + me.confid + ')'; > + } > + > + me.items = [ipanel]; > + > + me.callParent(); > + /* 'data' is assigned an empty array in same file, and here we > + * use it like an object > + */ > + me.load({ > + success: function(response, options) { > + ipanel.setVMConfig(response.result.data); > + if (me.confid) { > + let value = response.result.data[me.confid]; > + let drive = PVE.Parser.parseQemuDrive(me.confid, value); > + if (!drive) { > + Ext.Msg.alert(gettext('Error'), 'Unable to parse drive options'); > + me.close(); > + return; > + } > + ipanel.setDrive(drive); > + me.isValid(); // trigger validation > + } > + }, > + }); > + }, > + getValues: function() { > + let values = this.callParent(); > + for (const [key, value] of Object.entries(values)) { > + const re = /ide\d+|sata\d+|virtio\d+|scsi\d+|import_sources/; > + if (key.match(re) && Array.isArray(value)) { > + // Collected from different panels => array > + // But API & some GUI functions expect not array > + const sep = key === 'import_sources' ? '\0' : ','; // for API > + values[key] = value.join(sep); > + } > + } > + return values; > + }, > +}); > diff --git a/www/manager6/window/Wizard.js b/www/manager6/window/Wizard.js > index 47d60b8e..de935fd0 100644 > --- a/www/manager6/window/Wizard.js > +++ b/www/manager6/window/Wizard.js > @@ -245,6 +245,8 @@ Ext.define('PVE.window.Wizard', { > }; > field.on('change', validcheck); > field.on('validitychange', validcheck); > + // Make available for fields that get added later > + me.validcheck = validcheck; > }); > }, > }); >