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

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

[Miguel Di Ciurcio Filho]

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
+- "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)
+- "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
+        - "qmp": the json equivalent type of the internal type (json-string)
+            - Possible values: integer, string, boolean
+
+Example:
+
+-> { "execute": "query-qdm" }
+<- {
+      "return": [
+        {
+           "name": "virtio-blk-pci",
+           "creatable": true,
+           "bus": "PCI",
+           "properties": [
+             {
+                 "name": "indirect_desc",
+                 "type": {
+                             "qdev": "bit",
+                             "qmp": "boolean"
+                         }
+             },
+             {
+                 "name": "logical_block_size",
+                 "type": {
+                             "qdev": "uint16",
+                             "qmp": "integer"
+                         }
+             },
+             {
+                 "name": "opt_io_size",
+                 "type": {
+                             "qdev": "uint32",
+                             "qmp": "integer"
+                         }
+             }
+             {
+                 "name": "drive",
+                 "type": {
+                             "qdev": "drive",
+                             "qmp": "string"
+                         }
+             }
+           ]
+        },
+        {
+           "name": "virtio-balloon-pci",
+           "creatable": true,
+           "bus": "PCI",
+           "properties": [
+             {
+                 "name": "indirect_desc",
+                 "type": {
+                             "qdev": "bit",
+                             "qmp": "boolean"
+                         }
+             }
+           ]
+        },
+      ....
+    ]
+
+EQMP
+
+STEXI
 @item info roms
 show roms
 @end table
-- 
1.7.1

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

[Luiz Capitulino]

On Fri,  9 Jul 2010 15:13:48 -0300
Miguel Di Ciurcio Filho <miguel.filho@gmail.com> wrote:

> 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

"System", do we really have this? If so, isn't it internal only?

> +- "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)
> +- "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)

I assume this is used in the command-line, right? It's better to say that
rather than "internal", as "internal" doesn't seem to have a purpose for clients.

Otherwise, looks good to me.

Markus, I need your ACK too.

> +            - Possible values: uint8, uint16, uint32, uint64, int32, macaddr,
> +              drive, chr, string, netdev, bit, taddr
> +        - "qmp": the json equivalent type of the internal type (json-string)
> +            - Possible values: integer, string, boolean
> +
> +Example:
> +
> +-> { "execute": "query-qdm" }
> +<- {
> +      "return": [
> +        {
> +           "name": "virtio-blk-pci",
> +           "creatable": true,
> +           "bus": "PCI",
> +           "properties": [
> +             {
> +                 "name": "indirect_desc",
> +                 "type": {
> +                             "qdev": "bit",
> +                             "qmp": "boolean"
> +                         }
> +             },
> +             {
> +                 "name": "logical_block_size",
> +                 "type": {
> +                             "qdev": "uint16",
> +                             "qmp": "integer"
> +                         }
> +             },
> +             {
> +                 "name": "opt_io_size",
> +                 "type": {
> +                             "qdev": "uint32",
> +                             "qmp": "integer"
> +                         }
> +             }
> +             {
> +                 "name": "drive",
> +                 "type": {
> +                             "qdev": "drive",
> +                             "qmp": "string"
> +                         }
> +             }
> +           ]
> +        },
> +        {
> +           "name": "virtio-balloon-pci",
> +           "creatable": true,
> +           "bus": "PCI",
> +           "properties": [
> +             {
> +                 "name": "indirect_desc",
> +                 "type": {
> +                             "qdev": "bit",
> +                             "qmp": "boolean"
> +                         }
> +             }
> +           ]
> +        },
> +      ....
> +    ]
> +
> +EQMP
> +
> +STEXI
>  @item info roms
>  show roms
>  @end table

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

[Markus Armbruster]

[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).

> +- "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"?

> +- "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
> +        - "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...

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.

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.

[...]

Note: query-qdm isn't just a straight port of info qdm to QMP.  It
additionally documents properties.  It's like info qdm + device_add D,?
for all D.  I'm fine with that.

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

[Miguel Di Ciurcio Filho]

On Tue, Jul 13, 2010 at 8:49 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> +- "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).

Ack.

>
>> +- "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"?

Ack.

>> +- "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
>> +        - "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...
>
> 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.
>

I think it is better not to expose too much the internals to. My
initial concern and Daniel's too was to make clear what is the meaning
of the property. Like, give me a proper formated MAC address or an id
of a properly created netdev.

But I think we could drop the qdev properties stuff and return the
proper error in case of parsing problems.

Regards,

Miguel

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

[Luiz Capitulino]

On Tue, 13 Jul 2010 10:30:24 -0300
Miguel Di Ciurcio Filho <miguel.filho@gmail.com> wrote:

[...]

> On Tue, Jul 13, 2010 at 8:49 AM, Markus Armbruster <armbru@redhat.com> wrote:
> >> +- "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
> >> +        - "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...
> >
> > 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.
> >
> 
> I think it is better not to expose too much the internals to. My
> initial concern and Daniel's too was to make clear what is the meaning
> of the property. Like, give me a proper formated MAC address or an id
> of a properly created netdev.
> 
> But I think we could drop the qdev properties stuff and return the
> proper error in case of parsing problems.

Then client writers will have to know how to encode the property ahead of
time, which makes self-documenting useless.

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

[Luiz Capitulino]

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..

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

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> 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?

What makes it special?

>> > +- "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"?

Stuff like i440FX: if your board comes with it, you already got it, if
not, you can't have it.

>> > +- "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.

Adding stuff we need when we need it is easier and less painfull than
taking out stuff when we realize we shouldn't have added it :)

> 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)

Let's start with this, and add more information as needed.

>     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?

I think the primary use for self-documentation is to let clients figure
out whether something they know how to use is actually there.  For that
use, we don't need to describe how MAC addresses look like.

Perhaps other uses could use such information.  For instance, a
hypothetical graphical front end to the QEMU monitor could let the user
configure arbitrary properties without understanding them.

For what it's worth, JSON schema has separate optional properties like
"format", "enum", "minimum", "maximum", and more.

"enum" seems particularly useful.

>   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...

"the int types"?

>> 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..

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

[Luiz Capitulino]

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

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > 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?
> 
> What makes it special?

Not sure I got what you meant.

> 
> >> > +- "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"?
> 
> Stuff like i440FX: if your board comes with it, you already got it, if
> not, you can't have it.

Ok, so it's about devices that can be dynamic created and hot plugged, right?
In this case I think the text should read something like:

 "the device can be dynamically created"

> >> > +- "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.
> 
> Adding stuff we need when we need it is easier and less painfull than
> taking out stuff when we realize we shouldn't have added it :)
> 
> > 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)
> 
> Let's start with this, and add more information as needed.

