qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)


From: Luiz Capitulino
Subject: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
Date: Wed, 16 Mar 2011 11:34:28 -0300

On Fri, 11 Mar 2011 17:05:30 -0600
Anthony Liguori <address@hidden> 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 :-)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]