[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpu
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast |
Date: |
Wed, 25 Apr 2018 14:30:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
On 04/25/18 08:39, Markus Armbruster wrote:
> Laszlo Ersek <address@hidden> 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