[pve-devel] [PATCH qemu-server 3/7] Add QEMU CPU flag querying helpers

[Fabian Grünbichler]

On September 2, 2019 4:27 pm, Stefan Reiter wrote:
> * query_understood_cpu_flags returns all flags that QEMU/KVM knows about
> * query_supported_cpu_flags returns all flags that QEMU/KVM can use on
>   this particular host.
> 
> To get supported flags, a temporary VM is started with QEMU, so we can
> issue the "query-cpu-model-expansion" QMP command. This is how libvirt
> queries supported flags for its "host-passthrough" CPU type.
> 
> query_supported_cpu_flags is thus rather slow and shouldn't be called
> unnecessarily.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> 
> Changes from RFC:
> 
> * Clearer regexes
> * Use existing QMP infrastructure
> * Add locking for temporary VM start
> * Add comments
> 
> 
>  PVE/QemuServer.pm | 84 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 8c519b5..97fa955 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3500,6 +3500,90 @@ sub get_command_for_arch($) {
>      return $cmd;
>  }
>  
> +# The format of the flags returned here uses dashes '-' as seperators,

nit: s/seperator/separator/a

> +# this is neither the default for 'query_supported_cpu_flags' below, nor for
> +# /proc/cpuinfo.
> +#
> +# To compare (or array_intersect) flags, it's a good idea to convert them all
> +# to a common format first (e.g. map s/\.|-/_/g).
> +sub query_understood_cpu_flags {

not used anywhere in this series? also, confusingly, this returns less 
values then the other helper (maybe because of some aliases?), and it's 
not a strict subset :-/

I guess this is in preparation of exposing it via the API? some sort of 
comment how _understood and _supported interact/are supposed to interact 
would be nice. either as comment or part of the commit message

> +    my $flags = [];
> +    my $flag_section = 0;
> +
> +    run_command(
> +	[get_command_for_arch('x86_64'), '-cpu', 'help'],

why not get_host_arch()?

> +	noerr => 1,
> +	quiet => 1,
> +	outfunc => sub {
> +	    my ($line) = @_;
> +
> +	    if ($flag_section) {
> +		return if $line =~ m/^\s*$/;
> +		$line =~ s/^\s*|\s*$//g;
> +		push @$flags, split(/\s+/, $line);
> +	    } elsif ($line =~ m/^\s*Recognized CPUID flags:\s*$/) {
> +		$flag_section = 1;
> +	    }
> +	}
> +    );
> +
> +    return $flags;
> +}
> +
> +# This function needs to start a temporary VM and is therefore rather expensive.
> +# If you need the value returned from this, you can get it much cheaper from
> +# pvestatd using PVE::Cluster::get_node_kv('cpuflags').

s/pvestatd/pmxcfs/

pvestatd is just the one putting it there ;)

> +#
> +# See also note to query_understood_cpu_flags above.
> +sub query_supported_cpu_flags {
> +    my $flags = [];
> +
> +    my $vmid = -1;
> +    my $pidfile = pidfile_name($vmid);
> +
> +    PVE::QemuConfig->lock_config($vmid, sub {
> +	# We start a temporary (frozen) VM with vmid -1 to allow us to send a QMP command
> +	my $rc = run_command([
> +	    get_command_for_arch('x86_64'),

again, why not get_host_arch()?

> +	    '-cpu', 'kvm64',
> +	    '-display', 'none',
> +	    '-chardev', "socket,id=qmp,path=/var/run/qemu-server/$vmid.qmp,server,nowait",
> +	    '-mon', 'chardev=qmp,mode=control',
> +	    '-pidfile', $pidfile,
> +	    '-S', '-daemonize'
> +	], noerr => 1, quiet => 1);
> +	return if $rc;
> +
> +	my $cmd_result = vm_mon_cmd_nocheck(
> +	    $vmid,
> +	    'query-cpu-model-expansion',
> +	    type => 'full',
> +	    model => { name => 'host' }
> +	);
> +
> +	my $props = $cmd_result->{model}->{props};
> +	if (%$props) {
> +	    foreach my $prop (keys %$props) {
> +		push @$flags, $prop if "$props->{$prop}" eq '1';
> +	    }
> +	}
> +
> +	# force stop with 10 sec timeout and 'nocheck'
> +	vm_stop(undef, $vmid, 1, 1, 10, 0, 1);
> +    });
> +
> +    # QEMU returns some flags multiple times, with '_', '.' or '-' as seperator
> +    # (e.g. lahf_lm and lahf-lm; sse4.2, sse4-2 and sse4_2; ...).
> +    # We only keep those with underscores, since they match the ones from
> +    # /proc/cpuinfo (they do the same thing, but we get rid of duplicates).
> +    @$flags = grep {
> +	my $replaced = (my $underscore = $_) =~ s/\.|-/_/g;
> +	!($replaced && grep { $_ eq $underscore } @$flags)
> +    } @$flags;
> +
> +    return $flags;
> +}
> +
>  sub get_cpu_options {
>      my ($conf, $arch, $kvm, $machine_type, $kvm_off, $kvmver, $winversion, $gpu_passthrough) = @_;
>  
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>