Re: [qemu-s390x] [Qemu-devel] [PATCH] S390: Expose s390-specific CPU info

[Luiz Capitulino]

On Thu, 8 Feb 2018 18:02:07 +0100
Viktor Mihajlovski <address@hidden> wrote:

> On 08.02.2018 17:22, Luiz Capitulino wrote:
> > On Thu, 8 Feb 2018 16:52:28 +0100
> > Viktor Mihajlovski <address@hidden> wrote:
> >   
> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> index 12c7dc8..0b36860 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -607,7 +607,27 @@
> >>  ##
> >>  { 'struct': 'CpuInfo2',
> >>    'data': {'cpu-index': 'int', '*halted': 'bool', 'qom-path': 'str',
> >> -           'thread-id': 'int', '*props': 'CpuInstanceProperties' } }
> >> +           'thread-id': 'int', '*props': 'CpuInstanceProperties',
> >> +           '*archdata': 'CpuInfoArchData' } }
> >> +
> >> +##
> >> +# @CpuInfoArchData:
> >> +#
> >> +# Architecure specific information about a virtual CPU
> >> +#
> >> +# Since: 2.12
> >> +#
> >> +##
> >> +{ 'union': 'CpuInfoArchData',
> >> +  'base': { 'arch': 'CpuInfoArch' },
> >> +  'discriminator': 'arch',
> >> +  'data': { 'x86': 'CpuInfoOther',
> >> +            'sparc': 'CpuInfoOther',
> >> +            'ppc': 'CpuInfoOther',
> >> +            'mips': 'CpuInfoOther',
> >> +            'tricore': 'CpuInfoOther',
> >> +            's390': 'CpuInfoS390',
> >> +            'other': 'CpuInfoOther' } }
> >>  
> >>  ##
> >>  # @query-cpus-fast:  
> > 
> > I don't think you need CpuInfoArchData, you can have S390CpuState
> > instead and ignore the other archs. It's not like all archs data
> > can be returned at the same time, and also you start having to
> > replicate that arch string list everywhere. Lastly, the arch name
> > is returned by query-target, so no need to duplicate that one either.
> >   
> I don't think I really understood your suggestion. Was it to assume that
> only s390 will have arch-specific data?. I.e. something along the lines of
> -           'thread-id': 'int', '*props': 'CpuInstanceProperties' } }
> +           'thread-id': 'int', '*props': 'CpuInstanceProperties',
> +           '*archdata': 'CpuInfoS390' } }
> 
> or some kind of in-line, anonymous union? I have to confess I'm pretty
> QAPI-illiterate, so I may have done it overly complicated. At least it
> feels that way.

Yes, what you propose above is what I had in mind. Maybe the QAPI has
some better way to do it though.