On 1/29/26 12:11 PM, Fiona Ebner wrote:
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.
yep noticed it too now, the reason is that the arch and nodename are not
set via 'binds' yet when the 'onMachineChange' is triggering
(at least not everytime, maybe a race condition? i swear it
worked at least once on my machine ;) )
I'll have to change how we get/save the arch/nodename there.
When that works, it'll not show any versions currently because
the backend does not return any 'virt' models.
i think we should fix that in the backend. I'd also add an 'arch'
parameter for the machines like you sent for the cputype
Signed-off-by: Dominik Csapak <[email protected]>
---
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.
make sense
+
+ 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.
as i wrote in the other mail, i think i'll change this completely
namely don't let each component calculate this seperately
+
+ 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.
i think this was just done as a heuristic without trying to replicate
the backend logic too tightly, but i might be misremembering
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/[email protected]/T/
i'll check it out
}
- 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';
}
}