From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 5D2C21FF0C1 for ; Wed, 26 Aug 2026 13:44:58 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2DA9D2144F; Wed, 26 Aug 2026 13:44:58 +0200 (CEST) Message-ID: <3cec4b70-3e06-434d-aa8f-0c9b07252de0@proxmox.com> Date: Wed, 26 Aug 2026 13:44:47 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-manager v2 1/4] ui: iso selector: add option to warn about virtio versions with issues To: Nicolas Frey , pve-devel@lists.proxmox.com References: <20260826074935.78437-1-n.frey@proxmox.com> <20260826074935.78437-2-n.frey@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260826074935.78437-2-n.frey@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787744685491 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.801 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 6JXR4U5O7SCCI7BEUELWGQ2SDHAXB4FK X-Message-ID-Hash: 6JXR4U5O7SCCI7BEUELWGQ2SDHAXB4FK X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > --- > 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 = ` + 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')}`; > + > + warningBox.setHtml( > + ' ' + > + 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 > > >