public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models
@ 2026-03-12  8:40 Arthur Bied-Charreton
  2026-03-12  8:40 ` [PATCH pve-manager 1/8] ui: VMCPUFlagSelector: Fix unknownFlags behaviour Arthur Bied-Charreton
                   ` (8 more replies)
  0 siblings, 9 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-12  8:40 UTC (permalink / raw)
  To: pve-devel

This is picked up from an old series [0] by Stefan Reiter.

As of before this series, the only way to create custom CPU models is by
editing `/etc/pve/virtual-guest/cpu-models.conf` manually.

This can be cumbersome for a few reasons, e.g., due to the fact that flags
misconfigurations are only caught when starting the VM.

`cpu-flags` endpoint:
The `cpu-flags` endpoint previously returned a list of hardcoded flags,
which is both non-exhaustive (some flags I should be able to set are missing),
and partly incorrect (some flags my host(s) do not support set are returned).
This is limiting and can lead to misconfigurations. The updated endpoint
intersects all flags QEMU accepts as `-cpu` arguments with all flags the host
hardware/emulation actually supports. This way, if I am able to set a flag in
the UI, I can be confident that the VM will actually be able to start.

Custom CPU model CRUD functionality:
Expose CRUD endpoints and UI flow to interact with `cpu-models.conf`. For each
flag, show a list of the cluster nodes supporting it, and only expose flags that
at least one node supports to avoid misconfigurations. Filter flags by
acceleration type (KVM/TCG).

[0] https://lore.proxmox.com/pve-devel/20211028114150.3245864-1-s.reiter@proxmox.com/


pve-manager:

Arthur Bied-Charreton (5):
  ui: VMCPUFlagSelector: Fix unknownFlags behaviour
  ui: CPUModelSelector: Fix dirty state on default
  ui: CPUModelSelector: Allow filtering out custom models
  ui: Add basic custom CPU model editor
  ui: Add CPU flag editor for custom models

 www/manager6/Makefile                  |   3 +
 www/manager6/dc/CPUTypeEdit.js         |  99 ++++++++++++++++
 www/manager6/dc/CPUTypeView.js         | 139 ++++++++++++++++++++++
 www/manager6/dc/Config.js              |   6 +
 www/manager6/form/CPUModelSelector.js  |  18 ++-
 www/manager6/form/PhysBitsSelector.js  | 153 +++++++++++++++++++++++++
 www/manager6/form/VMCPUFlagSelector.js | 135 ++++++++++++++++++----
 7 files changed, 529 insertions(+), 24 deletions(-)
 create mode 100644 www/manager6/dc/CPUTypeEdit.js
 create mode 100644 www/manager6/dc/CPUTypeView.js
 create mode 100644 www/manager6/form/PhysBitsSelector.js


qemu-server:

Arthur Bied-Charreton (3):
  qemu: Add helpers for new custom models endpoints
  api: qemu: Extend cpu-flags endpoint to return actually supported
    flags
  api: qemu: Add CRUD handlers for custom CPU models

 src/PVE/API2/Qemu/CPU.pm        | 236 +++++++++++++++++++++++++++++++-
 src/PVE/API2/Qemu/CPUFlags.pm   | 108 ++++++++++++++-
 src/PVE/QemuServer/CPUConfig.pm |  35 ++++-
 3 files changed, 374 insertions(+), 5 deletions(-)


Summary over all repositories:
  10 files changed, 903 insertions(+), 29 deletions(-)

-- 
Generated by murpp 0.9.0



^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH pve-manager 1/8] ui: VMCPUFlagSelector: Fix unknownFlags behaviour
  2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
@ 2026-03-12  8:40 ` Arthur Bied-Charreton
  2026-03-25 15:57   ` Fiona Ebner
  2026-03-12  8:40 ` [PATCH pve-manager 2/8] ui: CPUModelSelector: Fix dirty state on default Arthur Bied-Charreton
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-12  8:40 UTC (permalink / raw)
  To: pve-devel; +Cc: Stefan Reiter

...and typo in name.

Original patch:
https://lore.proxmox.com/pve-devel/20211028114150.3245864-6-s.reiter@proxmox.com/

Originally-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 www/manager6/form/VMCPUFlagSelector.js | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
index 922fcfa6..74b1a2c4 100644
--- a/www/manager6/form/VMCPUFlagSelector.js
+++ b/www/manager6/form/VMCPUFlagSelector.js
@@ -14,7 +14,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     scrollable: 'y',
     height: 200,
 
-    unkownFlags: [],
+    unknownFlags: [],
 
     store: {
         type: 'store',
@@ -62,32 +62,32 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
             }
         });
 
-        flags += me.unkownFlags.join(';');
+        flags += (me.unknownFlags.length > 0 ? ';' : '') + me.unknownFlags.join(';');
 
         return flags;
     },
 
-    // Adjusts the store for the current value and determines the unkown flags based on what the
+    // Adjusts the store for the current value and determines the unknown flags based on what the
     // store does not know.
     adjustStoreForValue: function () {
         let me = this;
         let store = me.getStore();
         let value = me.value;
 
-        me.unkownFlags = [];
+        me.unknownFlags = [];
 
         store.getData().each((rec) => rec.set('state', '='));
 
         let flags = value ? value.split(';') : [];
         flags.forEach(function (flag) {
             let sign = flag.substr(0, 1);
-            flag = flag.substr(1);
+            let flagName = flag.substr(1);
 
-            let rec = store.findRecord('name', flag, 0, false, true, true);
+            let rec = store.findRecord('name', flagName, 0, false, true, true);
             if (rec !== null) {
                 rec.set('state', sign);
             } else {
-                me.unkownFlags.push(flag);
+                me.unknownFlags.push(flag);
             }
         });
 
-- 
2.47.3




^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH pve-manager 2/8] ui: CPUModelSelector: Fix dirty state on default
  2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
  2026-03-12  8:40 ` [PATCH pve-manager 1/8] ui: VMCPUFlagSelector: Fix unknownFlags behaviour Arthur Bied-Charreton
