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

Reply via email to