| Message ID | 1279558287-9446-3-git-send-email-miguel.filho@gmail.com |
|---|---|
| State | New |
| Headers | show |
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",
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
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.
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 >
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",
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(-)