public inbox for pve-devel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal