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

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

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