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
>
