Re: [PATCH pve-manager v4 12/17] ui: add basic custom CPU model editor

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

On Mon, May 11, 2026 at 11:27:06AM +0200, Fiona Ebner wrote:
> Am 30.04.26 um 6:00 PM schrieb Arthur Bied-Charreton:
> > diff --git a/www/manager6/dc/CPUTypeEdit.js b/www/manager6/dc/CPUTypeEdit.js
> > new file mode 100644
> > index 00000000..594772ed
> > --- /dev/null
> > +++ b/www/manager6/dc/CPUTypeEdit.js
> > +    items: [
> > +        {
> > +            xtype: 'inputpanel',
> > +            onGetValues: function (values) {
> > +                let win = this.up('window');
> > +
> > +                PVE.Utils.delete_if_default(values, 'reported-model', '', win.isCreate);
> > +                PVE.Utils.delete_if_default(values, 'hv-vendor-id', '', win.isCreate);
> > +                PVE.Utils.delete_if_default(values, 'phys-bits', '', win.isCreate);
> > +                PVE.Utils.delete_if_default(values, 'flags', '', win.isCreate);
> 
> Should we also do this for the 'hidden' property?
> 
Yea, would make sense, will add it
> > +
> > +                if (win.isCreate) {
> > +                    delete values.delete;
> > +                }
> > +
> > +                return values;
> > +            },
> > +            column1: [
> > +                {
> > +                    xtype: 'pmxDisplayEditField',
> > +                    fieldLabel: gettext('Name'),
> > +                    cbind: {
> > +                        editable: '{isCreate}',
> > +                        value: '{cputype}',
> > +                    },
> > +                    name: 'cputype',
> > +                    renderer: (val) => val.replace(/^custom-/, ''),
> > +                    allowBlank: false,
> > +                },
> > +                {
> > +                    xtype: 'CPUModelSelector',
> > +                    fieldLabel: gettext('Base Model'),
> > +                    showCustomModels: false,
> > +                    name: 'reported-model',
> > +                    autoEl: {
> > +                        tag: 'div',
> > +                        'data-qtip': gettext(
> > +                            'CPU model the rest of the configuration will be based on.',
> 
> Nit: s/will be/is/
> This in particular makes it sound slightly more correct when editing and
> we can still use the same gettext for the grid below.
> 
> Since the historic (back-end) default is the non-recommended kvm64
> model, I think we should have the user make an actual choice here and
> disallow empty for the model selector field. At least in the isCreate
> case, otherwise I guess we need to keep it working for compat with
> pre-existing manually created custom models.
> 
Alright, makes sense, will add in v5 - thanks for the context!
> > diff --git a/www/manager6/dc/CPUTypeView.js b/www/manager6/dc/CPUTypeView.js
> > new file mode 100644
> > index 00000000..207673a7
> > --- /dev/null
> > +++ b/www/manager6/dc/CPUTypeView.js
> > +        {
> > +            header: gettext('Base Model'),
> > +            flex: 1,
> > +            dataIndex: 'reported-model',
> > +            autoEl: {
> > +                tag: 'div',
> > +                'data-qtip': gettext('CPU model the rest of the configuration will be based on.'),
> 
> Nit: s/will be/is/
> 
ack
> > +            },
> > +        },
> > +        {
> > +            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/cluster/qemu/custom-cpu-models/',
> > +            getRecordName: (rec) => rec.data.cputype,
> > +            getUrl: function (rec) {
> > +                let me = this;
> > +                return me.baseurl + rec.data.cputype;
> > +            },
> > +            confirmMsg: function (rec) {
> > +                return Ext.String.format(
> > +                    gettext('Are you sure you want to remove the custom CPU model {0}'),
> > +                    `"${rec.data.cputype.replace(/^custom-/, '')}"?`,
> 
> The question mark should be part of the template string
> 
ack
> > 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
> > +    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,
> > +                },
> 
> I feel like we could be a bit more descriptive here, maybe:
> Default from QEMU
> Inherit from host CPU
> Custom number
> ?
> 
Sounds good! Will add in v5
> > +                {
> > +                    xtype: 'numberfield',
> > +                    width: 60,
> > +                    margin: '0 0 0 10px',
> > +                    minValue: 8,
> > +                    maxValue: 64,
> > +                    reference: 'customNum',
> > +                    allowBlank: false,
> > +                    isFormField: false,
> > +                    disabled: true,
> > +                },
> > +            ],
> > +        },
> > +    ],
> > +});
>