I would be ok with that, I guess.

> >     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?
> 
> I think the primary use for self-documentation is to let clients figure
> out whether something they know how to use is actually there.  

Then documenting types is not needed.

I think it's beyond that, one possible advanced usage is to allow clients
to automatically catch up with protocol additions.

If we add only the json type, then client writers will have to know beforehand
the qdev type of all properties, which implies that clients will have to be
modified to learn about new properties.

If the problem is the key's name ("qdev"), then we could have:

- "type": a json-object with type information, contains the following:
   - "json": the JSON type of the property, to be used when setting it
             through QMP (json-string)
   - "format": only present when the property requires special formatting
               (json-string, optional)
                 Possible values: macaddr, drive, chardev, netdev, taddr

Now, I only know the format of a macaddr and am assuming that the others
need special formatting too.

> For that use, we don't need to describe how MAC addresses look like.

I didn't actually mean that we should describe how MAC addresses look like,
but that we should say when a property _is_ a MAC address.

> Perhaps other uses could use such information.  For instance, a
> hypothetical graphical front end to the QEMU monitor could let the user
> configure arbitrary properties without understanding them.

That's exactly my point.

> For what it's worth, JSON schema has separate optional properties like
> "format", "enum", "minimum", "maximum", and more.
> 
> "enum" seems particularly useful.
> 
> >   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...
> 
> "the int types"?

From Miguel's proposal:

+        - "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

Those int types don't make sense under QMP.

> 
> >> 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..
> 

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

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Tue, 13 Jul 2010 18:20:20 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>> 
>> > 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?
>> 
>> What makes it special?
>
> Not sure I got what you meant.

What makes bus "System" different from other buses?  Unless there's
significant difference, there's no reason to treat it specially.

>> >> > +- "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"?
>> 
>> Stuff like i440FX: if your board comes with it, you already got it, if
>> not, you can't have it.
>
> Ok, so it's about devices that can be dynamic created and hot plugged, right?

It's not about hot-plug.  For instance, isa-serial is "creatable", but
can't be hot-plugged.  -device can cold-plug it.  device_add doesn't
know how to cold-plug, but that's fixable.

> In this case I think the text should read something like:
>
>  "the device can be dynamically created"

"dynamically" suggests hot-plug.

>> >> > +- "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.
>> 
>> Adding stuff we need when we need it is easier and less painfull than
>> taking out stuff when we realize we shouldn't have added it :)
>> 
>> > 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)
>> 
>> Let's start with this, and add more information as needed.
>
> I would be ok with that, I guess.
>
>> >     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?
>> 
>> I think the primary use for self-documentation is to let clients figure
>> out whether something they know how to use is actually there.  
>
> Then documenting types is not needed.

... for this particular use case.  There are others.

> I think it's beyond that, one possible advanced usage is to allow clients
> to automatically catch up with protocol additions.
>
> If we add only the json type, then client writers will have to know beforehand
> the qdev type of all properties, which implies that clients will have to be
> modified to learn about new properties.
>
> If the problem is the key's name ("qdev"), then we could have:
>
> - "type": a json-object with type information, contains the following:
>    - "json": the JSON type of the property, to be used when setting it
>              through QMP (json-string)
>    - "format": only present when the property requires special formatting
>                (json-string, optional)
>                  Possible values: macaddr, drive, chardev, netdev, taddr
>
> Now, I only know the format of a macaddr and am assuming that the others
> need special formatting too.

enum PropertyType has little to do with JSON, and should not be exposed
here.  Its purpose is to declare how to parse the argument value from a
*string* (think -device) and store the resulting value in the
DeviceState.

Aside: device_add actually converts all property values to string.
That's because -device came first, and it takes only strings.  Not nice.

What you seem to have in mind is how to give clients more information
about acceptable argument values than just "type".  I agree that such
information may be useful for some applications.  But PropertyType is
not the solution, not even a partial solution (I can elaborate if you
want).  Check out JSON Schema for an attempt at a more complete
solution.

Let's do just the obvious "type" (json-string) now.

[...]