Re: [PATCH pve-qemu 2/3] build: query Hyper-V enlightenment flags for CPU flags list

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

Am 22.05.26 um 3:21 PM schrieb Arthur Bied-Charreton:
> The recognized CPU flags list shipped with pve-qemu-kvm must include
> Hyper-V enlightenment flags so the custom CPU models API can offer them.
> 
> Query them from the 'host-x86_64-cpu' type via QMP (qom-list-properties)
> in parse-cpu-flags.pl, filtering for properties with the 'hv-' prefix,
> and include them in the recognized flags list.
> 
> Filter out non-boolean enlightenments, since the custom CPU model config
> only supports enable/disable.
> 
> Suggested-by: Fiona Ebner <f.ebner@proxmox.com>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  debian/parse-cpu-flags.pl | 42 +++++++++++++++++++++++++++++++++++++++
>  debian/rules              |  2 +-
>  2 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/debian/parse-cpu-flags.pl b/debian/parse-cpu-flags.pl
> index 1847b3e..2b5d32a 100755
> --- a/debian/parse-cpu-flags.pl
> +++ b/debian/parse-cpu-flags.pl
> @@ -2,7 +2,10 @@
>  
>  use warnings;
>  use strict;

Style nit: please add a blank line here.

You could go ahead and use v5.36 and function signatures while at it.

> +use IPC::Open2;
> +use JSON;
>  
> +my ($qemu_bin) = @ARGV;
>  my @flags = ();
>  my $got_flags_section;
>  
> @@ -20,4 +23,43 @@ while (<STDIN>) {
>  
>  die "no QEMU/KVM CPU flags detected from STDIN input" if scalar (@flags) <= 0;
>  
> +my $pid = open2(
> +    my $out,
> +    my $in,
> +    $qemu_bin,
> +    '-machine',
> +    'none',
> +    '-display',
> +    'none',
> +    '-S',
> +    '-qmp',
> +    'stdio',

I'd add a -nodefaults for good measure

> +);
> +
> +sub qmp {
> +    my ($cmd, %args) = @_;
> +    print $in encode_json({ execute => $cmd, %args ? (arguments => \%args) : () }), "\n";
> +    while (my $line = <$out>) {
> +        my $msg = decode_json($line);
> +        next if $msg->{event};
> +        return $msg->{return} if exists $msg->{return};

Style nit: use parentheses for exists()

> +        die "QMP error: " . encode_json($msg->{error}) if $msg->{error};
> +    }
> +}
> +
> +<$out>;
> +qmp('qmp_capabilities');
> +my $props = qmp('qom-list-properties', typename => 'host-x86_64-cpu');

Nice!

> +for my $qo ($props->@*) {
> +    # Filter out non-boolean flags, our custom CPU model config only supports
> +    # enable/disable.
> +    if (index($qo->{name}, 'hv-') == 0 && $qo->{type} eq 'bool') {
> +        push @flags, $qo->{name};
> +    }
> +}
> +qmp('quit');
> +waitpid($pid, 0);
> +
> +@flags = sort @flags;
> +
>  print join("\n", @flags) or die "$!\n";
> diff --git a/debian/rules b/debian/rules
> index c90db29..e3eebe8 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -123,7 +123,7 @@ install: build
>  	rm -f $(destdir)/usr/lib/kvm/virtfs-proxy-helper
>  
>  	# CPU flags are static for QEMU version, allows avoiding more costly checks
> -	$(destdir)/usr/bin/qemu-system-x86_64 -cpu help | ./debian/parse-cpu-flags.pl > $(flagfile)
> +	$(destdir)/usr/bin/qemu-system-x86_64 -cpu help | ./debian/parse-cpu-flags.pl $(destdir)/usr/bin/qemu-system-x86_64 > $(flagfile)

Should we rather also do the call to -cpu help inside the script then?
It feels strange to mix things like this (half way via STDIN, half way
execute command with passed-in binary).

I'm also thinking about whether we should go ahead and use just the QOM
CPU properties rather than relying on the -cpu help output. We would
then need a hardcoded blacklist of boolean properties that are not to be
expose as flags (and also the aliases to avoid duplication). It's not
that long of a list, but going forward, we'll need to be slightly more
careful not to add anything as a flag that we don't want. In practice,
after patch 3/3, every new boolean option needs to be looked at anyways
and then the person doing the rebase needs to check if it is actually a
flag. I'm not fully sure, but it would avoid the mixing info from
different sources altogether. What do you think?

>  
>  	# Supported machine versions are static for a given QEMU binary.
>  	$(destdir)/usr/bin/qemu-system-x86_64 -machine help | ./debian/parse-machines.pl > $(machine_file_x86_64)