@ 2026-03-12  8:40 ` Arthur Bied-Charreton
  2026-03-26  9:53   ` Fiona Ebner
  2026-03-12  8:40 ` [PATCH pve-manager 3/8] ui: CPUModelSelector: Allow filtering out custom models Arthur Bied-Charreton
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-12  8:40 UTC (permalink / raw)
  To: pve-devel; +Cc: Stefan Reiter

'originalValue' is set to null in case it's the default, but getValue
returns an empty string. This means that when editing a VM's CPU config
when the model is 'default', the form would always be marked dirty.

Original patch:
https://lore.proxmox.com/pve-devel/20211028114150.3245864-7-s.reiter@proxmox.com/

Originally-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 www/manager6/form/CPUModelSelector.js | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js
index 2ebd08eb..737b6ff4 100644
--- a/www/manager6/form/CPUModelSelector.js
+++ b/www/manager6/form/CPUModelSelector.js
@@ -39,7 +39,11 @@ Ext.define('PVE.form.CPUModelSelector', {
         ],
         width: 360,
     },
-
+    getValue: function () {
+        let me = this;
+        let val = me.callParent();
+        return val === '' ? null : val;
+    },
     store: {
         autoLoad: true,
         model: 'PVE.data.CPUModel',
-- 
2.47.3




^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH pve-manager 3/8] ui: CPUModelSelector: Allow filtering out custom models
  2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
  2026-03-12  8:40 ` [PATCH pve-manager 1/8] ui: VMCPUFlagSelector: Fix unknownFlags behaviour Arthur Bied-Charreton
  2026-03-12  8:40 ` [PATCH pve-manager 2/8] ui: CPUModelSelector: Fix dirty state on default Arthur Bied-Charreton
@ 2026-03-12  8:40 ` Arthur Bied-Charreton
  2026-03-26  9:59   ` Fiona Ebner
  2026-03-12  8:40 ` [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor Arthur Bied-Charreton
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-12  8:40 UTC (permalink / raw)
  To: pve-devel; +Cc: Stefan Reiter

Add 'allowCustom' configuration parameter. When set to false, only
default CPU models will be shown.

Original patch:
https://lore.proxmox.com/pve-devel/20211028114150.3245864-8-s.reiter@proxmox.com/

Originally-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 www/manager6/form/CPUModelSelector.js | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js
index 737b6ff4..2154ff46 100644
--- a/www/manager6/form/CPUModelSelector.js
+++ b/www/manager6/form/CPUModelSelector.js
@@ -19,7 +19,9 @@ Ext.define('PVE.form.CPUModelSelector', {
     autoSelect: false,
 
     deleteEmpty: true,
-
+    config: {
+        allowCustom: true,
+    },
     listConfig: {
         columns: [
             {
@@ -100,4 +102,11 @@ Ext.define('PVE.form.CPUModelSelector', {
             },
         },
     },
+    initComponent: function () {
+        let me = this;
+        me.callParent();
+        if (!me.allowCustom) {
+            me.getStore().addFilter({ filterFn: (rec) => !rec.data.custom });
+        }
+    },
 });
-- 
2.47.3




^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor
  2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
                   ` (2 preceding siblings ...)
  2026-03-12  8:40 ` [PATCH pve-manager 3/8] ui: CPUModelSelector: Allow filtering out custom models Arthur Bied-Charreton
@ 2026-03-12  8:40 ` Arthur Bied-Charreton
  2026-03-26 15:10   ` Fiona Ebner
  2026-03-12  8:40 ` [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models Arthur Bied-Charreton
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-12  8:40 UTC (permalink / raw)
  To: pve-devel; +Cc: Stefan Reiter

Add components for creating/updating custom CPU models, allowing to set
the values that would normally have to be set manually in
`/etc/pve/virtual-guest/cpu-models.conf` [0].

Original patch:
https://lore.proxmox.com/pve-devel/20211028114150.3245864-9-s.reiter@proxmox.com/

[0] https://pve.proxmox.com/wiki/Manual:_cpu-models.conf

Originally-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 www/manager6/Makefile                 |   3 +
 www/manager6/dc/CPUTypeEdit.js        |  90 +++++++++++++++
 www/manager6/dc/CPUTypeView.js        | 139 +++++++++++++++++++++++
 www/manager6/dc/Config.js             |   6 +
 www/manager6/form/PhysBitsSelector.js | 153 ++++++++++++++++++++++++++
 5 files changed, 391 insertions(+)
 create mode 100644 www/manager6/dc/CPUTypeEdit.js
 create mode 100644 www/manager6/dc/CPUTypeView.js
 create mode 100644 www/manager6/form/PhysBitsSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 4558d53e..fbde2327 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -58,6 +58,7 @@ JSSRC= 							\
 	form/PCISelector.js				\
 	form/PCIMapSelector.js				\
 	form/PermPathSelector.js			\
+	form/PhysBitsSelector.js			\
 	form/PoolSelector.js				\
 	form/PreallocationSelector.js			\
 	form/PrivilegesSelector.js			\
@@ -170,6 +171,8 @@ JSSRC= 							\
 	dc/CmdMenu.js					\
 	dc/Config.js					\
 	dc/CorosyncLinkEdit.js				\
+	dc/CPUTypeEdit.js				\
+	dc/CPUTypeView.js				\
 	dc/GroupEdit.js					\
 	dc/GroupView.js					\
 	dc/Guests.js					\
diff --git a/www/manager6/dc/CPUTypeEdit.js b/www/manager6/dc/CPUTypeEdit.js
new file mode 100644
index 00000000..8cf508b4
--- /dev/null
+++ b/www/manager6/dc/CPUTypeEdit.js
@@ -0,0 +1,90 @@
+Ext.define('PVE.dc.CPUTypeEdit', {
+    extend: 'Proxmox.window.Edit',
+    alias: ['widget.pveCpuTypeEdit'],
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    subject: gettext('CPU Type'),
+
+    // Avoid default-focusing the reported model dropdown while still
+    // focusing the name textfield if it is editable
+    defaultFocus: 'textfield',
+
+    height: 600,
+    width: 800,
+
+    cbindData: {
+        cputype: '',
+        isCreate: (cfg) => !cfg.cputype,
+    },
+
+    cbind: {
+        autoLoad: (get) => !get('isCreate'),
+        url: (get) => `/api2/extjs/nodes/localhost/capabilities/qemu/cpu/model/${get('cputype')}`,
+        method: (get) => (get('isCreate') ? 'POST' : 'PUT'),
+        isCreate: (get) => get('isCreate'),
+    },
+
+    getValues: function () {
+        let me = this;
+        let values = me.callParent();
+
+        PVE.Utils.delete_if_default(values, 'reported-model', '', me.isCreate);
+        PVE.Utils.delete_if_default(values, 'hv-vendor-id', '', me.isCreate);
+        PVE.Utils.delete_if_default(values, 'phys-bits', '', me.isCreate);
+        PVE.Utils.delete_if_default(values, 'flags', '', me.isCreate);
+
+        if (me.isCreate && !values.cputype.match(/^custom-/)) {
+            values.cputype = 'custom-' + values.cputype;
+        }
+
+        return values;
+    },
+
+    items: [
+        {
+            xtype: 'inputpanel',
+            column1: [
+                {
+                    xtype: 'pmxDisplayEditField',
+                    fieldLabel: gettext('Name'),
+                    cbind: {
+                        editable: '{isCreate}',
+                        value: '{cputype}',
+                    },
+                    name: 'cputype',
+                    renderer: (val) => val.replace(/^custom-/, ''),
+                    allowBlank: false,
+                },
+                {
+                    xtype: 'CPUModelSelector',
+                    fieldLabel: gettext('Reported Model'),
+                    allowCustom: false,
+                    name: 'reported-model',
+                },
+                {
+                    xtype: 'textfield',
+                    fieldLabel: gettext('Hyper-V Vendor'),
+                    name: 'hv-vendor-id',
+                    allowBlank: true,
+                    emptyText: gettext('None'),
+                    maxLength: 12,
+                },
+            ],
+            column2: [
+                {
+                    xtype: 'checkbox',
+                    fieldLabel: gettext('Hidden'),
+                    name: 'hidden',
+                    inputValue: 1,
+                    uncheckedValue: 0,
+                },
+                {
+                    xtype: 'PhysBitsSelector',
+                    fieldLabel: gettext('Phys-Bits'),
+                    name: 'phys-bits',
+                },
+            ],
+
+        },
+    ],
+});
diff --git a/www/manager6/dc/CPUTypeView.js b/www/manager6/dc/CPUTypeView.js
new file mode 100644
index 00000000..c79ce690
--- /dev/null
+++ b/www/manager6/dc/CPUTypeView.js
@@ -0,0 +1,139 @@
+Ext.define('pve-custom-cpu-type', {
+    extend: 'Ext.data.Model',
+    fields: [
+        'cputype',
+        'reported-model',
+        'hv-vendor-id',
+        'flags',
+        'phys-bits',
+        { name: 'hidden', type: 'boolean' },
+    ],
+});
+
+Ext.define('PVE.dc.CPUTypeView', {
+    extend: 'Ext.grid.GridPanel',
+    alias: ['widget.pveCPUTypeView'],
+
+    onlineHelp: 'qm_cpu',
+
+    store: {
+        model: 'pve-custom-cpu-type',
+        proxy: {
+            type: 'proxmox',
+            url: '/api2/json/nodes/localhost/capabilities/qemu/cpu/model',
+        },
+        autoLoad: true,
+        sorters: ['cputype'],
+    },
+
+    controller: {
+        xclass: 'Ext.app.ViewController',
+
+        getSelection: function () {
+            let me = this;
+            let grid = me.getView();
+            let selection = grid.getSelection();
+            if (selection.length === 1) {
+                return selection[0].data;
+            }
+            return null;
+        },
+
+        showEditor: function (cputype) {
+            let me = this;
+            let param = cputype ? { cputype } : {};
+            let win = Ext.create('PVE.dc.CPUTypeEdit', param);
+            win.on('destroy', () => me.reload());
+            win.show();
+        },
+
+        onAdd: function () {
+            let me = this;
+            me.showEditor();
+        },
+
+        onEdit: function () {
+            let me = this;
+            let selection = me.getSelection();
+            me.showEditor(selection.cputype);
+        },
+
+        reload: function () {
+            let me = this;
+            me.getView().getStore().reload();
+        },
+    },
+
+    columns: [
+        {
+            header: gettext('Name'),
+            flex: 1,
+            dataIndex: 'cputype',
+            renderer: (val) => val.replace(/^custom-/, ''),
+        },
+        {
+            header: gettext('Reported Model'),
+            flex: 1,
+            dataIndex: 'reported-model',
+        },
+        {
+            header: gettext('Phys-Bits'),
+            flex: 1,
+            dataIndex: 'phys-bits',
+        },
+        {
+            header: gettext('Hidden'),
+            flex: 1,
+            dataIndex: 'hidden',
+        },
+        {
+            header: gettext('HyperV-Vendor'),
+            flex: 1,
+            dataIndex: 'hv-vendor-id',
+        },
+        {
+            header: gettext('Flags'),
+            flex: 2,
+            dataIndex: 'flags',
+        },
+    ],
+
+    tbar: [
+        {
+            text: gettext('Add'),
+            handler: 'onAdd',
+        },
+        '-',
+        {
+            xtype: 'proxmoxStdRemoveButton',
+            baseurl: '/api2/extjs/nodes/localhost/capabilities/qemu/cpu/model/',
+            getRecordName: (rec) => rec.data.cputype,
+            getUrl: function (rec) {
+                let me = this;
+                return me.baseurl + rec.data.cputype;
+            },
+            callback: 'reload',
+        },
+        {
+            text: gettext('Edit'),
+            handler: 'onEdit',
+        },
+    ],
+
+    selModel: {
+        xtype: 'rowmodel',
+    },
+
+    listeners: {
+        itemdblclick: function (_, rec) {
+            let me = this;
+            me.getController().showEditor(rec.data.cputype);
+        },
+    },
+
+    initComponent: function () {
+        let me = this;
+        me.callParent();
+        Proxmox.Utils.monStoreErrors(me, me.store);
+    },
+});
diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index b5e27a21..629e4fc8 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -146,6 +146,12 @@ Ext.define('PVE.dc.Config', {
                     title: gettext('Replication'),
                     itemId: 'replication',
                 },
+                {
+                    xtype: 'pveCPUTypeView',
+                    iconCls: 'fa fa-microchip',
+                    title: gettext('Custom CPU models'),
+                    itemId: 'cputypes',
+                },
                 {
                     xtype: 'pveACLView',
                     title: gettext('Permissions'),
diff --git a/www/manager6/form/PhysBitsSelector.js b/www/manager6/form/PhysBitsSelector.js
new file mode 100644
index 00000000..78f932a6
--- /dev/null
+++ b/www/manager6/form/PhysBitsSelector.js
@@ -0,0 +1,153 @@
+Ext.define('PVE.form.PhysBitsSelector', {
+    extend: 'Ext.form.FieldContainer',
+    alias: 'widget.PhysBitsSelector',
+    mixins: ['Ext.form.field.Field'],
+
+    layout: 'vbox',
+    originalValue: '',
+
+    controller: {
+        xclass: 'Ext.app.ViewController',
+
+        onRadioChange: function (radio, value) {
+            let me = this;
+            if (value === undefined) {
+                return;
+            }
+
+            ['modeDefault', 'modeHost', 'modeCustom'].forEach(function (ref) {
+                let r = me.lookupReference(ref);
+                if (r !== radio) {
+                    r.suspendEvents();
+                    r.setValue(false);
+                    r.resumeEvents();
+                }
+            });
+
+            me.updateNumberField();
+        },
+
+        updateNumberField: function () {
+            let me = this;
+            let modeCustom = me.lookupReference('modeCustom');
+            let customNum = me.lookupReference('customNum');
+
+            customNum.setDisabled(!modeCustom.getValue());
+            me.getView().validate();
+        },
+
+        listen: {
+            component: {
+                '*': {
+                    change: function () {
+                        let me = this;
+                        me.getView().checkChange();
+                    },
+                },
+            },
+        },
+    },
+
+    getValue: function () {
+        let me = this;
+        let ctrl = me.getController();
+        if (ctrl.lookupReference('modeDefault').getValue()) {
+            return '';
+        } else if (ctrl.lookupReference('modeHost').getValue()) {
+            return 'host';
+        } else if (ctrl.lookupReference('modeCustom').getValue()) {
+            return ctrl.lookupReference('customNum').getValue();
+        }
+        return ''; // shouldn't happen
+    },
+
+    setValue: function (value) {
+        let me = this;
+        let ctrl = me.getController();
+        let modeField;
+
+        if (!value) {
+            modeField = ctrl.lookupReference('modeDefault');
+        } else if (value === 'host') {
+            modeField = ctrl.lookupReference('modeHost');
+        } else {
+            let customNum = ctrl.lookupReference('customNum');
+            customNum.setValue(value);
+            modeField = ctrl.lookupReference('modeCustom');
+        }
+
+        modeField.setValue(true);
+        me.checkChange();
+
+        return value;
+    },
+
+    getErrors: function () {
+        let me = this;
+        let ctrl = me.getController();
+        if (ctrl.lookupReference('modeCustom').getValue()) {
+            return ctrl.lookupReference('customNum').getErrors();
+        }
+        return [];
+    },
+
+    isValid: function () {
+        let me = this;
+        let ctrl = me.getController();
+        if (ctrl.lookupReference('modeCustom').getValue()) {
+            return ctrl.lookupReference('customNum').isValid();
+        }
+        return true;
+    },
+
+    items: [
+        {
+            xtype: 'radiofield',
+            boxLabel: gettext('Default'),
+            inputValue: 'default',
+            checked: true,
+            reference: 'modeDefault',
+            listeners: {
+                change: 'onRadioChange',
+            },
+            isFormField: false,
+        },
+        {
+            xtype: 'radiofield',
+            boxLabel: gettext('Host'),
+            inputValue: 'host',
+            reference: 'modeHost',
+            listeners: {
+                change: 'onRadioChange',
+            },
+            isFormField: false,
+        },
+        {
+            xtype: 'fieldcontainer',
+            layout: 'hbox',
+            items: [
+                {
+                    xtype: 'radiofield',
+                    boxLabel: gettext('Custom'),
+                    inputValue: 'custom',
+                    listeners: {
+                        change: 'onRadioChange',
+                    },
+                    reference: 'modeCustom',
+                    isFormField: false,
+                },
+                {
+                    xtype: 'numberfield',
+                    width: 60,
+                    margin: '0 0 0 10px',
+                    minValue: 8,
+                    maxValue: 64,
+                    reference: 'customNum',
+                    allowBlank: false,
+                    isFormField: false,
+                    disabled: true,
+                },
+            ],
+        },
+    ],
+});
-- 
2.47.3




^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models
  2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
                   ` (3 preceding siblings ...)
  2026-03-12  8:40 ` [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor Arthur Bied-Charreton
@ 2026-03-12  8:40 ` Arthur Bied-Charreton
  2026-03-26 15:22   ` Fiona Ebner
  2026-03-26 15:40   ` Maximiliano Sandoval
  2026-03-12  8:40 ` [PATCH qemu-server 6/8] qemu: Add helpers for new custom models endpoints Arthur Bied-Charreton
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-12  8:40 UTC (permalink / raw)
  To: pve-devel; +Cc: Stefan Reiter

Add CPU flag editor to the CPUTypeEdit component, using the VMCPUFlagSelector
also used in the VM creation flow. By default, only show the CPU flags that
are currently meant to be shown in the VM creation window, see [0]. When in
CPUTypeEdit, show all available flags.

For each flag in VMCPUFlagSelector, also display which node(s) it is available
on to limit misconfigurations.

Original patch:
https://lore.proxmox.com/pve-devel/20211028114150.3245864-10-s.reiter@proxmox.com

[0] https://git.proxmox.com/?p=qemu-server.git;a=blob;f=src/PVE/QemuServer/CPUConfig.pm;h=32ec495422791422f20caa928d6b632b3de4fcd9;hb=refs/heads/master#l349

Originally-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 www/manager6/dc/CPUTypeEdit.js         |  11 ++-
 www/manager6/form/CPUModelSelector.js  |   1 +
 www/manager6/form/VMCPUFlagSelector.js | 121 ++++++++++++++++++++++---
 3 files changed, 117 insertions(+), 16 deletions(-)

diff --git a/www/manager6/dc/CPUTypeEdit.js b/www/manager6/dc/CPUTypeEdit.js
index 8cf508b4..d6e06601 100644
--- a/www/manager6/dc/CPUTypeEdit.js
+++ b/www/manager6/dc/CPUTypeEdit.js
@@ -84,7 +84,16 @@ Ext.define('PVE.dc.CPUTypeEdit', {
                     name: 'phys-bits',
                 },
             ],
-
+            columnB: [
+                {
+                    xtype: 'vmcpuflagselector',
+                    fieldLabel: gettext('Extra CPU flags'),
+                    name: 'flags',
+                    restrictToVMFlags: false,
+                    height: 380,
+                    hideHeaders: false,
+                },
+            ],
         },
     ],
 });
diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js
index 2154ff46..683fa469 100644
--- a/www/manager6/form/CPUModelSelector.js
+++ b/www/manager6/form/CPUModelSelector.js
@@ -17,6 +17,7 @@ Ext.define('PVE.form.CPUModelSelector', {
     anyMatch: true,
     forceSelection: true,
     autoSelect: false,
+    triggerAction: 'query',
 
     deleteEmpty: true,
     config: {
diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
index 74b1a2c4..06c9d9f1 100644
--- a/www/manager6/form/VMCPUFlagSelector.js
+++ b/www/manager6/form/VMCPUFlagSelector.js
@@ -1,3 +1,19 @@
+const VM_CPU_FLAGS_SUBSET = {
+    aes: true,
+    'amd-no-ssb': true,
+    'amd-ssbd': true,
+    'hv-evmcs': true,
+    'hv-tlbflush': true,
+    ibpb: true,
+    'md-clear': true,
+    'nested-virt': true,
+    pcid: true,
+    pdpe1gb: true,
+    'spec-ctrl': true,
+    ssbd: true,
+    'virt-ssbd': true,
+};
+
 Ext.define('PVE.form.VMCPUFlagSelector', {
     extend: 'Ext.grid.Panel',
     alias: 'widget.vmcpuflagselector',
@@ -6,6 +22,10 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
         field: 'Ext.form.field.Field',
     },
 
+    config: {
+        restrictToVMFlags: true,
+    },
+
     disableSelection: true,
     columnLines: false,
     selectable: false,
@@ -17,27 +37,18 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     unknownFlags: [],
 
     store: {
-        type: 'store',
-        fields: ['name', { name: 'state', defaultValue: '=' }, 'description'],
-        autoLoad: true,
+        fields: ['name', { name: 'state', defaultValue: '=' }, 'description', 'supported-on'],
+        autoLoad: false,
         proxy: {
             type: 'proxmox',
             url: '/api2/json/nodes/localhost/capabilities/qemu/cpu-flags',
+            extraParams: { accel: 'kvm' },
         },
         listeners: {
             update: function () {
                 this.commitChanges();
             },
-            refresh: function (store, eOpts) {
-                let me = this;
-                let view = me.view;
-
-                if (store.adjustedForValue !== view.value) {
-                    view.adjustStoreForValue();
-                }
-            },
         },
-        adjustedForValue: undefined,
     },
 
     getValue: function () {
@@ -86,14 +97,18 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
             let rec = store.findRecord('name', flagName, 0, false, true, true);
             if (rec !== null) {
                 rec.set('state', sign);
+                rec.commit();
             } else {
                 me.unknownFlags.push(flag);
             }
         });
 
-        store.adjustedForValue = value;
+        me.getView().refresh();
+    },
+    isDirty: function () {
+        let me = this;
+        return me.originalValue !== me.getValue();
     },
-
     setValue: function (value) {
         let me = this;
 
@@ -109,6 +124,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     },
     columns: [
         {
+            text: gettext('State'),
             dataIndex: 'state',
             renderer: function (v) {
                 switch (v) {
@@ -125,6 +141,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
             width: 65,
         },
         {
+            text: gettext('Set'),
             xtype: 'widgetcolumn',
             dataIndex: 'state',
             width: 95,
@@ -171,22 +188,96 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
             },
         },
         {
+            text: gettext('Flag'),
             dataIndex: 'name',
             width: 100,
         },
         {
+            text: gettext('Description'),
             dataIndex: 'description',
+            sortable: false,
+            cellWrap: true,
+            flex: 3,
+        },
+        {
+            text: gettext('Supported On'),
+            dataIndex: 'supported-on',
             cellWrap: true,
             flex: 1,
+            renderer: (v) => (Array.isArray(v) ? v.join(', ') : ''),
         },
     ],
 
     initComponent: function () {
         let me = this;
 
+        me.unknownFlags = [];
         me.value = me.originalValue = '';
-        me.store.view = me;
+
+        me.dockedItems = [
+            {
+                xtype: 'toolbar',
+                dock: 'top',
+                items: [
+                    {
+                        xtype: 'tbtext',
+                        text: gettext('Acceleration:'),
+                        autoEl: {
+                            tag: 'span',
+                            'data-qtip': gettext(
+                                'A custom CPU model using acceleration-specific flags should only be assigned to VMs configured with the matching acceleration type, i.e., `kvm: off` for TCG, or `kvm: on` for KVM.',
+                            ),
+                        },
+                    },
+                    {
+                        xtype: 'radiogroup',
+                        layout: 'hbox',
+                        validateOnChange: false,
+                        items: [
+                            {
+                                boxLabel: 'KVM',
+                                inputValue: 'kvm',
+                                name: 'accel',
+                                checked: true,
+                                isFormField: false,
+                            },
+                            {
+                                boxLabel: 'TCG',
+                                inputValue: 'tcg',
+                                name: 'accel',
+                                margin: '0 0 0 10',
+                                isFormField: false,
+                            },
+                        ],
+                        listeners: {
+                            change: function (_, value) {
+                                let view = this.up('grid');
+                                let proxy = view.getStore().getProxy();
+                                let accel = value.accel;
+                                if (accel) {
+                                    proxy.setExtraParam('accel', accel);
+                                } else {
+                                    delete proxy.extraParams.accel;
+                                }
+                                view.getStore().load();
+                            },
+                        },
+                    },
+                ],
+            },
+        ];
 
         me.callParent(arguments);
+
+        me.getStore().on('load', function (store, _, success) {
+            if (success) {
+                if (me.restrictToVMFlags) {
+                    store.filterBy((rec) => VM_CPU_FLAGS_SUBSET[rec.get('name')] === true);
+                }
+                me.adjustStoreForValue();
+                me.checkDirty();
+            }
+        });
+        me.getStore().load();
     },
 });
-- 
2.47.3




^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH qemu-server 6/8] qemu: Add helpers for new custom models endpoints
  2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
                   ` (4 preceding siblings ...)
  2026-03-12  8:40 ` [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models Arthur Bied-Charreton
@ 2026-03-12  8:40 ` Arthur Bied-Charreton
  2026-03-20 17:20   ` Fiona Ebner
  2026-03-12  8:40 ` [PATCH qemu-server 7/8] api: qemu: Extend cpu-flags endpoint to return actually supported flags Arthur Bied-Charreton
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-12  8:40 UTC (permalink / raw)
  To: pve-devel; +Cc: Stefan Reiter

Add functionality to lock & write the custom CPU config and some other
helpers that will be needed in custom CPU models CRUD endpoints.

Original patch:
https://lore.proxmox.com/pve-devel/20211028114150.3245864-5-s.reiter@proxmox.com/

Originally-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 src/PVE/QemuServer/CPUConfig.pm | 35 ++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index 32ec4954..2b6665a7 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -6,10 +6,10 @@ use warnings;
 use JSON;
 
 use PVE::JSONSchema qw(json_bool);
-use PVE::Cluster qw(cfs_register_file cfs_read_file);
+use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
 use PVE::ProcFSTools;
 use PVE::RESTEnvironment qw(log_warn);
-use PVE::Tools qw(run_command);
+use PVE::Tools qw(run_command lock_file);
 
 use PVE::QemuServer::Helpers qw(min_version);
 
@@ -51,6 +51,19 @@ sub load_custom_model_conf {
     return cfs_read_file($default_filename);
 }
 
+sub write_custom_model_conf {
+    my ($conf) = @_;
+    cfs_write_file($default_filename, $conf);
+}
+
+sub lock_cpu_config {
+    my ($code) = @_;
+    cfs_lock_file($default_filename, undef, $code);
+    if (my $err = $@) {
+        die $err;
+    }
+}
+
 #builtin models : reported-model is mandatory
 my $builtin_models_by_arch = {
     x86_64 => {
@@ -298,6 +311,19 @@ sub get_supported_cpu_flags {
     return $supported_cpu_flags_by_arch->{$arch};
 }
 
+sub description_by_flag {
+    my ($arch) = @_;
+    return { map { $_->{name} => $_->{description} } get_supported_cpu_flags($arch)->@* };
+}
+
+sub get_cpu_vendor {
+    my ($cputype, $arch) = @_;
+    $arch = $host_arch if !defined($arch);
+    my $retval = $cpu_models_by_arch->{$arch}->{$cputype}
+        or die "Built-in CPU type '$cputype' does not exist";
+    return $retval;
+}
+
 my $all_supported_cpu_flags = {};
 for my $arch ($supported_cpu_flags_by_arch->%*) {
     for my $flag ($supported_cpu_flags_by_arch->{$arch}->@*) {
@@ -375,6 +401,7 @@ my $cpu_fmt = {
         optional => 1,
     },
 };
+PVE::JSONSchema::register_standard_option('pve-qemu-cpu', $cpu_fmt);
 
 my $sev_fmt = {
     type => {
@@ -564,6 +591,7 @@ sub write_config {
 
         # saved in section header
         delete $model_conf->{cputype};
+        $model_conf->{type} = $class->type();
     }
 
     $class->SUPER::write_config($filename, $cfg);
@@ -612,7 +640,8 @@ sub get_cpu_models {
 
     my $conf = load_custom_model_conf();
     for my $custom_model (keys %{ $conf->{ids} }) {
-        my $reported_model = $conf->{ids}->{$custom_model}->{'reported-model'};
+        my $model = $conf->{ids}->{$custom_model};
+        my $reported_model = $model->{'reported-model'};
         $reported_model //= $cpu_fmt->{'reported-model'}->{default};
         my $vendor = $all_cpu_models->{$reported_model};
         push @$models,
-- 
2.47.3




^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH qemu-server 7/8] api: qemu: Extend cpu-flags endpoint to return actually supported flags
  2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
                   ` (5 preceding siblings ...)
  2026-03-12  8:40 ` [PATCH qemu-server 6/8] qemu: Add helpers for new custom models endpoints Arthur Bied-Charreton
@ 2026-03-12  8:40 ` Arthur Bied-Charreton
  2026-03-20 17:20   ` Fiona Ebner
  2026-03-12  8:40 ` [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models Arthur Bied-Charreton
  2026-03-26 14:54 ` [PATCH manager/qemu-server 0/8] Add API and UI " Fiona Ebner
  8 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-12  8:40 UTC (permalink / raw)
  To: pve-devel; +Cc: Stefan Reiter

Previously the endpoint returned a hardcoded list of flags. It now
returns flags that are both recognized by QEMU, and supported by at
least one cluster node to give a somewhat accurate picture of what flags
can actually be used on the current cluster.

The 'nested-virt` entry is prepended. An optional 'accel' parameter
filters results by virtualization type (kvm or tcg) to help avoid
misconfigurations when assigning flags to VMs with a specific
acceleration setting.

This deviates from the original patch [0] by adding the functionality to
the `cpu-flags` endpoint instead of adding new endpoints. This is
because we never need the understood/supported flags alone in the
frontend, only their intersection. This also improves the VM CPU flag
selector by letting users select from all possible flags in their
cluster.

When passed `aarch64` as argument for `arch`, the index returns an empty
list, which is consistent with the behavior from before this patch.

[0]
https://lore.proxmox.com/pve-devel/20211028114150.3245864-3-s.reiter@proxmox.com/

Originally-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 src/PVE/API2/Qemu/CPUFlags.pm | 108 +++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Qemu/CPUFlags.pm b/src/PVE/API2/Qemu/CPUFlags.pm
index 672bd2d2..9baf6c3e 100644
--- a/src/PVE/API2/Qemu/CPUFlags.pm
+++ b/src/PVE/API2/Qemu/CPUFlags.pm
@@ -10,6 +10,9 @@ use PVE::QemuServer::CPUConfig;
 
 use base qw(PVE::RESTHandler);
 
+# vmx/svm are already represented by the nested-virt synthetic entry
+my %NESTED_VIRT_ALIASES = map { $_ => 1 } qw(vmx svm);
+
 __PACKAGE__->register_method({
     name => 'index',
     path => '',
@@ -21,6 +24,13 @@ __PACKAGE__->register_method({
         properties => {
             node => get_standard_option('pve-node'),
             arch => get_standard_option('pve-qm-cpu-arch', { optional => 1 }),
+            accel => {
+                description =>
+                    'Virtualization type to filter flags by. If not provided, return all flags.',
+                type => 'string',
+                enum => [qw(kvm tcg)],
+                optional => 1,
+            },
         },
     },
     returns => {
@@ -35,6 +45,13 @@ __PACKAGE__->register_method({
                 description => {
                     type => 'string',
                     description => "Description of the CPU flag.",
+                    optional => 1,
+                },
+                'supported-on' => {
+                    description => 'List of nodes supporting the flag.',
+                    type => 'array',
+                    items => get_standard_option('pve-node'),
+                    optional => 1,
                 },
             },
         },
@@ -42,10 +59,99 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
+        my $accel = extract_param($param, 'accel');
         my $arch = extract_param($param, 'arch');
 
-        return PVE::QemuServer::CPUConfig::get_supported_cpu_flags($arch);
+        if (defined($arch) && $arch eq 'aarch64') {
+            return [];
+        }
+
+        my $descriptions = PVE::QemuServer::CPUConfig::description_by_flag($arch);
+
+        my $nested_virt = {
+            name => 'nested-virt',
+            description => $descriptions->{'nested-virt'},
+        };
+
+        return [$nested_virt, @{ extract_flags($descriptions, $accel) }];
     },
 });
 
+# As described here [0], in order to get an accurate picture of which flags can actually be used, we need
+# to intersect:
+#
+# 1. The understood CPU flags, i.e., all flags QEMU accepts in theory, but that may not be actually
+#   supported by the host CPU, and
+# 2. The supported CPU flags, which returns some settings/flags that cannot be used as `-cpu` arguments.
+#
+# [0] https://git.proxmox.com/?p=qemu-server.git;a=blob;f=src/PVE/QemuServer.pm;h=09e7a19b2f11ef48d2cfc11837b70338c306817c;hb=refs/heads/master#l2916
+sub extract_flags($descriptions, $accel = undef) {
+    my $recognized = extract_understood($descriptions);
+    my $supported = extract_supported($descriptions, $accel);
+
+    my %recognized_set = map { $_->{name} => 1 } @$recognized;
+
+    return [
+        map { {
+            name => $_->{name},
+            'supported-on' => $_->{'supported-on'},
+            (defined($_->{description}) ? (description => $_->{description}) : ()),
+        } } grep {
+            $recognized_set{ $_->{name} }
+        } @$supported
+    ];
+}
+
+sub extract_understood($descriptions) {
+    my $understood_cpu_flags = PVE::QemuServer::query_understood_cpu_flags();
+
+    return [
+        map {
+            my $entry = { name => $_ };
+            $entry->{description} = $descriptions->{$_} if $descriptions->{$_};
+            $entry;
+        } grep {
+            !$NESTED_VIRT_ALIASES{$_}
+        } @$understood_cpu_flags
+    ];
+}
+
+# We do not use `PVE::QemuServer::CPUConfig::query_supported_cpu_flags`, which is quite expensive since
+# it needs to spawn QEMU instances in order to check which flags are supported. Rather, we use its cached
+# output, which is stored by `pvestatd` [0].
+#
+# [0] https://git.proxmox.com/?p=pve-manager.git;a=blob;f=PVE/Service/pvestatd.pm;h=d0719446e3b9a5f1bd3c861dbe768432cb3d7a0e;hb=refs/heads/master#l87
+sub extract_supported($descriptions, $accel = undef) {
+    my %hash;
+
+    my sub add_flags($flags_by_node) {
+        for my $node (keys %$flags_by_node) {
+            # This depends on `pvestatd` storing the flags in space-separated format, which is the case
+            # at the time of this commit.
+            for (split(' ', $flags_by_node->{$node})) {
+                if ($hash{$_}) {
+                    $hash{$_}->{'supported-on'}->{$node} = 1;
+                } else {
+                    $hash{$_} = { 'supported-on' => { $node => 1 }, name => $_ };
+                }
+            }
+        }
+    }
+
+    add_flags(PVE::Cluster::get_node_kv('cpuflags-kvm')) if !defined($accel) || $accel eq 'kvm';
+    add_flags(PVE::Cluster::get_node_kv('cpuflags-tcg')) if !defined($accel) || $accel eq 'tcg';
+
+    return [
+        map {
+            my $entry = { %$_, 'supported-on' => [sort keys %{ $_->{'supported-on'} }] };
+            $entry->{description} = $descriptions->{ $_->{name} }
+                if $descriptions->{ $_->{name} };
+            $entry;
+        } sort {
+            $a->{name} cmp $b->{name}
+        } grep {
+            !$NESTED_VIRT_ALIASES{ $_->{name} }
+        } values %hash
+    ];
+}
 1;
-- 
2.47.3




^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models
  2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
                   ` (6 preceding siblings ...)
  2026-03-12  8:40 ` [PATCH qemu-server 7/8] api: qemu: Extend cpu-flags endpoint to return actually supported flags Arthur Bied-Charreton
@ 2026-03-12  8:40 ` Arthur Bied-Charreton
  2026-03-23 14:46   ` Fiona Ebner
  2026-03-26 14:54 ` [PATCH manager/qemu-server 0/8] Add API and UI " Fiona Ebner
  8 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-12  8:40 UTC (permalink / raw)
  To: pve-devel; +Cc: Stefan Reiter

Add GET handlers for both all custom CPU models and specific ones, POST
handler for creating custom CPU models, PUT handler for updating them,
and DELETE handler for deleting them.

Original patches:
https://lore.proxmox.com/pve-devel/20211028114150.3245864-4-s.reiter@proxmox.com/
https://lore.proxmox.com/pve-devel/20211028114150.3245864-5-s.reiter@proxmox.com/

Originally-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 src/PVE/API2/Qemu/CPU.pm | 236 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 235 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Qemu/CPU.pm b/src/PVE/API2/Qemu/CPU.pm
index f8a7e11d..9d89504d 100644
--- a/src/PVE/API2/Qemu/CPU.pm
+++ b/src/PVE/API2/Qemu/CPU.pm
@@ -52,7 +52,7 @@ __PACKAGE__->register_method({
                 },
             },
         },
-        links => [{ rel => 'child', href => '{name}' }],
+        links => [{ rel => 'child', href => 'model/{cputype}' }],
     },
     code => sub {
         my ($param) = @_;
@@ -67,4 +67,238 @@ __PACKAGE__->register_method({
     },
 });
 
+__PACKAGE__->register_method({
+    name => 'config',
+    path => 'model',
+    method => 'GET',
+    description => 'Read all custom CPU models definitions',
+    permissions => {
+        check => ['perm', '/nodes', ['Sys.Audit']],
+    },
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+        },
+    },
+    returns => {
+        type => 'array',
+        items => {
+            type => 'object',
+            properties => get_standard_option('pve-qemu-cpu'),
+        },
+    },
+    code => sub {
+        my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf();
+        delete $conf->{order};
+        my $ids = [];
+        foreach my $id (keys %{ $conf->{ids} }) {
+            delete $conf->{ids}->{$id}->{type};
+            push @$ids, $conf->{ids}->{$id};
+        }
+        return $ids;
+    },
+});
+
+__PACKAGE__->register_method({
+    name => 'create',
+    path => 'model',
+    method => 'POST',
+    protected => 1,
+    description => 'Add a custom CPU model definition',
+    permissions => {
+        check => ['perm', '/nodes', ['Sys.Console']],
+    },
+    parameters => {
+        additionalProperties => 0,
+        properties => PVE::QemuServer::CPUConfig::add_cpu_json_properties({
+            digest => get_standard_option('pve-config-digest'),
+            node => get_standard_option('pve-node'),
+        }),
+    },
+    returns => { type => 'null' },
+    code => sub {
+        my ($param) = @_;
+        delete $param->{node};
+
+        my $digest = extract_param($param, 'digest');
+
+        my $name = $param->{cputype};
+        die "custom cpu models 'cputype' must start with 'custom-' prefix\n"
+            if $name !~ m/^custom-/;
+        (my $name_no_prefix = $name) =~ s/^custom-//;
+
+        PVE::QemuServer::CPUConfig::lock_cpu_config(sub {
+            my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf();
+            PVE::SectionConfig::assert_if_modified($conf, $digest);
+
+            die "custom CPU model '$name' already exists\n"
+                if defined($conf->{ids}->{$name_no_prefix});
+            $conf->{ids}->{$name_no_prefix} = $param;
+
+            PVE::QemuServer::CPUConfig::write_custom_model_conf($conf);
+        });
+    },
+});
+
+__PACKAGE__->register_method({
+    name => 'delete',
+    path => 'model/{cputype}',
+    method => 'DELETE',
+    protected => 1,
+    description => 'Delete a custom CPU model definition',
+    permissions => {
+        check => ['perm', '/nodes', ['Sys.Console']],
+    },
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+            digest => get_standard_option('pve-config-digest'),
+            cputype => {
+                type => 'string',
+                description => "The custom model to delete. Must be prefixed with 'custom-'",
+            },
+        },
+    },
+    returns => { type => 'null' },
+    code => sub {
+        my ($param) = @_;
+
+        my $digest = extract_param($param, 'digest');
+
+        my $name = $param->{cputype};
+        die "'cputype' must start with 'custom-' prefix\n" if $name !~ m/^custom-/;
+        (my $name_no_prefix = $name) =~ s/^custom-//;
+
+        PVE::QemuServer::CPUConfig::lock_cpu_config(sub {
+            my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf();
+            PVE::SectionConfig::assert_if_modified($conf, $digest);
+
+            die "custom CPU model '$name_no_prefix' does not exist\n"
+                if !defined($conf->{ids}->{$name_no_prefix});
+            delete $conf->{ids}->{$name_no_prefix};
+
+            PVE::QemuServer::CPUConfig::write_custom_model_conf($conf);
+        });
+
+    },
+});
+
+__PACKAGE__->register_method({
+    name => 'update',
+    path => 'model/{cputype}',
+    method => 'PUT',
+    protected => 1,
+    description => "Update a custom CPU model definition.",
+    permissions => {
+        check => ['perm', '/nodes', ['Sys.Console']],
+    },
+    parameters => {
+        additionalProperties => 0,
+        properties => PVE::QemuServer::CPUConfig::add_cpu_json_properties({
+            node => get_standard_option('pve-node'),
+            digest => get_standard_option('pve-config-digest'),
+            delete => {
+                type => 'string',
+                format => 'pve-configid-list',
+                description => "A list of properties to delete.",
+                optional => 1,
+            },
+        }),
+    },
+    returns => { type => 'null' },
+    code => sub {
+        my ($param) = @_;
+        delete $param->{node};
+
+        my $digest = extract_param($param, 'digest');
+        my $delete = extract_param($param, 'delete') // '';
+        my %delete_hash = map { $_ => 1 } PVE::Tools::split_list($delete);
+
+        my $name = $param->{cputype};
+        die "custom CPU model's 'cputype' must start with 'custom-' prefix\n"
+            if $name !~ m/^custom-/;
+
+        (my $name_no_prefix = $name) =~ s/^custom-//;
+
+        PVE::QemuServer::CPUConfig::lock_cpu_config(sub {
+            my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf();
+
+            PVE::SectionConfig::assert_if_modified($conf, $digest);
+
+            my $model = $conf->{ids}->{$name_no_prefix};
+            die "custom CPU model '$name_no_prefix' does not exist\n" if !defined($model);
+
+            my $props = PVE::QemuServer::CPUConfig::add_cpu_json_properties({});
+            for my $p (keys %$props) {
+                if ($delete_hash{$p}) {
+                    die "cannot delete 'cputype' property\n" if $p eq 'cputype';
+                    die "cannot set and delete property '$p' at once\n" if $param->{$p};
+                    delete $model->{$p};
+                } elsif (defined($param->{$p})) {
+                    $model->{$p} = $param->{$p};
+                }
+            }
+
+            PVE::QemuServer::CPUConfig::write_custom_model_conf($conf);
+        });
+    },
+});
+
+__PACKAGE__->register_method({
+    name => 'info',
+    path => 'model/{cputype}',
+    method => 'GET',
+    description => 'Retrieve details about a specific CPU model',
+    permissions => {
+        check => ['perm', '/nodes', ['Sys.Audit']],
+    },
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+            cputype => {
+                type => 'string',
+                description =>
+                    "Name of the CPU model to query. Prefix with 'custom-' for custom models.",
+            },
+        },
+    },
+    returns => {
+        type => 'object',
+        properties => PVE::QemuServer::CPUConfig::add_cpu_json_properties({
+            vendor => {
+                type => 'string',
+                description => 'The CPU vendor reported to the guest OS. Only'
+                    . ' relevant for default models.',
+                optional => 1,
+            },
+            digest => get_standard_option('pve-config-digest'),
+        }),
+    },
+    code => sub {
+        my ($param) = @_;
+        my $cputype = $param->{cputype};
+
+        if ($cputype =~ m/^custom-/) {
+            my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf();
+            my $digest = $conf->{digest};
+            my $retval = PVE::QemuServer::CPUConfig::get_custom_model($cputype);
+            $retval->{digest} = $digest;
+            return $retval;
+        }
+
+        # this correctly errors in case $cputype is unknown
+        my $vendor = PVE::QemuServer::CPUConfig::get_cpu_vendor($cputype);
+
+        my $retval = {
+            cputype => $cputype,
+            vendor => $vendor,
+        };
+
+        return $retval;
+    },
+});
+
 1;
-- 
2.47.3




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH qemu-server 6/8] qemu: Add helpers for new custom models endpoints
  2026-03-12  8:40 ` [PATCH qemu-server 6/8] qemu: Add helpers for new custom models endpoints Arthur Bied-Charreton
@ 2026-03-20 17:20   ` Fiona Ebner
  2026-03-23  6:56     ` Arthur Bied-Charreton
  0 siblings, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-20 17:20 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pve-devel

Nit: the qemu-server patches should come first, since the UI patches
depend on them. And it's always good to mention in the cover letter
which inter-package dependencies there are, i.e. pve-manager needs a
versioned dependency bump for qemu-server. It's quite obvious in this
case, but still helps and mostly writing this for the future.

The prefix 'qemu' doesn't really add any information here. It should
rather be something like 'cpu config:'

Am 12.03.26 um 9:39 AM schrieb Arthur Bied-Charreton:
> Add functionality to lock & write the custom CPU config and some other
> helpers that will be needed in custom CPU models CRUD endpoints.
> 
> Original patch:
> https://lore.proxmox.com/pve-devel/20211028114150.3245864-5-s.reiter@proxmox.com/
> 
> Originally-by: Stefan Reiter <s.reiter@proxmox.com>

If you want, you can put a short high-level changelog here to document
how the patch changed, e.g.:

[AB: split patch into adding helpers and adding API endpoints
     some other change]

This can help follow history better later on.

> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  src/PVE/QemuServer/CPUConfig.pm | 35 ++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
> index 32ec4954..2b6665a7 100644
> --- a/src/PVE/QemuServer/CPUConfig.pm
> +++ b/src/PVE/QemuServer/CPUConfig.pm
> @@ -6,10 +6,10 @@ use warnings;
>  use JSON;
>  
>  use PVE::JSONSchema qw(json_bool);
> -use PVE::Cluster qw(cfs_register_file cfs_read_file);
> +use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
>  use PVE::ProcFSTools;
>  use PVE::RESTEnvironment qw(log_warn);
> -use PVE::Tools qw(run_command);
> +use PVE::Tools qw(run_command lock_file);
>  
>  use PVE::QemuServer::Helpers qw(min_version);
>  
> @@ -51,6 +51,19 @@ sub load_custom_model_conf {
>      return cfs_read_file($default_filename);
>  }
>  
> +sub write_custom_model_conf {
> +    my ($conf) = @_;
> +    cfs_write_file($default_filename, $conf);

Pre-existing, but could you add a preparatory commit renaming the
variable $default_filename to something more telling? Maybe
$cpu_models_filename?

> +}
> +
> +sub lock_cpu_config {
> +    my ($code) = @_;
> +    cfs_lock_file($default_filename, undef, $code);
> +    if (my $err = $@) {
> +        die $err;
> +    }

Style nit: could also just be
die $@ if $@;

> +}
> +
>  #builtin models : reported-model is mandatory
>  my $builtin_models_by_arch = {
>      x86_64 => {
> @@ -298,6 +311,19 @@ sub get_supported_cpu_flags {
>      return $supported_cpu_flags_by_arch->{$arch};
>  }
>  
> +sub description_by_flag {
> +    my ($arch) = @_;
> +    return { map { $_->{name} => $_->{description} } get_supported_cpu_flags($arch)->@* };
> +}
> +
> +sub get_cpu_vendor {
> +    my ($cputype, $arch) = @_;
> +    $arch = $host_arch if !defined($arch);

The $host_arch variable was recently dropped, needs a rebase.

> +    my $retval = $cpu_models_by_arch->{$arch}->{$cputype}
> +        or die "Built-in CPU type '$cputype' does not exist";
> +    return $retval;
> +}

These are quite small helpers and I do not see a big reason why they
should be added in a separate commit rather than just directly in the
commits with their users. It can be okay to add helpers up-front,
depending on the scenario, but here:
* 2 of the helpers are for config file handling
* 2 of the helpers are for cpu model property details, but also quite
unrelated
* the helpers are not complex

So I don't see enough semantic grouping that would justify having this
as a separate patch.

> +
>  my $all_supported_cpu_flags = {};
>  for my $arch ($supported_cpu_flags_by_arch->%*) {
>      for my $flag ($supported_cpu_flags_by_arch->{$arch}->@*) {
> @@ -375,6 +401,7 @@ my $cpu_fmt = {
>          optional => 1,
>      },
>  };
> +PVE::JSONSchema::register_standard_option('pve-qemu-cpu', $cpu_fmt);

I'd rather have this as a separate commit or squashed into the commit
with its first user.

>  
>  my $sev_fmt = {
>      type => {
> @@ -564,6 +591,7 @@ sub write_config {
>  
>          # saved in section header
>          delete $model_conf->{cputype};
> +        $model_conf->{type} = $class->type();

The commit message should at least describe why this is necessary. I
suppose for writing to actually work? Could also be a separate fix then,
but its fine to add together with the helper.

>      }
>  
>      $class->SUPER::write_config($filename, $cfg);
> @@ -612,7 +640,8 @@ sub get_cpu_models {
>  
>      my $conf = load_custom_model_conf();
>      for my $custom_model (keys %{ $conf->{ids} }) {
> -        my $reported_model = $conf->{ids}->{$custom_model}->{'reported-model'};
> +        my $model = $conf->{ids}->{$custom_model};
> +        my $reported_model = $model->{'reported-model'};

This change is unrelated to the rest.

>          $reported_model //= $cpu_fmt->{'reported-model'}->{default};
>          my $vendor = $all_cpu_models->{$reported_model};
>          push @$models,





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH qemu-server 7/8] api: qemu: Extend cpu-flags endpoint to return actually supported flags
  2026-03-12  8:40 ` [PATCH qemu-server 7/8] api: qemu: Extend cpu-flags endpoint to return actually supported flags Arthur Bied-Charreton
@ 2026-03-20 17:20   ` Fiona Ebner
  2026-03-23  7:25     ` Arthur Bied-Charreton
  0 siblings, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-20 17:20 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pve-devel; +Cc: Wolfgang Bumiller

Similar comment regarding the 'qemu' prefix as in patch 6/8

Note that I'm not finished with the review and will continue next week
with patch 8/8 and a quick look at the UI patches. Still sending this
already for discussion.

Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> Previously the endpoint returned a hardcoded list of flags. It now
> returns flags that are both recognized by QEMU, and supported by at
> least one cluster node to give a somewhat accurate picture of what flags
> can actually be used on the current cluster.
> 
> The 'nested-virt` entry is prepended. An optional 'accel' parameter
> filters results by virtualization type (kvm or tcg) to help avoid
> misconfigurations when assigning flags to VMs with a specific
> acceleration setting.
> 
> This deviates from the original patch [0] by adding the functionality to
> the `cpu-flags` endpoint instead of adding new endpoints. This is
> because we never need the understood/supported flags alone in the
> frontend, only their intersection. This also improves the VM CPU flag
> selector by letting users select from all possible flags in their
> cluster.
> 
> When passed `aarch64` as argument for `arch`, the index returns an empty
> list, which is consistent with the behavior from before this patch.
> 
> [0]
> https://lore.proxmox.com/pve-devel/20211028114150.3245864-3-s.reiter@proxmox.com/
> 
> Originally-by: Stefan Reiter <s.reiter@proxmox.com>

I don't think there is enough left of the original patch/approach in
this case to keep that trailer.

> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  src/PVE/API2/Qemu/CPUFlags.pm | 108 +++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/API2/Qemu/CPUFlags.pm b/src/PVE/API2/Qemu/CPUFlags.pm
> index 672bd2d2..9baf6c3e 100644
> --- a/src/PVE/API2/Qemu/CPUFlags.pm
> +++ b/src/PVE/API2/Qemu/CPUFlags.pm
> @@ -10,6 +10,9 @@ use PVE::QemuServer::CPUConfig;

Missing use statements for PVE::QemuServer and PVE::Cluster

>  
>  use base qw(PVE::RESTHandler);
>  
> +# vmx/svm are already represented by the nested-virt synthetic entry
> +my %NESTED_VIRT_ALIASES = map { $_ => 1 } qw(vmx svm);

I'd prefer a short helper function with a bit more generic name, like
flag_is_aliased(). And/or maybe have a filter_cpu_flags() function.

> +
>  __PACKAGE__->register_method({
>      name => 'index',
>      path => '',
> @@ -21,6 +24,13 @@ __PACKAGE__->register_method({
>          properties => {
>              node => get_standard_option('pve-node'),
>              arch => get_standard_option('pve-qm-cpu-arch', { optional => 1 }),
> +            accel => {
> +                description =>
> +                    'Virtualization type to filter flags by. If not provided, return all flags.',
> +                type => 'string',
> +                enum => [qw(kvm tcg)],
> +                optional => 1,
> +            },
>          },
>      },
>      returns => {
> @@ -35,6 +45,13 @@ __PACKAGE__->register_method({
>                  description => {
>                      type => 'string',
>                      description => "Description of the CPU flag.",
> +                    optional => 1,
> +                },
> +                'supported-on' => {
> +                    description => 'List of nodes supporting the flag.',
> +                    type => 'array',
> +                    items => get_standard_option('pve-node'),
> +                    optional => 1,
>                  },
>              },
>          },
> @@ -42,10 +59,99 @@ __PACKAGE__->register_method({
>      code => sub {
>          my ($param) = @_;
>  
> +        my $accel = extract_param($param, 'accel');
>          my $arch = extract_param($param, 'arch');
>  
> -        return PVE::QemuServer::CPUConfig::get_supported_cpu_flags($arch);

NAK: this endpoint is used for the VM-specific selection right now and
you can only configure flags that are allow-listed for a VM-specific CPU
model, i.e. the get_supported_cpu_flags() flags, not all flags. Some
flags do have security implications for the host too, that's why
creating custom models with arbitrary flags must be a more privileged
operation. I also think it would be a bit overkill to have all flags
there, rather than the ones we deem most useful to expose.

So the API call should not start providing other flags by default,
because the non-allow-listed flags cannot be set anyways for a
VM-specific model. We can add a new API parameter for this, maybe
"kind/type/something-else?" being an enum with values 'all|vm-specific'
and by default having 'vm-specific'.

I don't think there ever is a case where we want all flags regardless of
accelerator, or is there? If not, we should make 'accel' have a default
value to avoid the case where it's not provided at all being treated
differently. I'd opt for 'kvm' as that's the more common use case.

> +        if (defined($arch) && $arch eq 'aarch64') {
> +            return [];

We should document this in the description and add a TODO comment for
the 'all flags' case. Not having any VM-specific flags is intentional
right now. Not having any usable flags is not. It's just that we don't
ship a list yet, as it's more difficult to obtain during QEMU build. For
x86_64, kvm -cpu help will list the flags.

> +        }
> +
> +        my $descriptions = PVE::QemuServer::CPUConfig::description_by_flag($arch);

We could avoid passing around the descriptions, by having a function
get_flag_description() and call that where needed. We can also have
CPUConfig cache it with a hash to avoid re-grepping every time. That
would feel a bit cleaner to me.

> +
> +        my $nested_virt = {
> +            name => 'nested-virt',
> +            description => $descriptions->{'nested-virt'},

Should we check which hosts do support it? I.e. check for svm|vmx and
declare support if either is present on a host.

> +        };
> +
> +        return [$nested_virt, @{ extract_flags($descriptions, $accel) }];
>      },
>  });
>  
> +# As described here [0], in order to get an accurate picture of which flags can actually be used, we need
> +# to intersect:
> +#
> +# 1. The understood CPU flags, i.e., all flags QEMU accepts in theory, but that may not be actually
> +#   supported by the host CPU, and
> +# 2. The supported CPU flags, which returns some settings/flags that cannot be used as `-cpu` arguments.
> +#
> +# [0] https://git.proxmox.com/?p=qemu-server.git;a=blob;f=src/PVE/QemuServer.pm;h=09e7a19b2f11ef48d2cfc11837b70338c306817c;hb=refs/heads/master#l2916

I'd prefer having "See the documentation of query_supported_cpu_flags()
and query_understood_cpu_flags() for more information." rather than
putting a web link. Then the reference stays up-to-date with any changes
there.

> +sub extract_flags($descriptions, $accel = undef) {

To me, the 'extract' in the function names here and for the others below
is rather confusing. The functions take the hash of supported flags
(currently called $descriptions) and adds more information/other flags.
The only thing it's extracting is the descriptions.

> +    my $recognized = extract_understood($descriptions);
> +    my $supported = extract_supported($descriptions, $accel);
> +
> +    my %recognized_set = map { $_->{name} => 1 } @$recognized;
> +
> +    return [
> +        map { {
> +            name => $_->{name},
> +            'supported-on' => $_->{'supported-on'},
> +            (defined($_->{description}) ? (description => $_->{description}) : ()),
> +        } } grep {
> +            $recognized_set{ $_->{name} }
> +        } @$supported
> +    ];
> +}
> +
> +sub extract_understood($descriptions) {
> +    my $understood_cpu_flags = PVE::QemuServer::query_understood_cpu_flags();
> +
> +    return [
> +        map {
> +            my $entry = { name => $_ };
> +            $entry->{description} = $descriptions->{$_} if $descriptions->{$_};
> +            $entry;
> +        } grep {
> +            !$NESTED_VIRT_ALIASES{$_}
> +        } @$understood_cpu_flags
> +    ];
> +}
> +
> +# We do not use `PVE::QemuServer::CPUConfig::query_supported_cpu_flags`, which is quite expensive since
> +# it needs to spawn QEMU instances in order to check which flags are supported. Rather, we use its cached
> +# output, which is stored by `pvestatd` [0].
> +#
> +# [0] https://git.proxmox.com/?p=pve-manager.git;a=blob;f=PVE/Service/pvestatd.pm;h=d0719446e3b9a5f1bd3c861dbe768432cb3d7a0e;hb=refs/heads/master#l87

Again, not a fan of putting a web reference here. I think it's enough to
mention that there are entries in the node kv store (regularly updated
by pvestatd).

> +sub extract_supported($descriptions, $accel = undef) {
> +    my %hash;
> +
> +    my sub add_flags($flags_by_node) {
> +        for my $node (keys %$flags_by_node) {
> +            # This depends on `pvestatd` storing the flags in space-separated format, which is the case
> +            # at the time of this commit.
> +            for (split(' ', $flags_by_node->{$node})) {
> +                if ($hash{$_}) {
> +                    $hash{$_}->{'supported-on'}->{$node} = 1;
> +                } else {
> +                    $hash{$_} = { 'supported-on' => { $node => 1 }, name => $_ };
> +                }
> +            }
> +        }
> +    }

I didn't know you could do nested subs directly and not just by
assigning a closure to a local variable. I'm not sure we have this
anywhere else in the code base and if there are any gotchas when doing
it like this.

@Wolfgang: anything to be aware of with this construct?

> +
> +    add_flags(PVE::Cluster::get_node_kv('cpuflags-kvm')) if !defined($accel) || $accel eq 'kvm';
> +    add_flags(PVE::Cluster::get_node_kv('cpuflags-tcg')) if !defined($accel) || $accel eq 'tcg';
> +
> +    return [
> +        map {
> +            my $entry = { %$_, 'supported-on' => [sort keys %{ $_->{'supported-on'} }] };
> +            $entry->{description} = $descriptions->{ $_->{name} }
> +                if $descriptions->{ $_->{name} };
> +            $entry;
> +        } sort {
> +            $a->{name} cmp $b->{name}
> +        } grep {
> +            !$NESTED_VIRT_ALIASES{ $_->{name} }
> +        } values %hash

Style nit: we usually write such things in a more mixed way rather than
purely functional. One advantage is that one doesn't have to read
bottom-to-top. Opinionated preference, but I think when you have
multiple instructions inside a map expression, it is a good sign that
it's better written as a loop.

> +    ];
> +}
>  1;





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH qemu-server 6/8] qemu: Add helpers for new custom models endpoints
  2026-03-20 17:20   ` Fiona Ebner
@ 2026-03-23  6:56     ` Arthur Bied-Charreton
  0 siblings, 0 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-23  6:56 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel

On Fri, Mar 20, 2026 at 06:20:38PM +0100, Fiona Ebner wrote:
> Nit: the qemu-server patches should come first, since the UI patches
> depend on them. And it's always good to mention in the cover letter
> which inter-package dependencies there are, i.e. pve-manager needs a
> versioned dependency bump for qemu-server. It's quite obvious in this
> case, but still helps and mostly writing this for the future.
> 
Makes sense, will reorder the patches in v2
> The prefix 'qemu' doesn't really add any information here. It should
> rather be something like 'cpu config:'
> 
> Am 12.03.26 um 9:39 AM schrieb Arthur Bied-Charreton:
> > Add functionality to lock & write the custom CPU config and some other
> > helpers that will be needed in custom CPU models CRUD endpoints.
> > 
> > Original patch:
> > https://lore.proxmox.com/pve-devel/20211028114150.3245864-5-s.reiter@proxmox.com/
> > 
> > Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> 
> If you want, you can put a short high-level changelog here to document
> how the patch changed, e.g.:
> 
> [AB: split patch into adding helpers and adding API endpoints
>      some other change]
> 
> This can help follow history better later on.
> 
Noted, thanks!
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> > ---
> >  src/PVE/QemuServer/CPUConfig.pm | 35 ++++++++++++++++++++++++++++++---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
> > index 32ec4954..2b6665a7 100644
> > --- a/src/PVE/QemuServer/CPUConfig.pm
> > +++ b/src/PVE/QemuServer/CPUConfig.pm
> > @@ -6,10 +6,10 @@ use warnings;
> >  use JSON;
> >  
> >  use PVE::JSONSchema qw(json_bool);
> > -use PVE::Cluster qw(cfs_register_file cfs_read_file);
> > +use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
> >  use PVE::ProcFSTools;
> >  use PVE::RESTEnvironment qw(log_warn);
> > -use PVE::Tools qw(run_command);
> > +use PVE::Tools qw(run_command lock_file);
> >  
> >  use PVE::QemuServer::Helpers qw(min_version);
> >  
> > @@ -51,6 +51,19 @@ sub load_custom_model_conf {
> >      return cfs_read_file($default_filename);
> >  }
> >  
> > +sub write_custom_model_conf {
> > +    my ($conf) = @_;
> > +    cfs_write_file($default_filename, $conf);
> 
> Pre-existing, but could you add a preparatory commit renaming the
> variable $default_filename to something more telling? Maybe
> $cpu_models_filename?
> 
Will do
> > +}
> > +
> > +sub lock_cpu_config {
> > +    my ($code) = @_;
> > +    cfs_lock_file($default_filename, undef, $code);
> > +    if (my $err = $@) {
> > +        die $err;
> > +    }
> 
> Style nit: could also just be
> die $@ if $@;
> 
Noted
> > +}
> > +
> >  #builtin models : reported-model is mandatory
> >  my $builtin_models_by_arch = {
> >      x86_64 => {
> > @@ -298,6 +311,19 @@ sub get_supported_cpu_flags {
> >      return $supported_cpu_flags_by_arch->{$arch};
> >  }
> >  
> > +sub description_by_flag {
> > +    my ($arch) = @_;
> > +    return { map { $_->{name} => $_->{description} } get_supported_cpu_flags($arch)->@* };
> > +}
> > +
> > +sub get_cpu_vendor {
> > +    my ($cputype, $arch) = @_;
> > +    $arch = $host_arch if !defined($arch);
> 
> The $host_arch variable was recently dropped, needs a rebase.
> 
Noted
> > +    my $retval = $cpu_models_by_arch->{$arch}->{$cputype}
> > +        or die "Built-in CPU type '$cputype' does not exist";
> > +    return $retval;
> > +}
> 
> These are quite small helpers and I do not see a big reason why they
> should be added in a separate commit rather than just directly in the
> commits with their users. It can be okay to add helpers up-front,
> depending on the scenario, but here:
> * 2 of the helpers are for config file handling
> * 2 of the helpers are for cpu model property details, but also quite
> unrelated
> * the helpers are not complex
> 
> So I don't see enough semantic grouping that would justify having this
> as a separate patch.
> 
Makes sense, will squash helpers into the commits that use them
> > +
> >  my $all_supported_cpu_flags = {};
> >  for my $arch ($supported_cpu_flags_by_arch->%*) {
> >      for my $flag ($supported_cpu_flags_by_arch->{$arch}->@*) {
> > @@ -375,6 +401,7 @@ my $cpu_fmt = {
> >          optional => 1,
> >      },
> >  };
> > +PVE::JSONSchema::register_standard_option('pve-qemu-cpu', $cpu_fmt);
> 
> I'd rather have this as a separate commit or squashed into the commit
> with its first user.
> 
Noted
> >  
> >  my $sev_fmt = {
> >      type => {
> > @@ -564,6 +591,7 @@ sub write_config {
> >  
> >          # saved in section header
> >          delete $model_conf->{cputype};
> > +        $model_conf->{type} = $class->type();
> 
> The commit message should at least describe why this is necessary. I
> suppose for writing to actually work? Could also be a separate fix then,
> but its fine to add together with the helper.
> 
> >      }
Yes, this was in the original commit but I somehow lost it, sorry! Will
be in v2.
> >  
> >      $class->SUPER::write_config($filename, $cfg);
> > @@ -612,7 +640,8 @@ sub get_cpu_models {
> >  
> >      my $conf = load_custom_model_conf();
> >      for my $custom_model (keys %{ $conf->{ids} }) {
> > -        my $reported_model = $conf->{ids}->{$custom_model}->{'reported-model'};
> > +        my $model = $conf->{ids}->{$custom_model};
> > +        my $reported_model = $model->{'reported-model'};
> 
> This change is unrelated to the rest.
> 
Will remove it, sorry about that.
> >          $reported_model //= $cpu_fmt->{'reported-model'}->{default};
> >          my $vendor = $all_cpu_models->{$reported_model};
> >          push @$models,
> 




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH qemu-server 7/8] api: qemu: Extend cpu-flags endpoint to return actually supported flags
  2026-03-20 17:20   ` Fiona Ebner
@ 2026-03-23  7:25     ` Arthur Bied-Charreton
  0 siblings, 0 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-23  7:25 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel, Wolfgang Bumiller

On Fri, Mar 20, 2026 at 06:20:40PM +0100, Fiona Ebner wrote:
> Similar comment regarding the 'qemu' prefix as in patch 6/8
> 
> Note that I'm not finished with the review and will continue next week
> with patch 8/8 and a quick look at the UI patches. Still sending this
> already for discussion.
> 
> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> > Previously the endpoint returned a hardcoded list of flags. It now
> > returns flags that are both recognized by QEMU, and supported by at
> > least one cluster node to give a somewhat accurate picture of what flags
> > can actually be used on the current cluster.
> > 
> > The 'nested-virt` entry is prepended. An optional 'accel' parameter
> > filters results by virtualization type (kvm or tcg) to help avoid
> > misconfigurations when assigning flags to VMs with a specific
> > acceleration setting.
> > 
> > This deviates from the original patch [0] by adding the functionality to
> > the `cpu-flags` endpoint instead of adding new endpoints. This is
> > because we never need the understood/supported flags alone in the
> > frontend, only their intersection. This also improves the VM CPU flag
> > selector by letting users select from all possible flags in their
> > cluster.
> > 
> > When passed `aarch64` as argument for `arch`, the index returns an empty
> > list, which is consistent with the behavior from before this patch.
> > 
> > [0]
> > https://lore.proxmox.com/pve-devel/20211028114150.3245864-3-s.reiter@proxmox.com/
> > 
> > Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> 
> I don't think there is enough left of the original patch/approach in
> this case to keep that trailer.
> 
Noted
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> > ---
> >  src/PVE/API2/Qemu/CPUFlags.pm | 108 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 107 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/PVE/API2/Qemu/CPUFlags.pm b/src/PVE/API2/Qemu/CPUFlags.pm
> > index 672bd2d2..9baf6c3e 100644
> > --- a/src/PVE/API2/Qemu/CPUFlags.pm
> > +++ b/src/PVE/API2/Qemu/CPUFlags.pm
> > @@ -10,6 +10,9 @@ use PVE::QemuServer::CPUConfig;
> 
> Missing use statements for PVE::QemuServer and PVE::Cluster
> 
Argh, thanks!
> >  
> >  use base qw(PVE::RESTHandler);
> >  
> > +# vmx/svm are already represented by the nested-virt synthetic entry
> > +my %NESTED_VIRT_ALIASES = map { $_ => 1 } qw(vmx svm);
> 
> I'd prefer a short helper function with a bit more generic name, like
> flag_is_aliased(). And/or maybe have a filter_cpu_flags() function.
> 
> > +
> >  __PACKAGE__->register_method({
> >      name => 'index',
> >      path => '',
> > @@ -21,6 +24,13 @@ __PACKAGE__->register_method({
> >          properties => {
> >              node => get_standard_option('pve-node'),
> >              arch => get_standard_option('pve-qm-cpu-arch', { optional => 1 }),
> > +            accel => {
> > +                description =>
> > +                    'Virtualization type to filter flags by. If not provided, return all flags.',
> > +                type => 'string',
> > +                enum => [qw(kvm tcg)],
> > +                optional => 1,
> > +            },
> >          },
> >      },
> >      returns => {
> > @@ -35,6 +45,13 @@ __PACKAGE__->register_method({
> >                  description => {
> >                      type => 'string',
> >                      description => "Description of the CPU flag.",
> > +                    optional => 1,
> > +                },
> > +                'supported-on' => {
> > +                    description => 'List of nodes supporting the flag.',
> > +                    type => 'array',
> > +                    items => get_standard_option('pve-node'),
> > +                    optional => 1,
> >                  },
> >              },
> >          },
> > @@ -42,10 +59,99 @@ __PACKAGE__->register_method({
> >      code => sub {
> >          my ($param) = @_;
> >  
> > +        my $accel = extract_param($param, 'accel');
> >          my $arch = extract_param($param, 'arch');
> >  
> > -        return PVE::QemuServer::CPUConfig::get_supported_cpu_flags($arch);
> 
> NAK: this endpoint is used for the VM-specific selection right now and
> you can only configure flags that are allow-listed for a VM-specific CPU
> model, i.e. the get_supported_cpu_flags() flags, not all flags. Some
> flags do have security implications for the host too, that's why
> creating custom models with arbitrary flags must be a more privileged
> operation. I also think it would be a bit overkill to have all flags
> there, rather than the ones we deem most useful to expose.
> 
> So the API call should not start providing other flags by default,
> because the non-allow-listed flags cannot be set anyways for a
> VM-specific model. We can add a new API parameter for this, maybe
> "kind/type/something-else?" being an enum with values 'all|vm-specific'
> and by default having 'vm-specific'.
> 
Yes, the UI part of this series filters the flags in the UI, which is
admittedly not very pretty. I like the enum idea, will introduce this in
v2, this will simplify the UI code quite a bit as well. Thanks!
> I don't think there ever is a case where we want all flags regardless of
> accelerator, or is there? If not, we should make 'accel' have a default
> value to avoid the case where it's not provided at all being treated
> differently. I'd opt for 'kvm' as that's the more common use case.
> 
I don't think so either, will add the paramter in v2. I also think `kvm`
is a good default.
> > +        if (defined($arch) && $arch eq 'aarch64') {
> > +            return [];
> 
> We should document this in the description and add a TODO comment for
> the 'all flags' case. Not having any VM-specific flags is intentional
> right now. Not having any usable flags is not. It's just that we don't
> ship a list yet, as it's more difficult to obtain during QEMU build. For
> x86_64, kvm -cpu help will list the flags.
> 
Makes sense, will add a TODO comment and update the endpoint description.
> > +        }
> > +
> > +        my $descriptions = PVE::QemuServer::CPUConfig::description_by_flag($arch);
> 
> We could avoid passing around the descriptions, by having a function
> get_flag_description() and call that where needed. We can also have
> CPUConfig cache it with a hash to avoid re-grepping every time. That
> would feel a bit cleaner to me.
> 
Yes, the reason why I passed it around was to avoid re-grepping, having
it cached in CPUConfig does sound better though.
> > +
> > +        my $nested_virt = {
> > +            name => 'nested-virt',
> > +            description => $descriptions->{'nested-virt'},
> 
> Should we check which hosts do support it? I.e. check for svm|vmx and
> declare support if either is present on a host.
> 
Good call, I made the wrong assumption that they would always be
available - will add a check in v2. 
> > +        };
> > +
> > +        return [$nested_virt, @{ extract_flags($descriptions, $accel) }];
> >      },
> >  });
> >  
> > +# As described here [0], in order to get an accurate picture of which flags can actually be used, we need
> > +# to intersect:
> > +#
> > +# 1. The understood CPU flags, i.e., all flags QEMU accepts in theory, but that may not be actually
> > +#   supported by the host CPU, and
> > +# 2. The supported CPU flags, which returns some settings/flags that cannot be used as `-cpu` arguments.
> > +#
> > +# [0] https://git.proxmox.com/?p=qemu-server.git;a=blob;f=src/PVE/QemuServer.pm;h=09e7a19b2f11ef48d2cfc11837b70338c306817c;hb=refs/heads/master#l2916
> 
> I'd prefer having "See the documentation of query_supported_cpu_flags()
> and query_understood_cpu_flags() for more information." rather than
> putting a web link. Then the reference stays up-to-date with any changes
> there.
> 
Noted, will update the doc comment
> > +sub extract_flags($descriptions, $accel = undef) {
> 
> To me, the 'extract' in the function names here and for the others below
> is rather confusing. The functions take the hash of supported flags
> (currently called $descriptions) and adds more information/other flags.
> The only thing it's extracting is the descriptions.
> 
I will think about better naming, I agree that these are not very
good.
> > +    my $recognized = extract_understood($descriptions);
> > +    my $supported = extract_supported($descriptions, $accel);
> > +
> > +    my %recognized_set = map { $_->{name} => 1 } @$recognized;
> > +
> > +    return [
> > +        map { {
> > +            name => $_->{name},
> > +            'supported-on' => $_->{'supported-on'},
> > +            (defined($_->{description}) ? (description => $_->{description}) : ()),
> > +        } } grep {
> > +            $recognized_set{ $_->{name} }
> > +        } @$supported
> > +    ];
> > +}
> > +
> > +sub extract_understood($descriptions) {
> > +    my $understood_cpu_flags = PVE::QemuServer::query_understood_cpu_flags();
> > +
> > +    return [
> > +        map {
> > +            my $entry = { name => $_ };
> > +            $entry->{description} = $descriptions->{$_} if $descriptions->{$_};
> > +            $entry;
> > +        } grep {
> > +            !$NESTED_VIRT_ALIASES{$_}
> > +        } @$understood_cpu_flags
> > +    ];
> > +}
> > +
> > +# We do not use `PVE::QemuServer::CPUConfig::query_supported_cpu_flags`, which is quite expensive since
> > +# it needs to spawn QEMU instances in order to check which flags are supported. Rather, we use its cached
> > +# output, which is stored by `pvestatd` [0].
> > +#
> > +# [0] https://git.proxmox.com/?p=pve-manager.git;a=blob;f=PVE/Service/pvestatd.pm;h=d0719446e3b9a5f1bd3c861dbe768432cb3d7a0e;hb=refs/heads/master#l87
> 
> Again, not a fan of putting a web reference here. I think it's enough to
> mention that there are entries in the node kv store (regularly updated
> by pvestatd).
> 
Noted
> > +sub extract_supported($descriptions, $accel = undef) {
> > +    my %hash;
> > +
> > +    my sub add_flags($flags_by_node) {
> > +        for my $node (keys %$flags_by_node) {
> > +            # This depends on `pvestatd` storing the flags in space-separated format, which is the case
> > +            # at the time of this commit.
> > +            for (split(' ', $flags_by_node->{$node})) {
> > +                if ($hash{$_}) {
> > +                    $hash{$_}->{'supported-on'}->{$node} = 1;
> > +                } else {
> > +                    $hash{$_} = { 'supported-on' => { $node => 1 }, name => $_ };
> > +                }
> > +            }
> > +        }
> > +    }
> 
> I didn't know you could do nested subs directly and not just by
> assigning a closure to a local variable. I'm not sure we have this
> anywhere else in the code base and if there are any gotchas when doing
> it like this.
> 
> @Wolfgang: anything to be aware of with this construct?
> 
> > +
> > +    add_flags(PVE::Cluster::get_node_kv('cpuflags-kvm')) if !defined($accel) || $accel eq 'kvm';
> > +    add_flags(PVE::Cluster::get_node_kv('cpuflags-tcg')) if !defined($accel) || $accel eq 'tcg';
> > +
> > +    return [
> > +        map {
> > +            my $entry = { %$_, 'supported-on' => [sort keys %{ $_->{'supported-on'} }] };
> > +            $entry->{description} = $descriptions->{ $_->{name} }
> > +                if $descriptions->{ $_->{name} };
> > +            $entry;
> > +        } sort {
> > +            $a->{name} cmp $b->{name}
> > +        } grep {
> > +            !$NESTED_VIRT_ALIASES{ $_->{name} }
> > +        } values %hash
> 
> Style nit: we usually write such things in a more mixed way rather than
> purely functional. One advantage is that one doesn't have to read
> bottom-to-top. Opinionated preference, but I think when you have
> multiple instructions inside a map expression, it is a good sign that
> it's better written as a loop.
> 
Noted, will change that
> > +    ];
> > +}
> >  1;
> 




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models
  2026-03-12  8:40 ` [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models Arthur Bied-Charreton
@ 2026-03-23 14:46   ` Fiona Ebner
  2026-03-23 16:04     ` Arthur Bied-Charreton
  0 siblings, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-23 14:46 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pve-devel

Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> Add GET handlers for both all custom CPU models and specific ones, POST
> handler for creating custom CPU models, PUT handler for updating them,
> and DELETE handler for deleting them.
> 
> Original patches:
> https://lore.proxmox.com/pve-devel/20211028114150.3245864-4-s.reiter@proxmox.com/
> https://lore.proxmox.com/pve-devel/20211028114150.3245864-5-s.reiter@proxmox.com/
> 
> Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  src/PVE/API2/Qemu/CPU.pm | 236 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 235 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/API2/Qemu/CPU.pm b/src/PVE/API2/Qemu/CPU.pm
> index f8a7e11d..9d89504d 100644
> --- a/src/PVE/API2/Qemu/CPU.pm
> +++ b/src/PVE/API2/Qemu/CPU.pm
> @@ -52,7 +52,7 @@ __PACKAGE__->register_method({
>                  },
>              },
>          },
> -        links => [{ rel => 'child', href => '{name}' }],
> +        links => [{ rel => 'child', href => 'model/{cputype}' }],

Pre-existing issues from the original series:

This does not work in pvesh, because the extract_children() function
there only matches:

if ($href =~ m/^\{(\S+)\}$/) {

Also, it's called 'name' in the returned result, not 'cputype'.

But this is the wrong place for a link to begin with, since we only want
to provide the link for custom models, it should be added to the 'model'
endpoint. There, it is called 'cputype' ;)

Having the 'model' endpoint below here seems wrong to me, for two reasons:

1. Since the 'cpu' endpoint already returns a result other than an index
of sub-endpoints, it cannot at the same time show in its result that the
sub-endpoint exists, and this breaks the directory structure.

2. It's below /nodes/ but it's touching cluster-wide configuration.

I think we should just have a new, dedicated endpoint, maybe
/cluster/qemu/custom-cpu-models? And then, we might want to drop the
need for specifying a 'custom-' prefix when using the calls?




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models
  2026-03-23 14:46   ` Fiona Ebner
@ 2026-03-23 16:04     ` Arthur Bied-Charreton
  2026-03-23 16:10       ` Arthur Bied-Charreton
  0 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-23 16:04 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel

On Mon, Mar 23, 2026 at 03:46:45PM +0100, Fiona Ebner wrote:
> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> > Add GET handlers for both all custom CPU models and specific ones, POST
> > handler for creating custom CPU models, PUT handler for updating them,
> > and DELETE handler for deleting them.
> > 
> > Original patches:
> > https://lore.proxmox.com/pve-devel/20211028114150.3245864-4-s.reiter@proxmox.com/
> > https://lore.proxmox.com/pve-devel/20211028114150.3245864-5-s.reiter@proxmox.com/
> > 
> > Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> > ---
> >  src/PVE/API2/Qemu/CPU.pm | 236 ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 235 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/PVE/API2/Qemu/CPU.pm b/src/PVE/API2/Qemu/CPU.pm
> > index f8a7e11d..9d89504d 100644
> > --- a/src/PVE/API2/Qemu/CPU.pm
> > +++ b/src/PVE/API2/Qemu/CPU.pm
> > @@ -52,7 +52,7 @@ __PACKAGE__->register_method({
> >                  },
> >              },
> >          },
> > -        links => [{ rel => 'child', href => '{name}' }],
> > +        links => [{ rel => 'child', href => 'model/{cputype}' }],
> 
> Pre-existing issues from the original series:
> 
> This does not work in pvesh, because the extract_children() function
> there only matches:
> 
> if ($href =~ m/^\{(\S+)\}$/) {
> 
I should have tested that, sorry..
> Also, it's called 'name' in the returned result, not 'cputype'.
> 
> But this is the wrong place for a link to begin with, since we only want
> to provide the link for custom models, it should be added to the 'model'
> endpoint. There, it is called 'cputype' ;)
> 
> Having the 'model' endpoint below here seems wrong to me, for two reasons:
> 
> 1. Since the 'cpu' endpoint already returns a result other than an index
> of sub-endpoints, it cannot at the same time show in its result that the
> sub-endpoint exists, and this breaks the directory structure.
> 
> 2. It's below /nodes/ but it's touching cluster-wide configuration.
> 
> I think we should just have a new, dedicated endpoint, maybe
> /cluster/qemu/custom-cpu-models? And then, we might want to drop the
> need for specifying a 'custom-' prefix when using the calls?
I agree that the current state feels weird. I went with the old patch's
approach but I should have rethought it a bit more, especially the
custom- prefix, the URL already carries that so it's quite redundant. 

I like the idea of a new /cluster/qemu/custom-cpu-models route, will
move the endpoint over to pve-manager for v2 and drop the custom- prefix
in the process. 

By that logic, the cpu-flags endpoint should probably also be cluster-wide, 
since it returns data for all nodes in the cluster? It technically takes
a node parameter, but that is ignored by the handler (both before and
after this series). What do you think?




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models
  2026-03-23 16:04     ` Arthur Bied-Charreton
@ 2026-03-23 16:10       ` Arthur Bied-Charreton
  2026-03-24  9:27         ` Fiona Ebner
  0 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-23 16:10 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel

On Mon, Mar 23, 2026 at 05:04:15PM +0100, Arthur Bied-Charreton wrote:
> On Mon, Mar 23, 2026 at 03:46:45PM +0100, Fiona Ebner wrote:
> > Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> > 
[...]
> > Having the 'model' endpoint below here seems wrong to me, for two reasons:
> > 
> > 1. Since the 'cpu' endpoint already returns a result other than an index
> > of sub-endpoints, it cannot at the same time show in its result that the
> > sub-endpoint exists, and this breaks the directory structure.
> > 
> > 2. It's below /nodes/ but it's touching cluster-wide configuration.
> > 
> > I think we should just have a new, dedicated endpoint, maybe
> > /cluster/qemu/custom-cpu-models? And then, we might want to drop the
> > need for specifying a 'custom-' prefix when using the calls?
> I agree that the current state feels weird. I went with the old patch's
> approach but I should have rethought it a bit more, especially the
> custom- prefix, the URL already carries that so it's quite redundant. 
> 
> I like the idea of a new /cluster/qemu/custom-cpu-models route, will
> move the endpoint over to pve-manager for v2 and drop the custom- prefix
> in the process. 
> 
> By that logic, the cpu-flags endpoint should probably also be cluster-wide, 
> since it returns data for all nodes in the cluster? It technically takes
> a node parameter, but that is ignored by the handler (both before and
> after this series). What do you think?
> 
Just realizing that this would be a breaking change since that endpoint
is pre-existing, would be better not to touch it, my bad...




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models
  2026-03-23 16:10       ` Arthur Bied-Charreton
@ 2026-03-24  9:27         ` Fiona Ebner
  0 siblings, 0 replies; 34+ messages in thread
From: Fiona Ebner @ 2026-03-24  9:27 UTC (permalink / raw)
  To: Arthur Bied-Charreton; +Cc: pve-devel

Am 23.03.26 um 5:09 PM schrieb Arthur Bied-Charreton:
> On Mon, Mar 23, 2026 at 05:04:15PM +0100, Arthur Bied-Charreton wrote:
>> On Mon, Mar 23, 2026 at 03:46:45PM +0100, Fiona Ebner wrote:
>>> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
>>>
> [...]
>>> Having the 'model' endpoint below here seems wrong to me, for two reasons:
>>>
>>> 1. Since the 'cpu' endpoint already returns a result other than an index
>>> of sub-endpoints, it cannot at the same time show in its result that the
>>> sub-endpoint exists, and this breaks the directory structure.
>>>
>>> 2. It's below /nodes/ but it's touching cluster-wide configuration.
>>>
>>> I think we should just have a new, dedicated endpoint, maybe
>>> /cluster/qemu/custom-cpu-models? And then, we might want to drop the
>>> need for specifying a 'custom-' prefix when using the calls?
>> I agree that the current state feels weird. I went with the old patch's
>> approach but I should have rethought it a bit more, especially the
>> custom- prefix, the URL already carries that so it's quite redundant. 
>>
>> I like the idea of a new /cluster/qemu/custom-cpu-models route, will
>> move the endpoint over to pve-manager for v2 and drop the custom- prefix
>> in the process. 
>>
>> By that logic, the cpu-flags endpoint should probably also be cluster-wide, 
>> since it returns data for all nodes in the cluster? It technically takes
>> a node parameter, but that is ignored by the handler (both before and
>> after this series). What do you think?
>>
> Just realizing that this would be a breaking change since that endpoint
> is pre-existing, would be better not to touch it, my bad...

No worries! If we would have a good reason to move it, we could add a
second endpoint, deprecate the old one and drop it with the next major
release.

But maybe we should keep that endpoint as-is too, since it is concerned
with the VM-specific flags on the node (it does depend on the installed
qemu-server version, even if new VM-specific flags are rarely addded),
and introduce a new one below /cluster/qemu/ for the cluster-wide flags
querying? We could still think about deprecating it in the long run and
also sending the VM-specific flags via kv and have them via the
cluster-wide endpoint, but not sure about that and I don't think it's
too relevant for the series. How does that sound?




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 1/8] ui: VMCPUFlagSelector: Fix unknownFlags behaviour
  2026-03-12  8:40 ` [PATCH pve-manager 1/8] ui: VMCPUFlagSelector: Fix unknownFlags behaviour Arthur Bied-Charreton
@ 2026-03-25 15:57   ` Fiona Ebner
  2026-03-26 13:47     ` Arthur Bied-Charreton
  0 siblings, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-25 15:57 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pve-devel

Am 12.03.26 um 9:39 AM schrieb Arthur Bied-Charreton:
> ...and typo in name.
> 
> Original patch:
> https://lore.proxmox.com/pve-devel/20211028114150.3245864-6-s.reiter@proxmox.com/
> 

Pre-existing, but the commit message/title should describe what exactly
is fixed. Also, might be nicer to have the renaming be a separate commit.

> Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  www/manager6/form/VMCPUFlagSelector.js | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
> index 922fcfa6..74b1a2c4 100644
> --- a/www/manager6/form/VMCPUFlagSelector.js
> +++ b/www/manager6/form/VMCPUFlagSelector.js
> @@ -14,7 +14,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>      scrollable: 'y',
>      height: 200,
>  
> -    unkownFlags: [],
> +    unknownFlags: [],
>  
>      store: {
>          type: 'store',
> @@ -62,32 +62,32 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>              }
>          });
>  
> -        flags += me.unkownFlags.join(';');
> +        flags += (me.unknownFlags.length > 0 ? ';' : '') + me.unknownFlags.join(';');
>  
>          return flags;
>      },
>  
> -    // Adjusts the store for the current value and determines the unkown flags based on what the
> +    // Adjusts the store for the current value and determines the unknown flags based on what the
>      // store does not know.
>      adjustStoreForValue: function () {
>          let me = this;
>          let store = me.getStore();
>          let value = me.value;
>  
> -        me.unkownFlags = [];
> +        me.unknownFlags = [];
>  
>          store.getData().each((rec) => rec.set('state', '='));
>  
>          let flags = value ? value.split(';') : [];
>          flags.forEach(function (flag) {
>              let sign = flag.substr(0, 1);
> -            flag = flag.substr(1);
> +            let flagName = flag.substr(1);
>  
> -            let rec = store.findRecord('name', flag, 0, false, true, true);
> +            let rec = store.findRecord('name', flagName, 0, false, true, true);
>              if (rec !== null) {
>                  rec.set('state', sign);
>              } else {
> -                me.unkownFlags.push(flag);
> +                me.unknownFlags.push(flag);
>              }
>          });
>  





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 2/8] ui: CPUModelSelector: Fix dirty state on default
  2026-03-12  8:40 ` [PATCH pve-manager 2/8] ui: CPUModelSelector: Fix dirty state on default Arthur Bied-Charreton
@ 2026-03-26  9:53   ` Fiona Ebner
  2026-03-26 14:14     ` Arthur Bied-Charreton
  0 siblings, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-26  9:53 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pve-devel

Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> 'originalValue' is set to null in case it's the default, but getValue
> returns an empty string. This means that when editing a VM's CPU config
> when the model is 'default', the form would always be marked dirty.
> 
> Original patch:
> https://lore.proxmox.com/pve-devel/20211028114150.3245864-7-s.reiter@proxmox.com/
> 

I wonder if this is (still) needed? If I remove the getValue() function,
and instead add the following in www/manager6/form/CPUModelSelector.js

    setValue: function (value) {

        let me = this;

        me.callParent([value]);

        console.log('val: ' + value + ' dirty: ' + me.isDirty());

    },


for debugging, it doesn't seem to be dirty when the default is set.

> Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  www/manager6/form/CPUModelSelector.js | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js
> index 2ebd08eb..737b6ff4 100644
> --- a/www/manager6/form/CPUModelSelector.js
> +++ b/www/manager6/form/CPUModelSelector.js
> @@ -39,7 +39,11 @@ Ext.define('PVE.form.CPUModelSelector', {
>          ],
>          width: 360,
>      },
> -
> +    getValue: function () {
> +        let me = this;
> +        let val = me.callParent();
> +        return val === '' ? null : val;
> +    },
>      store: {
>          autoLoad: true,
>          model: 'PVE.data.CPUModel',





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 3/8] ui: CPUModelSelector: Allow filtering out custom models
  2026-03-12  8:40 ` [PATCH pve-manager 3/8] ui: CPUModelSelector: Allow filtering out custom models Arthur Bied-Charreton
@ 2026-03-26  9:59   ` Fiona Ebner
  2026-03-26 14:17     ` Arthur Bied-Charreton
  0 siblings, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-26  9:59 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pve-devel

Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> Add 'allowCustom' configuration parameter. When set to false, only
> default CPU models will be shown.
> 
> Original patch:
> https://lore.proxmox.com/pve-devel/20211028114150.3245864-8-s.reiter@proxmox.com/
> 
> Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  www/manager6/form/CPUModelSelector.js | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js
> index 737b6ff4..2154ff46 100644
> --- a/www/manager6/form/CPUModelSelector.js
> +++ b/www/manager6/form/CPUModelSelector.js
> @@ -19,7 +19,9 @@ Ext.define('PVE.form.CPUModelSelector', {
>      autoSelect: false,
>  
>      deleteEmpty: true,
> -
> +    config: {
> +        allowCustom: true,

Tiny nit: I' slightly prefer showCustom or even slightly more
showCustomModels

> +    },
>      listConfig: {
>          columns: [
>              {
> @@ -100,4 +102,11 @@ Ext.define('PVE.form.CPUModelSelector', {
>              },
>          },
>      },
> +    initComponent: function () {
> +        let me = this;
> +        me.callParent();
> +        if (!me.allowCustom) {
> +            me.getStore().addFilter({ filterFn: (rec) => !rec.data.custom });
> +        }
> +    },
>  });





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 1/8] ui: VMCPUFlagSelector: Fix unknownFlags behaviour
  2026-03-25 15:57   ` Fiona Ebner
@ 2026-03-26 13:47     ` Arthur Bied-Charreton
  0 siblings, 0 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-26 13:47 UTC (permalink / raw)
  To: Fiona Ebner, y; +Cc: pve-devel

On Wed, Mar 25, 2026 at 04:57:42PM +0100, Fiona Ebner wrote:
> Am 12.03.26 um 9:39 AM schrieb Arthur Bied-Charreton:
> > ...and typo in name.
> > 
> > Original patch:
> > https://lore.proxmox.com/pve-devel/20211028114150.3245864-6-s.reiter@proxmox.com/
> > 
> 
> Pre-existing, but the commit message/title should describe what exactly
> is fixed. Also, might be nicer to have the renaming be a separate commit.
> 
ACK, will make this 2 commits and clarify the fix
> > Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
[...]




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 2/8] ui: CPUModelSelector: Fix dirty state on default
  2026-03-26  9:53   ` Fiona Ebner
@ 2026-03-26 14:14     ` Arthur Bied-Charreton
  0 siblings, 0 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-26 14:14 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel

On Thu, Mar 26, 2026 at 10:53:01AM +0100, Fiona Ebner wrote:
> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> > 'originalValue' is set to null in case it's the default, but getValue
> > returns an empty string. This means that when editing a VM's CPU config
> > when the model is 'default', the form would always be marked dirty.
> > 
> > Original patch:
> > https://lore.proxmox.com/pve-devel/20211028114150.3245864-7-s.reiter@proxmox.com/
> > 
> 
> I wonder if this is (still) needed? If I remove the getValue() function,
> and instead add the following in www/manager6/form/CPUModelSelector.js
> 
>     setValue: function (value) {
> 
>         let me = this;
> 
>         me.callParent([value]);
> 
>         console.log('val: ' + value + ' dirty: ' + me.isDirty());
> 
>     },
> 
> 
> for debugging, it doesn't seem to be dirty when the default is set.
> 
You're right, I cannot reproduce it either. Dropping this in the next
iteration.

[...]




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 3/8] ui: CPUModelSelector: Allow filtering out custom models
  2026-03-26  9:59   ` Fiona Ebner
@ 2026-03-26 14:17     ` Arthur Bied-Charreton
  0 siblings, 0 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-26 14:17 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel

On Thu, Mar 26, 2026 at 10:59:24AM +0100, Fiona Ebner wrote:
> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> > Add 'allowCustom' configuration parameter. When set to false, only
> > default CPU models will be shown.
> > 
> > Original patch:
> > https://lore.proxmox.com/pve-devel/20211028114150.3245864-8-s.reiter@proxmox.com/
> > 
> > Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> > ---
> >  www/manager6/form/CPUModelSelector.js | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js
> > index 737b6ff4..2154ff46 100644
> > --- a/www/manager6/form/CPUModelSelector.js
> > +++ b/www/manager6/form/CPUModelSelector.js
> > @@ -19,7 +19,9 @@ Ext.define('PVE.form.CPUModelSelector', {
> >      autoSelect: false,
> >  
> >      deleteEmpty: true,
> > -
> > +    config: {
> > +        allowCustom: true,
> 
> Tiny nit: I' slightly prefer showCustom or even slightly more
> showCustomModels
> 
ACK, will update the variable name in the next iteration of the series
> > +    },
> >      listConfig: {
> >          columns: [
> >              {
> > @@ -100,4 +102,11 @@ Ext.define('PVE.form.CPUModelSelector', {
> >              },
> >          },
> >      },
> > +    initComponent: function () {
> > +        let me = this;
> > +        me.callParent();
> > +        if (!me.allowCustom) {
> > +            me.getStore().addFilter({ filterFn: (rec) => !rec.data.custom });
> > +        }
> > +    },
> >  });
> 




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models
  2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
                   ` (7 preceding siblings ...)
  2026-03-12  8:40 ` [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models Arthur Bied-Charreton
@ 2026-03-26 14:54 ` Fiona Ebner
  2026-03-27 13:07   ` Arthur Bied-Charreton
  8 siblings, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-26 14:54 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pve-devel

Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> This is picked up from an old series [0] by Stefan Reiter.
> 
> As of before this series, the only way to create custom CPU models is by
> editing `/etc/pve/virtual-guest/cpu-models.conf` manually.
> 
> This can be cumbersome for a few reasons, e.g., due to the fact that flags
> misconfigurations are only caught when starting the VM.
> 
> `cpu-flags` endpoint:
> The `cpu-flags` endpoint previously returned a list of hardcoded flags,
> which is both non-exhaustive (some flags I should be able to set are missing),
> and partly incorrect (some flags my host(s) do not support set are returned).
> This is limiting and can lead to misconfigurations. The updated endpoint
> intersects all flags QEMU accepts as `-cpu` arguments with all flags the host
> hardware/emulation actually supports. This way, if I am able to set a flag in
> the UI, I can be confident that the VM will actually be able to start.
> 
> Custom CPU model CRUD functionality:
> Expose CRUD endpoints and UI flow to interact with `cpu-models.conf`. For each
> flag, show a list of the cluster nodes supporting it, and only expose flags that
> at least one node supports to avoid misconfigurations. Filter flags by
> acceleration type (KVM/TCG).
> 
> [0] https://lore.proxmox.com/pve-devel/20211028114150.3245864-1-s.reiter@proxmox.com/

With a pre-existing, manually added model:

[I] root@pve9a1 ~# cat /etc/pve/virtual-guest/cpu-models.conf
cpu-model: nested-for-wsl
	flags +svm
	hidden 1
	hv-vendor-id AuthenticAMD
	reported-model EPYC

When opening it up in the UI, the editor seems to immediately detect a
change, i.e. the OK button is clickable and when I click it, I get the
following error:

Parameter verification failed. (400)
flags: value does not match the regex pattern

It seems to be because of the 'svm' flag not being in the flags grid.
Since users might've already added such models manually, it would be
nice if it would work. Maybe list the unknown flags in the grid without
description?




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor
  2026-03-12  8:40 ` [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor Arthur Bied-Charreton
@ 2026-03-26 15:10   ` Fiona Ebner
  2026-03-27  9:23     ` Arthur Bied-Charreton
  0 siblings, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-26 15:10 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pve-devel

Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> diff --git a/www/manager6/dc/CPUTypeEdit.js b/www/manager6/dc/CPUTypeEdit.js
> new file mode 100644
> index 00000000..8cf508b4
> --- /dev/null
> +++ b/www/manager6/dc/CPUTypeEdit.js

Could also use an onlineHelp reference

---snip 8<---

> +                {
> +                    xtype: 'CPUModelSelector',
> +                    fieldLabel: gettext('Reported Model'),

What about 'Base Model' with a tooltip that it's reported to the guest
(if that is even necessary)? I feel like 'Reported Model' doesn't make
it clear that the rest of the configuration is applied based off that model.

> +                    allowCustom: false,
> +                    name: 'reported-model',
> +                },

---snip 8<---

> diff --git a/www/manager6/dc/CPUTypeView.js b/www/manager6/dc/CPUTypeView.js
> new file mode 100644
> index 00000000..c79ce690
> --- /dev/null
> +++ b/www/manager6/dc/CPUTypeView.js
> @@ -0,0 +1,139 @@
> +Ext.define('pve-custom-cpu-type', {
> +    extend: 'Ext.data.Model',
> +    fields: [
> +        'cputype',
> +        'reported-model',
> +        'hv-vendor-id',
> +        'flags',
> +        'phys-bits',
> +        { name: 'hidden', type: 'boolean' },
> +    ],
> +});
> +
> +Ext.define('PVE.dc.CPUTypeView', {
> +    extend: 'Ext.grid.GridPanel',
> +    alias: ['widget.pveCPUTypeView'],
> +
> +    onlineHelp: 'qm_cpu',

Maybe link directly to the CPU type section?

---snip 8<---

> +    columns: [
> +        {
> +            header: gettext('Name'),
> +            flex: 1,
> +            dataIndex: 'cputype',
> +            renderer: (val) => val.replace(/^custom-/, ''),
> +        },
> +        {
> +            header: gettext('Reported Model'),

Same as above, I'd prefer "Base Model".

> +            flex: 1,
> +            dataIndex: 'reported-model',
> +        },
> +        {
> +            header: gettext('Phys-Bits'),
> +            flex: 1,
> +            dataIndex: 'phys-bits',
> +        },
> +        {
> +            header: gettext('Hidden'),
> +            flex: 1,
> +            dataIndex: 'hidden',
> +        },
> +        {
> +            header: gettext('HyperV-Vendor'),
> +            flex: 1,
> +            dataIndex: 'hv-vendor-id',
> +        },
> +        {
> +            header: gettext('Flags'),
> +            flex: 2,
> +            dataIndex: 'flags',
> +        },
> +    ],
> +
> +    tbar: [
> +        {
> +            text: gettext('Add'),
> +            handler: 'onAdd',
> +        },
> +        '-',
> +        {
> +            xtype: 'proxmoxStdRemoveButton',
> +            baseurl: '/api2/extjs/nodes/localhost/capabilities/qemu/cpu/model/',
> +            getRecordName: (rec) => rec.data.cputype,
> +            getUrl: function (rec) {
> +                let me = this;
> +                return me.baseurl + rec.data.cputype;
> +            },
> +            callback: 'reload',

Currently, the confirm dialog shows:
"Are you sure you want to remove entry 'custom-nested-for-wsl'?"
Would be nicer along the lines of
"Are you sure you want to remove the custom CPU model 'nested-for-wsl'"
if that can be done without much effort. Otherwise, not too important.

> +        },
> +        {
> +            text: gettext('Edit'),
> +            handler: 'onEdit',
> +        },
> +    ],
> +
> +    selModel: {
> +        xtype: 'rowmodel',
> +    },
> +
> +    listeners: {
> +        itemdblclick: function (_, rec) {
> +            let me = this;
> +            me.getController().showEditor(rec.data.cputype);
> +        },
> +    },
> +
> +    initComponent: function () {
> +        let me = this;
> +        me.callParent();
> +        Proxmox.Utils.monStoreErrors(me, me.store);
> +    },
> +});
> diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
> index b5e27a21..629e4fc8 100644
> --- a/www/manager6/dc/Config.js
> +++ b/www/manager6/dc/Config.js
> @@ -146,6 +146,12 @@ Ext.define('PVE.dc.Config', {
>                      title: gettext('Replication'),
>                      itemId: 'replication',
>                  },
> +                {
> +                    xtype: 'pveCPUTypeView',
> +                    iconCls: 'fa fa-microchip',
> +                    title: gettext('Custom CPU models'),
> +                    itemId: 'cputypes',
> +                },

I feel like this might better fit further below, after the directory and
resource mappings items.

I wonder if we should collect the two mappings and this in a common
section, but I can't come up with a good name right now, something akin
to "Guest Resources/Hardware"? But that is something to be further
discussed so should be ordered at the end of the series or as a follow-up.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models
  2026-03-12  8:40 ` [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models Arthur Bied-Charreton
@ 2026-03-26 15:22   ` Fiona Ebner
  2026-03-27  9:34     ` Arthur Bied-Charreton
  2026-03-26 15:40   ` Maximiliano Sandoval
  1 sibling, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-26 15:22 UTC (permalink / raw)
  To: Arthur Bied-Charreton, pve-devel

Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> Add CPU flag editor to the CPUTypeEdit component, using the VMCPUFlagSelector
> also used in the VM creation flow. By default, only show the CPU flags that
> are currently meant to be shown in the VM creation window, see [0]. When in

In the VM Hardware/creation window, I think we should not allow
selecting the accelerator for the flags by the user, but use the
accelerator that the VM currently has configured. With a
hint/description that this is the currently configured one.

Nice work so far from both you and the original author!

> CPUTypeEdit, show all available flags.
> 
> For each flag in VMCPUFlagSelector, also display which node(s) it is available
> on to limit misconfigurations.
> 

---snip 8<---

> diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
> index 74b1a2c4..06c9d9f1 100644
> --- a/www/manager6/form/VMCPUFlagSelector.js
> +++ b/www/manager6/form/VMCPUFlagSelector.js
> @@ -1,3 +1,19 @@
> +const VM_CPU_FLAGS_SUBSET = {
> +    aes: true,
> +    'amd-no-ssb': true,
> +    'amd-ssbd': true,
> +    'hv-evmcs': true,
> +    'hv-tlbflush': true,
> +    ibpb: true,
> +    'md-clear': true,
> +    'nested-virt': true,
> +    pcid: true,
> +    pdpe1gb: true,
> +    'spec-ctrl': true,
> +    ssbd: true,
> +    'virt-ssbd': true,

I'd rather not have the list hard-coded here if it can be avoided and
from what we discussed for the qemu-server patches I think it won't be
needed anymore, right?




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models
  2026-03-12  8:40 ` [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models Arthur Bied-Charreton
  2026-03-26 15:22   ` Fiona Ebner
@ 2026-03-26 15:40   ` Maximiliano Sandoval
  2026-03-27  7:48     ` Arthur Bied-Charreton
  1 sibling, 1 reply; 34+ messages in thread
From: Maximiliano Sandoval @ 2026-03-26 15:40 UTC (permalink / raw)
  To: Arthur Bied-Charreton; +Cc: pve-devel

Arthur Bied-Charreton <a.bied-charreton@proxmox.com> writes:

> Add CPU flag editor to the CPUTypeEdit component, using the VMCPUFlagSelector
> also used in the VM creation flow. By default, only show the CPU flags that
> are currently meant to be shown in the VM creation window, see [0]. When in
> CPUTypeEdit, show all available flags.
>
> For each flag in VMCPUFlagSelector, also display which node(s) it is available
> on to limit misconfigurations.
>
> Original patch:
> https://lore.proxmox.com/pve-devel/20211028114150.3245864-10-s.reiter@proxmox.com
>
> [0] https://git.proxmox.com/?p=qemu-server.git;a=blob;f=src/PVE/QemuServer/CPUConfig.pm;h=32ec495422791422f20caa928d6b632b3de4fcd9;hb=refs/heads/master#l349
>
> Originally-by: Stefan Reiter <s.reiter@proxmox.com>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  www/manager6/dc/CPUTypeEdit.js         |  11 ++-
>  www/manager6/form/CPUModelSelector.js  |   1 +
>  www/manager6/form/VMCPUFlagSelector.js | 121 ++++++++++++++++++++++---
>  3 files changed, 117 insertions(+), 16 deletions(-)
>
> diff --git a/www/manager6/dc/CPUTypeEdit.js b/www/manager6/dc/CPUTypeEdit.js
> index 8cf508b4..d6e06601 100644
> --- a/www/manager6/dc/CPUTypeEdit.js
> +++ b/www/manager6/dc/CPUTypeEdit.js
> @@ -84,7 +84,16 @@ Ext.define('PVE.dc.CPUTypeEdit', {
>                      name: 'phys-bits',
>                  },
>              ],
> -
> +            columnB: [
> +                {
> +                    xtype: 'vmcpuflagselector',
> +                    fieldLabel: gettext('Extra CPU flags'),
> +                    name: 'flags',
> +                    restrictToVMFlags: false,
> +                    height: 380,
> +                    hideHeaders: false,
> +                },
> +            ],
>          },
>      ],
>  });
> diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js
> index 2154ff46..683fa469 100644
> --- a/www/manager6/form/CPUModelSelector.js
> +++ b/www/manager6/form/CPUModelSelector.js
> @@ -17,6 +17,7 @@ Ext.define('PVE.form.CPUModelSelector', {
>      anyMatch: true,
>      forceSelection: true,
>      autoSelect: false,
> +    triggerAction: 'query',
>  
>      deleteEmpty: true,
>      config: {
> diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
> index 74b1a2c4..06c9d9f1 100644
> --- a/www/manager6/form/VMCPUFlagSelector.js
> +++ b/www/manager6/form/VMCPUFlagSelector.js
> @@ -1,3 +1,19 @@
> +const VM_CPU_FLAGS_SUBSET = {
> +    aes: true,
> +    'amd-no-ssb': true,
> +    'amd-ssbd': true,
> +    'hv-evmcs': true,
> +    'hv-tlbflush': true,
> +    ibpb: true,
> +    'md-clear': true,
> +    'nested-virt': true,
> +    pcid: true,
> +    pdpe1gb: true,
> +    'spec-ctrl': true,
> +    ssbd: true,
> +    'virt-ssbd': true,
> +};
> +
>  Ext.define('PVE.form.VMCPUFlagSelector', {
>      extend: 'Ext.grid.Panel',
>      alias: 'widget.vmcpuflagselector',
> @@ -6,6 +22,10 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>          field: 'Ext.form.field.Field',
>      },
>  
> +    config: {
> +        restrictToVMFlags: true,
> +    },
> +
>      disableSelection: true,
>      columnLines: false,
>      selectable: false,
> @@ -17,27 +37,18 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>      unknownFlags: [],
>  
>      store: {
> -        type: 'store',
> -        fields: ['name', { name: 'state', defaultValue: '=' }, 'description'],
> -        autoLoad: true,
> +        fields: ['name', { name: 'state', defaultValue: '=' }, 'description', 'supported-on'],
> +        autoLoad: false,
>          proxy: {
>              type: 'proxmox',
>              url: '/api2/json/nodes/localhost/capabilities/qemu/cpu-flags',
> +            extraParams: { accel: 'kvm' },
>          },
>          listeners: {
>              update: function () {
>                  this.commitChanges();
>              },
> -            refresh: function (store, eOpts) {
> -                let me = this;
> -                let view = me.view;
> -
> -                if (store.adjustedForValue !== view.value) {
> -                    view.adjustStoreForValue();
> -                }
> -            },
>          },
> -        adjustedForValue: undefined,
>      },
>  
>      getValue: function () {
> @@ -86,14 +97,18 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>              let rec = store.findRecord('name', flagName, 0, false, true, true);
>              if (rec !== null) {
>                  rec.set('state', sign);
> +                rec.commit();
>              } else {
>                  me.unknownFlags.push(flag);
>              }
>          });
>  
> -        store.adjustedForValue = value;
> +        me.getView().refresh();
> +    },
> +    isDirty: function () {
> +        let me = this;
> +        return me.originalValue !== me.getValue();
>      },
> -
>      setValue: function (value) {
>          let me = this;
>  
> @@ -109,6 +124,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>      },
>      columns: [
>          {
> +            text: gettext('State'),
>              dataIndex: 'state',
>              renderer: function (v) {
>                  switch (v) {
> @@ -125,6 +141,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>              width: 65,
>          },
>          {
> +            text: gettext('Set'),

This would benefit from a TRANSLATORS comment. Is this a noun (set as in
set theory)? is it a verb (to set)? From the point of view of
translators there is not enough context to decide.

>              xtype: 'widgetcolumn',
>              dataIndex: 'state',
>              width: 95,
> @@ -171,22 +188,96 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>              },
>          },
>          {
> +            text: gettext('Flag'),
>              dataIndex: 'name',
>              width: 100,
>          },
>          {
> +            text: gettext('Description'),
>              dataIndex: 'description',
> +            sortable: false,
> +            cellWrap: true,
> +            flex: 3,
> +        },
> +        {
> +            text: gettext('Supported On'),
> +            dataIndex: 'supported-on',
>              cellWrap: true,
>              flex: 1,
> +            renderer: (v) => (Array.isArray(v) ? v.join(', ') : ''),
>          },
>      ],
>  
>      initComponent: function () {
>          let me = this;
>  
> +        me.unknownFlags = [];
>          me.value = me.originalValue = '';
> -        me.store.view = me;
> +
> +        me.dockedItems = [
> +            {
> +                xtype: 'toolbar',
> +                dock: 'top',
> +                items: [
> +                    {
> +                        xtype: 'tbtext',
> +                        text: gettext('Acceleration:'),
> +                        autoEl: {
> +                            tag: 'span',
> +                            'data-qtip': gettext(
> +                                'A custom CPU model using acceleration-specific flags should only be assigned to VMs configured with the matching acceleration type, i.e., `kvm: off` for TCG, or `kvm: on` for KVM.',

I would recommend to use quotes `"`, single-quotes `'` in user-facing
strings. Backticks are a markup/markdown concept. See [1].

off-topic: In principle one should use “” rather than "", but I don't
think there is precedent for this the codebase.

[1] https://en.wikipedia.org/wiki/Quotation_mark#In_English

> +                            ),
> +                        },
> +                    },
> +                    {
> +                        xtype: 'radiogroup',
> +                        layout: 'hbox',
> +                        validateOnChange: false,
> +                        items: [
> +                            {
> +                                boxLabel: 'KVM',
> +                                inputValue: 'kvm',
> +                                name: 'accel',
> +                                checked: true,
> +                                isFormField: false,
> +                            },
> +                            {
> +                                boxLabel: 'TCG',
> +                                inputValue: 'tcg',
> +                                name: 'accel',
> +                                margin: '0 0 0 10',
> +                                isFormField: false,
> +                            },
> +                        ],
> +                        listeners: {
> +                            change: function (_, value) {
> +                                let view = this.up('grid');
> +                                let proxy = view.getStore().getProxy();
> +                                let accel = value.accel;
> +                                if (accel) {
> +                                    proxy.setExtraParam('accel', accel);
> +                                } else {
> +                                    delete proxy.extraParams.accel;
> +                                }
> +                                view.getStore().load();
> +                            },
> +                        },
> +                    },
> +                ],
> +            },
> +        ];
>  
>          me.callParent(arguments);
> +
> +        me.getStore().on('load', function (store, _, success) {
> +            if (success) {
> +                if (me.restrictToVMFlags) {
> +                    store.filterBy((rec) => VM_CPU_FLAGS_SUBSET[rec.get('name')] === true);
> +                }
> +                me.adjustStoreForValue();
> +                me.checkDirty();
> +            }
> +        });
> +        me.getStore().load();
>      },
>  });

-- 
Maximiliano




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models
  2026-03-26 15:40   ` Maximiliano Sandoval
@ 2026-03-27  7:48     ` Arthur Bied-Charreton
  0 siblings, 0 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-27  7:48 UTC (permalink / raw)
  To: Maximiliano Sandoval; +Cc: pve-devel

On Thu, Mar 26, 2026 at 04:40:12PM +0100, Maximiliano Sandoval wrote:
> Arthur Bied-Charreton <a.bied-charreton@proxmox.com> writes:
> 
[...]
> >          {
> > +            text: gettext('Set'),
> 
> This would benefit from a TRANSLATORS comment. Is this a noun (set as in
> set theory)? is it a verb (to set)? From the point of view of
> translators there is not enough context to decide.
> 
It is 'to set' (German 'setzen'), but on second thought it does not feel
right to describe a value with a verb like this, maybe something like
'Value' would be better. Thanks for the feedback!
[...]
> > +                                'A custom CPU model using acceleration-specific flags should only be assigned to VMs configured with the matching acceleration type, i.e., `kvm: off` for TCG, or `kvm: on` for KVM.',
> 
> I would recommend to use quotes `"`, single-quotes `'` in user-facing
> strings. Backticks are a markup/markdown concept. See [1].
> 
> off-topic: In principle one should use “” rather than "", but I don't
> think there is precedent for this the codebase.
> 
> [1] https://en.wikipedia.org/wiki/Quotation_mark#In_English
> 
You're right, those look weird when not rendered. Will change that in
v2, thanks!
> 
[...]
> -- 
> Maximiliano




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor
  2026-03-26 15:10   ` Fiona Ebner
@ 2026-03-27  9:23     ` Arthur Bied-Charreton
  2026-03-27  9:32       ` Fiona Ebner
  0 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-27  9:23 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel

On Thu, Mar 26, 2026 at 04:10:34PM +0100, Fiona Ebner wrote:

Thanks for the feedback! 

Regarding the onlineHelp feedback, I will add an anchor to CPU Type 
section in qm.adoc and link to that, qm_cpu is currently the 
closest we can link to otherwise.

> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> 
[...]
> > +                {
> > +                    xtype: 'CPUModelSelector',
> > +                    fieldLabel: gettext('Reported Model'),
> 
> What about 'Base Model' with a tooltip that it's reported to the guest
> (if that is even necessary)? I feel like 'Reported Model' doesn't make
> it clear that the rest of the configuration is applied based off that model.
> 
I agree that "Base Model" makes more sense than "Reported Model",
however the latter is better aligned with the SectionConfig key.

In order for pvesh to be consistent with the UI, we would need to expose 
`base-model` in the `custom-cpu-models` API and translate it to 
`reported-model` in the handlers. Which would however still not be 
consistent with the actual config file content and might lead to confusion 
for users who are/were manually editing the file.

`reported-model` seems to be quite sticky, changing the SectionConfig
key looks like a pretty big refactor? 

What do you think? Would we be okay with the naming inconcistency, and
if so at what level should the break happen? Otherwise we could keep
"Reported Model" and add a tooltip explaining it to avoid confusion.

> > +                    allowCustom: false,
> > +                    name: 'reported-model',
> > +                },
[...]
> 
> Currently, the confirm dialog shows:
> "Are you sure you want to remove entry 'custom-nested-for-wsl'?"
> Would be nicer along the lines of
> "Are you sure you want to remove the custom CPU model 'nested-for-wsl'"
> if that can be done without much effort. Otherwise, not too important.
> 
It's possible, I thought one could only pass static strings to
`proxmoxStdRemoveButton`'s `confirmMsg` property, but turns out it also
accepts callbacks (`function(rec)`). Will be updated in v2.
> > +        },
[...]
> > +                {
> > +                    xtype: 'pveCPUTypeView',
> > +                    iconCls: 'fa fa-microchip',
> > +                    title: gettext('Custom CPU models'),
> > +                    itemId: 'cputypes',
> > +                },
> 
> I feel like this might better fit further below, after the directory and
> resource mappings items.
Makes sense, will move it down.
> 
> I wonder if we should collect the two mappings and this in a common
> section, but I can't come up with a good name right now, something akin
> to "Guest Resources/Hardware"? But that is something to be further
> discussed so should be ordered at the end of the series or as a follow-up.

I appended a commit to v2 implementing this so we can iterate on it :)





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor
  2026-03-27  9:23     ` Arthur Bied-Charreton
@ 2026-03-27  9:32       ` Fiona Ebner
  2026-03-27  9:34         ` Arthur Bied-Charreton
  0 siblings, 1 reply; 34+ messages in thread
From: Fiona Ebner @ 2026-03-27  9:32 UTC (permalink / raw)
  To: Arthur Bied-Charreton; +Cc: pve-devel

Am 27.03.26 um 10:22 AM schrieb Arthur Bied-Charreton:
> On Thu, Mar 26, 2026 at 04:10:34PM +0100, Fiona Ebner wrote:
>> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
>>
> [...]
>>> +                {
>>> +                    xtype: 'CPUModelSelector',
>>> +                    fieldLabel: gettext('Reported Model'),
>>
>> What about 'Base Model' with a tooltip that it's reported to the guest
>> (if that is even necessary)? I feel like 'Reported Model' doesn't make
>> it clear that the rest of the configuration is applied based off that model.
>>
> I agree that "Base Model" makes more sense than "Reported Model",
> however the latter is better aligned with the SectionConfig key.
> 
> In order for pvesh to be consistent with the UI, we would need to expose 
> `base-model` in the `custom-cpu-models` API and translate it to 
> `reported-model` in the handlers. Which would however still not be 
> consistent with the actual config file content and might lead to confusion 
> for users who are/were manually editing the file.
> 
> `reported-model` seems to be quite sticky, changing the SectionConfig
> key looks like a pretty big refactor? 
> 
> What do you think? Would we be okay with the naming inconcistency, and
> if so at what level should the break happen? Otherwise we could keep
> "Reported Model" and add a tooltip explaining it to avoid confusion.
> 

You don't need to change it in the backend. There's no real need for
user-facing strings in the UI to be consistent with property keys in the
API schema. Things may be called differently in the UI if those names
are better from a user perspective.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models
  2026-03-26 15:22   ` Fiona Ebner
@ 2026-03-27  9:34     ` Arthur Bied-Charreton
  0 siblings, 0 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-27  9:34 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel

On Thu, Mar 26, 2026 at 04:22:24PM +0100, Fiona Ebner wrote:
> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> > Add CPU flag editor to the CPUTypeEdit component, using the VMCPUFlagSelector
> > also used in the VM creation flow. By default, only show the CPU flags that
> > are currently meant to be shown in the VM creation window, see [0]. When in
> 
> In the VM Hardware/creation window, I think we should not allow
> selecting the accelerator for the flags by the user, but use the
> accelerator that the VM currently has configured. With a
> hint/description that this is the currently configured one.
> 
Good idea, I will add a `kvm` config property to VMCPUFlagSelector so we
can differentiate and show the hint.

> Nice work so far from both you and the original author!
Thanks :)
> 
> > CPUTypeEdit, show all available flags.
> > 
> > For each flag in VMCPUFlagSelector, also display which node(s) it is available
> > on to limit misconfigurations.
> > 
> 
> ---snip 8<---
> 
> > diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
> > index 74b1a2c4..06c9d9f1 100644
> > --- a/www/manager6/form/VMCPUFlagSelector.js
> > +++ b/www/manager6/form/VMCPUFlagSelector.js
> > @@ -1,3 +1,19 @@
> > +const VM_CPU_FLAGS_SUBSET = {
> > +    aes: true,
> > +    'amd-no-ssb': true,
> > +    'amd-ssbd': true,
> > +    'hv-evmcs': true,
> > +    'hv-tlbflush': true,
> > +    ibpb: true,
> > +    'md-clear': true,
> > +    'nested-virt': true,
> > +    pcid: true,
> > +    pdpe1gb: true,
> > +    'spec-ctrl': true,
> > +    ssbd: true,
> > +    'virt-ssbd': true,
> 
> I'd rather not have the list hard-coded here if it can be avoided and
> from what we discussed for the qemu-server patches I think it won't be
> needed anymore, right?
Yea this is ugly & already dropped from v2, I moved everything to the
backend when addressing your feedback for the cpu-flags endpoint :)




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor
  2026-03-27  9:32       ` Fiona Ebner
@ 2026-03-27  9:34         ` Arthur Bied-Charreton
  0 siblings, 0 replies; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-27  9:34 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel

On Fri, Mar 27, 2026 at 10:32:52AM +0100, Fiona Ebner wrote:
> Am 27.03.26 um 10:22 AM schrieb Arthur Bied-Charreton:
> > On Thu, Mar 26, 2026 at 04:10:34PM +0100, Fiona Ebner wrote:
> >> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> >>
> > [...]
> >>> +                {
> >>> +                    xtype: 'CPUModelSelector',
> >>> +                    fieldLabel: gettext('Reported Model'),
> >>
> >> What about 'Base Model' with a tooltip that it's reported to the guest
> >> (if that is even necessary)? I feel like 'Reported Model' doesn't make
> >> it clear that the rest of the configuration is applied based off that model.
> >>
> > I agree that "Base Model" makes more sense than "Reported Model",
> > however the latter is better aligned with the SectionConfig key.
> > 
> > In order for pvesh to be consistent with the UI, we would need to expose 
> > `base-model` in the `custom-cpu-models` API and translate it to 
> > `reported-model` in the handlers. Which would however still not be 
> > consistent with the actual config file content and might lead to confusion 
> > for users who are/were manually editing the file.
> > 
> > `reported-model` seems to be quite sticky, changing the SectionConfig
> > key looks like a pretty big refactor? 
> > 
> > What do you think? Would we be okay with the naming inconcistency, and
> > if so at what level should the break happen? Otherwise we could keep
> > "Reported Model" and add a tooltip explaining it to avoid confusion.
> > 
> 
> You don't need to change it in the backend. There's no real need for
> user-facing strings in the UI to be consistent with property keys in the
> API schema. Things may be called differently in the UI if those names
> are better from a user perspective.
Okay, will only change it in the UI then, thanks!




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models
  2026-03-26 14:54 ` [PATCH manager/qemu-server 0/8] Add API and UI " Fiona Ebner
@ 2026-03-27 13:07   ` Arthur Bied-Charreton
  2026-03-27 13:28     ` Fiona Ebner
  0 siblings, 1 reply; 34+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-27 13:07 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: pve-devel

On Thu, Mar 26, 2026 at 03:54:53PM +0100, Fiona Ebner wrote:
> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
> > This is picked up from an old series [0] by Stefan Reiter.
> > 
> > As of before this series, the only way to create custom CPU models is by
> > editing `/etc/pve/virtual-guest/cpu-models.conf` manually.
> > 
> > This can be cumbersome for a few reasons, e.g., due to the fact that flags
> > misconfigurations are only caught when starting the VM.
> > 
> > `cpu-flags` endpoint:
> > The `cpu-flags` endpoint previously returned a list of hardcoded flags,
> > which is both non-exhaustive (some flags I should be able to set are missing),
> > and partly incorrect (some flags my host(s) do not support set are returned).
> > This is limiting and can lead to misconfigurations. The updated endpoint
> > intersects all flags QEMU accepts as `-cpu` arguments with all flags the host
> > hardware/emulation actually supports. This way, if I am able to set a flag in
> > the UI, I can be confident that the VM will actually be able to start.
> > 
> > Custom CPU model CRUD functionality:
> > Expose CRUD endpoints and UI flow to interact with `cpu-models.conf`. For each
> > flag, show a list of the cluster nodes supporting it, and only expose flags that
> > at least one node supports to avoid misconfigurations. Filter flags by
> > acceleration type (KVM/TCG).
> > 
> > [0] https://lore.proxmox.com/pve-devel/20211028114150.3245864-1-s.reiter@proxmox.com/
> 
> With a pre-existing, manually added model:
> 
> [I] root@pve9a1 ~# cat /etc/pve/virtual-guest/cpu-models.conf
> cpu-model: nested-for-wsl
> 	flags +svm
> 	hidden 1
> 	hv-vendor-id AuthenticAMD
> 	reported-model EPYC
> 
> When opening it up in the UI, the editor seems to immediately detect a
> change, i.e. the OK button is clickable and when I click it, I get the
> following error:
> 
> Parameter verification failed. (400)
> flags: value does not match the regex pattern
> 
Thanks a lot for catching that!

> It seems to be because of the 'svm' flag not being in the flags grid.
Yes, just confirmed that.
> Since users might've already added such models manually, it would be
> nice if it would work. Maybe list the unknown flags in the grid without
> description?
Yes, I will display unknown flags in the grid with empty supported-on
fields. 

On this topic: I currently manually filter out occurrences of svm/vmx to 
replace them with nested-virt. Would it make sense to still display them
in the flags selector, even if they overlap with our abstraction? 




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models
  2026-03-27 13:07   ` Arthur Bied-Charreton
@ 2026-03-27 13:28     ` Fiona Ebner
  0 siblings, 0 replies; 34+ messages in thread
From: Fiona Ebner @ 2026-03-27 13:28 UTC (permalink / raw)
  To: Arthur Bied-Charreton; +Cc: pve-devel

Am 27.03.26 um 2:06 PM schrieb Arthur Bied-Charreton:
> On Thu, Mar 26, 2026 at 03:54:53PM +0100, Fiona Ebner wrote:
>> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
>>> This is picked up from an old series [0] by Stefan Reiter.
>>>
>>> As of before this series, the only way to create custom CPU models is by
>>> editing `/etc/pve/virtual-guest/cpu-models.conf` manually.
>>>
>>> This can be cumbersome for a few reasons, e.g., due to the fact that flags
>>> misconfigurations are only caught when starting the VM.
>>>
>>> `cpu-flags` endpoint:
>>> The `cpu-flags` endpoint previously returned a list of hardcoded flags,
>>> which is both non-exhaustive (some flags I should be able to set are missing),
>>> and partly incorrect (some flags my host(s) do not support set are returned).
>>> This is limiting and can lead to misconfigurations. The updated endpoint
>>> intersects all flags QEMU accepts as `-cpu` arguments with all flags the host
>>> hardware/emulation actually supports. This way, if I am able to set a flag in
>>> the UI, I can be confident that the VM will actually be able to start.
>>>
>>> Custom CPU model CRUD functionality:
>>> Expose CRUD endpoints and UI flow to interact with `cpu-models.conf`. For each
>>> flag, show a list of the cluster nodes supporting it, and only expose flags that
>>> at least one node supports to avoid misconfigurations. Filter flags by
>>> acceleration type (KVM/TCG).
>>>
>>> [0] https://lore.proxmox.com/pve-devel/20211028114150.3245864-1-s.reiter@proxmox.com/
>>
>> With a pre-existing, manually added model:
>>
>> [I] root@pve9a1 ~# cat /etc/pve/virtual-guest/cpu-models.conf
>> cpu-model: nested-for-wsl
>> 	flags +svm
>> 	hidden 1
>> 	hv-vendor-id AuthenticAMD
>> 	reported-model EPYC
>>
>> When opening it up in the UI, the editor seems to immediately detect a
>> change, i.e. the OK button is clickable and when I click it, I get the
>> following error:
>>
>> Parameter verification failed. (400)
>> flags: value does not match the regex pattern
>>
> Thanks a lot for catching that!
> 
>> It seems to be because of the 'svm' flag not being in the flags grid.
> Yes, just confirmed that.
>> Since users might've already added such models manually, it would be
>> nice if it would work. Maybe list the unknown flags in the grid without
>> description?
> Yes, I will display unknown flags in the grid with empty supported-on
> fields. 
> 
> On this topic: I currently manually filter out occurrences of svm/vmx to 
> replace them with nested-virt. Would it make sense to still display them
> in the flags selector, even if they overlap with our abstraction? 

I think we can still show them and don't need to filter them out. In
particular, not having the 'supported-on' for those flags can also lead
to confusion, as can (artificially) hiding them from users. I don't see
why we should force people to use the abstract 'nested-virt'. It can be
convenient for mixed-CPU-vendor clusters, and having the flag be part of
the VM-specific ones was the main motivation to add it.




^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2026-03-27 13:28 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI for custom CPU models Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 1/8] ui: VMCPUFlagSelector: Fix unknownFlags behaviour Arthur Bied-Charreton
2026-03-25 15:57   ` Fiona Ebner
2026-03-26 13:47     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 2/8] ui: CPUModelSelector: Fix dirty state on default Arthur Bied-Charreton
2026-03-26  9:53   ` Fiona Ebner
2026-03-26 14:14     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 3/8] ui: CPUModelSelector: Allow filtering out custom models Arthur Bied-Charreton
2026-03-26  9:59   ` Fiona Ebner
2026-03-26 14:17     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor Arthur Bied-Charreton
2026-03-26 15:10   ` Fiona Ebner
2026-03-27  9:23     ` Arthur Bied-Charreton
2026-03-27  9:32       ` Fiona Ebner
2026-03-27  9:34         ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models Arthur Bied-Charreton
2026-03-26 15:22   ` Fiona Ebner
2026-03-27  9:34     ` Arthur Bied-Charreton
2026-03-26 15:40   ` Maximiliano Sandoval
2026-03-27  7:48     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH qemu-server 6/8] qemu: Add helpers for new custom models endpoints Arthur Bied-Charreton
2026-03-20 17:20   ` Fiona Ebner
2026-03-23  6:56     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH qemu-server 7/8] api: qemu: Extend cpu-flags endpoint to return actually supported flags Arthur Bied-Charreton
2026-03-20 17:20   ` Fiona Ebner
2026-03-23  7:25     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models Arthur Bied-Charreton
2026-03-23 14:46   ` Fiona Ebner
2026-03-23 16:04     ` Arthur Bied-Charreton
2026-03-23 16:10       ` Arthur Bied-Charreton
2026-03-24  9:27         ` Fiona Ebner
2026-03-26 14:54 ` [PATCH manager/qemu-server 0/8] Add API and UI " Fiona Ebner
2026-03-27 13:07   ` Arthur Bied-Charreton
2026-03-27 13:28     ` Fiona Ebner

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