From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=38689 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PLaE6-00016H-NP
	for qemu-devel@nongnu.org; Thu, 25 Nov 2010 06:42:59 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1PLaE5-0004pS-L7
	for qemu-devel@nongnu.org; Thu, 25 Nov 2010 06:42:58 -0500
Received: from mx1.redhat.com ([209.132.183.28]:20536)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1PLaE5-0004pJ-Ch
	for qemu-devel@nongnu.org; Thu, 25 Nov 2010 06:42:57 -0500
Received: from int-mx12.intmail.prod.int.phx2.redhat.com
	(int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25])
	by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oAPBgtgk016944
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <qemu-devel@nongnu.org>; Thu, 25 Nov 2010 06:42:56 -0500
Message-ID: <4CEE4BBC.5050204@redhat.com>
Date: Thu, 25 Nov 2010 12:42:52 +0100
From: Gerd Hoffmann <kraxel@redhat.com>
MIME-Version: 1.0
References: <1289479772-19961-1-git-send-email-kraxel@redhat.com>
	<20101112135834.63129825@doriath>
In-Reply-To: <20101112135834.63129825@doriath>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [RfC PATCH] spice: qmp windup: connection events &
	info command.
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: qemu-devel@nongnu.org

   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