Re: [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor

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

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 :)