[pve-devel] [PATCH storage/manager] guest import improvements

[pve-devel] [PATCH storage/manager] guest import improvements

[Dominik Csapak]

some sensible improvements for the guest import

pve-storage:

Dominik Csapak (2):
  esxi: add warning for losing efi state
  esxi: only add scsihw if it's defined

 src/PVE/Storage/ESXiPlugin.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

pve-manager:

Dominik Csapak (4):
  ui: guest import: fix isWindows check
  ui: guest import: auto activate virtio preparation for win + ovmf
  ui: guest import: correctly set default scsihw value
  ui: guest import: add warning for losing efi state

 www/manager6/window/GuestImport.js | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

-- 
2.39.2

[pve-devel] [PATCH storage 1/2] esxi: add warning for losing efi state

[Dominik Csapak]

we cannot import the state of the efivars (e.g. boot order)
so add a warning for that

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/Storage/ESXiPlugin.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
index 3f47577..e448760 100644
--- a/src/PVE/Storage/ESXiPlugin.pm
+++ b/src/PVE/Storage/ESXiPlugin.pm
@@ -955,6 +955,7 @@ sub get_create_args {
     if ($firmware eq 'efi') {
 	$create_args->{bios} = 'ovmf';
 	$create_disks->{efidisk0} = 1;
+	$warn->('efi-state-lost', key => "bios", value => "ovmf");
     } else {
 	$create_args->{bios} = 'seabios';
     }
-- 
2.39.2

[pve-devel] applied: [PATCH storage 1/2] esxi: add warning for losing efi state

[Thomas Lamprecht]

On 19/03/2024 14:00, Dominik Csapak wrote:
> we cannot import the state of the efivars (e.g. boot order)
> so add a warning for that
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/Storage/ESXiPlugin.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
>

applied, thanks!

but I squashed in adding the new warning also to the return schema,
as the comment above the $warn closure tries to remind:

> # NOTE: all types must be added to the return schema of the import-metadata API endpoint

but yeah, that's not that visible if one copies a $warn->() call and
sadly not easily enforceable at package build time.

[pve-devel] [PATCH storage 2/2] esxi: only add scsihw if it's defined

[Dominik Csapak]

otherwise we get `scsihw: null` from the api, which is not a valid
value, so just omit it.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/Storage/ESXiPlugin.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
index e448760..b36cea8 100644
--- a/src/PVE/Storage/ESXiPlugin.pm
+++ b/src/PVE/Storage/ESXiPlugin.pm
@@ -1073,7 +1073,7 @@ sub get_create_args {
 	    $warn->('ovmf-with-lsi-unsupported', key => 'scsihw', value => "$scsihw");
 	}
     }
-    $create_args->{scsihw} = $scsihw;
+    $create_args->{scsihw} = $scsihw if defined($scsihw);
 
     $create_args->{boot} = "order=$boot_order";
 
-- 
2.39.2

[pve-devel] applied: [PATCH storage 2/2] esxi: only add scsihw if it's defined

[Thomas Lamprecht]

On 19/03/2024 14:00, Dominik Csapak wrote:
> otherwise we get `scsihw: null` from the api, which is not a valid
> value, so just omit it.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/Storage/ESXiPlugin.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!

[pve-devel] [PATCH manager 1/4] ui: guest import: fix isWindows check

[Dominik Csapak]

