From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=56415 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pzro4-0002Si-3J for qemu-devel@nongnu.org; Wed, 16 Mar 2011 10:34:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pzro2-0006SZ-Jx for qemu-devel@nongnu.org; Wed, 16 Mar 2011 10:34:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19148) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pzro2-0006RW-7x for qemu-devel@nongnu.org; Wed, 16 Mar 2011 10:34:34 -0400 Date: Wed, 16 Mar 2011 11:34:28 -0300 From: Luiz Capitulino Message-ID: <20110316113428.21c599a3@doriath> In-Reply-To: <1299884745-521-1-git-send-email-aliguori@us.ibm.com> References: <1299884745-521-1-git-send-email-aliguori@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2) List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Markus Armbruster , Adam Litke , qemu-devel@nongnu.org, Avi Kivity On Fri, 11 Mar 2011 17:05:30 -0600 Anthony Liguori wrote: > For more information about the background of QAPI, see > http://wiki.qemu.org/Features/QAPI > > This series depends on 'QAPI Round 0' which I posted earlier. > > Since v2, the major changes are: > > - Switch to a multiline code emitter to ease readability > - Use named parameters for escape sequences > - Add support for proxy commands > - Add support for asynchronous commands > > This version still adds a -qmp2 option. This is the only practical way I know > to have testable code while not merging 200 patches all at once. I've started reviewing this and my first impression is that this seems to be real good. However, there's a lot of code here and some parts of it are a bit complicated, so I need more time to do a thorough review and testing. Having said that, my only immediate concern is weather this will have any negative side effects on the wire protocol, today or in the future. I mean, a C library has different extensibility constraints and functionality requirements than a high-level protocol and tying/mixing the two can have bad side effects, like this small one (patch 12/15): +## +# @put_event: +# +# Disconnect a signal. This command is used to disconnect from a signal based +# on the handle returned by a signal accessor. +# +# @tag: the handle returned by a signal accessor. +# +# Returns: Nothing on success. +# If @tag is not a valid handle, InvalidParameterValue +# +# Since: 0.15.0 The name 'signal' (at least today) doesn't make sense on the wire protocol, 'put_event' probably doesn't make sense in the C library, nor does 'event'. Another detail is that, event extension is more important than command extension, because it's probably going to happen. I think it would be very bad to add new events just because we wanted to add a new field. Most of these problems seems to go away just by making libqmp internal to QEMU, then I think all this work would rock big time :-)