From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 33B9F1FF13F for ; Thu, 29 Jan 2026 12:12:33 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 209BA270F6; Thu, 29 Jan 2026 12:12:59 +0100 (CET) Message-ID: <60129254-d70c-48d5-8f3c-10d3e320da28@proxmox.com> Date: Thu, 29 Jan 2026 12:12:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [pve-devel] [PATCH manager 07/10] ui: qemu: make machine panels/fields architecture aware To: Proxmox VE development discussion , Dominik Csapak References: <20260128123035.2576774-1-d.csapak@proxmox.com> <20260128123035.2576774-8-d.csapak@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260128123035.2576774-8-d.csapak@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: 1769685070442 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.017 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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: QVU4UJRJZ4LO63G2LMDSU7LSJ3QQXVWO X-Message-ID-Hash: QVU4UJRJZ4LO63G2LMDSU7LSJ3QQXVWO 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 28.01.26 um 1:30 PM schrieb Dominik Csapak: > For this, introduce a new 'QemuMachineSelector', which does the same > thing as the scsihw selector by filtering the kv store with a predefined > list for each architecture. > > Since the backend default of the machine is actually different for > x86_64 vs aarch64, there is some logic to handle the 'default' value > for the machine (iow. replace 'pc' with the default machine for each > architecture) > Setting a machine version for an existing aarch64 VM is broken, e.g. the result will be "pc-i440fx-10.1" instead. > Signed-off-by: Dominik Csapak > --- > www/manager6/Makefile | 1 + > www/manager6/Utils.js | 18 ++++++- > www/manager6/form/QemuMachineSelector.js | 64 ++++++++++++++++++++++++ > www/manager6/qemu/HardwareView.js | 3 +- > www/manager6/qemu/MachineEdit.js | 63 +++++++++++++++++------ > www/manager6/qemu/SystemEdit.js | 10 ++-- > 6 files changed, 136 insertions(+), 23 deletions(-) > create mode 100644 www/manager6/form/QemuMachineSelector.js > > diff --git a/www/manager6/Makefile b/www/manager6/Makefile > index 4558d53e..7670b95d 100644 > --- a/www/manager6/Makefile > +++ b/www/manager6/Makefile > @@ -62,6 +62,7 @@ JSSRC= \ > form/PreallocationSelector.js \ > form/PrivilegesSelector.js \ > form/QemuBiosSelector.js \ > + form/QemuMachineSelector.js \ > form/SDNControllerSelector.js \ > form/SDNZoneSelector.js \ > form/SDNVnetSelector.js \ > diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js > index de1ee0ba..613a7113 100644 > --- a/www/manager6/Utils.js > +++ b/www/manager6/Utils.js > @@ -509,8 +509,22 @@ Ext.define('PVE.Utils', { > return agentstring; > }, > > - render_qemu_machine: function (value) { > - return value || Proxmox.Utils.defaultText + ' (i440fx)'; > + defaultMachines: { > + x86_64: 'pc', > + aarch64: 'virt', > + }, > + > + machineText: { > + pc: 'i440fx', > + }, These new entries should mention QEMU somewhere in their name, because it's in the very general/stuffed Utils class. > + > + render_qemu_machine: function (value, arch = 'x86_64') { > + if (!value) { > + let machine = PVE.Utils.defaultMachines[arch]; > + let machineText = PVE.Utils.machineText[machine] ?? machine; > + return `${Proxmox.Utils.defaultText} (${machineText})`; > + } > + return value; > }, > > render_qemu_bios: function (value) { > diff --git a/www/manager6/form/QemuMachineSelector.js b/www/manager6/form/QemuMachineSelector.js > new file mode 100644 > index 00000000..2852b585 > --- /dev/null > +++ b/www/manager6/form/QemuMachineSelector.js > @@ -0,0 +1,64 @@ > +Ext.define('PVE.form.QemuMachineSelector', { > + extend: 'Proxmox.form.KVComboBox', > + alias: 'widget.pveQemuMachineSelector', > + > + comboItems: [ > + ['__default__', PVE.Utils.render_qemu_machine('')], > + ['q35', 'q35'], > + ], > + > + machinesPerArchitecture: { > + x86_64: ['__default__', 'q35'], // __default__ is i440fx > + aarch64: ['__default__'], // __default__ is virt > + }, > + > + // defaults to hostArch > + arch: undefined, > + > + // depends on the node > + hostArch: 'x86_64', Sounds like it should start as undefined and we should require a nodename to be present at init time then and set hostArch accordingly? Otherwise, it seems a bit brittle and might be wrong if setNodename() is forgotten to be called by a use site. > + > + nodename: undefined, > + > + setNodename: function (nodename) { > + let me = this; > + let node = PVE.data.ResourceStore.getNodeById(nodename); > + if (node) { > + me.hostArch = node.data.architecture; > + me.setArch(me.arch); // recalculate the filter > + } > + }, > + > + setArch: function (arch) { > + let me = this; > + if (arch === '__default__') { > + arch = undefined; > + } > + me.arch = arch; > + arch ??= me.hostArch; > + let wasValid = me.isValid(); > + me.store.clearFilter(); > + let allowedMachines = me.machinesPerArchitecture[arch]; > + me.store.addFilter((rec) => allowedMachines.indexOf(rec.data.key) !== -1); > + // update default value with new arch > + let record = me.store.findRecord('key', '__default__'); > + if (record) { > + record.set('value', PVE.Utils.render_qemu_machine('', arch)); > + record.commit(); > + } > + let isValid = me.isValid(); > + > + // for some reason, adding/changing filters does not trigger this, even though > + // it show the field as invalid, so simply track and fire the event manually. > + if (wasValid !== isValid) { > + me.fireEvent('validitychange', isValid); > + } > + }, > + > + initComponent: function () { > + var me = this; > + > + me.callParent(); > + me.setArch(me.arch); > + }, > +}); > diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js > index b7cc7856..1e2cd026 100644 > --- a/www/manager6/qemu/HardwareView.js > +++ b/www/manager6/qemu/HardwareView.js > @@ -202,13 +202,14 @@ Ext.define('PVE.qemu.HardwareView', { > defaultValue: '', > renderer: function (value, metaData, record, rowIndex, colIndex, store, pending) { > let ostype = me.getObjectValue('ostype', undefined, pending); > + let arch = PVE.Utils.getArchitecture(me.getObjectValue('arch'), nodename); > if ( > PVE.Utils.is_windows(ostype) && > (!value || value === 'pc' || value === 'q35') > ) { > return value === 'q35' ? 'pc-q35-5.1' : 'pc-i440fx-5.1'; Unrelated to the patch, but I noticed now that this is actually wrong. Since QEMU 9.1+ the backend defaults to the creation version if present. But related to the series, it should default to virt for ARM of course, which it doesn't yet, but I sent a patch: https://lore.proxmox.com/pve-devel/20260129110607.65416-1-f.ebner@proxmox.com/T/ > } > - return PVE.Utils.render_qemu_machine(value); > + return PVE.Utils.render_qemu_machine(value, arch); > }, > }, > scsihw: { > diff --git a/www/manager6/qemu/MachineEdit.js b/www/manager6/qemu/MachineEdit.js > index 1b1989e8..89d13886 100644 > --- a/www/manager6/qemu/MachineEdit.js > +++ b/www/manager6/qemu/MachineEdit.js > @@ -23,8 +23,16 @@ Ext.define('PVE.qemu.MachineInputPanel', { > let me = this; > let version = me.lookup('version'); > let store = version.getStore(); > + > let oldRec = store.findRecord('id', version.getValue(), 0, false, false, true); > - let type = value === 'q35' ? 'q35' : 'i440fx'; > + > + let vm = me.getViewModel(); > + let arch = PVE.Utils.getArchitecture(vm.get('arch'), vm.get('nodename')); > + let defaultMachine = PVE.Utils.defaultMachines[arch]; > + if (defaultMachine === 'pc') { > + defaultMachine = 'i440fx'; // the default in the backend is 'pc' which means 'i440fx' for the qemu machinetype' Style nit: please move the comment to its own line. Comment has a trailing excessive single quote > + } > + let type = value === 'q35' ? 'q35' : defaultMachine; > store.clearFilter(); > store.addFilter((val) => val.data.id === 'latest' || val.data.type === type); > if (!me.getView().isWindows) { > @@ -45,9 +53,12 @@ Ext.define('PVE.qemu.MachineInputPanel', { > }, > > onGetValues: function (values) { > + let me = this; > + let arch = PVE.Utils.getArchitecture(values.arch, me.nodename); > + delete values.arch; > if (values.delete === 'machine' && values.viommu) { > delete values.delete; > - values.machine = 'pc'; > + values.machine = PVE.Utils.defaultMachines[arch]; > } > if (values.version && values.version !== 'latest') { > values.machine = values.version; > @@ -68,8 +79,10 @@ Ext.define('PVE.qemu.MachineInputPanel', { > let machineConf = PVE.Parser.parsePropertyString(values.machine, 'type'); > values.machine = machineConf.type; > > + let arch = PVE.Utils.getArchitecture(values.arch, me.nodename); > + let defaultMachine = PVE.Utils.defaultMachines[arch]; > me.isWindows = values.isWindows; > - if (values.machine === 'pc') { > + if (values.machine === defaultMachine) { > values.machine = '__default__'; > } > Similar to above, unrelated but the pinning does not match the backend, but will also need to be adapted once the default for virt is fixed in the backend. if (me.isWindows) { if (values.machine === '__default__') { values.version = 'pc-i440fx-5.1'; } else if (values.machine === 'q35') { values.version = 'pc-q35-5.1'; } }