Re: [PATCH pve-manager v2 4/4] partially fix #7801: ui: hardware view: warn about problematic virtio versions

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

Am 26.08.26 um 9:49 AM schrieb Nicolas Frey:
> based on whether the ostype is windows or not. extends the
> `editorFactory` to allow extraOptions to be a callback to allow for lazy
> evaluation. this is needed as 'ostype' is not yet instantiated when
> eagerly evaluating.
> 
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7801
> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
> ---
> changes since v1:
> * actually lazily evaluate the ostype by putting it in
>   the function body that gets returned (i.e. the factory)
> * add missing warnVirtio to commonOpts
> 
>  www/manager6/qemu/HardwareView.js | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
> index e6c02299..9e900746 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -436,6 +436,7 @@ Ext.define('PVE.qemu.HardwareView', {
>                  listeners: {
>                      destroy: () => me.reload(),
>                  },
> +                warnVirtio: PVE.Utils.is_windows(me.getObjectValue('ostype')),

Seems a bit out of place in the commonOpts. Can't we do this more
locally in the definition below? Might need splitting out like the
efidisk_menuitem?

>              };
> 
>              if (Ext.isString(editor)) {
> @@ -805,7 +806,6 @@ Ext.define('PVE.qemu.HardwareView', {
>          };
> 
>          let editorFactory = (classPath, extraOptions) => {
> -            extraOptions = extraOptions || {};
>              return () =>
>                  Ext.create(`PVE.qemu.${classPath}`, {
>                      autoShow: true,
> @@ -816,7 +816,7 @@ Ext.define('PVE.qemu.HardwareView', {
>                      },
>                      isAdd: true,
>                      isCreate: true,
> -                    ...extraOptions,
> +                    ...(Ext.isFunction(extraOptions) ? extraOptions() : extraOptions || {}),

This is rather hacky too.

>                  });
>          };
> 
> @@ -847,7 +847,9 @@ Ext.define('PVE.qemu.HardwareView', {
>                                  text: gettext('CD/DVD Drive'),
>                                  iconCls: 'pve-itype-icon-cdrom',
>                                  disabled: !caps.vms['VM.Config.CDROM'],
> -                                handler: editorFactory('CDEdit'),
> +                                handler: editorFactory('CDEdit', () => ({
> +                                    warnVirtio: PVE.Utils.is_windows(me.getObjectValue('ostype')),
> +                                })),
>                              },
>                              {
>                                  text: gettext('Network Device'),
> --
> 2.47.3
> 
> 
>