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

[Fiona Ebner <f.ebner@proxmox.com>]

Am 16.02.26 um 12:10 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. 'arch' defaults to
> x86_64 for backwards compatibility.
> 
> Update get_cpu_models to only return custom models that match the
> arch to allow querying custom models for a given host architecture.
> 

Please also add a check in validate_cpu_conf() that the 'reported-model'
is available for the configured architecture. It's nicer UX if this
fails early when configuring rather than only later when using.

get_cpu_options() could also get a check that the arch of the custom
model matches the VM arch.

> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  src/PVE/QemuServer/CPUConfig.pm | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
> index 32ec4954..728c5562 100644
> --- a/src/PVE/QemuServer/CPUConfig.pm
> +++ b/src/PVE/QemuServer/CPUConfig.pm
> @@ -374,6 +374,13 @@ my $cpu_fmt = {
>              . " note that doing so will break live migration to CPUs with other values.",
>          optional => 1,
>      },
> +    arch => {

To avoid getting out of sync, you could re-use the existing
'pve-qm-cpu-arch' standard option, overwriting the 'description',
'default' and 'optional' properties.

> +        type => 'string',
> +        enum => [qw(x86_64 aarch64)],
> +        default => 'x86_64',
> +        description => 'The architecture the CPU model belongs to.',
> +        optional => 1,
> +    },
>  };
>  
>  my $sev_fmt = {
> @@ -612,9 +619,14 @@ sub get_cpu_models {
>  
>      my $conf = load_custom_model_conf();
>      for my $custom_model (keys %{ $conf->{ids} }) {
> +        my $custom_model_arch = $conf->{ids}->{$custom_model}->{'arch'};
> +        $custom_model_arch //= $cpu_fmt->{'arch'}->{default};

Style nit: we usually avoid single quotes when not needed for hash keys.
Note that you are not doing it for 'default' either. It's easier to read
if consistent with the other code in the module.

> +        next if ($custom_model_arch ne $arch);

Style nit: no need for parentheses for post-if expression

> +
>          my $reported_model = $conf->{ids}->{$custom_model}->{'reported-model'};
>          $reported_model //= $cpu_fmt->{'reported-model'}->{default};
>          my $vendor = $all_cpu_models->{$reported_model};
> +
>          push @$models,
>              {
>                  name => "custom-$custom_model",