[Qemu-devel] Re: [RfC PATCH] spice: qmp windup: connection events & info command.

[Gerd Hoffmann]

  Hi,

> The first thing we have to check with Daniel is whether or not we're
> providing the info they would need/expect,

Daniel?

> apart from that I have the
> following general comments:
>
>   1. It's missing documentation in QMP/qmp-events.txt and qmp-commands.hx
>      (yeah, docs are far from code, hope to fix soon)

Ok.

>   2. Can you please split this in two patches? One adding the events and
>      the other adding the query command

Doesn't make that much sense IMHO as the both provide quite simliar 
informations (i.e. "info spice" gives you a list of connections with 
pretty much the same info provided by the events).

> > +/*
> > + * generic print handler for hmp 'info $what'
> > + * simply pretty-print the josn representation
> > + */
> > +#if defined(CONFIG_SPICE) /* because 'info spice' is the only user */
> > +static void do_info_generic_print(Monitor *mon, const QObject *data)
> > +{
> > +    QString *json = qobject_to_json_pretty(data);
> > +    monitor_printf(mon, "%s\n", qstring_get_str(json));
> > +    QDECREF(json);
> > +}
> > +#endif
>
> We definitely need a generic print handler, but I don't that stringifying
> JSON makes a minimal good user interface.

Well, this is the pretty json version which prints stuff multi-line and 
with intention.  Certainly not perfect but reasonable readable with 
minimum effort.  We can replace it with something else when it shows up.

> > +static QList *channel_list_get(void)
> > +{
> > +    ChannelList *item;
> > +    QList *list;
> > +    QDict *dict;
> > +
> > +    list = qlist_new();
> > +    QTAILQ_FOREACH(item,&channel_list, link) {
> > +        dict = qdict_new();
> > +        add_addr_info(dict,&item->info->paddr, item->info->plen);
> > +        add_channel_info(dict, item->info);
> > +        qlist_append_obj(list, QOBJECT(dict));
>
> You can use qlist_append() and drop the QOBJECT() usage.

Ok.

cheers,
  Gerd