[PATCH manager v2 0/4] fix #7801: ui: warn about problematic virtio versions on iso select

[PATCH manager v2 0/4] fix #7801: ui: warn about problematic virtio versions on iso select

[Nicolas Frey]

fixes #7801 by scanning for virtio filename regex and warning the user
if the version has known issues attributed to it [0].

I tried the following approaches to warn the user:
* via a (quite intrusive) pop-up
* a warning box after the iso selector
* an inline hoverable icon on the selector field

I felt the second approach resulted in the best UX, so I went with
that in this series.

Changes since v1 (thanks Maximiliano!):
* use single gettext call for warning message
* remove the detailed hints about which version to switch to,
  instead rely on the link to the wiki
* actually lazily evaluate the ostype in patch [4/4] by putting it in
  the function body that gets returned (i.e. the factory)
* add missing warnVirtio to commonOpts

[0] https://pve.proxmox.com/wiki/Windows_VirtIO_Drivers

pve-manager:

Nicolas Frey (4):
  ui: iso selector: add option to warn about virtio versions with issues
  ui: cd edit: add `warnVirtio` option to pass to iso selector
  partially fix #7801: ui: ospanel: warn about problematic virtio
    versions
  partially fix #7801: ui: hardware view: warn about problematic virtio
    versions

 www/manager6/form/IsoSelector.js  | 70 ++++++++++++++++++++++++++++++-
 www/manager6/qemu/CDEdit.js       |  4 ++
 www/manager6/qemu/HardwareView.js |  8 ++--
 www/manager6/qemu/OSPanel.js      |  1 +
 4 files changed, 78 insertions(+), 5 deletions(-)


Summary over all repositories:
  4 files changed, 78 insertions(+), 5 deletions(-)

--
Generated by murpp 0.12.1

[PATCH pve-manager v2 1/4] ui: iso selector: add option to warn about virtio versions with issues

[Nicolas Frey]

based on the known issues as seen on the wiki. If warnVirtio is true,
the filename is matched against "virtio-win-x.x.x.iso" and checks
whether the version number in the filename is in any "from - to" version
range in `virtioIssues`.

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
changes since v1:
* use single gettext call for warning message
* remove the detailed hints about which version to switch to,
  instead rely on the link to the wiki

 www/manager6/form/IsoSelector.js | 70 +++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/www/manager6/form/IsoSelector.js b/www/manager6/form/IsoSelector.js