while most of our 'windiows' ostypes start with 'win' not all of them do
(wxp, wvista), so just shorten the condition to 'starts with `w`', this
covers all our windows ostypes, while not including others.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/window/GuestImport.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/www/manager6/window/GuestImport.js b/www/manager6/window/GuestImport.js
index 443bc886..81e9074e 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -198,7 +198,7 @@ Ext.define('PVE.window.GuestImport', {
 	    let collection = store.getData().getSource() ?? store.getData();
 	    let rec = collection.find('autogenerated', true);
 
-	    let isWindows = (value ?? '').startsWith('win');
+	    let isWindows = (value ?? '').startsWith('w');
 	    if (rec) {
 		rec.set('hidden', !isWindows);
 		rec.commit();
@@ -275,7 +275,7 @@ Ext.define('PVE.window.GuestImport', {
 	        + get('warnings').map(w => `<li>${w}</li>`).join('') + '</ul>',
 	    liveImportNote: get => !get('liveImport') ? ''
 	        : gettext('Note: If anything goes wrong during the live-import, new data written by the VM may be lost.'),
-	    isWindows: get => (get('os') ?? '').startsWith('win'),
+	    isWindows: get => (get('os') ?? '').startsWith('w'),
 	},
     },
 
@@ -925,7 +925,7 @@ Ext.define('PVE.window.GuestImport', {
 		    me.additionalCdIdx = additionalCdIdx;
 		    me.lookup('cdGrid').getStore().add({
 			enable: true,
-			hidden: !(me.vmConfig.ostype ?? '').startsWith('win'),
+			hidden: !(me.vmConfig.ostype ?? '').startsWith('w'),
 			id: additionalCdIdx,
 			autogenerated: true,
 		    });
-- 
2.39.2

[pve-devel] [PATCH manager 2/4] ui: guest import: auto activate virtio preparation for win + ovmf

[Dominik Csapak]

it seems on esxi, most windows vms with uefi are automatically
configured with an lsi scsi controller, which we can't currently support
(ovmf driver issue) so automatically activate the sata mapping + virtio
preparation in this case

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/window/GuestImport.js | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/www/manager6/window/GuestImport.js b/www/manager6/window/GuestImport.js
index 81e9074e..719bab31 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -644,7 +644,8 @@ Ext.define('PVE.window.GuestImport', {
 			    fieldLabel: gettext('Prepare for VirtIO-SCSI'),
 			    labelWidth: 200,
 			    reference: 'mapSata',
-			    isFormField: false,
+			    name: 'mapSata',
+			    submitValue: false,
 			    disabled: true,
 			    bind: {
 				disabled: '{!isWindows}',
@@ -934,11 +935,15 @@ Ext.define('PVE.window.GuestImport', {
 		me.getViewModel().set('warnings', data.warnings.map(w => renderWarning(w)));
 
 		let osinfo = PVE.Utils.get_kvm_osinfo(me.vmConfig.ostype ?? '');
+		let mapSata = (me.vmConfig.ostype ?? '').startsWith('w') && (me.vmConfig.bios ?? '').indexOf('ovmf') !== -1;
 
 		me.setValues({
 		    osbase: osinfo.base,
 		    ...me.vmConfig,
 		});
+
+
+		me.lookup('mapSata').setValue(mapSata);
 	    },
 	});
     },
-- 
2.39.2

[pve-devel] [PATCH manager 3/4] ui: guest import: correctly set default scsihw value

[Dominik Csapak]

we have to set it to '__default__' if we didn't get one from the api

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/window/GuestImport.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/www/manager6/window/GuestImport.js b/www/manager6/window/GuestImport.js
index 719bab31..297e2198 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -204,7 +204,8 @@ Ext.define('PVE.window.GuestImport', {
 		rec.commit();
 	    }
 	    let prepareVirtio = me.lookup('mapSata').getValue();
-	    me.lookup('scsihw').setValue(prepareVirtio && isWindows ? 'virtio-scsi-single' : me.getView().vmConfig.scsihw);
+	    let defaultScsiHw = me.getView().vmConfig.scsihw ?? '__default__';
+	    me.lookup('scsihw').setValue(prepareVirtio && isWindows ? 'virtio-scsi-single' : defaultScsiHw );
 
 	    me.refreshGrids();
 	},
@@ -662,6 +663,7 @@ Ext.define('PVE.window.GuestImport', {
 			    xtype: 'pveScsiHwSelector',
 			    reference: 'scsihw',
 			    name: 'scsihw',
+			    value: '__default__',
 			    submitValue: false,
 			    fieldLabel: gettext('SCSI Controller'),
 			},
-- 
2.39.2

[pve-devel] [PATCH manager 4/4] ui: guest import: add warning for losing efi state

[Dominik Csapak]

and add a link to recreate the boot entries in ovmf

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/window/GuestImport.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/window/GuestImport.js b/www/manager6/window/GuestImport.js
index 297e2198..83288dad 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -867,6 +867,7 @@ Ext.define('PVE.window.GuestImport', {
 		'ovmf-with-lsi-unsupported': gettext("OVMF is built without LSI drivers, scsi hardware was set to '{1}'"),
 		'serial-port-socket-only': gettext("Serial socket '{0}' will be mapped to a socket"),
 		'guest-is-running': gettext('Virtual guest seems to be running on source host. Import might fail or have inconsistent state!'),
+		'efi-state-lost': Ext.String.format(gettext('EFI state cannot be imported, you may need to reconfigure the boot order (see {0})'), '<a href="https://pve.proxmox.com/wiki/OVMF/UEFI_Boot_Entries">OVMF/UEFI Boot Entries</a>'),
 	    };
             let message = warningsCatalogue[w.type];
 	    if (!w.type || !message) {
-- 
2.39.2

[pve-devel] applied: [PATCH storage/manager] guest import improvements

[Thomas Lamprecht]

On 19/03/2024 14:00, Dominik Csapak wrote:
> some sensible improvements for the guest import
> 
> pve-storage:
> 
> Dominik Csapak (2):
>   esxi: add warning for losing efi state
>   esxi: only add scsihw if it's defined
> 
>  src/PVE/Storage/ESXiPlugin.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> pve-manager:
> 
> Dominik Csapak (4):
>   ui: guest import: fix isWindows check
>   ui: guest import: auto activate virtio preparation for win + ovmf
>   ui: guest import: correctly set default scsihw value
>   ui: guest import: add warning for losing efi state
> 
>  www/manager6/window/GuestImport.js | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 


applied now also the UI patches, thanks!

I squashed in a fix to a "extra whitespace in parenthesis" eslint warning
in patch 3/4 and split up the overly long line in 4/4.