Re: [Qemu-devel] [PATCH 1/2] qmp: add query-cpus-fast

[Eduardo Habkost <ehabkost@redhat.com>]

On Thu, Feb 08, 2018 at 02:59:17PM -0600, Eric Blake wrote:
> On 02/08/2018 01:59 PM, Eduardo Habkost wrote:
> > On Wed, Feb 07, 2018 at 12:50:13PM -0500, Luiz Capitulino wrote:
> > > The query-cpus command has an extremely serious side effect:
> > > it always interrupt all running vCPUs so that they can run
> > > ioctl calls. This can cause a huge performance degradation for
> > > some workloads. And most of the information retrieved by the
> > > ioctl calls are not even used by query-cpus.
> > > 
> > > This commit introduces a replacement for query-cpus called
> > > query-cpus-fast, which has the following features:
> > > 
> 
> > > +# Notes: @halted is a transient state that changes frequently.  By the time the
> > > +#        data is sent to the client, the guest may no longer be halted.
> > > +##
> > > +{ 'struct': 'CpuInfo2',
> > > +  'data': {'cpu-index': 'int', '*halted': 'bool', 'qom-path': 'str',
> > > +           'thread-id': 'int', '*props': 'CpuInstanceProperties' } }
> > 
> > This will require duplicating struct fields every time we add a
> > new field to query-cpus-fast (e.g. how would VIktor's
> > CpuInfoS390State patch[1] look like if rebased on top of yours?).
> > 
> > One way to avoid that is to use CpuInfo for both, and make all
> > "slow"  fields optional.  Another option is to use QAPI
> > inheritance, but it could be a little complicated if unions are
> > involved?
> 
> Inheritance is better than optional fields for the sake of introspection
> learning which fields to expect.
> 
> Put the common fields to both interfaces in the base class, then have the
> slower (older) CpuInfo class extend the base class to add the additional
> fields.
> 
> Unions should be able to inherit just fine from structs (after all, a flat
> union requires a struct base); but if we need two layers of unions, we'll
> need to enhance QAPI code generation first.

If we can't do union-union inheritance yet, maybe we can work around it this
way:

# fields that are always returned by both query-cpus and query-cpus-fast
{ 'struct': 'BothCpuInfoBase',
  'data': {'cpu': 'int', 'qom_path': 'str', 'thread_id': 'int',
           '*props': 'CpuInstanceProperties' } }

# fields that are always returned by query-cpus
{ 'struct': 'CpuInfoBase',
  'base': 'BothCpuInfoBase',
  'data': {'current': 'bool', 'halted': 'bool', 'arch': 'CpuInfoArch' } }

# query-cpus return value
{ 'union': 'CpuInfo',
  'base': 'CpuInfoBase',
  'discriminator': 'arch',
  'data': { 'x86': 'CpuInfoX86',
            's390': 'CpuInfoS390',
            'sparc': 'CpuInfoSPARC',
            'ppc': 'CpuInfoPPC',
            'mips': 'CpuInfoMIPS',
            'tricore': 'CpuInfoTricore',
            'other': 'CpuInfoOther' } }

# fields that are always returned by query-cpus-fast
{ 'struct': 'FastCpuInfoBase',
  'base': 'BothCpuInfoBase',
  'data': { 'arch': 'CpuInfoArch' } }

# return value of query-cpus-fast
{ 'union': 'FastCpuInfo',
  'base': 'FastCpuInfoBase',
  'discriminator': 'arch',
  'data': { 'x86': 'CpuInfoOther',
            's390': 'FastCpuInfoS390',
            'sparc': 'CpuInfoOther',
            'ppc': 'CpuInfoOther',
            'mips': 'CpuInfoOther',
            'tricore': 'CpuInfoOther',
            'other': 'CpuInfoOther' } }

# fields returned by both query-cpus and query-cpus-fast on s390
{ 'struct': 'FastCpuInfoS390',
  'data': { 'fast_field': 'int' } }

# fields returned by query-cpus on s390
{ 'struct': 'CpuInfoS390',
  'base': 'FastCpuInfoS390',
  'data': { 'slow_field': 'int' } }

-- 
Eduardo