From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60572) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScNu0-0005P9-Ty for qemu-devel@nongnu.org; Wed, 06 Jun 2012 17:36:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ScNty-0002aM-Jb for qemu-devel@nongnu.org; Wed, 06 Jun 2012 17:36:28 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:50943) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScNty-0002a6-9q for qemu-devel@nongnu.org; Wed, 06 Jun 2012 17:36:26 -0400 Received: by obbwd20 with SMTP id wd20so12096036obb.4 for ; Wed, 06 Jun 2012 14:36:24 -0700 (PDT) Sender: fluxion Date: Wed, 6 Jun 2012 16:36:18 -0500 From: Michael Roth Message-ID: <20120606213618.GR2916@illuin> References: <1338858018-17189-1-git-send-email-mdroth@linux.vnet.ibm.com> <1338858018-17189-2-git-send-email-mdroth@linux.vnet.ibm.com> <4FCDDA12.4040907@redhat.com> <20120605211128.GM2916@illuin> <4FCF076C.8040905@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FCF076C.8040905@redhat.com> Subject: Re: [Qemu-devel] [PATCH 01/17] qidl: add QEMU IDL processor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: aliguori@us.ibm.com, yamahata@valinux.co.jp, quintela@redhat.com, qemu-devel@nongnu.org, owasserm@redhat.com, pbonzini@redhat.com, akong@redhat.com, afaerber@suse.de 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/-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. > > >> > >> > >> > >> 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 >