Re: [PATCH v24 01/21] qapi: machine.json: change docs regarding CpuInstanceProperties

[Nina Schoetterl-Glausch]

On Thu, 2023-10-12 at 07:59 +0200, Markus Armbruster wrote:
> Nina Schoetterl-Glausch <nsg@linux.ibm.com> writes:
> 
> > Clarify roles of different architectures.
> > Also change things a bit in anticipation of additional members being
> > added.
> > 
> > Suggested-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > ---
> > 
> > 
> > Reference to s390x docs added in patch 14
> > 
> > 
> >  qapi/machine.json | 17 +++++++++--------
> >  1 file changed, 9 insertions(+), 8 deletions(-)
> > 
> > diff --git a/qapi/machine.json b/qapi/machine.json
> > index a08b6576ca..3c074c9902 100644
> > --- a/qapi/machine.json
> > +++ b/qapi/machine.json
> > @@ -70,9 +70,9 @@
> >  # @qom-path: path to the CPU object in the QOM tree
> >  #
> >  # @thread-id: ID of the underlying host thread
> > -#
> > -# @props: properties describing to which node/socket/core/thread
> > -#     virtual CPU belongs to, provided if supported by board
> > +
> 
> Keep the '#', please.
> 
> > +# @props: properties of type CpuInstanceProperties associated with a
> > +#     virtual CPU, e.g. the socket id
> >  #
> >  # @target: the QEMU system emulation target, which determines which
> >  #     additional fields will be listed (since 3.0)
> > @@ -899,6 +899,9 @@
> >  # should be passed by management with device_add command when a CPU is
> >  # being hotplugged.
> >  #
> > +# Which members are optional and which mandatory depends on the 
> > architecture
> > +# and board.
> 
> Long line.  Wrap like this:
> 
>    # Which members are optional and which mandatory depends on the
>    # architecture and board.
> 
> Would "depends on the machine type" be more precise?

Depends on the definition of "machine type" I guess.
"architecture and board" seems more descriptive to me.

> 
> > +#
> >  # @node-id: NUMA node ID the CPU belongs to
> >  #
> >  # @socket-id: socket number within node/board the CPU belongs to
> > @@ -912,15 +915,13 @@
> >  #
> >  # @thread-id: thread number within core the CPU belongs to
> >  #
> > -# Note: currently there are 6 properties that could be present but
> > -#     management should be prepared to pass through other properties
> > -#     with device_add command to allow for future interface extension.
> > -#     This also requires the filed names to be kept in sync with the
> > -#     properties passed to -device/device_add.
> > +# Note: management should be prepared to pass through additional
> > +# properties with device_add.
> 
> Indent the second line for consistency with other Note: sections:
> 
>    # Note: management should be prepared to pass through additional
>    #     properties with device_add.
> 
> >  #
> >  # Since: 2.7
> >  ##
> >  { 'struct': 'CpuInstanceProperties',
> > +  # Keep these in sync with the properties device_add accepts
> >    'data': { '*node-id': 'int',
> >              '*socket-id': 'int',
> >              '*die-id': 'int',
>