From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56330) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TNne4-00035I-PR for qemu-devel@nongnu.org; Mon, 15 Oct 2012 12:36:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TNndy-0004oL-MC for qemu-devel@nongnu.org; Mon, 15 Oct 2012 12:36:00 -0400 Received: from mail-oa0-f45.google.com ([209.85.219.45]:60616) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TNndy-0004oA-Hi for qemu-devel@nongnu.org; Mon, 15 Oct 2012 12:35:54 -0400 Received: by mail-oa0-f45.google.com with SMTP id i18so4989149oag.4 for ; Mon, 15 Oct 2012 09:35:53 -0700 (PDT) Sender: fluxion Date: Mon, 15 Oct 2012 11:35:46 -0500 From: Michael Roth Message-ID: <20121015163546.GA27254@illuin> References: <1350076268-18461-1-git-send-email-mdroth@linux.vnet.ibm.com> <1350076268-18461-25-git-send-email-mdroth@linux.vnet.ibm.com> <507BC568.10504@redhat.com> <507C0AE3.7080903@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <507C0AE3.7080903@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, peter.maydell@linaro.org, aliguori@us.ibm.com, qemu-devel@nongnu.org, blauwirbel@gmail.com On Mon, Oct 15, 2012 at 03:08:51PM +0200, Paolo Bonzini wrote: > Il 15/10/2012 10:12, Paolo Bonzini ha scritto: > > Il 12/10/2012 23:11, Michael Roth ha scritto: > >> + elif field['type'].startswith('enum '): > >> + typename = 'int' > > > > Note that there is support for enum properties in qdev. Please consider > > adding it, though it can be done as a follow-up. > > > > I'm going to play a bit with the series and convert 1 or 2 devices > > myself to see how it looks, then I'll give my acked-by. > > Ok, so now I played with it a bit. My main comments, which can all be > tackled as a follow-up, are: > > - immutable/derived/broken/elsewhere (and the default, let's call it > serialized) are really five cases of the same QIDL property. Perhaps > this could be enforced in the extended syntax like this: > > #define q_immutable QIDL(serialize(immutable)) > #define q_derived QIDL(serialize(derived)) > #define q_broken QIDL(serialize(broken)) > #define q_elsewhere QIDL(serialize(elsewhere)) > > I would also make it possible to explicitly specify the fifth state, if > only for symmetry. Agreed, that's a more proper grouping. Though, for consistency with QIDL(property, ...), I would do QIDL(serialize, ...) > > I'm not sure what your plans are for q_derived vs. VMState. If a field > X is set in pre_save hooks based on field Y, how should the fields be > set? X is usually not up-to-date, so it should be q_derived. But Y > cannot be serialized as is, so it should be q_elsewhere. One of the > two is wrong, which one? :) > > - q_properties are also always q_immutable. I think this should be > enforced in the code generator. Agreed. > > - it would be _much_ better if you could automatically derive properties > information for embedded structs. For example, Notifiers and qemu_irqs > are always q_immutable. NotifierLists probably are always q_elsewhere, > because the owner of the notifiers should add themselves back. q_inherit maybe? Otherwise we're overriding "q_default" in subtle ways that may not always be desired. > > In general, if struct X is QIDL_DECLAREd and only has q_immutable > fields, it can be taken as q_immutable. Hence for example the base > class should not need any decoration; ISADevice will be seen as > q_immutable, but PCIDevice will be seen as serialized. But even if a > struct is not QIDL_DECLAREd, it should be possible to apply a tag to a > typedef, and have it always applied to the members. I'd prefer to stick with a common declaration macro to handle tagging. How about QIDL_DECLARE(MyImmutableType, q_immutable) to apply a tag to all members? > > Paolo >