[1/2] QMP: Introduce the documentation for query-qdm

---
 qemu-monitor.hx |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)

On 07/03/2010 12:27 AM, Miguel Di Ciurcio Filho wrote:
> ---
>   qemu-monitor.hx |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 68 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index 9f62b94..5348899 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -2490,6 +2490,74 @@ 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.
>    

Why the name qdm?

'query-available-devices' instead?

Otherwise looks pretty nice.

On Sun, Jul 4, 2010 at 2:14 AM, Avi Kivity <avi@redhat.com> wrote:
>>  show device tree
>>  @item info qdm
>>  show qdev device model list
>> +ETEXI
>> +SQMP
>> +query-qdm
>> +---------
>> +
>> +Describe the capabilities of all devices registered with qdev.
>>
>
> Why the name qdm?
>
> 'query-available-devices' instead?
>
> Otherwise looks pretty nice.

No special reason, but it is already a known command that provides
this information in the monitor.

Regards,

Miguel

On Fri,  2 Jul 2010 18:27:02 -0300
Miguel Di Ciurcio Filho <miguel.filho@gmail.com> wrote:

> ---
>  qemu-monitor.hx |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 68 insertions(+), 0 deletions(-)
> 
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index 9f62b94..5348899 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -2490,6 +2490,74 @@ 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 list, each element is a json-object describing a single
> +device type.

s/The returned output is a list/The returned value is a json-array

> +
> +Each json-object contains the following:
> +
> +- "name": the short name of the device (json-string)

Why short? Isn't it the name itself?

> +- "bus": the name of the bus type for the device (json-string)

Do we need a list o possible values?

> +- "alias": an alias by which the device is also known (json-string, optional)
> +- "description": a long description the device  (json-string, optional)
> +- "creatable": whether this device can be created on command line (json-boolean)
> +- "props": a list where each element is an json-object that describes a property
> +of the device. Each json-object contains the following:

Suggest using "properties" (vs. "props")

> +     - "name": the short name of the property (json-string)

Why short? Isn't it the name itself?

> +     - "info": short description of the property (json-string)

You sure it's a description of the property? It seems to describe how to
set it (related, but slightly different).

Also, most of the time it seems to be an exact copy of "type". I suggest
to make it optional and only show it when it differs from "type".

> +     - "type": the data type of the property value (json-string)

We need a list o possible values, with a small explanation of each one.
Do we need the equivalent in json too?

