Re: [Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator

[Paolo Bonzini]

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