index b2d94ed3..ba7b6953 100644
--- a/www/manager6/form/IsoSelector.js
+++ b/www/manager6/form/IsoSelector.js
@@ -10,6 +10,7 @@ Ext.define('PVE.form.IsoSelector', {

     nodename: undefined,
     insideWizard: false,
+    warnVirtio: false,
     labelWidth: undefined,
     labelAlign: 'right',

@@ -63,6 +64,63 @@ Ext.define('PVE.form.IsoSelector', {
         return me.callParent([disabled]);
     },

+    virtioIssues: [
+        {
+            from: '0.1.215',
+            to: '0.1.262',
+        },
+        {
+            from: '0.1.285',
+            to: '0.1.285',
+        },
+    ],
+
+    checkVirtioVersion: function (filename) {
+        let me = this;
+        let warningBox = me.lookup('virtioWarning');
+
+        const compareVersions = (a, b) => {
+            let pa = a.split('.').map(Number);
+            let pb = b.split('.').map(Number);
+            let max = Math.max(pa.length, pb.length);
+            for (let i = 0; i < max; i++) {
+                let cmp = (pa[i] || 0) - (pb[i] || 0);
+                if (cmp !== 0) {
+                    return cmp;
+                }
+            }
+            return 0;
+        };
+
+        let match = (filename || '').match(/virtio-win[_-](\d+\.\d+\.\d+)/i);
+        let version = match && match[1];
+        let issue =
+            version &&
+            me.virtioIssues.some(
+                (i) => compareVersions(version, i.from) >= 0 && compareVersions(version, i.to) <= 0,
+            );
+
+        if (!issue) {
+            warningBox.setHtml('');
+            return;
+        }
+
+        let atag = `<a
+            href="https://pve.proxmox.com/wiki/Windows_VirtIO_Drivers#Known_issues"
+            target="_blank">${gettext('known issues')}</a>`;
+
+        warningBox.setHtml(
+            '<i class="fa fa-exclamation-triangle warning"></i> ' +
+                Ext.String.format(
+                    gettext(
+                        'Version {0} of the VirtIO drivers is known to cause issues. See {1} for more details.',
+                    ),
+                    version,
+                    atag,
+                ),
+        );
+    },
+
     referenceHolder: true,

     items: [
@@ -105,10 +163,18 @@ Ext.define('PVE.form.IsoSelector', {
             },
             allowBlank: false,
             listeners: {
-                change: function () {
-                    this.up('pveIsoSelector').checkChange();
+                change: function (_field, value) {
+                    let selector = this.up('pveIsoSelector');
+                    if (selector.warnVirtio) {
+                        selector.checkVirtioVersion(value);
+                    }
+                    selector.checkChange();
                 },
             },
         },
+        {
+            xtype: 'displayfield',
+            reference: 'virtioWarning',
+        },
     ],
 });
--
2.47.3

Re: [PATCH pve-manager v2 1/4] ui: iso selector: add option to warn about virtio versions with issues

[Fiona Ebner]

Am 26.08.26 um 9:49 AM schrieb Nicolas Frey:
> based on the known issues as seen on the wiki. If warnVirtio is true,
> the filename is matched against "virtio-win-x.x.x.iso" and checks
> whether the version number in the filename is in any "from - to" version
> range in `virtioIssues`.
> 
> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
> ---
> changes since v1:
> * use single gettext call for warning message
> * remove the detailed hints about which version to switch to,
>   instead rely on the link to the wiki
> 
>  www/manager6/form/IsoSelector.js | 70 +++++++++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/form/IsoSelector.js b/www/manager6/form/IsoSelector.js
> index b2d94ed3..ba7b6953 100644
> --- a/www/manager6/form/IsoSelector.js
> +++ b/www/manager6/form/IsoSelector.js
> @@ -10,6 +10,7 @@ Ext.define('PVE.form.IsoSelector', {
> 
>      nodename: undefined,
>      insideWizard: false,
> +    warnVirtio: false,

Nit: I'd use a slightly more explicit name "warnVirtioWin" and similarly
for other names below

>      labelWidth: undefined,
>      labelAlign: 'right',
> 
> @@ -63,6 +64,63 @@ Ext.define('PVE.form.IsoSelector', {
>          return me.callParent([disabled]);
>      },
> 
> +    virtioIssues: [
> +        {
> +            from: '0.1.215',
> +            to: '0.1.262',
> +        },
> +        {
> +            from: '0.1.285',
> +            to: '0.1.285',
> +        },
> +    ],
> +
> +    checkVirtioVersion: function (filename) {
> +        let me = this;
> +        let warningBox = me.lookup('virtioWarning');
> +
> +        const compareVersions = (a, b) => {
> +            let pa = a.split('.').map(Number);
> +            let pb = b.split('.').map(Number);
> +            let max = Math.max(pa.length, pb.length);
> +            for (let i = 0; i < max; i++) {
> +                let cmp = (pa[i] || 0) - (pb[i] || 0);
> +                if (cmp !== 0) {
> +                    return cmp;
> +                }
> +            }
> +            return 0;
> +        };
> +
> +        let match = (filename || '').match(/virtio-win[_-](\d+\.\d+\.\d+)/i);
> +        let version = match && match[1];
> +        let issue =
> +            version &&
> +            me.virtioIssues.some(
> +                (i) => compareVersions(version, i.from) >= 0 && compareVersions(version, i.to) <= 0,
> +            );
> +
> +        if (!issue) {
> +            warningBox.setHtml('');

Nit: I think it would be nice to hide the displayfield if there is no
warning.

> +            return;
> +        }
> +
> +        let atag = `<a
> +            href="https://pve.proxmox.com/wiki/Windows_VirtIO_Drivers#Known_issues"

Typo: the i should be capitalized, i.e. "#Known_Issues"

> +            target="_blank">${gettext('known issues')}</a>`;
> +
> +        warningBox.setHtml(
> +            '<i class="fa fa-exclamation-triangle warning"></i> ' +
> +                Ext.String.format(
> +                    gettext(
> +                        'Version {0} of the VirtIO drivers is known to cause issues. See {1} for more details.',
> +                    ),
> +                    version,
> +                    atag,
> +                ),
> +        );
> +    },
> +
>      referenceHolder: true,
> 
>      items: [
> @@ -105,10 +163,18 @@ Ext.define('PVE.form.IsoSelector', {
>              },
>              allowBlank: false,
>              listeners: {
> -                change: function () {
> -                    this.up('pveIsoSelector').checkChange();
> +                change: function (_field, value) {
> +                    let selector = this.up('pveIsoSelector');
> +                    if (selector.warnVirtio) {
> +                        selector.checkVirtioVersion(value);
> +                    }
> +                    selector.checkChange();
>                  },
>              },
>          },
> +        {
> +            xtype: 'displayfield',

Please use
userCls: 'pmx-hint',
like we do for other warnings. And it should start out as hidden,
otherwise there is suddenly new spacing below all ISO selectors.

> +            reference: 'virtioWarning',
> +        },
>      ],
>  });
> --
> 2.47.3
> 
> 
> 

Re: [PATCH pve-manager v2 1/4] ui: iso selector: add option to warn about virtio versions with issues

[Nicolas Frey]

On Wed Aug 26, 2026 at 1:44 PM CEST, Fiona Ebner wrote:
> Am 26.08.26 um 9:49 AM schrieb Nicolas Frey:
>> based on the known issues as seen on the wiki. If warnVirtio is true,
>> the filename is matched against "virtio-win-x.x.x.iso" and checks
>> whether the version number in the filename is in any "from - to" version
>> range in `virtioIssues`.
>>
>> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
>> ---
>> changes since v1:
>> * use single gettext call for warning message
>> * remove the detailed hints about which version to switch to,
>>   instead rely on the link to the wiki
>>
>>  www/manager6/form/IsoSelector.js | 70 +++++++++++++++++++++++++++++++-
>>  1 file changed, 68 insertions(+), 2 deletions(-)
>>
>> diff --git a/www/manager6/form/IsoSelector.js b/www/manager6/form/IsoSelector.js
>> index b2d94ed3..ba7b6953 100644
>> --- a/www/manager6/form/IsoSelector.js
>> +++ b/www/manager6/form/IsoSelector.js
>> @@ -10,6 +10,7 @@ Ext.define('PVE.form.IsoSelector', {
>>
>>      nodename: undefined,
>>      insideWizard: false,
>> +    warnVirtio: false,
>
> Nit: I'd use a slightly more explicit name "warnVirtioWin" and similarly
> for other names below

ack

>
>>      labelWidth: undefined,
>>      labelAlign: 'right',

[snip]

>> +        let atag = `<a
>> +            href="https://pve.proxmox.com/wiki/Windows_VirtIO_Drivers#Known_issues"
>
> Typo: the i should be capitalized, i.e. "#Known_Issues"
>

will change in next revision, good catch!

>> +            target="_blank">${gettext('known issues')}</a>`;
>> +
>> +        warningBox.setHtml(
>> +            '<i class="fa fa-exclamation-triangle warning"></i> ' +
>> +                Ext.String.format(
>> +                    gettext(
>> +                        'Version {0} of the VirtIO drivers is known to cause issues. See {1} for more details.',
>> +                    ),
>> +                    version,
>> +                    atag,
>> +                ),
>> +        );
>> +    },
>> +
>>      referenceHolder: true,
>>
>>      items: [
>> @@ -105,10 +163,18 @@ Ext.define('PVE.form.IsoSelector', {
>>              },
>>              allowBlank: false,
>>              listeners: {
>> -                change: function () {
>> -                    this.up('pveIsoSelector').checkChange();
>> +                change: function (_field, value) {
>> +                    let selector = this.up('pveIsoSelector');
>> +                    if (selector.warnVirtio) {
>> +                        selector.checkVirtioVersion(value);
>> +                    }
>> +                    selector.checkChange();
>>                  },
>>              },
>>          },
>> +        {
>> +            xtype: 'displayfield',
>
> Please use
> userCls: 'pmx-hint',
> like we do for other warnings. And it should start out as hidden,
> otherwise there is suddenly new spacing below all ISO selectors.
>

Ah I didn't notice that, sorry. Will do in a v3, thanks!

>> +            reference: 'virtioWarning',
>> +        },
>>      ],
>>  });
>> --
>> 2.47.3
>>
>>
>>

[PATCH pve-manager v2 2/4] ui: cd edit: add `warnVirtio` option to pass to iso selector

[Nicolas Frey]

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 www/manager6/qemu/CDEdit.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/www/manager6/qemu/CDEdit.js b/www/manager6/qemu/CDEdit.js
index c185de53..76712f44 100644
--- a/www/manager6/qemu/CDEdit.js
+++ b/www/manager6/qemu/CDEdit.js
@@ -3,6 +3,7 @@ Ext.define('PVE.qemu.CDInputPanel', {
     alias: 'widget.pveQemuCDInputPanel',
 
     insideWizard: false,
+    warnVirtio: false,
 
     onGetValues: function (values) {
         var me = this;
@@ -100,6 +101,7 @@ Ext.define('PVE.qemu.CDInputPanel', {
         me.isosel = Ext.create('PVE.form.IsoSelector', {
             nodename: me.nodename,
             insideWizard: me.insideWizard,
+            warnVirtio: me.warnVirtio,
             name: 'cdimage',
         });
 
@@ -129,6 +131,7 @@ Ext.define('PVE.qemu.CDEdit', {
     extend: 'Proxmox.window.Edit',
 
     width: 400,
+    warnVirtio: false,
 
     initComponent: function () {
         var me = this;
@@ -143,6 +146,7 @@ Ext.define('PVE.qemu.CDEdit', {
         var ipanel = Ext.create('PVE.qemu.CDInputPanel', {
             confid: me.confid,
             nodename: nodename,
+            warnVirtio: me.warnVirtio,
         });
 
         Ext.applyIf(me, {
-- 
2.47.3

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

[Nicolas Frey]

only on the virtio iso selector

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7801
Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 www/manager6/qemu/OSPanel.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/qemu/OSPanel.js b/www/manager6/qemu/OSPanel.js
index 556ad4c3..dc861746 100644
--- a/www/manager6/qemu/OSPanel.js
+++ b/www/manager6/qemu/OSPanel.js
@@ -161,6 +161,7 @@ Ext.define('PVE.qemu.OSPanel', {
                             reference: 'isoSelector',
                             name: 'ide0',
                             insideWizard: true,
+                            warnVirtio: true,
                             hidden: true,
                             disabled: true,
                             bind: {
-- 
2.47.3

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

[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')),
             };

             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 || {}),
                 });
         };

@@ -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

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

[Fiona Ebner]

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
> 
> 
> 

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

[Nicolas Frey]

On Wed Aug 26, 2026 at 1:44 PM CEST, Fiona Ebner wrote:
> 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?
>

doesn't efidisk_menuitem just apply to the "Add" dropdown menu? the commonOpts
are IMO not ideal, but there is no option to pass in any additional options:

```
if (Ext.isString(editor)) {
    Ext.create(editor, commonOpts);
}
```

where the `CDEdit` editor is determined (in `run_editor`) like so:

```
if (rowdef.isOnStorageBus) {
    let value = me.getObjectValue(rec.data.key, '', true);
    if (isCloudInitKey(value)) {
        return;
    } else if (value.match(/media=cdrom/)) {
        editor = 'PVE.qemu.CDEdit';
    } else if (!diskCap) {
        return;
    }
}
```

I'd have to rewrite a bunch of stuff here to allow for that and I'm not
sure it's worth the hassle here. what do you think?

>>              };
>>
>>              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.
>

do you mean the way it's written with a ternary or generally the lazy evaluation?

>>                  });
>>          };
>>
>> @@ -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
>>
>>
>>

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

[Fiona Ebner]

Am 26.08.26 um 3:37 PM schrieb Nicolas Frey:
> On Wed Aug 26, 2026 at 1:44 PM CEST, Fiona Ebner wrote:
>> 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?
>>
> 
> doesn't efidisk_menuitem just apply to the "Add" dropdown menu?

Hmm, right.

> the commonOpts
> are IMO not ideal, but there is no option to pass in any additional options:
> 
> ```
> if (Ext.isString(editor)) {
>     Ext.create(editor, commonOpts);
> }
> ```
> 
> where the `CDEdit` editor is determined (in `run_editor`) like so:
> 
> ```
> if (rowdef.isOnStorageBus) {
>     let value = me.getObjectValue(rec.data.key, '', true);
>     if (isCloudInitKey(value)) {
>         return;
>     } else if (value.match(/media=cdrom/)) {
>         editor = 'PVE.qemu.CDEdit';

Can we define an instance with the extra option up-front and then assign
that one instead of the string here?

>     } else if (!diskCap) {
>         return;
>     }
> }
> ```
> 
> I'd have to rewrite a bunch of stuff here to allow for that and I'm not
> sure it's worth the hassle here. what do you think?
> 
>>>              };
>>>
>>>              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.
>>
> 
> do you mean the way it's written with a ternary or generally the lazy evaluation?
> 

I meant the overloading of the parameter to either be a dictionary or a
function. But to be fair, it is JavaScript, so nothing unheard of.

>>>                  });
>>>          };
>>>
>>> @@ -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
>>>
>>>
>>>

Re: [PATCH manager v2 0/4] fix #7801: ui: warn about problematic virtio versions on iso select

[Dominik Csapak]

high level question: could it make sense to have this
warning also on the backend, e.g. in the start task?

that way a user only using the cli/api would also see it, not
only the ui users.

if that makes sense, we might want to keep a list
of versions on the backend and expose it via the api
for the gui?

on the backend we could even match the sha256 sum or something like
that (if that's not too much of a performance hit on start) to
check if it's really the right version

On 8/26/26 9:49 AM, Nicolas Frey wrote:
> fixes #7801 by scanning for virtio filename regex and warning the user
> if the version has known issues attributed to it [0].
> 
> I tried the following approaches to warn the user:
> * via a (quite intrusive) pop-up
> * a warning box after the iso selector
> * an inline hoverable icon on the selector field
> 
> I felt the second approach resulted in the best UX, so I went with
> that in this series.
> 
> Changes since v1 (thanks Maximiliano!):
> * use single gettext call for warning message
> * remove the detailed hints about which version to switch to,
>    instead rely on the link to the wiki
> * actually lazily evaluate the ostype in patch [4/4] by putting it in
>    the function body that gets returned (i.e. the factory)
> * add missing warnVirtio to commonOpts
> 
> [0] https://pve.proxmox.com/wiki/Windows_VirtIO_Drivers
> 
> pve-manager:
> 
> Nicolas Frey (4):
>    ui: iso selector: add option to warn about virtio versions with issues
>    ui: cd edit: add `warnVirtio` option to pass to iso selector
>    partially fix #7801: ui: ospanel: warn about problematic virtio
>      versions
>    partially fix #7801: ui: hardware view: warn about problematic virtio
>      versions
> 
>   www/manager6/form/IsoSelector.js  | 70 ++++++++++++++++++++++++++++++-
>   www/manager6/qemu/CDEdit.js       |  4 ++
>   www/manager6/qemu/HardwareView.js |  8 ++--
>   www/manager6/qemu/OSPanel.js      |  1 +
>   4 files changed, 78 insertions(+), 5 deletions(-)
> 
> 
> Summary over all repositories:
>    4 files changed, 78 insertions(+), 5 deletions(-)
> 
> --
> Generated by murpp 0.12.1

Re: [PATCH manager v2 0/4] fix #7801: ui: warn about problematic virtio versions on iso select

[Nicolas Frey]

On Thu Aug 27, 2026 at 8:14 AM CEST, Dominik Csapak wrote:
> high level question: could it make sense to have this
> warning also on the backend, e.g. in the start task?
>
> that way a user only using the cli/api would also see it, not
> only the ui users.
>
> if that makes sense, we might want to keep a list
> of versions on the backend and expose it via the api
> for the gui?

I think both of these points make sense to implement, I can add it in the
next revision

>
> on the backend we could even match the sha256 sum or something like
> that (if that's not too much of a performance hit on start) to
> check if it's really the right version
>

while I guess this is a better heuristic to check against instead of
just relying on the filename, I think unless performing the sha256
sum once and caching it, it would be too much of an unnecessary
performance hit.

[snip]