[pve-devel] [PATCH v3 qemu 02/12] Write understood CPU flags into static file

[Stefan Reiter]

On 10/17/19 11:07 AM, Thomas Lamprecht wrote:
> On 10/15/19 4:12 PM, Stefan Reiter wrote:
>> located at /usr/share/kvm/cpu-flags-understood-$arch
>>
>> This file can be read by qemu-server's "query_understood_cpu_flags"
>> function, avoiding a more expensive call to QEMU.
>>
>> For now, only x86_64 is implemented, since aarch64 doesn't print any flags when
>> called this way.
>>
>> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
>> ---
>>
>> v5: check for empty file
>>
>>   debian/rules | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/debian/rules b/debian/rules
>> index 8f428c7..bc43ade 100755
>> --- a/debian/rules
>> +++ b/debian/rules
>> @@ -21,6 +21,8 @@ ARCH ?= $(shell dpkg-architecture -qDEB_HOST_GNU_CPU)
>>   PACKAGE=pve-qemu-kvm
>>   destdir := $(CURDIR)/debian/$(PACKAGE)
>>   
>> +flagfile := $(destdir)/usr/share/kvm/cpu-flags-understood-x86_64
>> +
>>   CFLAGS = -Wall
>>   
>>   ifneq (,$(findstring noopt,$(DEB_BUILD_OPTIONS)))
>> @@ -128,6 +130,18 @@ install: build
>>   	rm -Rf $(destdir)/usr/include
>>   	rm -Rf $(destdir)/usr/lib*
>>   
>> +	# write files for understood CPU flags, since these are static for every
>> +	# version (saves a QEMU call in qemu-server at runtime)
>> +	$(destdir)/usr/bin/qemu-system-x86_64 -cpu help \
>> +		| perl -0777 -pe ' \
>> +			s/^.*Recognized CPUID flags://s; # remove up to flags \
>> +			s/\n{2,}.*$$//s; # remove any trailing text \
>> +			s/\s{2,}|\n/\s/g; # remove unnecessary whitespace \
> 
> yeah, replacing with \s won't magically use the whitespace detected before
> but writes an actual s ;) so this is broken.

Whoops, makes sense :)

> 
> But actually I'd like a bit of a different approach, in doing this and
> saving this.
> 
> First, I would prefer a newline separated list, this makes it much easier
> to spot what differs between updates, e.g., with diffoscope.
> 

vimdiff handles space-seperated files well too, since QEMU prints flags 
in alphabetical order - but no objections here.

> Secondly, a debian/ hosted perl script would be preferred, e.g., something
> like debian/parse-cpu-flags.pl:
> ----8<----
> #!/usr/bin/perl
> 
> use warnings;
> use strict;
> 
> my @flags = ();
> my $got_flags_section;
> 
> while (<STDIN>) {
>      if (/^\s*Recognized CPUID flags:/) {
>          $got_flags_section = 1;
>          next;
>      }
>      next if !$got_flags_section;
> 
>      s/^\s+//;
> 
>      push @flags, split(/\s+/);
> }
> 
> print join("\n", @flags) or die "$!\n";
> ---->8----
> 
> you could move the "assert there flags at all" check there too, then a
> simple:
> $(destdir)/usr/bin/qemu-system-x86_64 -cpu help | ./debian/parse-cpu-flags.pl > $(flagfile)
> would be enough in debian rules.

e.g.

die "no CPU flags detected\n" if !scalar(@flags);

in your script above?

> 
> this would let us allow to keep the qemu-server patch as is. As you use
> `split /\s+/` there, it still matches.
> 
> if you agree this can be send outside of this series revision, or done
> by me, whatever you prefer.
> 

Sounds good to me, I'd say if a v6 is needed I'll include it there, if 
the series is applied as is you can just put it in, if it isn't too much 
hassle.

>> +		' > $(flagfile)
>> +
>> +	# check that file is not empty
>> +	[ -s $(flagfile) ]
>> +
>>   # Build architecture-independent files here.
>>   binary-indep: build install
>>   # We have nothing to do by default.
>>
>