Re: [PATCH v16 08/11] qapi/s390x/cpu topology: set-cpu-topology monitor command

[Nina Schoetterl-Glausch]

On Mon, 2023-02-27 at 08:59 +0100, Thomas Huth wrote:
> On 24/02/2023 18.15, Nina Schoetterl-Glausch wrote:
> > On Wed, 2023-02-22 at 15:21 +0100, Pierre Morel wrote:
> > > The modification of the CPU attributes are done through a monitor
> > > command.
> > > 
> > > It allows to move the core inside the topology tree to optimize
> > > the cache usage in the case the host's hypervisor previously
> > > moved the CPU.
> > > 
> > > The same command allows to modify the CPU attributes modifiers
> > > like polarization entitlement and the dedicated attribute to notify
> > > the guest if the host admin modified scheduling or dedication of a vCPU.
> > > 
> > > With this knowledge the guest has the possibility to optimize the
> > > usage of the vCPUs.
> > > 
> > > The command has a feature unstable for the moment.
> > > 
> > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > > ---
> > >   qapi/machine-target.json |  35 +++++++++
> > >   include/monitor/hmp.h    |   1 +
> > >   hw/s390x/cpu-topology.c  | 154 +++++++++++++++++++++++++++++++++++++++
> > >   hmp-commands.hx          |  17 +++++
> > >   4 files changed, 207 insertions(+)
> > > 
> > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> > > index a52cc32f09..baa9d273cf 100644
> > > --- a/qapi/machine-target.json
> > > +++ b/qapi/machine-target.json
> > > @@ -354,3 +354,38 @@
> > >   { 'enum': 'CpuS390Polarization',
> > >     'prefix': 'S390_CPU_POLARIZATION',
> > >     'data': [ 'horizontal', 'vertical' ] }
> > > +
> > > +##
> > > +# @set-cpu-topology:
> > > +#
> > > +# @core-id: the vCPU ID to be moved
> > > +# @socket-id: optional destination socket where to move the vCPU
> > > +# @book-id: optional destination book where to move the vCPU
> > > +# @drawer-id: optional destination drawer where to move the vCPU
> > > +# @entitlement: optional entitlement
> > > +# @dedicated: optional, if the vCPU is dedicated to a real CPU
> > > +#
> > > +# Features:
> > > +# @unstable: This command may still be modified.
> > > +#
> > > +# Modifies the topology by moving the CPU inside the topology
> > > +# tree or by changing a modifier attribute of a CPU.
> > > +# Default value for optional parameter is the current value
> > > +# used by the CPU.
> > > +#
> > > +# Returns: Nothing on success, the reason on failure.
> > > +#
> > > +# Since: 8.0
> > > +##
> > > +{ 'command': 'set-cpu-topology',
> > > +  'data': {
> > > +      'core-id': 'uint16',
> > > +      '*socket-id': 'uint16',
> > > +      '*book-id': 'uint16',
> > > +      '*drawer-id': 'uint16',
> > > +      '*entitlement': 'str',
> > 
> > How about you add a machine-common.json and define CpuS390Entitlement there,
> > and then include it from both machine.json and machine-target.json?
> 
> I'm not sure whether double inclusion works with the QAPI parser (since this 
> might code to be generated twice) ... have you tried?

I haven't, the documentation says:

> Include directives
> ------------------
> 
> Syntax::
> 
>     INCLUDE = { 'include': STRING }
> 
> The QAPI schema definitions can be modularized using the 'include' directive::
> 
>  { 'include': 'path/to/file.json' }
> 
> The directive is evaluated recursively, and include paths are relative
> to the file using the directive.  Multiple includes of the same file
> are idempotent.

Which is why I thought it should work, but I guess this is a statement about
including the same file twice in another file and not about including the same
file from two files.

But then, as far as I can tell, the build system only builds qapi-schema.json,
which includes all other files, so it could apply.
> 
> 
>   Thomas
>