Re: [PATCH qemu-server v4 03/17] cpu flags: move cpu flags-related utilities to their own module

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

Am 30.04.26 um 6:01 PM schrieb Arthur Bied-Charreton:
> -sub get_supported_cpu_flags {

The caller in src/PVE/API2/Qemu/CPUFlags.pm still does:
PVE::QemuServer::CPUConfig::get_supported_cpu_flags($arch)
and should be updated as part of this patch.

> -    my ($arch) = @_;
> -    $arch = get_host_arch() if !defined($arch);

You changed the logic in the moved function and make $arch required.
Again, the caller in src/PVE/API2/Qemu/CPUFlags.pm would need to be
updated for this.

---snip 8<---

> +my $all_supported_vm_specific_cpu_flags = {};
> +for my $arch ($supported_vm_specific_cpu_flags_by_arch->%*) {
> +    for my $flag ($supported_vm_specific_cpu_flags_by_arch->{$arch}->@*) {
> +        $all_supported_vm_specific_cpu_flags->{ $flag->{name} } = 1;
> +    }
> +}
> +
> +# Understood CPU flags are written to a file at 'pve-qemu' compile time

Nit: pre-existing, but maybe add "and shipped below this directory by
the pve-qemu-kvm package"

> +my $understood_cpu_flag_dir = "/usr/share/kvm";
> +
> +sub supported_cpu_flags_names() {
> +    return sort keys $all_supported_vm_specific_cpu_flags->%*;

Nit: perlcritic complains here:
"return" statement followed by "sort" at line 98, column 5.  Behavior is
undefined if called in scalar context.  (Severity: 5)

Also, you can do the sort once upon module load and then just return
that result.

> +}
> +
> +sub cpu_flag_supported_re() {
> +    return qr/([+-])(@{[join('|', supported_cpu_flags_names())]})/;
> +}
> +
> +sub cpu_flag_any_re() {
> +    return qr/([+-])([a-zA-Z0-9\-_\.]+)/;
> +}
> +
> +# Return supported VM-specific CPU flags.
> +sub get_supported_cpu_flags($arch) {
> +    return $supported_vm_specific_cpu_flags_by_arch->{$arch};
> +}
> +
> +sub query_understood_cpu_flags($arch) {
> +    my $filepath = "$understood_cpu_flag_dir/recognized-CPUID-flags-$arch";
> +
> +    die "Cannot query understood QEMU CPU flags for architecture: $arch (file not found)\n"
> +        if !-e $filepath;
> +
> +    my $raw = PVE::Tools::file_get_contents($filepath);

Nit: please use PVE::File instead

> +    $raw =~ s/^\s+|\s+$//g;
> +    my @flags = split(/\s+/, $raw);
> +
> +    return \@flags;
> +}

Nit: please insert a blank here

> +1;