all lists on lists.proxmox.com
 help / color / mirror / Atom feed
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);
>               },
>           });





      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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal