Re: [Qemu-devel] [PATCH v2] QMP: Introduce the documentation for query-qdm

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue, 13 Jul 2010 13:49:46 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> [cc: kraxel]
> 
> I didn't get around to review v1.  Sorry.
> 
> Miguel Di Ciurcio Filho <miguel.filho@gmail.com> writes:
> 
> > Changelog from v1
> > -----------------
> > - renamed "props" to "properties"
> > - updated the examples
> > - reworded the explanations of "name" and "description"
> > - split "type" into a json-object, adding "qmp" and "qdev"
> > - list all possible values for "bus"
> > - list all possible values for "qdev" on "type"
> > - list all possible values for "qmp" on "type"
> > ---
> >  qemu-monitor.hx |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 88 insertions(+), 0 deletions(-)
> >
> > diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> > index 2af3de6..0055d0a 100644
> > --- a/qemu-monitor.hx
> > +++ b/qemu-monitor.hx
> > @@ -2490,6 +2490,94 @@ STEXI
> >  show device tree
> >  @item info qdm
> >  show qdev device model list
> > +ETEXI
> > +SQMP
> > +query-qdm
> > +---------
> > +
> > +Describe the capabilities of all devices registered with qdev.
> > +
> > +The returned output is a json-array, each element is a json-object describing
> > +a single device type.
> > +
> > +Each json-object contains the following:
> > +
> > +- "name": name of the device (json-string)
> > +- "bus": the name of the bus type for the device (json-string)
> > +    - Possible values: PCI, SCSI, I2C, ISA, SSI, USB, virtio-serial-bus, System
> 
> Missing: IDE (hw/ide/qdev.c) and s390-virtio (hw/s390-virtio-bus.c).

What about "System", is it ok to expose it?

> > +- "alias": an alias by which the device is also known (json-string, optional)
> > +- "description": description of the device  (json-string, optional)
> > +- "creatable": whether this device can be created on command line (json-boolean)
> 
> "on the command line" is misleading; it applies to monitor (human & QMP)
> as well.
> 
> "by the user"?

When is a device not "creatable"?

> > +- "properties": a list where each element is an json-object that describes a
> > +  property of the device. Each json-object contains the following:
> > +     - "name": the name of the property (json-string)
> > +     - "type": a json-object that contains the following:
> > +        - "qdev": the internal name of the type of the property (json-string)
> > +            - Possible values: uint8, uint16, uint32, uint64, int32, macaddr,
> > +              drive, chr, string, netdev, bit, taddr

Didn't see this before, but we should not use too short names like "chr" and
"taddr".

> > +        - "qmp": the json equivalent type of the internal type (json-string)
> > +            - Possible values: integer, string, boolean
> 
> Fairly close to JSON Schema, but there are differences.
> 
> Do we need "qdev"?  Is exposing it wise?  Smells a bit too much of
> internal detail for comfort...

Agreed, although this was a suggestion from me.

> Could we use "type" just like JSON Schema?  Drop "qdev" or move it out
> of "type", then make "type" what its member "qmp" is now.

The most important point here is to try to figure out how clients are
going to use this. Actually, the fact that libvirt doesn't actually
need this makes me wonder if we should have it in the first place, at
the same time listing the property name w/o saying what type it accepts
doesn't seem to be that useful either.

Let's look at all possibilities:

 1. Drop "qmp" key and rename "qdev" to "type" (ie. use the qdev type only)

    This was the original proposal. I don't like it because its values
    are quite low-level and have no meaning in json.

    If this is needed, then I think it needs a complement specifying the
    json type for it.

 2. Drop "qdev" key and rename "qmp" to "type" (ie. use the json type only)

    This is what makes sense when self-documenting the protocol, however,
    given the original proposal, I wonder if this is going to be enough.

    That is, say a property takes a MAC address. By using the json type
    we will just say "string", how is the client going to know that it's
    actually a MAC address in a string format?

  3. Use both "qdev" and "qmp" keys

    Then clients can have a matrix of qdev and qmp types, then when
    query-qdm says a property takes qdev=macaddr and qmp=string, clients
    can automatically build the command's argument.

    Now, I'm wondering if the int types make sense...

> Note: qdev property documentation overlaps with QMP command
> self-documentation, because qdev properties are also arguments of
> command device_add.  We may wish to revisit this when we do
> self-documentation.

Hm, maybe this is also true for other commands Daniel has converted..