From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51778) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TNkPz-00027P-M5 for qemu-devel@nongnu.org; Mon, 15 Oct 2012 09:09:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TNkPt-0005F5-UM for qemu-devel@nongnu.org; Mon, 15 Oct 2012 09:09:15 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:42557) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TNkPt-0005Et-O7 for qemu-devel@nongnu.org; Mon, 15 Oct 2012 09:09:09 -0400 Received: by mail-pb0-f45.google.com with SMTP id rp2so4855314pbb.4 for ; Mon, 15 Oct 2012 06:09:08 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <507C0AE3.7080903@redhat.com> Date: Mon, 15 Oct 2012 15:08:51 +0200 From: Paolo Bonzini MIME-Version: 1.0 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> In-Reply-To: <507BC568.10504@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit 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: , Cc: kwolf@redhat.com, peter.maydell@linaro.org, aliguori@us.ibm.com, Michael Roth , qemu-devel@nongnu.org, blauwirbel@gmail.com 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. 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. - 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. 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. Paolo