Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations

[Michael Roth]

On Fri, Oct 05, 2012 at 05:07:46PM +0200, Paolo Bonzini wrote:
> Il 04/10/2012 19:33, Michael Roth ha scritto:
> > Signed-off-by: Michael Roth <address@hidden>
> > ---
> >  qidl.h |  113 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 113 insertions(+)
> >  create mode 100644 qidl.h
> > 
> > diff --git a/qidl.h b/qidl.h
> > new file mode 100644
> > index 0000000..eae0202
> > --- /dev/null
> > +++ b/qidl.h
> > @@ -0,0 +1,113 @@
> > +/*
> > + * QEMU IDL Macros/stubs
> > + *
> > + * See docs/qidl.txt for usage information.
> > + *
> > + * Copyright IBM, Corp. 2012
> > + *
> > + * Authors:
> > + *  Michael Roth    <address@hidden>
> > + *
> > + * This work is licensed under the terms of the GNU GPLv2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef QIDL_H
> > +#define QIDL_H
> > +
> > +#include <glib.h>
> > +#include "qapi/qapi-visit-core.h"
> > +#include "qemu/object.h"
> > +#include "hw/qdev-properties.h"
> > +
> > +#ifdef QIDL_GEN
> > +
> > +/* we pass the code through the preprocessor with QIDL_GEN defined to parse
> > + * structures as they'd appear after preprocessing, and use the following
> > + * definitions mostly to re-insert the initial macros/annotations so they
> > + * stick around for the parser to process
> > + */
> > +#define QIDL(...) QIDL(__VA_ARGS__)
> > +#define QIDL_START(name, ...) QIDL_START(name, ##__VA_ARGS__)
> > +
> > +#define QIDL_VISIT_TYPE(name, v, s, f, e)
> > +#define QIDL_SCHEMA_ADD_LINK(name, obj, path, errp)
> > +#define QIDL_PROPERTIES(name)
> 
> Ok, a few questions...
> 
> Why do you need these to expand to nothing in the QIDL_GEN case?
> 

They don't need to, I was just trying to be explicit about what
directives were relevant to the parser and which ones were relevant to
the actually compiled code. It was more a development "aid" than
anything else though, so I think we can drop the special handling and
clean these up a bit.

> > +#define QIDL_DECLARE(name, ...) \
> 
> Can QIDL_DECLARE replace QIDL_ENABLED as the magic detection string for
> qidl compilation?
> 

In some cases the declarations will come via #include'd headers, so the
only way to do that reliable is to run it through the preprocessor
first, which is how things were done in v1. But running everything
through cpp adds substantial overhead, and just because a QIDL-fied
struct is included in a C file, it doesn't mean that the C file intends
to use any qidl-generated code.

So that's why the QIDL_ENABLE() directive was added, so we can scan for
that before doing the pass through cpp and the qidl parser to process all
the qidl declarations.

> > +    QIDL_START(name, ##__VA_ARGS__) \
> > +    struct name
> > +
> > +#else /* !QIDL_GEN */
> > +
> > +#define QIDL(...)
> > +#ifdef QIDL_ENABLED
> > +#define QIDL_START(name, ...) \
> > +    static struct { \
> > +        void (*visitor)(Visitor *, struct name **, const char *, Error 
> > **); \
> > +        const char *schema_json_text; \
> > +        Object *schema_obj; \
> > +        Property *properties; \
> > +    } qidl_data_##name;
> > +#else
> > +#define QIDL_START(name,  ...)
> > +#endif
> > +
> > +#define QIDL_DECLARE(name, ...) \
> > +    QIDL_START(name, ##__VA_ARGS__) \
> > +    struct name
> 
> This is the same definition as in the QIDL_GEN case.  Please include it
> just once.
> 

Will do.

> > +#define QIDL_VISIT_TYPE(name, v, s, f, e) \
> > +    g_assert(qidl_data_##name.visitor); \
> > +    qidl_data_##name.visitor(v, s, NULL, e)
> > +
> > +#define QIDL_SCHEMA_ADD_LINK(name, obj, path, errp) \
> > +    g_assert(qidl_data_##name.schema_obj); \
> > +    object_property_add_link(obj, path, "container", \
> > +                             &qidl_data_##name.schema_obj, errp)
> 
> Where will QIDL_SCHEMA_ADD_LINK be used?

In general, in the init functions of any qom objects that expose a
"state" property in the qom tree, so that in every case where we expose
serialized state information for an object/device via qom, we also provide
a link to a schema describing it's structure.

> 
> > +#define QIDL_PROPERTIES(name) \
> > +    qidl_data_##name.properties
> > +
> > +#endif /* QIDL_GEN */
> > +
> > +/* must be "called" in any C files that make use of QIDL-generated code */
> > +#define QIDL_ENABLE()
> > +
> > +/* QIDL annotations/markers
> > + *
> > + * q_immutable: state is fully restorable via device
> > + *   [re-]initialization/realization
> > + *
> > + * q_derived: state can be fully reconstructed from other fields (and will 
> > be,
> > + *   via [re-]initialization of the device or a separate hook)
> > + *
> > + * q_broken: state should (or possibly should) be saved, but isn't. mostly 
> > an aid
> > + *   for device developers having issues with serialization of a particular
> > + *   field, committed code should contain these except in special 
> > circumstances
> > + *
> > + * q_optional: <field> should only be serialized if the field by the name 
> > of
> > + *   has_<field> is true
> > + *
> > + * q_elsewhere: state should be serialized, but is done so elsewhere (for
> > + *   instance, by another device with a pointer to the same data)
> > + *
> > + * q_size(field): for static/dynamically-allocated arrays. specifies the 
> > field
> > + *   in the structure containing the number of elements that should be
> > + *   serialized. if argument is wrapped in parenthesis it is instead 
> > interpreted
> > + *   as an expression that should be invaluated to determine the size.
> > + *
> > + * q_property(<property name> [, <default value>]): specifies that field 
> > is a
> > + *   qdev-style property. all properties of the struct are then accessible 
> > via
> > + *   QIDL_PROPERTIES(<device name>) macro.
> > + */
> > +
> > +#define q_immutable QIDL(immutable)
> > +#define q_derived QIDL(derived)
> > +#define q_broken QIDL(broken)
> > +#define q_optional QIDL(optional)
> > +#define q_elsewhere QIDL(elsewhere)
> > +#define q_size(...) QIDL(size_is, ##__VA_ARGS__)
> > +#define q_property(name, ...) QIDL(property, name, ##__VA_ARGS__)
> > +
> > +#endif
> > 
> 
>