Re: [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast

[Laszlo Ersek <lersek@redhat.com>]

On 04/25/18 08:39, Markus Armbruster wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
> 
>> Commit ca230ff33f89 added added the @arch field to @CpuInfoFast, but it
>> failed to set the new field in qmp_query_cpus_fast(), when TARGET_S390X
>> was not defined. The updated @query-cpus-fast example in
>> "qapi-schema.json" showed "arch":"x86" only because qmp_query_cpus_fast()
>> calls g_malloc0() to allocate CpuInfoFast, and the CPU_INFO_ARCH_X86 enum
>> constant is generated with value 0.
>>
>> All @arch values other than @s390 implied the @CpuInfoOther sub-struct for
>> @CpuInfoFast -- at the time of writing the patch --, thus no fields other
>> than @arch needed to be set when TARGET_S390X was not defined. Set @arch
>> now, by copying the corresponding assignments from qmp_query_cpus().
> 
> Now I'm confused.
> 
> In the schema, @arch "riscv" implies CpuInfoRISCV:
> 
>     { 'union': 'CpuInfoFast',
>       'base': {'cpu-index': 'int', 'qom-path': 'str',
>                'thread-id': 'int', '*props': 'CpuInstanceProperties',
>                'arch': 'CpuInfoArch' },
>       'discriminator': 'arch',
>       'data': { 'x86': 'CpuInfoOther',
>                 'sparc': 'CpuInfoOther',
>                 'ppc': 'CpuInfoOther',
>                 'mips': 'CpuInfoOther',
>                 'tricore': 'CpuInfoOther',
>                 's390': 'CpuInfoS390',
>                 'riscv': 'CpuInfoRISCV',
>                 'other': 'CpuInfoOther' } }
> 
> In qmp_query_cpus_fast(), it can't imply anything, because can't occur.
> That's a bug, and this patch fixes it.  Except it sets @arch to "other"
> instead of "riscv" when defined(TARGET_RISCV).  Why?  Oh, I see, that
> gets fixed in the next patch.  Please explain that in your commit
> message, or squash the two patches.  The latter feels simpler, so that's
> what I'd do.

I figured I'd keep one "Fixes:" tag per patch, but I see this separation
confused at least three reviewers, so I'll squash #1 and #2, and will
list two "Fixes:" tags.

Thanks!
Laszlo