From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MabhO-00019l-Hr
	for qemu-devel@nongnu.org; Mon, 10 Aug 2009 16:42:30 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MabhK-00017d-6Z
	for qemu-devel@nongnu.org; Mon, 10 Aug 2009 16:42:30 -0400
Received: from [199.232.76.173] (port=44242 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MabhJ-00017X-Tw
	for qemu-devel@nongnu.org; Mon, 10 Aug 2009 16:42:25 -0400
Received: from mx2.redhat.com ([66.187.237.31]:52347)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <lcapitulino@redhat.com>) id 1MabhJ-0001Xs-9i
	for qemu-devel@nongnu.org; Mon, 10 Aug 2009 16:42:25 -0400
Date: Mon, 10 Aug 2009 17:42:13 -0300
From: Luiz Capitulino <lcapitulino@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v1 00/25] Monitor handlers new structure
	phase 1
Message-ID: <20090810174213.616e6c09@doriath>
In-Reply-To: <4A80803F.4010303@codemonkey.ws>
References: <1249318642-19324-1-git-send-email-lcapitulino@redhat.com>
	<4A80803F.4010303@codemonkey.ws>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
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.gnu.org/pipermail/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: Anthony Liguori <anthony@codemonkey.ws>
Cc: jan.kiszka@siemens.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, avi@redhat.com

On Mon, 10 Aug 2009 15:17:03 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> Luiz Capitulino wrote:
> >  Hi there,
> >
> >  In the long QEMU Monitor Protocol (QMP) thread people have agreed that,
> > whichever protocol we are going to use, the first step that needs to be
> > done is to improve current Monitor's code, so that command handlers
> > support 'structured' input and output.
> >   
> 
> I think one of the goals was for there to be type safety.  qdicts store 
> void *s which is pretty clumsy.

 Yes.

> 
> I think you may be waiting to introduce QObject, but in the interim, you 
> should at least introduce a boxed type and have proper accessors in 
> qdict.  For instance, instead of:
> 
> int f = (long)qdict_get(foo, "bar");
> 
> It should be:
> 
> int f = qdict_get_int(foo, "bar");

 It will be provided by the QObject patches I'm already
working on, why can't we wait for it?

 The current code also stores handler arguments as void pointers
and does the casting when passing them as arguments.

 So, unless you have caught a real bug in the series, I think
it's safe to merge it as is. This will contribute to have
QObject sooner. :)

 Also, it's a good idea to get this tested before introducing
another layer on top of it.