Re: [pve-devel] [PATCH qemu-server v2 2/4] cpu config: factor out gathering common cpu properties

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

Am 02.09.25 um 1:23 PM schrieb Daniel Kral:
> The same logic is already present in print_cpu_device(...),
> get_cpu_options(...), and get_cpu_bitness(...) and will also be used in
> a new helper the next patch, so factor it out in preparation.
> 
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

with two comments below

> ---
>  src/PVE/QemuServer/CPUConfig.pm | 54 +++++++++++----------------------
>  1 file changed, 17 insertions(+), 37 deletions(-)
> 
> diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
> index 786a99d8..f57275dd 100644
> --- a/src/PVE/QemuServer/CPUConfig.pm
> +++ b/src/PVE/QemuServer/CPUConfig.pm
> @@ -492,30 +492,15 @@ sub print_cpu_device {
>      die "Hotplug of non x86_64 CPU not yet supported" if $arch ne 'x86_64';
>  
>      my $kvm = $conf->{kvm} // is_native_arch($arch);
> -    my $cpu = get_default_cpu_type('x86_64', $kvm);
> -    if (my $cputype = $conf->{cpu}) {
> -        my $cpuconf = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $cputype)
> -            or die "Cannot parse cpu description: $cputype\n";
> -        $cpu = $cpuconf->{cputype};
> -
> -        if (my $model = $builtin_models->{$cpu}) {
> -            $cpu = $model->{'reported-model'};
> -        } elsif (is_custom_model($cputype)) {
> -            my $custom_cpu = get_custom_model($cpu);
> -
> -            $cpu = $custom_cpu->{'reported-model'} // $cpu_fmt->{'reported-model'}->{default};
> -        }
> -        if (my $replacement_type = $depreacated_cpu_map->{$cpu}) {
> -            $cpu = $replacement_type;
> -        }
> -    }
> +    my ($cputype) = get_cpu_properties($conf->{cpu}, 'x86_64', $kvm);

Nit: even if it's the only possible value right now, I'd still use $arch
instead of hardcoding 'x86_64'.

>  
>      my $cores = $conf->{cores} || 1;
>  
>      my $current_core = ($id - 1) % $cores;
>      my $current_socket = int(($id - 1 - $current_core) / $cores);
>  
> -    return "$cpu-x86_64-cpu,id=cpu$id,socket-id=$current_socket,core-id=$current_core,thread-id=0";
> +    return
> +        "$cputype-x86_64-cpu,id=cpu$id,socket-id=$current_socket,core-id=$current_core,thread-id=0";
>  }
>  
>  # Resolves multiple arrays of hashes representing CPU flags with metadata to a
> @@ -597,9 +582,8 @@ sub parse_cpuflag_list {
>      return $res;
>  }
>  
> -# Calculate QEMU's '-cpu' argument from a given VM configuration
> -sub get_cpu_options {
> -    my ($conf, $arch, $kvm, $kvm_off, $machine_version, $winversion, $gpu_passthrough) = @_;
> +sub get_cpu_properties {
> +    my ($cpu_prop_str, $arch, $kvm, $kvm_off) = @_;

Alternatively, we could not pass $kvm_off and have the single caller
override its own $kvm_off only when the returned value is defined. Not
sure if that's cleaner, both seem slightly awkward.

>  
>      my $cputype = get_default_cpu_type($arch, $kvm);
>  
> @@ -607,7 +591,7 @@ sub get_cpu_options {
>      my $custom_cpu;
>      my $builtin_cpu;
>      my $hv_vendor_id;
> -    if (my $cpu_prop_str = $conf->{cpu}) {
> +    if ($cpu_prop_str) {
>          $cpu = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $cpu_prop_str)
>              or die "Cannot parse cpu description: $cpu_prop_str\n";
>  
> @@ -632,6 +616,16 @@ sub get_cpu_options {
>          $hv_vendor_id = $cpu->{'hv-vendor-id'} if defined($cpu->{'hv-vendor-id'});
>      }
>  
> +    return ($cputype, $cpu, $custom_cpu, $builtin_cpu, $kvm_off, $hv_vendor_id);
> +}
> +
> +# Calculate QEMU's '-cpu' argument from a given VM configuration
> +sub get_cpu_options {
> +    my ($conf, $arch, $kvm, $kvm_off, $machine_version, $winversion, $gpu_passthrough) = @_;
> +
> +    (my $cputype, my $cpu, my $custom_cpu, my $builtin_cpu, $kvm_off, my $hv_vendor_id) =
> +        get_cpu_properties($conf->{cpu}, $arch, $kvm, $kvm_off);
> +
>      my $pve_flags = get_pve_cpu_flags($conf, $kvm, $cputype, $arch, $machine_version);
>  
>      my $hv_flags =
> @@ -842,21 +836,7 @@ sub get_cpu_bitness {
>  
>      $arch //= get_host_arch();
>  
> -    my $cputype = get_default_cpu_type($arch, 0);
> -
> -    if ($cpu_prop_str) {
> -        my $cpu = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $cpu_prop_str)
> -            or die "Cannot parse cpu description: $cpu_prop_str\n";
> -
> -        $cputype = $cpu->{cputype};
> -
> -        if (my $model = $builtin_models->{$cputype}) {
> -            $cputype = $model->{'reported-model'};
> -        } elsif (is_custom_model($cputype)) {
> -            my $custom_cpu = get_custom_model($cputype);
> -            $cputype = $custom_cpu->{'reported-model'} // $cpu_fmt->{'reported-model'}->{default};
> -        }
> -    }
> +    my ($cputype) = get_cpu_properties($cpu_prop_str, $arch);
>  
>      return $cputypes_32bit->{$cputype} ? 32 : 64 if $arch eq 'x86_64';
>      return 64 if $arch eq 'aarch64';



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel