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 <d.k...@proxmox.com>
> ---
> 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
