[Qemu-devel] [PATCH v3 0/2] QMP: Introduce query-qdm

[Qemu-devel] [PATCH v3 0/2] QMP: Introduce query-qdm

[Miguel Di Ciurcio Filho]

This series introduces the documentation for the query-qdm command and the
conversion of the monitor command 'info qdm' to QMP.

The documentation and code are based on a patch previously sent to qemu-devel
by Daniel P. Berrange:

http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00931.html

Changelog from v2
-----------------
- added IDE and s390-virtio as possible values for "bus"
- specify that the "properties" list is optional, in case the device doesn't
have anything to be setup
- reworded the explanation of "creatable"
- reverted the qdev/qmp split, just use "type" and its json equivalent
representation
- do not list legacy stuff: PROP_TYPE_(VLAN|PTR|UNSPEC)

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"

Changes from the Daniel's original patch:
- Split the patch in two, taking out the documentation from the code
- Reworded some parts of the documentation and added data types
- Small cleanups and renamed do_info_devices() to do_info_qdm()
- Added do_info_qdm_print() to be used in the monitor

Regards,

Miguel

---

Miguel Di Ciurcio Filho (2):
  QMP: Introduce the documentation for query-qdm
  monitor: Convert 'info qdm' to QMP

 hw/qdev.c       |  110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/qdev.h       |    3 +-
 monitor.c       |    3 +-
 qemu-monitor.hx |   71 +++++++++++++++++++++++++++++++++++
 4 files changed, 183 insertions(+), 4 deletions(-)

[Qemu-devel] [PATCH v3 1/2] QMP: Introduce the documentation for query-qdm

[Miguel Di Ciurcio Filho]

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

diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 2af3de6..4e6062b 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -2490,6 +2490,77 @@ 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,
+                        IDE, s390-virtio
+- "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 by the user (json-boolean)
+- "properties": a json-array where each item is a json-object that describes a
+  property of the device. If the device has no property to be setup, this item
+  will not be present. Each json-object contains the following:
+     - "name": the name of the property (json-string)
+     - "type": the json type of the property (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": "boolean" 
+             },
+             {
+                 "name": "logical_block_size",
+                 "type": "integer"
+             },
+             {
+                 "name": "opt_io_size",
+                 "type": "integer"
+             },
+             {
+                 "name": "drive",
+                 "type": "string"
+             }
+           ]
+        },
+        {
+           "name": "virtio-balloon-pci",
+           "creatable": true,
+           "bus": "PCI",
+           "properties": [
+             {
+                 "name": "indirect_desc",
+                 "type": "boolean"
+             }
+           ]
+        },
+      ....
+    ]
+
+EQMP
+
+STEXI
 @item info roms
 show roms
 @end table
-- 
1.7.1

[Qemu-devel] [PATCH v3 2/2] monitor: Convert 'info qdm' to QMP

[Miguel Di Ciurcio Filho]

Converts the 'info qdm' command to QMP, allowing the discovery of all devices
known to the QEMU binary without relying on command line paramaters like
-device ? and -device devtype,?

This change does not modify the output of the 'info qdm' monitor command.

Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
---
 hw/qdev.c |  110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/qdev.h |    3 +-
 monitor.c |    3 +-
 3 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index e99c73f..d24d42a 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,6 +29,7 @@
 #include "qdev.h"
 #include "sysemu.h"
 #include "monitor.h"
+#include "qjson.h"
 
 static int qdev_hotplug = 0;
 
@@ -779,13 +780,118 @@ void do_info_qtree(Monitor *mon)
         qbus_print(mon, main_system_bus, 0);
 }
 
