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

[Anthony Liguori]

On 03/16/2011 09:34 AM, Luiz Capitulino wrote:
> 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):

C library is not quite as important as C interface.  I want QMP to be an 
interface that we consume internally because that will make QMP a strong 
external interface.

A fundamental design characteristic for me is that first and foremost, 
QMP should be a good C interface and that the wire representation should 
be easy to support in a good C interface.

This is a shift in our direction but the good news is that the practical 
impact is small.  But I don't think there's a lot of value of focusing 
on non-C consumers because any non-C consumer is capable of consuming a 
good C interface (but the inverse is not true).

> +##
> +# @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'.

I tried very hard to make events useful without changing the wire 
protocol significantly but I've failed there.

I've got a new proposal for handling events that introduces the concept 
of a signal on the wire and the notion of connecting and disconnecting 
from signals.

> 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.

The way this is typically handled is that signals tend to pass 
structures instead of lots of fields.  For instance, most of the GDK 
events just pass a structure for the event (like GdkButtonEvent).

> 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 :-)

For 0.15.0, libqmp is internal to QEMU.  We need to think very hard 
before making it an external interface.

But the same sort of compatibility considerations apply to using QMP 
within QEMU.  If you add a new field to a function call, we need to 
modify any internal usage of the API.  If we add a field to a structure, 
as long as we use feature flags (we do), only the places that care to 
set that field need to worry about it.

Regards,

Anthony Liguori