On Tue, 13 Feb 2018 13:30:02 +0100 Viktor Mihajlovski <mihaj...@linux.vnet.ibm.com> wrote:
> On 12.02.2018 19:15, Luiz Capitulino wrote: > > On Mon, 12 Feb 2018 13:14:32 +0100 > > Viktor Mihajlovski <mihaj...@linux.vnet.ibm.com> wrote: > > > >> -{ 'struct': 'CpuInfoFast', > >> - 'data': {'cpu-index': 'int', 'qom-path': 'str', > >> - 'thread-id': 'int', '*props': 'CpuInstanceProperties' } } > >> +{ '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': 'CpuInfoS390Fast', > >> + 'other': 'CpuInfoOther' } } > > > > Consider this a minor comment (and QMP maintainers can give a much > > better advice than me), but I think this arch list has problems. For > > one thing, it's incomplete. And the second problem is the 'other' > > field. What happens when QEMU starts supporting a new arch? 'other' > > becomes the new arch. Is this compatible? I don't know. > This seems to be the customary approach, see > https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg01986.html > > > > I don't know if this would work with the QAPI, but you could have > > a '*arch-data' field in the CpuInfoFast definition, like: > > > > 'data': { ..., '*arch-data': 'CpuInfoFastArchData' } > > > > where 'CpuInfoFastArchData' is defined by each arch that supports > > the field. An arch supporting the field could also export a > > query_cpus_fast_arch() function, which is called by qmp_query_cpus_fast(). > > > I had it like this in my first reply to your initial patch. However, > that adds an additional hierarchy level in the return data. This > prevents clients like libvirt to reuse the data extraction code when > they switch over to using query-cpus-fast. OK, fair enough.