> +
> +Example:
> +
> +-> { "execute": "query-qdm" }
> +<- {
> +      "return": [
> +        {
> +           "name": "virtio-9p-pci",
> +           "creatable": true,
> +           "bus": "PCI",
> +           "props": [
> +             {
> +                 "name": "indirect_desc",
> +                 "type": "bit",
> +                 "info": "on/off"
> +             },
> +             {
> +                 "name": "mount_tag",
> +                 "type": "string",
> +                 "info": "string"
> +             },
> +             {
> +                 "name": "fsdev",
> +                 "type": "string",
> +                 "info": "string"
> +             }
> +           ]
> +        },
> +        {
> +           "name": "virtio-balloon-pci",
> +           "creatable": true,
> +           "bus": "PCI",
> +           "props": [
> +             {
> +               "name": "indirect_desc",
> +               "type": "bit",
> +               "info": "on/off"
> +             }
> +           ]
> +        },
> +      ....
> +    ]

Suggest a NOTE saying this the equivalent of command-line options -device ?
and -device devname,?

> +
> +EQMP
> +
> +STEXI
>  @item info roms
>  show roms
>  @end table

On Mon, Jul 5, 2010 at 12:22 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> +
>> +Describe the capabilities of all devices registered with qdev.
>> +
>> +The returned output is a list, each element is a json-object describing a single
>> +device type.
>
> s/The returned output is a list/The returned value is a json-array
>

Ack.

>> +
>> +Each json-object contains the following:
>> +
>> +- "name": the short name of the device (json-string)
>
> Why short? Isn't it the name itself?

Yeah, it is just the a name. I think the initial use for "short" from
Daniel was to distinguish from "name" and "description".

>
>> +- "bus": the name of the bus type for the device (json-string)
>
> Do we need a list o possible values?
>

I didn't find a central location for all the possible values. Although
running `egrep -C1 "static struct BusInfo" *.c` shows most names, I
hope. Does anyone more experienced could confirm that this is how I
can find out all bus types?

>> +- "alias": an alias by which the device is also known (json-string, optional)
>> +- "description": a long description the device  (json-string, optional)
>> +- "creatable": whether this device can be created on command line (json-boolean)
>> +- "props": a list where each element is an json-object that describes a property
>> +of the device. Each json-object contains the following:
>
> Suggest using "properties" (vs. "props")

Better indeed, ack.

>
>> +     - "name": the short name of the property (json-string)
>
> Why short? Isn't it the name itself?
>

No need for short here, IMHO. Ack.

>> +     - "info": short description of the property (json-string)
>
> You sure it's a description of the property? It seems to describe how to
> set it (related, but slightly different).
>
> Also, most of the time it seems to be an exact copy of "type". I suggest
> to make it optional and only show it when it differs from "type".
>
>> +     - "type": the data type of the property value (json-string)
>

Looking deeper into it I think it is a bit clear now.

DeviceInfo
    |__Property
    |       | char name
    |       | PropertyInfo info
    |               |  char name
    |               |  enum PropertyType type
    |__Property
            | char name
            | PropertyInfo info
                    |  char name
                    |  enum PropertyType type

So, for something like this:

"properties": [
             {
                 "name": "indirect_desc",
                 "type": "bit",
                 "info": "on/off"
             },

"name" is Property->name
"type" is Property->info->type
"info" is Property->info->name

"name" and "type" are relevant, but I think "info" is not.

e.g.:
$ qemu -device e1000,?
e1000.mac=macaddr
e1000.vlan=vlan
e1000.netdev=netdev

The strings after the "=" sign come from Property->info->name. So, I
think the attribute "info" is really not needed.

> We need a list o possible values, with a small explanation of each one.
> Do we need the equivalent in json too?

Possible values for "type" are defined in the patch on the
qdev_property_type_to_string() function. To spot them in the current
code, hw/qdev.c:77:

enum PropertyType {
    PROP_TYPE_UNSPEC = 0,
    PROP_TYPE_UINT8,
    PROP_TYPE_UINT16,
    PROP_TYPE_UINT32,
    PROP_TYPE_INT32,
    PROP_TYPE_UINT64,
    PROP_TYPE_TADDR,
    PROP_TYPE_MACADDR,
    PROP_TYPE_DRIVE,
    PROP_TYPE_CHR,
    PROP_TYPE_STRING,
    PROP_TYPE_NETDEV,
    PROP_TYPE_VLAN,
    PROP_TYPE_PTR,
    PROP_TYPE_BIT,
};

So it is a mix of json-(string|integer|boolean). It seams to me that a
device_add using QMP will use just use strings. Need to confirm that.

>
> Suggest a NOTE saying this the equivalent of command-line options -device ?

Ack.

Regards,

Miguel

On Mon, 5 Jul 2010 16:34:22 -0300
Miguel Di Ciurcio Filho <miguel.filho@gmail.com> wrote:

> Possible values for "type" are defined in the patch on the
> qdev_property_type_to_string() function. To spot them in the current
> code, hw/qdev.c:77:
> 
> enum PropertyType {
>     PROP_TYPE_UNSPEC = 0,
>     PROP_TYPE_UINT8,
>     PROP_TYPE_UINT16,
>     PROP_TYPE_UINT32,
>     PROP_TYPE_INT32,
>     PROP_TYPE_UINT64,
>     PROP_TYPE_TADDR,
>     PROP_TYPE_MACADDR,
>     PROP_TYPE_DRIVE,
>     PROP_TYPE_CHR,
>     PROP_TYPE_STRING,
>     PROP_TYPE_NETDEV,
>     PROP_TYPE_VLAN,
>     PROP_TYPE_PTR,
>     PROP_TYPE_BIT,
> };
> 
> So it is a mix of json-(string|integer|boolean). It seams to me that a
> device_add using QMP will use just use strings. Need to confirm that.

There are integers too.

Daniel, can you clarify how libvirt is going to use this member?

Maybe we could have something like this:

 "type": { "qdev": "macaddr", "qmp": "string" }

On Wed, Jul 07, 2010 at 10:07:09AM -0300, Luiz Capitulino wrote:
> On Mon, 5 Jul 2010 16:34:22 -0300
> Miguel Di Ciurcio Filho <miguel.filho@gmail.com> wrote:
> 
> > Possible values for "type" are defined in the patch on the
> > qdev_property_type_to_string() function. To spot them in the current
> > code, hw/qdev.c:77:
> > 
> > enum PropertyType {
> >     PROP_TYPE_UNSPEC = 0,
> >     PROP_TYPE_UINT8,
> >     PROP_TYPE_UINT16,
> >     PROP_TYPE_UINT32,
> >     PROP_TYPE_INT32,
> >     PROP_TYPE_UINT64,
> >     PROP_TYPE_TADDR,
> >     PROP_TYPE_MACADDR,
> >     PROP_TYPE_DRIVE,
> >     PROP_TYPE_CHR,
> >     PROP_TYPE_STRING,
> >     PROP_TYPE_NETDEV,
> >     PROP_TYPE_VLAN,
> >     PROP_TYPE_PTR,
> >     PROP_TYPE_BIT,
> > };
> > 
> > So it is a mix of json-(string|integer|boolean). It seams to me that a
> > device_add using QMP will use just use strings. Need to confirm that.
> 
> There are integers too.
> 
> Daniel, can you clarify how libvirt is going to use this member?

We're not actively planning to use this field. When I wrote the patch
originally, I was aiming to provide the maximim semantically useful
information possible, rather than just the generic json data type.
This ensures that this is fully self-documenting.


> Maybe we could have something like this:
> 
>  "type": { "qdev": "macaddr", "qmp": "string" }

Daniel