Some suggestions inline. Skimmed over the code to spot style issues, correctness
was not really checked.
On 2024-10-16 18:47, Daniel Kral wrote:
> Adds cache and async I/O selectors to the CDROM Drive Edit modal window
> in the "Hardware" tab. This allows users to set these options in the
> WebGUI when the VM fails to start because the underlying storage
> (driver) does not support a specific set of configurations.
>
> Signed-off-by: Daniel Kral <d.k...@proxmox.com>
> ---
> www/manager6/qemu/CDEdit.js | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/www/manager6/qemu/CDEdit.js b/www/manager6/qemu/CDEdit.js
> index 3cc16205..f15905e7 100644
> --- a/www/manager6/qemu/CDEdit.js
> +++ b/www/manager6/qemu/CDEdit.js
> @@ -7,6 +7,7 @@ Ext.define('PVE.qemu.CDInputPanel', {
> onGetValues: function(values) {
> var me = this;
>
> + var params = {};
Note: For new code we should use `let` or `const` insted of `var` [1].
When touching existing code can IMO also change it to `let`, but of course
familiarize yourself first with the differences between the three and make sure
that you don't introduce any unwanted changes (e.g. due to changed scoping).
[1]: https://pve.proxmox.com/wiki/Javascript_Style_Guide
> var confid = me.confid || values.controller + values.deviceid;
>
> me.drive.media = 'cdrom';
> @@ -18,7 +19,8 @@ Ext.define('PVE.qemu.CDInputPanel', {
> me.drive.file = 'none';
> }
>
> - var params = {};
> + PVE.Utils.propertyStringSet(me.drive, values.cache, 'cache');
> + PVE.Utils.propertyStringSet(me.drive, values.aio, 'aio');
>
> params[confid] = PVE.Parser.printQemuDrive(me.drive);
>
> @@ -46,6 +48,9 @@ Ext.define('PVE.qemu.CDInputPanel', {
> values.cdimage = drive.file;
> }
>
> + values.cache = drive.cache || '__default__';
> + values.aio = drive.aio || '__default__';
> +
> me.drive = drive;
>
> me.setValues(values);
> @@ -118,6 +123,27 @@ Ext.define('PVE.qemu.CDInputPanel', {
>
> me.items = items;
>
> + // those are only useful for specific niche use cases
> + if (!me.insideWizard) {
> + me.advancedColumn1 = [
> + {
> + xtype: 'CacheTypeSelector',
> + name: 'cache',
> + value: '__default__',
> + fieldLabel: gettext('Cache'),
> + },
> + ];
> + me.advancedColumn2 = [
> + {
> + xtype: 'AsyncIOTypeSelector',
> + name: 'aio',
> + value: '__default__',
> + fieldLabel: gettext('Async IO'),
> + allowBlank: false,
> + },
> + ];
> + }
> +
> me.callParent();
> },
> });
> @@ -125,7 +151,7 @@ Ext.define('PVE.qemu.CDInputPanel', {
> Ext.define('PVE.qemu.CDEdit', {
> extend: 'Proxmox.window.Edit',
>
> - width: 400,
> + width: 550,
This change is not mentioned in the commit message and should probably
be split out into its own commit.
>
> initComponent: function() {
> var me = this;
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
