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 9D3DE1FF09B for ; Mon, 31 Aug 2026 09:11:41 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 6A04D2156A; Mon, 31 Aug 2026 09:11:41 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 31 Aug 2026 09:11:37 +0200 Message-Id: Subject: Re: [PATCH manager v2] fix #7896: ui: qemu: show effective machine version for Windows guests From: "Jakob Klocker" To: "Dominik Csapak" , X-Mailer: aerc 0.20.0 References: <20260824130155.249596-1-j.klocker@proxmox.com> <382da81b-b48b-4047-9fb0-a9d355573902@proxmox.com> In-Reply-To: <382da81b-b48b-4047-9fb0-a9d355573902@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788160284869 X-SPAM-LEVEL: Spam detection results: 0 AWL 1.757 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: WJGR4QH3RZXHSZ2MVNSALBMSDIBDGFB7 X-Message-ID-Hash: WJGR4QH3RZXHSZ2MVNSALBMSDIBDGFB7 X-MailFrom: j.klocker@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: On Wed Aug 26, 2026 at 10:36 AM CEST, Dominik Csapak wrote: > one comment inline > > On 8/24/26 3:01 PM, Jakob Klocker wrote: > [snip] >> q35: (get) =3D> get('type') =3D=3D=3D 'q35', >> @@ -102,10 +104,21 @@ Ext.define('PVE.qemu.MachineInputPanel', { >> } >> =20 >> if (me.isWindows) { >> - if (values.machine =3D=3D=3D '__default__') { >> - values.version =3D 'pc-i440fx-5.1'; >> - } else if (values.machine =3D=3D=3D 'q35') { >> - values.version =3D 'pc-q35-5.1'; >> + if (values.machine =3D=3D=3D '__default__' || values.machin= e =3D=3D=3D 'q35') { >> + let effective =3D PVE.Utils.qemu_implicit_machine_versi= on( >> + values.machine, >> + values.creationQemu, >> + values.arch, >> + ); >> + me.getViewModel().set({ >> + effectiveVersionLabel: effective + ' ' + gettext('(= implicit)'), >> + implicitVersion: true, >> + }); >> + me.setAdvancedVisible(true); > > >> + // flush the binding, then re-baseline the reset value = to it >> + // otherwise the Reset button reverts to the field's em= pty initial value >> + me.getViewModel().notify(); >> + me.lookup('effectiveVersion').resetOriginalValue(); > > under which circumstances is this necessary? > > even with those two statements deleted, i can't trigger it to have > the reset button enabled? > > in general, we probably shouldn't mess with the 'resetOriginalValue' too > much, but if we do, having the steps where this is necessary > would be good (does not have to be a comment, in the commit message is > fine for me) Than for taking a look at the patch. I'm not quite sure what you mean with 'reset button enabled'. The reset button enables just like in other UI forms; in the Machine case that's=20 when you select a different value in the dropdowns (e.g. change=20 `Machine`, `Version` or `vIOMMU`). I'm talking about the=20 `Reset from Data` button on the top right. The `resetOriginalValue` is necessary because the=20 `effectiveVersionLabel` value is empty on page load, and only gets set when `setValues` is called. Since `setValues` is not called on a reset, this would display an empty current version on a reset. Therefore I=20 replace the field's empty initial value with the actual current version on page load, so a reset reverts to the correct value instead of an empty one. I thought adding the comment above the code, saying that on reset the value is empty made this clear -- if not, I can mention why exactly I used this in the commit message as well.