-void do_info_qdm(Monitor *mon)
+static void qdm_list_iter(QObject *obj, void *opaque)
+{
+
+    Monitor *mon = opaque;
+    QDict *dev = qobject_to_qdict(obj);
+
+    monitor_printf(mon, "name \"%s\", bus %s", qdict_get_str(dev, "name"),
+            qdict_get_str(dev, "bus"));
+
+    if (qdict_haskey(dev, "alias")) {
+        monitor_printf(mon, ", alias \"%s\"", qdict_get_str(dev, "alias"));
+    }
+
+    if (qdict_haskey(dev, "description")) {
+        monitor_printf(mon, ", desc \"%s\"", qdict_get_str(dev, "description"));
+    }
+
+    if (!qdict_get_bool(dev, "creatable")) {
+        monitor_printf(mon, ", no-user");
+    }
+
+    monitor_printf(mon, "\n");
+}
+
+void do_info_qdm_print(Monitor *mon, const QObject *ret_data)
+{
+    QList *devs;
+
+    devs = qobject_to_qlist(ret_data);
+    qlist_iter(devs, qdm_list_iter, mon);
+}
+
+static const char *qdev_property_type_to_string(int type)
+{
+    switch (type) {
+    case PROP_TYPE_UINT8:
+    case PROP_TYPE_UINT16:
+    case PROP_TYPE_UINT32:
+    case PROP_TYPE_INT32:
+    case PROP_TYPE_UINT64:
+        return "integer";
+    case PROP_TYPE_TADDR:
+    case PROP_TYPE_MACADDR:
+    case PROP_TYPE_DRIVE:
+    case PROP_TYPE_CHR:
+    case PROP_TYPE_STRING:
+    case PROP_TYPE_NETDEV:
+        return "string";
+    case PROP_TYPE_BIT:
+	    return "boolean";
+    case PROP_TYPE_UNSPEC:
+    case PROP_TYPE_VLAN:
+    case PROP_TYPE_PTR:
+        return NULL;
+    }
+
+    return NULL;
+}
+
+void do_info_qdm(Monitor *mon, QObject **ret_data)
 {
     DeviceInfo *info;
+    QList *devs = qlist_new();
 
     for (info = device_info_list; info != NULL; info = info->next) {
-        qdev_print_devinfo(info);
+        QObject *obj;
+        QDict *dev;
+        QList *props = qlist_new();
+        Property *prop;
+
+        for (prop = info->props; prop && prop->name; prop++) {
+            QObject *entry;
+            /*
+             * TODO: skip old and hackish stuff, they will be removed some day.
+             */
+            if (!prop->info->parse || prop->info->type == PROP_TYPE_VLAN
+                || prop->info->type == PROP_TYPE_PTR
+                || prop->info->type == PROP_TYPE_UNSPEC) {
+                continue;
+            }
+
+            const char *type = qdev_property_type_to_string(prop->info->type);
+
+            entry = qobject_from_jsonf("{ 'name': %s, 'type': %s }",
+                           prop->name, type);
+
+            qlist_append_obj(props, entry);
+        }
+
+        obj = qobject_from_jsonf("{ 'name': %s, 'bus': %s, 'creatable': %i }",
+                     info->name,
+                     info->bus_info->name,
+                     info->no_user ? 0 : 1);
+
+        dev = qobject_to_qdict(obj);
+
+        if (!qlist_empty(props)) {
+            qdict_put(dev, "properties", props);
+        }
+
+        if (info->alias) {
+            qdict_put(dev, "alias", qstring_from_str(info->alias));
+        }
+
+        if (info->desc) {
+            qdict_put(dev, "description", qstring_from_str(info->desc));
+        }
+
+        qlist_append(devs, dev);
     }
+
+    *ret_data = QOBJECT(devs);
 }
 
 int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/hw/qdev.h b/hw/qdev.h
index 678f8b7..3b0382b 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -184,7 +184,8 @@ void qbus_free(BusState *bus);
 /*** monitor commands ***/
 
 void do_info_qtree(Monitor *mon);
-void do_info_qdm(Monitor *mon);
+void do_info_qdm_print(Monitor *mon, const QObject *ret_data);
+void do_info_qdm(Monitor *mon, QObject **ret_data);
 int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
diff --git a/monitor.c b/monitor.c
index 45fd482..66810f2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2565,7 +2565,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show qdev device model list",
-        .mhandler.info = do_info_qdm,
+        .user_print = do_info_qdm_print,
+        .mhandler.info_new = do_info_qdm,
     },
     {
         .name       = "roms",
-- 
1.7.1

[Qemu-devel] Re: [PATCH v3 2/2] monitor: Convert 'info qdm' to QMP

[Luiz Capitulino]

On Mon, 19 Jul 2010 13:51:27 -0300
Miguel Di Ciurcio Filho <miguel.filho@gmail.com> wrote:

> Converts the 'info qdm' command to QMP, allowing the discovery of all devices
> known to the QEMU binary without relying on command line paramaters like
> -device ? and -device devtype,?
> 
> This change does not modify the output of the 'info qdm' monitor command.
> 
> Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
> ---
>  hw/qdev.c |  110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  hw/qdev.h |    3 +-
>  monitor.c |    3 +-
>  3 files changed, 112 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index e99c73f..d24d42a 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -29,6 +29,7 @@
>  #include "qdev.h"
>  #include "sysemu.h"
>  #include "monitor.h"
> +#include "qjson.h"
>  
>  static int qdev_hotplug = 0;
>  
> @@ -779,13 +780,118 @@ void do_info_qtree(Monitor *mon)
>          qbus_print(mon, main_system_bus, 0);
>  }
>  
> -void do_info_qdm(Monitor *mon)
> +static void qdm_list_iter(QObject *obj, void *opaque)
> +{
> +
> +    Monitor *mon = opaque;
> +    QDict *dev = qobject_to_qdict(obj);
> +
> +    monitor_printf(mon, "name \"%s\", bus %s", qdict_get_str(dev, "name"),
> +            qdict_get_str(dev, "bus"));
> +
> +    if (qdict_haskey(dev, "alias")) {
> +        monitor_printf(mon, ", alias \"%s\"", qdict_get_str(dev, "alias"));
> +    }
> +
> +    if (qdict_haskey(dev, "description")) {
> +        monitor_printf(mon, ", desc \"%s\"", qdict_get_str(dev, "description"));
> +    }
> +
> +    if (!qdict_get_bool(dev, "creatable")) {
> +        monitor_printf(mon, ", no-user");
> +    }
> +
> +    monitor_printf(mon, "\n");
> +}
> +
> +void do_info_qdm_print(Monitor *mon, const QObject *ret_data)
> +{
> +    QList *devs;
> +
> +    devs = qobject_to_qlist(ret_data);
> +    qlist_iter(devs, qdm_list_iter, mon);
> +}
> +
> +static const char *qdev_property_type_to_string(int type)
> +{
> +    switch (type) {
> +    case PROP_TYPE_UINT8:
> +    case PROP_TYPE_UINT16:
> +    case PROP_TYPE_UINT32:
> +    case PROP_TYPE_INT32:
> +    case PROP_TYPE_UINT64:
> +        return "integer";
> +    case PROP_TYPE_TADDR:
> +    case PROP_TYPE_MACADDR:
> +    case PROP_TYPE_DRIVE:
> +    case PROP_TYPE_CHR:
> +    case PROP_TYPE_STRING:
> +    case PROP_TYPE_NETDEV:
> +        return "string";
> +    case PROP_TYPE_BIT:
> +	    return "boolean";
> +    case PROP_TYPE_UNSPEC:
> +    case PROP_TYPE_VLAN:
> +    case PROP_TYPE_PTR:
> +        return NULL;

Shouldn't you just drop UNSPEC, VLAN and PRT?

> +    }
> +
> +    return NULL;
> +}
> +
> +void do_info_qdm(Monitor *mon, QObject **ret_data)
>  {
>      DeviceInfo *info;
> +    QList *devs = qlist_new();
>  
>      for (info = device_info_list; info != NULL; info = info->next) {
> -        qdev_print_devinfo(info);
> +        QObject *obj;
> +        QDict *dev;
> +        QList *props = qlist_new();
> +        Property *prop;
> +
> +        for (prop = info->props; prop && prop->name; prop++) {
> +            QObject *entry;
> +            /*
> +             * TODO: skip old and hackish stuff, they will be removed some day.
> +             */
> +            if (!prop->info->parse || prop->info->type == PROP_TYPE_VLAN
> +                || prop->info->type == PROP_TYPE_PTR
> +                || prop->info->type == PROP_TYPE_UNSPEC) {
> +                continue;
> +            }
> +
> +            const char *type = qdev_property_type_to_string(prop->info->type);

Variable definitions should be on the top and that function can return
NULL. You should put an assert() here if that's impossible to happen,
otherwise qdev_property_type_to_string() should return "unknown" and that
should be documented.

I think the assert() is fine.

> +
> +            entry = qobject_from_jsonf("{ 'name': %s, 'type': %s }",
> +                           prop->name, type);
> +
> +            qlist_append_obj(props, entry);
> +        }
> +
> +        obj = qobject_from_jsonf("{ 'name': %s, 'bus': %s, 'creatable': %i }",
> +                     info->name,
> +                     info->bus_info->name,
> +                     info->no_user ? 0 : 1);
> +
> +        dev = qobject_to_qdict(obj);
> +
> +        if (!qlist_empty(props)) {
> +            qdict_put(dev, "properties", props);
> +        }

We'll leak 'props' when it's empty.

> +
> +        if (info->alias) {
> +            qdict_put(dev, "alias", qstring_from_str(info->alias));
> +        }
> +
> +        if (info->desc) {
> +            qdict_put(dev, "description", qstring_from_str(info->desc));
> +        }
> +
> +        qlist_append(devs, dev);
>      }
> +
> +    *ret_data = QOBJECT(devs);
>  }
>  
>  int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 678f8b7..3b0382b 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -184,7 +184,8 @@ void qbus_free(BusState *bus);
>  /*** monitor commands ***/
>  
>  void do_info_qtree(Monitor *mon);
> -void do_info_qdm(Monitor *mon);
> +void do_info_qdm_print(Monitor *mon, const QObject *ret_data);
> +void do_info_qdm(Monitor *mon, QObject **ret_data);
>  int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  
> diff --git a/monitor.c b/monitor.c
> index 45fd482..66810f2 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2565,7 +2565,8 @@ static const mon_cmd_t info_cmds[] = {
>          .args_type  = "",
>          .params     = "",
>          .help       = "show qdev device model list",
> -        .mhandler.info = do_info_qdm,
> +        .user_print = do_info_qdm_print,
> +        .mhandler.info_new = do_info_qdm,

Haven't we agreed on calling this query-available-devices or something
like that?

>      },
>      {
>          .name       = "roms",

[Qemu-devel] Re: [PATCH v3 2/2] monitor: Convert 'info qdm' to QMP

[Daniel P. Berrange]

On Wed, Jul 21, 2010 at 02:42:28PM -0300, Luiz Capitulino wrote:
> On Mon, 19 Jul 2010 13:51:27 -0300
> Miguel Di Ciurcio Filho <miguel.filho@gmail.com> wrote:
> 
> > Converts the 'info qdm' command to QMP, allowing the discovery of all devices
> > known to the QEMU binary without relying on command line paramaters like
> > -device ? and -device devtype,?
> > 
> > This change does not modify the output of the 'info qdm' monitor command.
> > 
> > Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
> > diff --git a/monitor.c b/monitor.c
> > index 45fd482..66810f2 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -2565,7 +2565,8 @@ static const mon_cmd_t info_cmds[] = {
> >          .args_type  = "",
> >          .params     = "",
> >          .help       = "show qdev device model list",
> > -        .mhandler.info = do_info_qdm,
> > +        .user_print = do_info_qdm_print,
> > +        .mhandler.info_new = do_info_qdm,
> 
> Haven't we agreed on calling this query-available-devices or something
> like that?

That's getting rather verbose for a name ! How about just
'query-dev-types' (anticipating future query-netdev-types,
query-chardev-types, commands etc, too)


Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

[Qemu-devel] Re: [PATCH v3 2/2] monitor: Convert 'info qdm' to QMP

[Luiz Capitulino]

On Wed, 21 Jul 2010 18:51:19 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Wed, Jul 21, 2010 at 02:42:28PM -0300, Luiz Capitulino wrote:
> > On Mon, 19 Jul 2010 13:51:27 -0300
> > Miguel Di Ciurcio Filho <miguel.filho@gmail.com> wrote:
> > 
> > > Converts the 'info qdm' command to QMP, allowing the discovery of all devices
> > > known to the QEMU binary without relying on command line paramaters like
> > > -device ? and -device devtype,?
> > > 
> > > This change does not modify the output of the 'info qdm' monitor command.
> > > 
> > > Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
> > > diff --git a/monitor.c b/monitor.c
> > > index 45fd482..66810f2 100644
> > > --- a/monitor.c
> > > +++ b/monitor.c
> > > @@ -2565,7 +2565,8 @@ static const mon_cmd_t info_cmds[] = {
> > >          .args_type  = "",
> > >          .params     = "",
> > >          .help       = "show qdev device model list",
> > > -        .mhandler.info = do_info_qdm,
> > > +        .user_print = do_info_qdm_print,
> > > +        .mhandler.info_new = do_info_qdm,
> > 
> > Haven't we agreed on calling this query-available-devices or something
> > like that?
> 
> That's getting rather verbose for a name ! How about just
> 'query-dev-types' (anticipating future query-netdev-types,
> query-chardev-types, commands etc, too)

I don't mind long names in the protocol, but I'm ok with your suggestion.

[Qemu-devel] Re: [PATCH v3 2/2] monitor: Convert 'info qdm' to QMP

[Anthony Liguori]

On 07/21/2010 12:51 PM, Daniel P. Berrange wrote:
> On Wed, Jul 21, 2010 at 02:42:28PM -0300, Luiz Capitulino wrote:
>    
>> On Mon, 19 Jul 2010 13:51:27 -0300
>> Miguel Di Ciurcio Filho<miguel.filho@gmail.com>  wrote:
>>
>>      
>>> Converts the 'info qdm' command to QMP, allowing the discovery of all devices
>>> known to the QEMU binary without relying on command line paramaters like
>>> -device ? and -device devtype,?
>>>
>>> This change does not modify the output of the 'info qdm' monitor command.
>>>
>>> Signed-off-by: Miguel Di Ciurcio Filho<miguel.filho@gmail.com>
>>> diff --git a/monitor.c b/monitor.c
>>> index 45fd482..66810f2 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -2565,7 +2565,8 @@ static const mon_cmd_t info_cmds[] = {
>>>           .args_type  = "",
>>>           .params     = "",
>>>           .help       = "show qdev device model list",
>>> -        .mhandler.info = do_info_qdm,
>>> +        .user_print = do_info_qdm_print,
>>> +        .mhandler.info_new = do_info_qdm,
>>>        
>> Haven't we agreed on calling this query-available-devices or something
>> like that?
>>      
> That's getting rather verbose for a name ! How about just
> 'query-dev-types' (anticipating future query-netdev-types,
> query-chardev-types, commands etc, too)
>    

I know we're still trying to recover from the great bit shortage of '09 
but I think we can afford to spare some here to make the names readable :-)

Good names are part of good documentation.  If it's not entirely obvious 
what the function does from it's name, it's probably a bad name (in the 
context of a wire protocol).

Regards,

Anthony Liguori

>
> Daniel
>