Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On Wed, Jun 06, 2012 at 10:31:56AM +0300, Avi Kivity wrote:
> On 06/06/2012 12:11 AM, Michael Roth wrote:
> >> 
> >> Is is possible to let the compiler process the .c file, with the IDL
> >> delimited by some marker?  I like how device models are self contained
> >> in one file now.
> > 
> > It's possible, but only if we inject the generated visitor code into the
> > devices via an #include "qapi-generated/<device>-qapi-visit.c";
> > 
> > I'm not sure how acceptable that is... but it does reduce the churn
> > quite a bit.
> 
> We could make qc add this #include (or even inject the code directly) by
> emitting a new C file (with #line directives to direct the debugger to
> the original) and compiling this intermediate file instead of the source.
> 
> >> > +There are three cases where state can be suppressed: when it is **immutable**,
> >> > +**derived**, or **broken**.  
> >> 
> >> There is a fourth class, non-guest-visible state (below).  There is a
> >> fifth class, migrated by other means, which includes memory and block
> >> device state, but of course it isn't interesting in this context.
> > 
> > There's a higher-level annotation, qc_declaration, which denotes what
> > devices/structs should be processed by the QIDL compiler (and follow
> > it's rules). So there's an implied "handled by other means" for
> > everything that falls outside this category.
> 
> Right, but within a qc_declaration struct there can be "other means" fields.
> 
> >> 
> >> <snip>
> >> 
> >> Suggestion: add a _guest marker for ordinary state.  Fail the build on
> >> unmarked fields.  This ensures that some thought is given to each field,
> >> instead of having a default that may be correct most of the time, but
> >> not always.
> > 
> > Hmm, I my general thought was that is doesn't hurt to send extra, which
> > made serialization a reasonable default course of action.
> > 
> > But there is indeed a risk of overwriting target state with garbage if
> > we don't verify what fields really should/shouldn't be sent. A marker to
> > track this does seem useful in that regard...
> 
> I don't think the default is unsafe.  I just dislike ABIs being cast
> into stone by carelessness, it can be hard to fix up later.
> 
> Suppose we have state X and derived state Y that is sent by mistake.
> But it can also be said that Y is the state and X derives from it, so
> can we ever remove one or the other?  It would be a bigger problem if
> there were multiple implementations of the protocol (instead of just
> qemu), but still, I'd rather see more thought going into the protcol
> when defining it rather than when trying to change it.
> 
> > 
> >> 
> >> Suggestion: add a mandatory position hint (_guest(7) or _pos(7)) that
> >> generates ordering within the fields.  This decouples the order of lines
> >> in the struct from the protocol, so you can add state where it make
> >> sense, or rearrange lines when they don't, and detect copy/paste errors.
> >> 
> > 
> > I'm in agreement with Gerd that the wire protocol we use should support
> > field names. I think device state constitutes a small enough percentage
> > of total migrated state that the performance impact would be negligable,
> > and migration will invariably add some negotiation/compatibility
> > functionality on top of the serialization that would make having field
> > names intact useful for analyzing/debugging things.
> > 
> > I personally like the idea of using compressed json, but I think we
> > could implement a QObject<->BER mechanism that would provide this as
> > well.
> 
> I'd like to see BER too.  But we will have to support the old protocol
> for quite some time (I'd say at least 3 years from the first release
> that supports the new protocol).
> 
> We could put the ordering some other place, but that makes it harder to
> write qc_declarations.
> 
> >> Surely there are available lexer/parser packages?
> > 
> > This seems promising:
> > 
> > http://pygments.org/docs/lexerdevelopment/
> 
> IMO some external tool is really needed.  I'm sure qc will pick up new
> features quickly, so separating the protocol description's description
> from the protocol description's parser is important.  You can't get a
> lot more meta than that.

There's also pyparsing.

It would need to be something fairly ubiquitous though, and pygments is
the more well-used library yet still isn't available on RHEL5 and
probably a few other distros we need to support.

Although, we *could* just probe/disable schema/vmstate generation lacking
such dependencies, but that absolutely requires us to check in modified
schemas/vmstate (which we already do) and build based on those rather
than autobuilt code. Based on comments elsewhere I think that's the
direction we wanna go anyway though, so we have some options. I'll look
into it.

> 
> -- 
> error compiling committee.c: too many arguments to function
>