Re: [PATCH qemu-server v3 1/2] cpu config: Add 'arch' property to cpu_fmt

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

On Thu, Feb 26, 2026 at 03:01:47PM +0100, Fiona Ebner wrote:
> Please always send a cover-letter if there is more than one patch, see
> [0]. This helps with tooling like b4 (e.g. a R-b for the cover-letter
> will be picked up for the whole series).
I was not aware of this, thank you!

> Am 17.02.26 um 3:11 PM schrieb Arthur Bied-Charreton:
> > Preparatory step for adding support for configuring custom CPU types in
> > the PVE UI.
> > 
> > Add optional property 'arch' (x86_64|aarch64) to cpu_fmt to allow custom
> > models to indicate which architecture they belong to, default to x86_64
> > for backwards compatibility.
> > 
> > Add checks to get_cpu_options and validate_cpu_conf to deny illegal
> > configs early.
> > 
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> 
> I still gave a review below, since the comments are worthwhile for the
> future too, but I'm really not sure anymore if we actually want this.
> Since this was my idea, sorry about that! We'd at least need to also
> check in validate_vm_cpu_conf(), since $cpu_fmt is used there too. But
> again, I don't think it's worthwhile:
> 
> The only use case for an explicit arch for custom models is when the
> reported-model cannot uniquely be associated to an arch. In all other
> cases, the option is just additional friction. Right now, the only
> affected models are 'host' and 'max' models. Supporting mixed arch
> clusters is not planned right now AFAIK, so actually just 'max'. And in
> practice, we can just associate the model to the arch of the VM its
> assigned to. If really needed, users can just define two custom models,
> one with flags for x86_64 and one with flags for aarch64. Even if at
> some point there is a model name conflict between archs for other
> models, it's gonna be very rare that users need both and if they do,
> they can just define two custom models.
> 
> What do you think?
> 
> If we really do see the need to add it (in the future), I think the
> default should rather be "arch of the reported-model and if that is not
> unique, arch of the VM it's assigned to".
> 
I agree, let's just add it if/when we need it. Thanks for the other
feedback as well, will keep it in mind for the future.

[...]
> > +    arch => {
> > +        %{ PVE::JSONSchema::get_standard_option('pve-qm-cpu-arch') },
> > +        default => 'x86_64',
> > +        description => 'The architecture the CPU model belongs to.',
> > +        optional => 1,
> > +    },
> 
> Please note that get_standard_option() has an explicit way to specify a
> base via its second argument, which is a bit more explicit.
> 
> >  };

> >  
> >  my $sev_fmt = {
> > @@ -475,6 +481,13 @@ sub validate_cpu_conf {
> >      my ($cpu) = @_;
> >      # required, but can't be forced in schema since it's encoded in section header for custom models
> >      die "CPU is missing cputype\n" if !$cpu->{cputype};
> > +
> > +    if (my $reported_model = $cpu->{'reported-model'}) {
> > +        my $arch = $cpu->{arch} // $cpu_fmt->{arch}->{default};
> 
> We use this logic three times and the $cpu_fmt name is a bit generic,
> which both suggest that having a helper function get_custom_cpu_arch()
> would be nice.
> 
[...]
> 
> [0]:
> https://lore.proxmox.com/all/176174696524.1776006.10760334798456181474.b4-ty@proxmox.com/