From: Dominik Csapak <d.csapak@proxmox.com>
To: Jakob Klocker <j.klocker@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [PATCH manager] fix #7896: ui: qemu: show effective machine version for Windows guests
Date: Fri, 21 Aug 2026 15:47:27 +0200 [thread overview]
Message-ID: <ff84536a-29ed-4540-b7ab-2365d3214c75@proxmox.com> (raw)
In-Reply-To: <20260819092833.114930-1-j.klocker@proxmox.com>
code looks good, but two smaller issues with the UX:
* when editing a VM in that state and just changing the
viommu, the value will be 'pc,viommu=virtio' or 'q35,v...'
this is then displayed in the hardware grid verbatim,
instead of the implicit value. see [0]
* when editing and then resetting the edit window,
the current version field + the hint vanishes.
not sure what the reason here is, but this doesn't feel very
nice
On 8/19/26 11:28 AM, Jakob Klocker wrote:
> Windows guests without an explicitly pinned machine version still run
> with a fixed one: guests created with QEMU 9.1 or newer keep the
> version they were created with, older ones fall back to 5.1.
>
> The GUI assumed the 5.1 fallback unconditionally, so guests created
> with a newer QEMU were shown a version they are not running. Derive
> the version from the QEMU version recorded at creation time instead,
> in both the hardware view and the machine edit dialog, and mark it as
> implicit so it stays distinguishable from an explicit pin.
>
> The edit dialog also pre-filled the version field with the fallback,
> which both suggested a version the guest might not be running and
> caused any other change in the dialog to submit that stale value.
> Leave the selector at 'latest' instead, so a version can be picked
> deliberately, and show the effective version as a read-only field
> together with a hint recommending an explicit pin. The advanced
> section is expanded so the hint is visible without further
> interaction.
>
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7896
> Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
> ---
> www/manager6/Utils.js | 21 +++++++++++++++++
> www/manager6/qemu/HardwareView.js | 19 +++++++++++++++-
> www/manager6/qemu/MachineEdit.js | 38 +++++++++++++++++++++++++++----
> 3 files changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index c86a00c5..4d6606af 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1822,6 +1822,27 @@ Ext.define('PVE.Utils', {
> return true;
> },
>
> + qemu_implicit_machine_version: function (machineType, creationQemu, arch) {
> + let baseVersion = '5.1';
> + let m = creationQemu?.match(/^(\d+)\.(\d+)/);
> + if (m) {
> + let major = parseInt(m[1], 10);
> + let minor = parseInt(m[2], 10);
> + if (major > 9 || (major === 9 && minor >= 1)) {
> + baseVersion = `${major}.${minor}`;
> + }
> + }
> +
> + let base;
> + if (machineType === 'q35') {
> + base = 'pc-q35';
> + } else {
> + let defaultMachine = PVE.qemu.Architecture.defaultMachines[arch];
> + base = defaultMachine === 'virt' ? 'virt' : 'pc-i440fx';
> + }
> + return `${base}-${baseVersion}`;
> + },
> +
> cleanEmptyObjectKeys: function (obj) {
> for (const propName of Object.keys(obj)) {
> if (obj[propName] === null || obj[propName] === undefined) {
> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
> index e6c02299..eeb86397 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -201,7 +201,21 @@ Ext.define('PVE.qemu.HardwareView', {
> PVE.Utils.is_windows(ostype) &&
> (!value || value === 'pc' || value === 'q35')
> ) {
[0]: here we check the value for === 'pc', but if this is 'pc,viommu...'
we don't get the implicit value
i think it would be easy to parse the machine property string before
this check and compare only the version
> - return value === 'q35' ? 'pc-q35-5.1' : 'pc-i440fx-5.1';
> + let meta = me.getObjectValue('meta', undefined, pending);
> + let creationQemu;
> + if (meta) {
> + creationQemu = PVE.Parser.parsePropertyString(meta)['creation-qemu'];
> + }
> + let machineType = value === 'q35' ? 'q35' : '__default__';
> + return (
> + PVE.Utils.qemu_implicit_machine_version(
> + machineType,
> + creationQemu,
> + arch,
> + ) +
> + ' ' +
> + gettext('(implicit)')
> + );
> }
> return PVE.Utils.render_qemu_machine(value, arch);
> },
> @@ -254,6 +268,9 @@ Ext.define('PVE.qemu.HardwareView', {
> ostype: {
> visible: false,
> },
> + meta: {
> + visible: false,
> + },
> affinity: {
> visible: false,
> },
> diff --git a/www/manager6/qemu/MachineEdit.js b/www/manager6/qemu/MachineEdit.js
> index 4b1a9e83..6fda7715 100644
> --- a/www/manager6/qemu/MachineEdit.js
> +++ b/www/manager6/qemu/MachineEdit.js
> @@ -6,6 +6,7 @@ Ext.define('PVE.qemu.MachineInputPanel', {
> viewModel: {
> data: {
> type: '__default__',
> + effectiveVersionLabel: '',
> },
> formulas: {
> q35: (get) => get('type') === 'q35',
> @@ -102,10 +103,17 @@ Ext.define('PVE.qemu.MachineInputPanel', {
> }
>
> if (me.isWindows) {
> - if (values.machine === '__default__') {
> - values.version = 'pc-i440fx-5.1';
> - } else if (values.machine === 'q35') {
> - values.version = 'pc-q35-5.1';
> + if (values.machine === '__default__' || values.machine === 'q35') {
> + let effective = PVE.Utils.qemu_implicit_machine_version(
> + values.machine,
> + values.creationQemu,
> + values.arch,
> + );
> + me.getViewModel().set(
> + 'effectiveVersionLabel',
> + effective + ' ' + gettext('(implicit)'),
> + );
> + me.setAdvancedVisible(true);
> }
> }
>
> @@ -176,6 +184,23 @@ Ext.define('PVE.qemu.MachineInputPanel', {
> },
> },
> },
> + {
> + xtype: 'displayfield',
> + fieldLabel: gettext('Current Version'),
> + bind: {
> + value: '{effectiveVersionLabel}',
> + hidden: '{!effectiveVersionLabel}',
> + },
> + },
> + {
> + xtype: 'displayfield',
> + userCls: 'pmx-hint',
> + value: gettext(
> + 'No fixed version is set; the version is chosen automatically based on when the VM' +
> + ' was created. Pinning a specific version is recommended.',
> + ),
> + bind: { hidden: '{!effectiveVersionLabel}' },
> + },
> {
> xtype: 'displayfield',
> fieldLabel: gettext('Note'),
> @@ -249,6 +274,11 @@ Ext.define('PVE.qemu.MachineEdit', {
> };
> values.isWindows = PVE.Utils.is_windows(conf.ostype);
> values.arch = PVE.qemu.Architecture.getGuestArchitecture(conf.arch, me.nodename);
> + if (conf.meta) {
> + let meta = PVE.Parser.parsePropertyString(conf.meta);
> + values.creationQemu = meta['creation-qemu'];
> + }
> +
> me.setValues(values);
> },
> });
prev parent reply other threads:[~2026-08-21 13:47 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-19 9:28 [PATCH manager] fix #7896: ui: qemu: show effective machine version for Windows guests Jakob Klocker
2026-08-21 13:47 ` Dominik Csapak [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ff84536a-29ed-4540-b7ab-2365d3214c75@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=j.klocker@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox