From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=53925 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1ObdJn-0005Pk-OM for qemu-devel@nongnu.org; Wed, 21 Jul 2010 13:43:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1ObdJa-0002nT-35 for qemu-devel@nongnu.org; Wed, 21 Jul 2010 13:42:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56905) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1ObdJZ-0002n9-Pm for qemu-devel@nongnu.org; Wed, 21 Jul 2010 13:42:41 -0400 Date: Wed, 21 Jul 2010 14:42:28 -0300 From: Luiz Capitulino Message-ID: <20100721144228.07969f8a@redhat.com> In-Reply-To: <1279558287-9446-3-git-send-email-miguel.filho@gmail.com> References: <1279558287-9446-1-git-send-email-miguel.filho@gmail.com> <1279558287-9446-3-git-send-email-miguel.filho@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH v3 2/2] monitor: Convert 'info qdm' to QMP List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Miguel Di Ciurcio Filho Cc: avi@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com On Mon, 19 Jul 2010 13:51:27 -0300 Miguel Di Ciurcio Filho 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 > --- > 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",