Re: [PATCH manager v2] ui: qemu: boot order: warn of missing pxe support for ovmf

[Fiona Ebner <f.ebner@proxmox.com>]

Am 09.06.26 um 2:11 PM schrieb Dominik Csapak:
> OVMF can only use PXE/network boot when a virtual RNG device is added
> for VMs, but this is not obvious from the UI or the error behavior.
> 
> Add a small warning to the boot order edit that says what is necessary.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

with some optional nits below:

> ---
> changes from v1:
> * removed stray debug log statement..., sry for the noise
> 
>  www/manager6/qemu/BootOrderEdit.js | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/www/manager6/qemu/BootOrderEdit.js b/www/manager6/qemu/BootOrderEdit.js
> index 521a3d6e..6ae1f506 100644
> --- a/www/manager6/qemu/BootOrderEdit.js
> +++ b/www/manager6/qemu/BootOrderEdit.js
> @@ -26,6 +26,7 @@ Ext.define('PVE.qemu.BootOrderPanel', {
>              let grid = me.lookup('grid');
>              let marker = me.lookup('marker');
>              let emptyWarning = me.lookup('emptyWarning');
> +            let rngPxeWarning = me.lookup('rngPxeWarning');
>  
>              marker.originalValue = undefined;
>  
> @@ -43,6 +44,18 @@ Ext.define('PVE.qemu.BootOrderPanel', {
>                          view.inUpdate = false;
>                          marker.checkDirty();
>                          emptyWarning.setHidden(val !== '');
> +
> +                        let showRngPxeWarning = false;
> +                        if (

Style nit: no need for the if block. You can assign the expression directly.

> +                            view.vmconfig?.bios?.toLowerCase().includes('ovmf') &&

Nit: No need for toLowerCase() and includes(), it can just be === 'ovmf'
like we already do in other places?

> +                            !view.vmconfig?.rng0 &&
> +                            val?.includes('net')
> +                        ) {
> +                            showRngPxeWarning = true;
> +                        }
> +
> +                        rngPxeWarning.setHidden(!showRngPxeWarning);
> +
>                          grid.getView().refresh();
>                      },
>                  },
> @@ -238,6 +251,15 @@ Ext.define('PVE.qemu.BootOrderPanel', {
>              userCls: 'pmx-hint',
>              value: gettext('Warning: No devices selected, the VM will probably not boot!'),
>          },
> +        {
> +            xtype: 'displayfield',
> +            reference: 'rngPxeWarning',
> +            userCls: 'pmx-hint',
> +            hidden: true,
> +            value: gettext(
> +                'Warning: PXE boot with OVMF only works when a Virtio RNG device is added!',

Nit: maybe use active voice?
For PXE boot with OVMF, you must add a VirtIO RNG device!

> +            ),
> +        },
>          {
>              // for dirty marking and 'reset' function
>              xtype: 'field',