Re: [Qemu-devel] [PATCH] target-i386: Improve x86_cpu_list output

[Andreas Färber]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 27.02.2013 08:52, schrieb Jan Kiszka:
> On 2013-02-27 08:37, Igor Mammedov wrote:
>> On Wed, 27 Feb 2013 00:26:38 -0300 Eduardo Habkost
>> <address@hidden> wrote:
>> 
>>> On Tue, Feb 26, 2013 at 10:57:56PM +0100, Igor Mammedov wrote:
>>>> On Sat, 23 Feb 2013 16:45:00 +0100 Jan Kiszka
>>>> <address@hidden> wrote:
>>>> 
>>>>> From: Jan Kiszka <address@hidden>
>>>>> 
>>>>> Several issues fixed: - We were missing a bunch of feature
>>>>> lists. Fix this by simply dumping the meta list
>>>>> feature_word_info. - kvm_enabled() cannot be true at this
>>>>> point because accelerators are initialized much later
>>>>> during init. Simply dump unconditionally.
>>>> Why not to move list_cpu after accelerators are initialized?
>>> 
>>> Because help output is simply documentation and shouldn't
>>> depend on any other config option parsing or accelerator
>>> initialization at all?
>> Don't see reason why it shouldn't. It's not a man page but a
>> program and can do pretty much everything.
> 
> Actually, requiring "-enable-kvm -cpu ?" to list the host type
> would be counterproductive - hardly any user will find this out, at
> best by chance. However ...
> 
>> 
>>> 
>>>> 
>>>>> - Add explanation for "host" CPU type.
>>>>> 
>>>>> Signed-off-by: Jan Kiszka <address@hidden> --- 
>>>>> target-i386/cpu.c |   20 +++++++++----------- 1 files
>>>>> changed, 9 insertions(+), 11 deletions(-)
>>>>> 
>>>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c index
>>>>> dfcf86e..6e742f0 100644 --- a/target-i386/cpu.c +++
>>>>> b/target-i386/cpu.c @@ -1453,18 +1453,16 @@ void
>>>>> x86_cpu_list(FILE *f, fprintf_function cpu_fprintf) 
>>>>> snprintf(buf, sizeof(buf), "%s", def->name); 
>>>>> (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf,
>>>>> def->model_id); } -    if (kvm_enabled()) { -
>>>>> (*cpu_fprintf)(f, "x86 %16s\n", "[host]"); -    } +
>>>>> (*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host", +
>>>>> "KVM processor with all supported host features"); +
>>>> that would make 'host' visible to users even if QEMU compiled
>>>> without KVM support. No big harm, but autotest could get
>>>> confused when it gets 'host' CPU but QEMU doesn't run because
>>>> it's not really supported.
>>> 
>>> Then we have to fix the autotest test code to not try it
>>> without KVM. :-)
>>> 
>>> Help output is not a probing mechanism (although we often
>>> misuse it as if it were), and I expect help output to be static
>>> and not depend on any subsystem initialization.
>> Then fix help output and add to "host" line something like " is
>> available with -enable-kvm on command line and if your build was
>> compiled --enable-kvm configure option", otherwise 'host' is
>> misleading. Now even without 'host' in output of -cpu 'help',
>> question why 'host' is not found periodically pops up on IRC.
>> This change will just increase frequency of it.
> 
> ...I will add "(only available in KVM mode)" here and wrap these
> lines in #ifdef CONFIG_KVM. That should be more acceptable, no?

I was about to ask for #ifdef CONFIG_KVM, yes please.

Can you move that into a second patch though? Then I can apply the
feature words fixes earlier. -cpu host has been a controversial and
known-broken topic for a long time already.

Thanks,
Andreas

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nrnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend￶rffer; HRB 16746 AG Nrnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)

iQIcBAEBAgAGBQJRLbzhAAoJEPou0S0+fgE/1lgQAI4RZM+hBIkVgFW9laosOmn6
lP7M+fy5037V4rGPdLuAY+F7BsuH3CBzr11o+QGJCmFJo+1IUI6AvnQNMQkA6S06
b/jmchu+LxQqxPXYF2v5F0MmznLpBOtX4C9A8NaPEW8+nXY7O0UQ1feRiN+vuA6L
PQnRQK6znsxhR/1dnZPEZMHtAqHN2VSYUD/VfUBzw1gYgXq7iyFIfetGEY9op3e4
ZuPh37pd0hkKebIoz7hUKlMf5OuM8ANsEt/exvZbnhH7nT/2UJhITFYLZXrCWqnB
KJoFlvQJjQ2VE8l18cr+fX0MVDpQKKks+svzVIasiPff4WM//Y+cnz44q8/JPlWB
XEAJTKnZHXECeunIw1bGafKZvf/Grrj7QaUGWPStohZO0+d5rkYgKCBNzsHBN468
Z5biu7StZUNW+r2z3YxdF64zg0wT1tXyN4CzI5NHNNKymmuyXt/b/srkGuNhronB
k7AsV4YqI8A7pQRfY9vc3RG17+Mzcbj+uHlnSxunwV8c6SiZ5zXoHpNR3oYnr6z7
5WB3LH9Yre2dcf0xBp+15SzMHkjd7XoGX5viVU1dbumNTvUValLRasQHshaNbdSP
skobPfFAJmXXqZabaGwgtfzo3ivRcFjzikbvAFacVtnx6zHpgpp+PdSZRWv+79uM
fr0q+VNKbnVfnYRyVh3k
=cbHS
-----END PGP SIGNATURE-----