]> git.proxmox.com Git - pve-manager.git/commitdiff
ui: guest import: rework windows virtio-scsi preparation
authorDominik Csapak <d.csapak@proxmox.com>
Wed, 13 Mar 2024 08:35:57 +0000 (09:35 +0100)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Thu, 14 Mar 2024 14:17:43 +0000 (15:17 +0100)
instead of having a separate iso selector that shows with an enabled
checkbox, just add a CDROM drive when windows is selected and there is
no such drive available yet.
The idea here is that the VM's OS is already fully set up, so a single
CDROM drive is enough to be used for installing VirtIO drivers, unlike
the VM create case, where the first one is already used for the
installation medium.

Also, rename the 'map to sata' checkbox to 'prepare for virtio-scsi'
that also changes the scsi controller to virtio-scsi-single

Additionally, change the positioning of the checkbox/scsihw selector
to be below the disk grid
With that we then only disable prepare-for-virtio checkbox for
non-windows OS types, as the scsi controller on the right looks like
it hangs in the air without any field on the left otherwise.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
 [ TL: rework commit message, squash in some fixes ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
www/manager6/window/GuestImport.js

index aac0c94a2f25617ac1ac779441edc925ad7d46fe..b846f00eb09cfad4b536aa9bbe8f9f23ef3dd8e8 100644 (file)
@@ -92,44 +92,69 @@ Ext.define('PVE.window.GuestImport', {
            summaryGrid.getStore().setData(Object.entries(values).map(([key, value]) => ({ key, value })));
        },
 
+       calculateAdditionalCDIdx: function() {
+           let me = this;
+
+           let maxIde = me.getMaxControllerId('ide');
+           let maxSata = me.getMaxControllerId('sata');
+           // only ide0 and ide2 can be used reliably for isos (e.g. for q35)
+           if (maxIde < 0) {
+               return 'ide0';
+           }
+           if (maxIde < 2) {
+               return 'ide2';
+           }
+           if (maxSata < PVE.Utils.diskControllerMaxIDs.sata - 1) {
+               return `sata${maxSata+1}`;
+           }
+
+           return '';
+       },
+
        // assume assigned sata disks indices are continuous, so without holes
-       getMaxSata: function() {
+       getMaxControllerId: function(controller) {
            let me = this;
            let view = me.getView();
-           if (view.maxSata !== undefined) {
-               return view.maxSata;
+           if (!controller) {
+               return -1;
+           }
+
+           let max = view[`max${controller}`];
+           if (max !== undefined) {
+               return max;
            }
 
-           view.maxSata = -1;
+           max = -1;
            for (const key of Object.keys(me.getView().vmConfig)) {
-               if (!key.toLowerCase().startsWith('sata')) {
+               if (!key.toLowerCase().startsWith(controller)) {
                    continue;
                }
-               let idx = parseInt(key.slice(4), 10);
-               if (idx > view.maxSata) {
-                   view.maxSata = idx;
+               let idx = parseInt(key.slice(controller.length), 10);
+               if (idx > max) {
+                   max = idx;
                }
            }
            me.lookup('diskGrid').getStore().each(rec => {
-               if (!rec.data.id.toLowerCase().startsWith('sata')) {
+               if (!rec.data.id.toLowerCase().startsWith(controller)) {
                    return;
                }
-               let idx = parseInt(rec.data.id.slice(4), 10);
-               if (idx > view.maxSata) {
-                   view.maxSata = idx;
+               let idx = parseInt(rec.data.id.slice(controller.length), 10);
+               if (idx > max) {
+                   max = idx;
                }
            });
            me.lookup('cdGrid').getStore().each(rec => {
-               if (!rec.data.id.toLowerCase().startsWith('sata')) {
+               if (!rec.data.id.toLowerCase().startsWith(controller) || rec.data.hidden) {
                    return;
                }
-               let idx = parseInt(rec.data.id.slice(4), 10);
-               if (idx > view.maxSata) {
-                   view.maxSata = idx;
+               let idx = parseInt(rec.data.id.slice(controller.length), 10);
+               if (idx > max) {
+                   max = idx;
                }
            });
 
-           return view.maxSata;
+           view[`max${controller}`] = max;
+           return max;
        },
 
        mapDisk: function(value, metaData) {
@@ -142,8 +167,12 @@ Ext.define('PVE.window.GuestImport', {
                return value;
            }
            let offset = parseInt(value.slice(4), 10);
-           let newIdx = offset + me.getMaxSata() + 1;
-           if (newIdx > PVE.Utils.diskControllerMaxIDs.sata) {
+           let newIdx = offset + me.getMaxControllerId('sata') + 1;
+           if (me.getViewModel().get('isWindows') && me.getView().additionalCdIdx?.startsWith('sata')) {
+               // additionalCdIdx takes the highest sata port
+               newIdx++;
+           }
+           if (newIdx >= PVE.Utils.diskControllerMaxIDs.sata) {
                let prefix = '';
                if (metaData !== undefined) {
                    // we're in the renderer so put a warning here
@@ -155,14 +184,45 @@ Ext.define('PVE.window.GuestImport', {
            return `sata${newIdx}`;
        },
 
-       refreshDiskGrid: function() {
+       refreshGrids: function() {
            this.lookup('diskGrid').reconfigure();
+           this.lookup('cdGrid').reconfigure();
+       },
+
+       onOSTypeChange: function(_cb, value) {
+           let me = this;
+           if (!value) {
+               return;
+           }
+           let store = me.lookup('cdGrid').getStore();
+           let collection = store.getData().getSource() ?? store.getData();
+           let rec = collection.find('autogenerated', true);
+
+           let isWindows = (value ?? '').startsWith('win');
+           if (rec) {
+               rec.set('hidden', !isWindows);
+               rec.commit();
+           }
+           let prepareVirtio = me.lookup('mapSata').getValue();
+           me.lookup('scsihw').setValue(prepareVirtio && isWindows ? 'virtio-scsi-single' : me.getView().vmConfig.scsihw);
+
+           me.refreshGrids();
        },
 
-       toggleIsoSelector: function(_cb, value) {
+       onPrepareVirtioChange: function(_cb, value) {
            let me = this;
-           me.lookup('isoSelector').setDisabled(!value);
-           me.lookup('isoSelector').setHidden(!value);
+
+           let scsihw = me.lookup('scsihw');
+           scsihw.suspendEvents();
+           scsihw.setValue(value ? 'virtio-scsi-single' : me.getView().vmConfig.scsihw);
+           scsihw.resumeEvents();
+
+           me.refreshGrids();
+       },
+
+       onScsiHwChange: function(_field, value) {
+           let me = this;
+           me.getView().vmConfig.scsihw = value;
        },
 
        control: {
@@ -187,13 +247,13 @@ Ext.define('PVE.window.GuestImport', {
                activate: 'calculateConfig',
            },
            'proxmoxcheckbox[reference=mapSata]': {
-               change: 'refreshDiskGrid',
+               change: 'onPrepareVirtioChange',
            },
            'combobox[name=ostype]': {
-               change: 'refreshDiskGrid',
+               change: 'onOSTypeChange',
            },
-           'proxmoxcheckbox[reference=enableSecondCD]': {
-               change: 'toggleIsoSelector',
+           'pveScsiHwSelector': {
+               change: 'onScsiHwChange',
            },
        },
     },
@@ -204,6 +264,7 @@ Ext.define('PVE.window.GuestImport', {
            socketCount: 1,
            liveImport: false,
            os: '',
+           maxCdDrives: false,
            warnings: [],
        },
 
@@ -323,27 +384,6 @@ Ext.define('PVE.window.GuestImport', {
                            config['live-restore'] = 1;
                        }
 
-                       if (grid.lookup('enableSecondCD')) {
-                           let idsToTry = ['ide0', 'ide2'];
-                           for (let i = 0; i <=PVE.Utils.diskControllerMaxIDs.sata; i++) {
-                               idsToTry.push(`sata{$i}`);
-                           }
-                           let found = false;
-                           for (const id of idsToTry) {
-                               if (!config[id]) {
-                                   config[id] = PVE.Parser.printQemuDrive({
-                                       media: 'cdrom',
-                                       file: grid.lookup('isoSelector').getValue(),
-                                   });
-                                   found = true;
-                                   break;
-                               }
-                           }
-                           if (!found) {
-                               console.warn('could not insert cd drive for virtio');
-                           }
-                       }
-
                        // remove __default__ values
                        for (const [key, value] of Object.entries(config)) {
                            if (value === '__default__') {
@@ -513,59 +553,8 @@ Ext.define('PVE.window.GuestImport', {
 
                    // the first inputpanel handles all values, so prevent value leakage here
                    onGetValues: () => ({}),
-                   column1: [
-                       {
-                           xtype: 'pveScsiHwSelector',
-                           reference: 'scsihw',
-                           name: 'scsihw',
-                           submitValue: false,
-                           fieldLabel: gettext('SCSI Controller'),
-                       },
-                       {
-                           xtype: 'proxmoxcheckbox',
-                           reference: 'enableSecondCD',
-                           isFormField: false,
-                           hidden: true,
-                           checked: false,
-                           boxLabel: gettext('Add additional drive for VirtIO drivers'),
-                           bind: {
-                               hidden: '{!isWindows}',
-                               disabled: '{!isWindows}',
-                           },
-                       },
-                   ],
 
-                   column2: [
-                       {
-                           xtype: 'proxmoxcheckbox',
-                           fieldLabel: gettext('Map SCSI to SATA'),
-                           labelWidth: 120,
-                           reference: 'mapSata',
-                           isFormField: false,
-                           hidden: true,
-                           disabled: true,
-                           bind: {
-                               hidden: '{!isWindows}',
-                               disabled: '{!isWindows}',
-                           },
-                           autoEl: {
-                               tag: 'div',
-                               'data-qtip': gettext('Useful for a quicker switch to VirtIO-SCSI attached disks'),
-                           },
-                       },
-                       {
-                           xtype: 'pveIsoSelector',
-                           reference: 'isoSelector',
-                           submitValue: false,
-                           labelWidth: 120,
-                           labelAlign: 'left',
-                           insideWizard: true,
-                           hidden: true,
-                           disabled: true,
-                       },
-                   ],
-
-                   columnB: [
+                   columnT: [
                        {
                            xtype: 'displayfield',
                            fieldLabel: gettext('Disks'),
@@ -648,6 +637,37 @@ Ext.define('PVE.window.GuestImport', {
                                },
                            ],
                        },
+                   ],
+
+                   column1: [
+                       {
+                           xtype: 'proxmoxcheckbox',
+                           fieldLabel: gettext('Prepare for VirtIO-SCSI'),
+                           labelWidth: 200,
+                           reference: 'mapSata',
+                           isFormField: false,
+                           disabled: true,
+                           bind: {
+                               disabled: '{!isWindows}',
+                           },
+                           autoEl: {
+                               tag: 'div',
+                               'data-qtip': gettext('Maps SCSI disks to SATA and changes the SCSI Controller. Useful for a quicker switch to VirtIO-SCSI attached disks'),
+                           },
+                       },
+                   ],
+
+                   column2: [
+                       {
+                           xtype: 'pveScsiHwSelector',
+                           reference: 'scsihw',
+                           name: 'scsihw',
+                           submitValue: false,
+                           fieldLabel: gettext('SCSI Controller'),
+                       },
+                   ],
+
+                   columnB: [
                        {
                            xtype: 'displayfield',
                            fieldLabel: gettext('CD/DVD Drives'),
@@ -666,6 +686,11 @@ Ext.define('PVE.window.GuestImport', {
                                sorters: [
                                    'id',
                                ],
+                               filters: [
+                                   function(rec) {
+                                       return !rec.data.hidden;
+                                   },
+                               ],
                            },
                            columns: [
                                {
@@ -835,7 +860,6 @@ Ext.define('PVE.window.GuestImport', {
 
        me.lookup('defaultStorage').setNodename(me.nodename);
        me.lookup('defaultBridge').setNodename(me.nodename);
-       me.lookup('isoSelector').setNodename(me.nodename);
 
        let renderWarning = w => {
            const warningsCatalogue = {
@@ -888,14 +912,29 @@ Ext.define('PVE.window.GuestImport', {
                    }
                    cdroms.push({
                        enable: true,
+                       hidden: false,
                        id,
                    });
                    delete me.vmConfig[id];
                }
+
                me.lookup('diskGrid').getStore().setData(disks);
                me.lookup('netGrid').getStore().setData(nets);
                me.lookup('cdGrid').getStore().setData(cdroms);
 
+               let additionalCdIdx = me.getController().calculateAdditionalCDIdx();
+               if (additionalCdIdx === '') {
+                   me.getViewModel().set('maxCdDrives', true);
+               } else if (cdroms.length === 0) {
+                   me.additionalCdIdx = additionalCdIdx;
+                   me.lookup('cdGrid').getStore().add({
+                       enable: true,
+                       hidden: !(me.vmConfig.ostype ?? '').startsWith('win'),
+                       id: additionalCdIdx,
+                       autogenerated: true,
+                   });
+               }
+
                me.getViewModel().set('warnings', data.warnings.map(w => renderWarning(w)));
 
                let osinfo = PVE.Utils.get_kvm_osinfo(me.vmConfig.ostype ?? '');