[Qemu-devel] [PATCH 0/3] qapi: Easier unboxed visits of subset of union type

[Qemu-devel] [PATCH 0/3] qapi: Easier unboxed visits of subset of union type

[Eric Blake]

Dan Berrange rightly pointed out that now that commit 544a373
unboxes the variants of unions, we will need a way for his pending
LUKS patches to access the fields of those variants without also
allocating wasted memory.

Patch 1 is a cleanup I noticed along the way, although in writing
this email, I now see it is only tangentially related.
Patch 2 provides the fix Dan needs, and patch 3 documents it,
along with other recent changes.

Note that this series has a conflict with my other pending series
(https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg04703.html);
but as it is shorter and is blocking Dan's patches, it may be easier
to review these first.  Depending on which series lands first, I don't
mind posting a rebased version of the other series.

Also available as a tag at this location:
git fetch git://repo.or.cz/qemu/ericb.git qapi-exportv1

and will soon be part of my branch at:
http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi

Eric Blake (3):
  qapi-dealloc: Reduce use outside of generated code
  qapi-visit: Expose visit_type_FOO_fields()
  qapi: Update docs to match recent generator changes

 scripts/qapi-visit.py     |  47 ++-----
 hw/acpi/core.c            |  11 +-
 net/net.c                 |  31 ++---
 numa.c                    |   9 +-
 tests/test-opts-visitor.c |  10 +-
 docs/qapi-code-gen.txt    | 308 ++++++++++++++++++++++++----------------------
 6 files changed, 188 insertions(+), 228 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH 1/3] qapi-dealloc: Reduce use outside of generated code

[Eric Blake]

No need to roll our own use of the dealloc visitors when we can
just directly use the qapi_free_FOO() functions that do what we
want in one line.

In net.c, inline net_visit() into its remaining lone caller.

After this patch, test-visitor-serialization.c is the only
non-generated file that needs to use a dealloc visitor, because
it is testing low level aspects of the visitor interface.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 hw/acpi/core.c            | 11 +----------
 net/net.c                 | 31 ++++++++++---------------------
 numa.c                    |  9 +--------
 tests/test-opts-visitor.c | 10 +---------
 4 files changed, 13 insertions(+), 48 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 3a14e90..3d9e5c4 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -26,7 +26,6 @@
 #include "hw/nvram/fw_cfg.h"
 #include "qemu/config-file.h"
 #include "qapi/opts-visitor.h"
-#include "qapi/dealloc-visitor.h"
 #include "qapi-visit.h"
 #include "qapi-event.h"

@@ -297,15 +296,7 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
 out:
     g_free(blob);
     g_strfreev(pathnames);
-
-    if (hdrs != NULL) {
-        QapiDeallocVisitor *dv;
-
-        dv = qapi_dealloc_visitor_new();
-        visit_type_AcpiTableOptions(qapi_dealloc_get_visitor(dv), NULL, &hdrs,
-                                    NULL);
-        qapi_dealloc_visitor_cleanup(dv);
-    }
+    qapi_free_AcpiTableOptions(hdrs);

     error_propagate(errp, err);
 }
diff --git a/net/net.c b/net/net.c
index aebf753..b0c832e 100644
--- a/net/net.c
+++ b/net/net.c
@@ -42,7 +42,6 @@
 #include "qemu/main-loop.h"
 #include "qapi-visit.h"
 #include "qapi/opts-visitor.h"
-#include "qapi/dealloc-visitor.h"
 #include "sysemu/sysemu.h"
 #include "net/filter.h"
 #include "qapi/string-output-visitor.h"
@@ -1043,38 +1042,28 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
 }


-static void net_visit(Visitor *v, int is_netdev, void **object, Error **errp)
-{
-    if (is_netdev) {
-        visit_type_Netdev(v, NULL, (Netdev **)object, errp);
-    } else {
-        visit_type_NetLegacy(v, NULL, (NetLegacy **)object, errp);
-    }
-}
-
-
 int net_client_init(QemuOpts *opts, int is_netdev, Error **errp)
 {
     void *object = NULL;
     Error *err = NULL;
     int ret = -1;
+    OptsVisitor *ov = opts_visitor_new(opts);
+    Visitor *v = opts_get_visitor(ov);

-    {
-        OptsVisitor *ov = opts_visitor_new(opts);
-
-        net_visit(opts_get_visitor(ov), is_netdev, &object, &err);
-        opts_visitor_cleanup(ov);
+    if (is_netdev) {
+        visit_type_Netdev(v, NULL, (Netdev **)&object, &err);
+    } else {
+        visit_type_NetLegacy(v, NULL, (NetLegacy **)&object, &err);
     }

     if (!err) {
         ret = net_client_init1(object, is_netdev, &err);
     }

-    if (object) {
-        QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
-
-        net_visit(qapi_dealloc_get_visitor(dv), is_netdev, &object, NULL);
-        qapi_dealloc_visitor_cleanup(dv);
+    if (is_netdev) {
+        qapi_free_Netdev(object);
+    } else {
+        qapi_free_NetLegacy(object);
     }

     error_propagate(errp, err);
diff --git a/numa.c b/numa.c
index 4c4f7f5..da27bf8 100644
--- a/numa.c
+++ b/numa.c
@@ -31,7 +31,6 @@
 #include "include/exec/cpu-common.h" /* for RAM_ADDR_FMT */
 #include "qapi-visit.h"
 #include "qapi/opts-visitor.h"
-#include "qapi/dealloc-visitor.h"
 #include "hw/boards.h"
 #include "sysemu/hostmem.h"
 #include "qmp-commands.h"
@@ -243,13 +242,7 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)

 error:
     error_report_err(err);
-
-    if (object) {
-        QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
-        visit_type_NumaOptions(qapi_dealloc_get_visitor(dv), NULL, &object,
-                               NULL);
-        qapi_dealloc_visitor_cleanup(dv);
-    }
+    qapi_free_NumaOptions(object);

     return -1;
 }
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index b7acf7d..297a02d 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -17,7 +17,6 @@
 #include "qemu/option.h"          /* qemu_opts_parse() */
 #include "qapi/opts-visitor.h"    /* opts_visitor_new() */
 #include "test-qapi-visit.h"      /* visit_type_UserDefOptions() */
-#include "qapi/dealloc-visitor.h" /* qapi_dealloc_visitor_new() */

 static QemuOptsList userdef_opts = {
     .name = "userdef",
@@ -55,14 +54,7 @@ setup_fixture(OptsVisitorFixture *f, gconstpointer test_data)
 static void
 teardown_fixture(OptsVisitorFixture *f, gconstpointer test_data)
 {
-    if (f->userdef != NULL) {
-        QapiDeallocVisitor *dv;
-
-        dv = qapi_dealloc_visitor_new();
-        visit_type_UserDefOptions(qapi_dealloc_get_visitor(dv), NULL,
-                                  &f->userdef, NULL);
-        qapi_dealloc_visitor_cleanup(dv);
-    }
+    qapi_free_UserDefOptions(f->userdef);
     error_free(f->err);
 }

-- 
2.5.0

Re: [Qemu-devel] [PATCH 1/3] qapi-dealloc: Reduce use outside of generated code

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> No need to roll our own use of the dealloc visitors when we can
> just directly use the qapi_free_FOO() functions that do what we
> want in one line.
>
> In net.c, inline net_visit() into its remaining lone caller.
>
> After this patch, test-visitor-serialization.c is the only
> non-generated file that needs to use a dealloc visitor, because
> it is testing low level aspects of the visitor interface.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Much better.

Applied to my qapi-next branch, thanks!

[Qemu-devel] [PATCH 2/3] qapi-visit: Expose visit_type_FOO_fields()

[Eric Blake]

Dan Berrange reported a case where he needs to work with a
QCryptoBlockOptions union type using the OptsVisitor, but only
visit one of the branches of that type (the discriminator is not
visited directly, but learned externally).  When things were
boxed, it was easy: just visit the variant directly, which took
care of both allocating the variant and visiting its fields.  But
now that things are unboxed, we need a way to visit the fields
without allocation, done by exposing visit_type_FOO_fields() to
the user.  Of course, this should only be done for objects, not
lists, so we need another flag to gen_visit_decl().

Since the function is now public, we no longer need to preserve
topological ordering via struct_fields_seen.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
Minor conflicts with pending series "qapi implicit types"; I can
rebase whichever series gets reviewed second.
---
 scripts/qapi-visit.py | 47 +++++++++++++----------------------------------
 1 file changed, 13 insertions(+), 34 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 2308268..35efe7c 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -15,52 +15,33 @@
 from qapi import *
 import re

-# visit_type_FOO_fields() is always emitted; track if a forward declaration
-# or implementation has already been output.
-struct_fields_seen = set()

-
-def gen_visit_decl(name, scalar=False):
+def gen_visit_decl(name, scalar=False, list=False):
+    ret = ''
     c_type = c_name(name) + ' *'
     if not scalar:
+        if not list:
+            ret += mcgen('''
+void visit_type_%(c_name)s_fields(Visitor *v, %(c_type)sobj, Error **errp);
+''',
+                         c_name=c_name(name), c_type=c_type)
         c_type += '*'
-    return mcgen('''
+    ret += mcgen('''
 void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_type)sobj, Error **errp);
 ''',
                  c_name=c_name(name), c_type=c_type)
-
-
-def gen_visit_fields_decl(typ):
-    if typ.name in struct_fields_seen:
-        return ''
-    struct_fields_seen.add(typ.name)
-    return mcgen('''
-
-static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s *obj, Error **errp);
-''',
-                 c_type=typ.c_name())
+    return ret


 def gen_visit_struct_fields(name, base, members, variants):
-    ret = ''
+    ret = mcgen('''

-    if base:
-        ret += gen_visit_fields_decl(base)
-    if variants:
-        for var in variants.variants:
-            # Ugly special case for simple union TODO get rid of it
-            if not var.simple_union_type():
-                ret += gen_visit_fields_decl(var.type)
-
-    struct_fields_seen.add(name)
-    ret += mcgen('''
-
-static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s *obj, Error **errp)
+void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s *obj, Error **errp)
 {
     Error *err = NULL;

 ''',
-                 c_name=c_name(name))
+                c_name=c_name(name))

     if base:
         ret += mcgen('''
@@ -173,8 +154,6 @@ def gen_visit_alternate(name, variants):
     for var in variants.variants:
         if var.type.alternate_qtype() == 'QTYPE_QINT':
             promote_int = 'false'
-        if isinstance(var.type, QAPISchemaObjectType):
-            ret += gen_visit_fields_decl(var.type)

     ret += mcgen('''

@@ -305,7 +284,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
             self.defn += gen_visit_enum(name)

     def visit_array_type(self, name, info, element_type):
-        decl = gen_visit_decl(name)
+        decl = gen_visit_decl(name, list=True)
         defn = gen_visit_list(name, element_type)
         if isinstance(element_type, QAPISchemaBuiltinType):
             self._btin += decl
-- 
2.5.0

Re: [Qemu-devel] [PATCH 2/3] qapi-visit: Expose visit_type_FOO_fields()

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> Dan Berrange reported a case where he needs to work with a
> QCryptoBlockOptions union type using the OptsVisitor, but only
> visit one of the branches of that type (the discriminator is not
> visited directly, but learned externally).  When things were
> boxed, it was easy: just visit the variant directly, which took
> care of both allocating the variant and visiting its fields.  But
> now that things are unboxed, we need a way to visit the fields
> without allocation, done by exposing visit_type_FOO_fields() to
> the user.  Of course, this should only be done for objects, not
> lists, so we need another flag to gen_visit_decl().
>
> Since the function is now public, we no longer need to preserve
> topological ordering via struct_fields_seen.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> Minor conflicts with pending series "qapi implicit types"; I can
> rebase whichever series gets reviewed second.
> ---
>  scripts/qapi-visit.py | 47 +++++++++++++----------------------------------
>  1 file changed, 13 insertions(+), 34 deletions(-)
>

Let me review how this thing works for an object type FOO before your
patch.

gen_visit_object() generates visit_type_FOO() with external linkage, and
gen_visit_struct_fields() generates visit_type_FOO_fields() with
internal linkage.

gen_visit_decl() generates a declaration of visit_type_FOO(), and
gen_visit_fields_decl() generates one for visit_type_FOO_fields() unless
it's already in scope.

visit_type_FOO_fields() is always called by visit_type_FOO(), and
sometimes called elsewhere.

We generate visit_type_FOO_fields() right before visit_type_FOO(), so
it's in scope there.  Anything that generates uses elsewhere must call
gen_visit_fields_decl().

Your patch generates visit_type_FOO_fields() declarations into the
header, which renders the "if already in scope" logic useless, along
with the need to call gen_visit_fields_decl() before generating
visit_type_FOO_fields() uses outside visit_type_FOO().

> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 2308268..35efe7c 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -15,52 +15,33 @@
>  from qapi import *
>  import re
>
> -# visit_type_FOO_fields() is always emitted; track if a forward declaration
> -# or implementation has already been output.
> -struct_fields_seen = set()
>
> -
> -def gen_visit_decl(name, scalar=False):
> +def gen_visit_decl(name, scalar=False, list=False):
> +    ret = ''
>      c_type = c_name(name) + ' *'
>      if not scalar:
> +        if not list:
> +            ret += mcgen('''
> +void visit_type_%(c_name)s_fields(Visitor *v, %(c_type)sobj, Error **errp);
> +''',
> +                         c_name=c_name(name), c_type=c_type)
>          c_type += '*'
> -    return mcgen('''
> +    ret += mcgen('''
>  void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_type)sobj, Error **errp);
>  ''',
>                   c_name=c_name(name), c_type=c_type)
> -
> -
> -def gen_visit_fields_decl(typ):
> -    if typ.name in struct_fields_seen:
> -        return ''
> -    struct_fields_seen.add(typ.name)
> -    return mcgen('''
> -
> -static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s *obj, Error **errp);
> -''',
> -                 c_type=typ.c_name())
> +    return ret

This folds gen_visit_fields_decl() into gen_visit_decl() with the
necessary changes: drop the "is already in scope" conditional, switch to
external linkage.

Since gen_visit_decl() is called for non-object types as well, it gets a
new optional parameter to suppress generation of
visit_type_FOO_fields().  The parameter is named @list, which makes no
sense to me.  See more below.

I don't like do-everything functions with suppressor flags much.  Can we
structure this as a set of do-one-thing functions?  Possibly with
convenience functions collecting common sets.

>  def gen_visit_struct_fields(name, base, members, variants):
> -    ret = ''
> +    ret = mcgen('''
>
> -    if base:
> -        ret += gen_visit_fields_decl(base)
> -    if variants:
> -        for var in variants.variants:
> -            # Ugly special case for simple union TODO get rid of it
> -            if not var.simple_union_type():
> -                ret += gen_visit_fields_decl(var.type)

Drop gen_visit_fields_decl() calls.  Good.

> -
> -    struct_fields_seen.add(name)

Drop code supporting "if already in scope" conditionals.  Good.

> -    ret += mcgen('''
> -
> -static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s *obj, Error **errp)
> +void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s *obj, Error **errp)

Change linkage.  Good.

>  {
>      Error *err = NULL;
>
>  ''',
> -                 c_name=c_name(name))
> +                c_name=c_name(name))
>
>      if base:
>          ret += mcgen('''
> @@ -173,8 +154,6 @@ def gen_visit_alternate(name, variants):
>      for var in variants.variants:
>          if var.type.alternate_qtype() == 'QTYPE_QINT':
>              promote_int = 'false'
> -        if isinstance(var.type, QAPISchemaObjectType):
> -            ret += gen_visit_fields_decl(var.type)

Drop gen_visit_fields_decl() calls.  Good.

>
>      ret += mcgen('''
>
> @@ -305,7 +284,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
       def visit_enum_type(self, name, info, values, prefix):
           # Special case for our lone builtin enum type
           # TODO use something cleaner than existence of info
           if not info:
               self._btin += gen_visit_decl(name, scalar=True)

No change, because scalar=True effectively implies list=True.

               if do_builtins:
                   self.defn += gen_visit_enum(name)
           else:
               self.decl += gen_visit_decl(name, scalar=True)

Likewise.

>              self.defn += gen_visit_enum(name)
>
>      def visit_array_type(self, name, info, element_type):
> -        decl = gen_visit_decl(name)
> +        decl = gen_visit_decl(name, list=True)


No change, because list=True suppresses it.

>          defn = gen_visit_list(name, element_type)
>          if isinstance(element_type, QAPISchemaBuiltinType):
>              self._btin += decl
               if do_builtins:
                   self.defn += defn
           else:
               self.decl += decl
               self.defn += defn

       def visit_object_type(self, name, info, base, members, variants):
           self.decl += gen_visit_decl(name)

Here, we now generate an additional visit_type_FOO_fields() declaration.

           self.defn += gen_visit_object(name, base, members, variants)

       def visit_alternate_type(self, name, info, variants):
           self.decl += gen_visit_decl(name)

Bug: here too.

    $ grep _BlockdevRef_fields q*[ch]
    qapi-visit.h:void visit_type_BlockdevRef_fields(Visitor *v, BlockdevRef *obj, Error **errp);

           self.defn += gen_visit_alternate(name, variants)

Your choice of an optional argument to keep things unchanged effectively
hid the places that changed.  Failed to fool me today, but don't expect
to remain lucky that way :)

One more thing: I never liked the name _fields.  C struct and union
types don't have fields, they have members.  Likewiese, JSON objects.
We shouldn't gratitously invent terminology.  We've done that quite a
bit around QMP (JSON object vs. C QDict, JSON array vs. QList, QFLoat is
really a double, ...).  Cleaning that up completely is probably hopeless
by now.  But we can rename visit_type_FOO_fields() to something more
sensible, say visit_type_FOO_members(), or even
visit_type_FOO_unboxed().

Re: [Qemu-devel] [PATCH 2/3] qapi-visit: Expose visit_type_FOO_fields()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 4971 bytes --]

On 02/24/2016 05:28 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Dan Berrange reported a case where he needs to work with a
>> QCryptoBlockOptions union type using the OptsVisitor, but only
>> visit one of the branches of that type (the discriminator is not
>> visited directly, but learned externally).  When things were
>> boxed, it was easy: just visit the variant directly, which took
>> care of both allocating the variant and visiting its fields.  But
>> now that things are unboxed, we need a way to visit the fields
>> without allocation, done by exposing visit_type_FOO_fields() to
>> the user.  Of course, this should only be done for objects, not
>> lists, so we need another flag to gen_visit_decl().
>>
>> Since the function is now public, we no longer need to preserve
>> topological ordering via struct_fields_seen.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>
>> ---
>> Minor conflicts with pending series "qapi implicit types"; I can
>> rebase whichever series gets reviewed second.

Sounds like you want this in first; I'll respin a single series with
both sets of patches, with this at the front.

>> ---
>>  scripts/qapi-visit.py | 47 +++++++++++++----------------------------------
>>  1 file changed, 13 insertions(+), 34 deletions(-)
>>
> 
> Let me review how this thing works for an object type FOO before your
> patch.
> 
> gen_visit_object() generates visit_type_FOO() with external linkage, and
> gen_visit_struct_fields() generates visit_type_FOO_fields() with
> internal linkage.
> 
> gen_visit_decl() generates a declaration of visit_type_FOO(), and
> gen_visit_fields_decl() generates one for visit_type_FOO_fields() unless
> it's already in scope.
> 
> visit_type_FOO_fields() is always called by visit_type_FOO(), and
> sometimes called elsewhere.
> 
> We generate visit_type_FOO_fields() right before visit_type_FOO(), so
> it's in scope there.  Anything that generates uses elsewhere must call
> gen_visit_fields_decl().
> 
> Your patch generates visit_type_FOO_fields() declarations into the
> header, which renders the "if already in scope" logic useless, along
> with the need to call gen_visit_fields_decl() before generating
> visit_type_FOO_fields() uses outside visit_type_FOO().

Correct. I'll try and incorporate some of this text in the commit
message for better justification.

>> +def gen_visit_decl(name, scalar=False, list=False):
>> +    ret = ''
>>      c_type = c_name(name) + ' *'
>>      if not scalar:
>> +        if not list:
>> +            ret += mcgen('''
>> +void visit_type_%(c_name)s_fields(Visitor *v, %(c_type)sobj, Error **errp);
>> +''',
>> +                         c_name=c_name(name), c_type=c_type)

> 
> This folds gen_visit_fields_decl() into gen_visit_decl() with the
> necessary changes: drop the "is already in scope" conditional, switch to
> external linkage.
> 
> Since gen_visit_decl() is called for non-object types as well, it gets a
> new optional parameter to suppress generation of
> visit_type_FOO_fields().  The parameter is named @list, which makes no
> sense to me.  See more below.
> 
> I don't like do-everything functions with suppressor flags much.  Can we
> structure this as a set of do-one-thing functions?  Possibly with
> convenience functions collecting common sets.

Sure; gen_visit_decl() stays unchanged, and gen_visit_decl_fields()
would be new and called only for objects.


> 
>        def visit_alternate_type(self, name, info, variants):
>            self.decl += gen_visit_decl(name)
> 
> Bug: here too.

Indeed - the declaration doesn't hurt, but there is no implementation to
back up the declaration, so it's better if I fix things to not add the
declaration.  v2 will fix it.

> 
>     $ grep _BlockdevRef_fields q*[ch]
>     qapi-visit.h:void visit_type_BlockdevRef_fields(Visitor *v, BlockdevRef *obj, Error **errp);
> 
>            self.defn += gen_visit_alternate(name, variants)
> 
> Your choice of an optional argument to keep things unchanged effectively
> hid the places that changed.  Failed to fool me today, but don't expect
> to remain lucky that way :)
> 
> One more thing: I never liked the name _fields.  C struct and union
> types don't have fields, they have members.  Likewiese, JSON objects.
> We shouldn't gratitously invent terminology.  We've done that quite a
> bit around QMP (JSON object vs. C QDict, JSON array vs. QList, QFLoat is
> really a double, ...).  Cleaning that up completely is probably hopeless
> by now.  But we can rename visit_type_FOO_fields() to something more
> sensible, say visit_type_FOO_members(), or even
> visit_type_FOO_unboxed().

visit_type_FOO_members() sounds slightly nicer to me.  I'll use that
terminology in v2 to see how things look.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH 3/3] qapi: Update docs to match recent generator changes

[Eric Blake]

Several commits have been changing the generator, but not updating
the docs to match:
- The implicit tag member is named "type", not "kind".  Screwed up in
commit 39a1815.
- Commit 9f08c8ec made list types lazy, and thereby dropped
UserDefOneList if nothing explicitly uses the list type.
- Commit 51e72bc1 switched the parameter order with 'name' occurring
earlier.
- Commit e65d89bf changed the layout of UserDefOneList.
- We now expose visit_type_FOO_fields() for objects.
- etc.

Rework the examples to show slightly more output (we don't want to
show too much; that's what the testsuite is for), and regenerate the
output to match all recent changes.  Also, rearrange output to show
.h files before .c (understanding the interface first often makes
the implementation easier to follow).

Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

---
Some content from Markus, hence his S-o-b.
A former version of this patch was posted with subset E v9.
---
 docs/qapi-code-gen.txt | 308 ++++++++++++++++++++++++++-----------------------
 1 file changed, 162 insertions(+), 146 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 999f3b9..e1dde87 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -1,7 +1,7 @@
 = How to use the QAPI code generator =

 Copyright IBM Corp. 2011
-Copyright (C) 2012-2015 Red Hat, Inc.
+Copyright (C) 2012-2016 Red Hat, Inc.

 This work is licensed under the terms of the GNU GPL, version 2 or
 later. See the COPYING file in the top-level directory.
@@ -656,7 +656,7 @@ Union types

     { "name": "BlockdevOptions", "meta-type": "object",
       "members": [
-          { "name": "kind", "type": "BlockdevOptionsKind" } ],
+          { "name": "type", "type": "BlockdevOptionsKind" } ],
       "tag": "type",
       "variants": [
           { "case": "file", "type": ":obj-FileOptions-wrapper" },
@@ -722,33 +722,39 @@ the names of built-in types.  Clients should examine member

 == Code generation ==

-Schemas are fed into four scripts to generate all the code/files that,
+Schemas are fed into five scripts to generate all the code/files that,
 paired with the core QAPI libraries, comprise everything required to
 take JSON commands read in by a Client JSON Protocol server, unmarshal
 the arguments into the underlying C types, call into the corresponding
-C function, and map the response back to a Client JSON Protocol
-response to be returned to the user.
+C function, map the response back to a Client JSON Protocol response
+to be returned to the user, and introspect the commands.

-As an example, we'll use the following schema, which describes a single
-complex user-defined type (which will produce a C struct, along with a list
-node structure that can be used to chain together a list of such types in
-case we want to accept/return a list of this type with a command), and a
-command which takes that type as a parameter and returns the same type:
+As an example, we'll use the following schema, which describes a
+single complex user-defined type, along with command which takes a
+list of that type as a parameter, and returns a single element of that
+type.  The user is responsible for writing the implementation of
+qmp_my_command(); everything else is produced by the generator.

     $ cat example-schema.json
     { 'struct': 'UserDefOne',
-      'data': { 'integer': 'int', 'string': 'str' } }
+      'data': { 'integer': 'int', '*string': 'str' } }

     { 'command': 'my-command',
-      'data':    {'arg1': 'UserDefOne'},
+      'data':    { 'arg1': ['UserDefOne'] },
       'returns': 'UserDefOne' }

     { 'event': 'MY_EVENT' }

+For a more thorough look at generated code, the testsuite includes
+tests/qapi-schema/qapi-schema-tests.json that covers more examples of
+what the generator will accept, and compiles the resulting C code as
+part of 'make check-unit'.
+
 === scripts/qapi-types.py ===

-Used to generate the C types defined by a schema. The following files are
-created:
+Used to generate the C types defined by a schema, as well as the
+functions for recursively cleaning up their resources, as
+qapi_free_FOO(). The following files are created:

 $(prefix)qapi-types.h - C types corresponding to types defined in
                         the schema you pass in
@@ -763,38 +769,6 @@ Example:

     $ python scripts/qapi-types.py --output-dir="qapi-generated" \
     --prefix="example-" example-schema.json
-    $ cat qapi-generated/example-qapi-types.c
-[Uninteresting stuff omitted...]
-
-    void qapi_free_UserDefOne(UserDefOne *obj)
-    {
-        QapiDeallocVisitor *qdv;
-        Visitor *v;
-
-        if (!obj) {
-            return;
-        }
-
-        qdv = qapi_dealloc_visitor_new();
-        v = qapi_dealloc_get_visitor(qdv);
-        visit_type_UserDefOne(v, &obj, NULL, NULL);
-        qapi_dealloc_visitor_cleanup(qdv);
-    }
-
-    void qapi_free_UserDefOneList(UserDefOneList *obj)
-    {
-        QapiDeallocVisitor *qdv;
-        Visitor *v;
-
-        if (!obj) {
-            return;
-        }
-
-        qdv = qapi_dealloc_visitor_new();
-        v = qapi_dealloc_get_visitor(qdv);
-        visit_type_UserDefOneList(v, &obj, NULL, NULL);
-        qapi_dealloc_visitor_cleanup(qdv);
-    }
     $ cat qapi-generated/example-qapi-types.h
 [Uninteresting stuff omitted...]

@@ -809,29 +783,59 @@ Example:

     struct UserDefOne {
         int64_t integer;
+        bool has_string;
         char *string;
     };

     void qapi_free_UserDefOne(UserDefOne *obj);

     struct UserDefOneList {
-        union {
-            UserDefOne *value;
-            uint64_t padding;
-        };
         UserDefOneList *next;
+        UserDefOne *value;
     };

     void qapi_free_UserDefOneList(UserDefOneList *obj);

     #endif
+    $ cat qapi-generated/example-qapi-types.c
+[Uninteresting stuff omitted...]
+
+    void qapi_free_UserDefOne(UserDefOne *obj)
+    {
+        QapiDeallocVisitor *qdv;
+        Visitor *v;
+
+        if (!obj) {
+            return;
+        }
+
+        qdv = qapi_dealloc_visitor_new();
+        v = qapi_dealloc_get_visitor(qdv);
+        visit_type_UserDefOne(v, NULL, &obj, NULL);
+        qapi_dealloc_visitor_cleanup(qdv);
+    }
+
+    void qapi_free_UserDefOneList(UserDefOneList *obj)
+    {
+        QapiDeallocVisitor *qdv;
+        Visitor *v;
+
+        if (!obj) {
+            return;
+        }
+
+        qdv = qapi_dealloc_visitor_new();
+        v = qapi_dealloc_get_visitor(qdv);
+        visit_type_UserDefOneList(v, NULL, &obj, NULL);
+        qapi_dealloc_visitor_cleanup(qdv);
+    }

 === scripts/qapi-visit.py ===

-Used to generate the visitor functions used to walk through and convert
-a QObject (as provided by QMP) to a native C data structure and
-vice-versa, as well as the visitor function used to dealloc a complex
-schema-defined C type.
+Used to generate the visitor functions used to walk through and
+convert between a native QAPI C data structure and some other format
+(such as QObject); the generated functions are named visit_type_FOO()
+and visit_type_FOO_fields().

 The following files are generated:

@@ -848,41 +852,62 @@ Example:

     $ python scripts/qapi-visit.py --output-dir="qapi-generated"
     --prefix="example-" example-schema.json
+    $ cat qapi-generated/example-qapi-visit.h
+[Uninteresting stuff omitted...]
+
+    #ifndef EXAMPLE_QAPI_VISIT_H
+    #define EXAMPLE_QAPI_VISIT_H
+
+[Visitors for built-in types omitted...]
+
+    void visit_type_UserDefOne_fields(Visitor *v, UserDefOne *obj, Error **errp);
+    void visit_type_UserDefOne(Visitor *v, const char *name, UserDefOne **obj, Error **errp);
+    void visit_type_UserDefOneList(Visitor *v, const char *name, UserDefOneList **obj, Error **errp);
+
+    #endif
     $ cat qapi-generated/example-qapi-visit.c
 [Uninteresting stuff omitted...]

-    static void visit_type_UserDefOne_fields(Visitor *v, UserDefOne **obj, Error **errp)
+    void visit_type_UserDefOne_fields(Visitor *v, UserDefOne *obj, Error **errp)
     {
         Error *err = NULL;

-        visit_type_int(v, &(*obj)->integer, "integer", &err);
+        visit_type_int(v, "integer", &obj->integer, &err);
         if (err) {
             goto out;
         }
-        visit_type_str(v, &(*obj)->string, "string", &err);
-        if (err) {
-            goto out;
-        }
-
-    out:
-        error_propagate(errp, err);
-    }
-
-    void visit_type_UserDefOne(Visitor *v, UserDefOne **obj, const char *name, Error **errp)
-    {
-        Error *err = NULL;
-
-        visit_start_struct(v, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), &err);
-        if (!err) {
-            if (*obj) {
-                visit_type_UserDefOne_fields(v, obj, errp);
+        if (visit_optional(v, "string", &obj->has_string)) {
+            visit_type_str(v, "string", &obj->string, &err);
+            if (err) {
+                goto out;
             }
-            visit_end_struct(v, &err);
         }
+
+    out:
+        error_propagate(errp, err);
+    }
+
+    void visit_type_UserDefOne(Visitor *v, const char *name, UserDefOne **obj, Error **errp)
+    {
+        Error *err = NULL;
+
+        visit_start_struct(v, name, (void **)obj, sizeof(UserDefOne), &err);
+        if (err) {
+            goto out;
+        }
+        if (!*obj) {
+            goto out_obj;
+        }
+        visit_type_UserDefOne_fields(v, *obj, &err);
+        error_propagate(errp, err);
+        err = NULL;
+    out_obj:
+        visit_end_struct(v, &err);
+    out:
         error_propagate(errp, err);
     }

-    void visit_type_UserDefOneList(Visitor *v, UserDefOneList **obj, const char *name, Error **errp)
+    void visit_type_UserDefOneList(Visitor *v, const char *name, UserDefOneList **obj, Error **errp)
     {
         Error *err = NULL;
         GenericList *i, **prev;
@@ -893,35 +918,24 @@ Example:
         }

         for (prev = (GenericList **)obj;
-             !err && (i = visit_next_list(v, prev, &err)) != NULL;
+             !err && (i = visit_next_list(v, prev, sizeof(**obj))) != NULL;
              prev = &i) {
             UserDefOneList *native_i = (UserDefOneList *)i;
-            visit_type_UserDefOne(v, &native_i->value, NULL, &err);
+            visit_type_UserDefOne(v, NULL, &native_i->value, &err);
         }

-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_list(v, &err);
+        visit_end_list(v);
     out:
         error_propagate(errp, err);
     }
-    $ cat qapi-generated/example-qapi-visit.h
-[Uninteresting stuff omitted...]
-
-    #ifndef EXAMPLE_QAPI_VISIT_H
-    #define EXAMPLE_QAPI_VISIT_H
-
-[Visitors for built-in types omitted...]
-
-    void visit_type_UserDefOne(Visitor *v, UserDefOne **obj, const char *name, Error **errp);
-    void visit_type_UserDefOneList(Visitor *v, UserDefOneList **obj, const char *name, Error **errp);
-
-    #endif

 === scripts/qapi-commands.py ===

-Used to generate the marshaling/dispatch functions for the commands defined
-in the schema. The following files are generated:
+Used to generate the marshaling/dispatch functions for the commands
+defined in the schema. The generated code implements
+qmp_marshal_COMMAND() (mentioned in qmp-commands.hx, and registered
+automatically), and declares qmp_COMMAND() that the user must
+implement.  The following files are generated:

 $(prefix)qmp-marshal.c: command marshal/dispatch functions for each
                         QMP command defined in the schema. Functions
@@ -939,6 +953,19 @@ Example:

     $ python scripts/qapi-commands.py --output-dir="qapi-generated"
     --prefix="example-" example-schema.json
+    $ cat qapi-generated/example-qmp-commands.h
+[Uninteresting stuff omitted...]
+
+    #ifndef EXAMPLE_QMP_COMMANDS_H
+    #define EXAMPLE_QMP_COMMANDS_H
+
+    #include "example-qapi-types.h"
+    #include "qapi/qmp/qdict.h"
+    #include "qapi/error.h"
+
+    UserDefOne *qmp_my_command(UserDefOneList *arg1, Error **errp);
+
+    #endif
     $ cat qapi-generated/example-qmp-marshal.c
 [Uninteresting stuff omitted...]

@@ -950,7 +977,7 @@ Example:
         Visitor *v;

         v = qmp_output_get_visitor(qov);
-        visit_type_UserDefOne(v, &ret_in, "unused", &err);
+        visit_type_UserDefOne(v, "unused", &ret_in, &err);
         if (err) {
             goto out;
         }
@@ -961,7 +988,7 @@ Example:
         qmp_output_visitor_cleanup(qov);
         qdv = qapi_dealloc_visitor_new();
         v = qapi_dealloc_get_visitor(qdv);
-        visit_type_UserDefOne(v, &ret_in, "unused", NULL);
+        visit_type_UserDefOne(v, "unused", &ret_in, NULL);
         qapi_dealloc_visitor_cleanup(qdv);
     }

@@ -972,10 +999,10 @@ Example:
         QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
         QapiDeallocVisitor *qdv;
         Visitor *v;
-        UserDefOne *arg1 = NULL;
+        UserDefOneList *arg1 = NULL;

         v = qmp_input_get_visitor(qiv);
-        visit_type_UserDefOne(v, &arg1, "arg1", &err);
+        visit_type_UserDefOne(v, "arg1", &arg1, &err);
         if (err) {
             goto out;
         }
@@ -992,7 +1019,7 @@ Example:
         qmp_input_visitor_cleanup(qiv);
         qdv = qapi_dealloc_visitor_new();
         v = qapi_dealloc_get_visitor(qdv);
-        visit_type_UserDefOne(v, &arg1, "arg1", NULL);
+        visit_type_UserDefOne(v, "arg1", &arg1, NULL);
         qapi_dealloc_visitor_cleanup(qdv);
     }

@@ -1002,24 +1029,12 @@ Example:
     }

     qapi_init(qmp_init_marshal);
-    $ cat qapi-generated/example-qmp-commands.h
-[Uninteresting stuff omitted...]
-
-    #ifndef EXAMPLE_QMP_COMMANDS_H
-    #define EXAMPLE_QMP_COMMANDS_H
-
-    #include "example-qapi-types.h"
-    #include "qapi/qmp/qdict.h"
-    #include "qapi/error.h"
-
-    UserDefOne *qmp_my_command(UserDefOne *arg1, Error **errp);
-
-    #endif

 === scripts/qapi-event.py ===

-Used to generate the event-related C code defined by a schema. The
-following files are created:
+Used to generate the event-related C code defined by a schema, with
+implementations for qapi_event_send_FOO(). The following files are
+created:

 $(prefix)qapi-event.h - Function prototypes for each event type, plus an
                         enumeration of all event names
@@ -1029,6 +1044,27 @@ Example:

     $ python scripts/qapi-event.py --output-dir="qapi-generated"
     --prefix="example-" example-schema.json
+    $ cat qapi-generated/example-qapi-event.h
+[Uninteresting stuff omitted...]
+
+    #ifndef EXAMPLE_QAPI_EVENT_H
+    #define EXAMPLE_QAPI_EVENT_H
+
+    #include "qapi/error.h"
+    #include "qapi/qmp/qdict.h"
+    #include "example-qapi-types.h"
+
+
+    void qapi_event_send_my_event(Error **errp);
+
+    typedef enum example_QAPIEvent {
+        EXAMPLE_QAPI_EVENT_MY_EVENT = 0,
+        EXAMPLE_QAPI_EVENT__MAX = 1,
+    } example_QAPIEvent;
+
+    extern const char *const example_QAPIEvent_lookup[];
+
+    #endif
     $ cat qapi-generated/example-qapi-event.c
 [Uninteresting stuff omitted...]

@@ -1054,27 +1090,6 @@ Example:
         [EXAMPLE_QAPI_EVENT_MY_EVENT] = "MY_EVENT",
         [EXAMPLE_QAPI_EVENT__MAX] = NULL,
     };
-    $ cat qapi-generated/example-qapi-event.h
-[Uninteresting stuff omitted...]
-
-    #ifndef EXAMPLE_QAPI_EVENT_H
-    #define EXAMPLE_QAPI_EVENT_H
-
-    #include "qapi/error.h"
-    #include "qapi/qmp/qdict.h"
-    #include "example-qapi-types.h"
-
-
-    void qapi_event_send_my_event(Error **errp);
-
-    typedef enum example_QAPIEvent {
-        EXAMPLE_QAPI_EVENT_MY_EVENT = 0,
-        EXAMPLE_QAPI_EVENT__MAX = 1,
-    } example_QAPIEvent;
-
-    extern const char *const example_QAPIEvent_lookup[];
-
-    #endif

 === scripts/qapi-introspect.py ===

@@ -1089,6 +1104,15 @@ Example:

     $ python scripts/qapi-introspect.py --output-dir="qapi-generated"
     --prefix="example-" example-schema.json
+    $ cat qapi-generated/example-qmp-introspect.h
+[Uninteresting stuff omitted...]
+
+    #ifndef EXAMPLE_QMP_INTROSPECT_H
+    #define EXAMPLE_QMP_INTROSPECT_H
+
+    extern const char example_qmp_schema_json[];
+
+    #endif
     $ cat qapi-generated/example-qmp-introspect.c
 [Uninteresting stuff omitted...]

@@ -1096,16 +1120,8 @@ Example:
         "{\"arg-type\": \"0\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
         "{\"arg-type\": \"1\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"2\"}, "
         "{\"members\": [], \"meta-type\": \"object\", \"name\": \"0\"}, "
-        "{\"members\": [{\"name\": \"arg1\", \"type\": \"2\"}], \"meta-type\": \"object\", \"name\": \"1\"}, "
-        "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"2\"}, "
+        "{\"members\": [{\"name\": \"arg1\", \"type\": \"[2]\"}], \"meta-type\": \"object\", \"name\": \"1\"}, "
+        "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"default\": null, \"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"2\"}, "
+        "{\"element-type\": \"2\", \"meta-type\": \"array\", \"name\": \"[2]\"}, "
         "{\"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": \"int\"}, "
         "{\"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": \"str\"}]";
-    $ cat qapi-generated/example-qmp-introspect.h
-[Uninteresting stuff omitted...]
-
-    #ifndef EXAMPLE_QMP_INTROSPECT_H
-    #define EXAMPLE_QMP_INTROSPECT_H
-
-    extern const char example_qmp_schema_json[];
-
-    #endif
-- 
2.5.0

Re: [Qemu-devel] [PATCH 3/3] qapi: Update docs to match recent generator changes

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> Several commits have been changing the generator, but not updating
> the docs to match:
> - The implicit tag member is named "type", not "kind".  Screwed up in
> commit 39a1815.
> - Commit 9f08c8ec made list types lazy, and thereby dropped
> UserDefOneList if nothing explicitly uses the list type.
> - Commit 51e72bc1 switched the parameter order with 'name' occurring
> earlier.
> - Commit e65d89bf changed the layout of UserDefOneList.
> - We now expose visit_type_FOO_fields() for objects.
> - etc.
>
> Rework the examples to show slightly more output (we don't want to
> show too much; that's what the testsuite is for), and regenerate the
> output to match all recent changes.  Also, rearrange output to show
> .h files before .c (understanding the interface first often makes
> the implementation easier to follow).
>
> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> ---
> Some content from Markus, hence his S-o-b.
> A former version of this patch was posted with subset E v9.
> ---
>  docs/qapi-code-gen.txt | 308 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 162 insertions(+), 146 deletions(-)
>
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 999f3b9..e1dde87 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -1,7 +1,7 @@
>  = How to use the QAPI code generator =
>
>  Copyright IBM Corp. 2011
> -Copyright (C) 2012-2015 Red Hat, Inc.
> +Copyright (C) 2012-2016 Red Hat, Inc.
>
>  This work is licensed under the terms of the GNU GPL, version 2 or
>  later. See the COPYING file in the top-level directory.
> @@ -656,7 +656,7 @@ Union types
>
>      { "name": "BlockdevOptions", "meta-type": "object",
>        "members": [
> -          { "name": "kind", "type": "BlockdevOptionsKind" } ],
> +          { "name": "type", "type": "BlockdevOptionsKind" } ],
>        "tag": "type",
>        "variants": [
>            { "case": "file", "type": ":obj-FileOptions-wrapper" },
> @@ -722,33 +722,39 @@ the names of built-in types.  Clients should examine member
>
>  == Code generation ==
>
> -Schemas are fed into four scripts to generate all the code/files that,
> +Schemas are fed into five scripts to generate all the code/files that,
>  paired with the core QAPI libraries, comprise everything required to
>  take JSON commands read in by a Client JSON Protocol server, unmarshal
>  the arguments into the underlying C types, call into the corresponding
> -C function, and map the response back to a Client JSON Protocol
> -response to be returned to the user.
> +C function, map the response back to a Client JSON Protocol response
> +to be returned to the user, and introspect the commands.
>
> -As an example, we'll use the following schema, which describes a single
> -complex user-defined type (which will produce a C struct, along with a list
> -node structure that can be used to chain together a list of such types in
> -case we want to accept/return a list of this type with a command), and a
> -command which takes that type as a parameter and returns the same type:
> +As an example, we'll use the following schema, which describes a
> +single complex user-defined type, along with command which takes a
> +list of that type as a parameter, and returns a single element of that
> +type.  The user is responsible for writing the implementation of
> +qmp_my_command(); everything else is produced by the generator.
>
>      $ cat example-schema.json
>      { 'struct': 'UserDefOne',
> -      'data': { 'integer': 'int', 'string': 'str' } }
> +      'data': { 'integer': 'int', '*string': 'str' } }
>
>      { 'command': 'my-command',
> -      'data':    {'arg1': 'UserDefOne'},
> +      'data':    { 'arg1': ['UserDefOne'] },
>        'returns': 'UserDefOne' }
>
>      { 'event': 'MY_EVENT' }
>
> +For a more thorough look at generated code, the testsuite includes
> +tests/qapi-schema/qapi-schema-tests.json that covers more examples of
> +what the generator will accept, and compiles the resulting C code as
> +part of 'make check-unit'.
> +
>  === scripts/qapi-types.py ===
>
> -Used to generate the C types defined by a schema. The following files are
> -created:
> +Used to generate the C types defined by a schema, as well as the
> +functions for recursively cleaning up their resources, as
> +qapi_free_FOO(). The following files are created:

Actually, it also creates conversion functions and enum lookup tables.
Suggest to be less specific:

    Used to generate the C types defined by a schema, along with
    supporting code.  The following files are created:

>
>  $(prefix)qapi-types.h - C types corresponding to types defined in
>                          the schema you pass in
> @@ -763,38 +769,6 @@ Example:
>
>      $ python scripts/qapi-types.py --output-dir="qapi-generated" \
>      --prefix="example-" example-schema.json
> -    $ cat qapi-generated/example-qapi-types.c
> -[Uninteresting stuff omitted...]
> -
> -    void qapi_free_UserDefOne(UserDefOne *obj)
> -    {
> -        QapiDeallocVisitor *qdv;
> -        Visitor *v;
> -
> -        if (!obj) {
> -            return;
> -        }
> -
> -        qdv = qapi_dealloc_visitor_new();
> -        v = qapi_dealloc_get_visitor(qdv);
> -        visit_type_UserDefOne(v, &obj, NULL, NULL);
> -        qapi_dealloc_visitor_cleanup(qdv);
> -    }
> -
> -    void qapi_free_UserDefOneList(UserDefOneList *obj)
> -    {
> -        QapiDeallocVisitor *qdv;
> -        Visitor *v;
> -
> -        if (!obj) {
> -            return;
> -        }
> -
> -        qdv = qapi_dealloc_visitor_new();
> -        v = qapi_dealloc_get_visitor(qdv);
> -        visit_type_UserDefOneList(v, &obj, NULL, NULL);
> -        qapi_dealloc_visitor_cleanup(qdv);
> -    }
>      $ cat qapi-generated/example-qapi-types.h
>  [Uninteresting stuff omitted...]
>
> @@ -809,29 +783,59 @@ Example:
>
>      struct UserDefOne {
>          int64_t integer;
> +        bool has_string;
>          char *string;
>      };
>
>      void qapi_free_UserDefOne(UserDefOne *obj);
>
>      struct UserDefOneList {
> -        union {
> -            UserDefOne *value;
> -            uint64_t padding;
> -        };
>          UserDefOneList *next;
> +        UserDefOne *value;
>      };
>
>      void qapi_free_UserDefOneList(UserDefOneList *obj);
>
>      #endif
> +    $ cat qapi-generated/example-qapi-types.c
> +[Uninteresting stuff omitted...]
> +
> +    void qapi_free_UserDefOne(UserDefOne *obj)
> +    {
> +        QapiDeallocVisitor *qdv;
> +        Visitor *v;
> +
> +        if (!obj) {
> +            return;
> +        }
> +
> +        qdv = qapi_dealloc_visitor_new();
> +        v = qapi_dealloc_get_visitor(qdv);
> +        visit_type_UserDefOne(v, NULL, &obj, NULL);
> +        qapi_dealloc_visitor_cleanup(qdv);
> +    }
> +
> +    void qapi_free_UserDefOneList(UserDefOneList *obj)
> +    {
> +        QapiDeallocVisitor *qdv;
> +        Visitor *v;
> +
> +        if (!obj) {
> +            return;
> +        }
> +
> +        qdv = qapi_dealloc_visitor_new();
> +        v = qapi_dealloc_get_visitor(qdv);
> +        visit_type_UserDefOneList(v, NULL, &obj, NULL);
> +        qapi_dealloc_visitor_cleanup(qdv);
> +    }
>
>  === scripts/qapi-visit.py ===
>
> -Used to generate the visitor functions used to walk through and convert
> -a QObject (as provided by QMP) to a native C data structure and
> -vice-versa, as well as the visitor function used to dealloc a complex
> -schema-defined C type.
> +Used to generate the visitor functions used to walk through and
> +convert between a native QAPI C data structure and some other format
> +(such as QObject); the generated functions are named visit_type_FOO()
> +and visit_type_FOO_fields().
>
>  The following files are generated:
>
> @@ -848,41 +852,62 @@ Example:
>
>      $ python scripts/qapi-visit.py --output-dir="qapi-generated"
>      --prefix="example-" example-schema.json
> +    $ cat qapi-generated/example-qapi-visit.h
> +[Uninteresting stuff omitted...]
> +
> +    #ifndef EXAMPLE_QAPI_VISIT_H
> +    #define EXAMPLE_QAPI_VISIT_H
> +
> +[Visitors for built-in types omitted...]
> +
> +    void visit_type_UserDefOne_fields(Visitor *v, UserDefOne *obj, Error **errp);
> +    void visit_type_UserDefOne(Visitor *v, const char *name, UserDefOne **obj, Error **errp);
> +    void visit_type_UserDefOneList(Visitor *v, const char *name, UserDefOneList **obj, Error **errp);
> +
> +    #endif
>      $ cat qapi-generated/example-qapi-visit.c
>  [Uninteresting stuff omitted...]
>
> -    static void visit_type_UserDefOne_fields(Visitor *v, UserDefOne **obj, Error **errp)
> +    void visit_type_UserDefOne_fields(Visitor *v, UserDefOne *obj, Error **errp)
>      {
>          Error *err = NULL;
>
> -        visit_type_int(v, &(*obj)->integer, "integer", &err);
> +        visit_type_int(v, "integer", &obj->integer, &err);
>          if (err) {
>              goto out;
>          }
> -        visit_type_str(v, &(*obj)->string, "string", &err);
> -        if (err) {
> -            goto out;
> -        }
> -
> -    out:
> -        error_propagate(errp, err);
> -    }
> -
> -    void visit_type_UserDefOne(Visitor *v, UserDefOne **obj, const char *name, Error **errp)
> -    {
> -        Error *err = NULL;
> -
> -        visit_start_struct(v, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), &err);
> -        if (!err) {
> -            if (*obj) {
> -                visit_type_UserDefOne_fields(v, obj, errp);
> +        if (visit_optional(v, "string", &obj->has_string)) {
> +            visit_type_str(v, "string", &obj->string, &err);
> +            if (err) {
> +                goto out;
>              }
> -            visit_end_struct(v, &err);
>          }
> +
> +    out:
> +        error_propagate(errp, err);
> +    }
> +
> +    void visit_type_UserDefOne(Visitor *v, const char *name, UserDefOne **obj, Error **errp)
> +    {
> +        Error *err = NULL;
> +
> +        visit_start_struct(v, name, (void **)obj, sizeof(UserDefOne), &err);
> +        if (err) {
> +            goto out;
> +        }
> +        if (!*obj) {
> +            goto out_obj;
> +        }
> +        visit_type_UserDefOne_fields(v, *obj, &err);
> +        error_propagate(errp, err);
> +        err = NULL;
> +    out_obj:
> +        visit_end_struct(v, &err);
> +    out:
>          error_propagate(errp, err);
>      }
>
> -    void visit_type_UserDefOneList(Visitor *v, UserDefOneList **obj, const char *name, Error **errp)
> +    void visit_type_UserDefOneList(Visitor *v, const char *name, UserDefOneList **obj, Error **errp)
>      {
>          Error *err = NULL;
>          GenericList *i, **prev;
> @@ -893,35 +918,24 @@ Example:
>          }
>
>          for (prev = (GenericList **)obj;
> -             !err && (i = visit_next_list(v, prev, &err)) != NULL;
> +             !err && (i = visit_next_list(v, prev, sizeof(**obj))) != NULL;
>               prev = &i) {
>              UserDefOneList *native_i = (UserDefOneList *)i;
> -            visit_type_UserDefOne(v, &native_i->value, NULL, &err);
> +            visit_type_UserDefOne(v, NULL, &native_i->value, &err);
>          }
>
> -        error_propagate(errp, err);
> -        err = NULL;
> -        visit_end_list(v, &err);
> +        visit_end_list(v);
>      out:
>          error_propagate(errp, err);
>      }
> -    $ cat qapi-generated/example-qapi-visit.h
> -[Uninteresting stuff omitted...]
> -
> -    #ifndef EXAMPLE_QAPI_VISIT_H
> -    #define EXAMPLE_QAPI_VISIT_H
> -
> -[Visitors for built-in types omitted...]
> -
> -    void visit_type_UserDefOne(Visitor *v, UserDefOne **obj, const char *name, Error **errp);
> -    void visit_type_UserDefOneList(Visitor *v, UserDefOneList **obj, const char *name, Error **errp);
> -
> -    #endif
>
>  === scripts/qapi-commands.py ===
>
> -Used to generate the marshaling/dispatch functions for the commands defined
> -in the schema. The following files are generated:
> +Used to generate the marshaling/dispatch functions for the commands
> +defined in the schema. The generated code implements
> +qmp_marshal_COMMAND() (mentioned in qmp-commands.hx, and registered
> +automatically), and declares qmp_COMMAND() that the user must
> +implement.  The following files are generated:
>
>  $(prefix)qmp-marshal.c: command marshal/dispatch functions for each
>                          QMP command defined in the schema. Functions
> @@ -939,6 +953,19 @@ Example:
>
>      $ python scripts/qapi-commands.py --output-dir="qapi-generated"
>      --prefix="example-" example-schema.json
> +    $ cat qapi-generated/example-qmp-commands.h
> +[Uninteresting stuff omitted...]
> +
> +    #ifndef EXAMPLE_QMP_COMMANDS_H
> +    #define EXAMPLE_QMP_COMMANDS_H
> +
> +    #include "example-qapi-types.h"
> +    #include "qapi/qmp/qdict.h"
> +    #include "qapi/error.h"
> +
> +    UserDefOne *qmp_my_command(UserDefOneList *arg1, Error **errp);
> +
> +    #endif
>      $ cat qapi-generated/example-qmp-marshal.c
>  [Uninteresting stuff omitted...]
>
> @@ -950,7 +977,7 @@ Example:
>          Visitor *v;
>
>          v = qmp_output_get_visitor(qov);
> -        visit_type_UserDefOne(v, &ret_in, "unused", &err);
> +        visit_type_UserDefOne(v, "unused", &ret_in, &err);
>          if (err) {
>              goto out;
>          }
> @@ -961,7 +988,7 @@ Example:
>          qmp_output_visitor_cleanup(qov);
>          qdv = qapi_dealloc_visitor_new();
>          v = qapi_dealloc_get_visitor(qdv);
> -        visit_type_UserDefOne(v, &ret_in, "unused", NULL);
> +        visit_type_UserDefOne(v, "unused", &ret_in, NULL);
>          qapi_dealloc_visitor_cleanup(qdv);
>      }
>
> @@ -972,10 +999,10 @@ Example:
>          QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
>          QapiDeallocVisitor *qdv;
>          Visitor *v;
> -        UserDefOne *arg1 = NULL;
> +        UserDefOneList *arg1 = NULL;
>
>          v = qmp_input_get_visitor(qiv);
> -        visit_type_UserDefOne(v, &arg1, "arg1", &err);
> +        visit_type_UserDefOne(v, "arg1", &arg1, &err);

UserDefOneList

>          if (err) {
>              goto out;
>          }
> @@ -992,7 +1019,7 @@ Example:
>          qmp_input_visitor_cleanup(qiv);
>          qdv = qapi_dealloc_visitor_new();
>          v = qapi_dealloc_get_visitor(qdv);
> -        visit_type_UserDefOne(v, &arg1, "arg1", NULL);
> +        visit_type_UserDefOne(v, "arg1", &arg1, NULL);

UserDefOneList

>          qapi_dealloc_visitor_cleanup(qdv);
>      }
>
> @@ -1002,24 +1029,12 @@ Example:
>      }
>
>      qapi_init(qmp_init_marshal);
> -    $ cat qapi-generated/example-qmp-commands.h
> -[Uninteresting stuff omitted...]
> -
> -    #ifndef EXAMPLE_QMP_COMMANDS_H
> -    #define EXAMPLE_QMP_COMMANDS_H
> -
> -    #include "example-qapi-types.h"
> -    #include "qapi/qmp/qdict.h"
> -    #include "qapi/error.h"
> -
> -    UserDefOne *qmp_my_command(UserDefOne *arg1, Error **errp);
> -
> -    #endif
>
>  === scripts/qapi-event.py ===
>
> -Used to generate the event-related C code defined by a schema. The
> -following files are created:
> +Used to generate the event-related C code defined by a schema, with
> +implementations for qapi_event_send_FOO(). The following files are
> +created:
>
>  $(prefix)qapi-event.h - Function prototypes for each event type, plus an
>                          enumeration of all event names
> @@ -1029,6 +1044,27 @@ Example:
>
>      $ python scripts/qapi-event.py --output-dir="qapi-generated"
>      --prefix="example-" example-schema.json
> +    $ cat qapi-generated/example-qapi-event.h
> +[Uninteresting stuff omitted...]
> +
> +    #ifndef EXAMPLE_QAPI_EVENT_H
> +    #define EXAMPLE_QAPI_EVENT_H
> +
> +    #include "qapi/error.h"
> +    #include "qapi/qmp/qdict.h"
> +    #include "example-qapi-types.h"
> +
> +
> +    void qapi_event_send_my_event(Error **errp);
> +
> +    typedef enum example_QAPIEvent {
> +        EXAMPLE_QAPI_EVENT_MY_EVENT = 0,
> +        EXAMPLE_QAPI_EVENT__MAX = 1,
> +    } example_QAPIEvent;
> +
> +    extern const char *const example_QAPIEvent_lookup[];
> +
> +    #endif
>      $ cat qapi-generated/example-qapi-event.c
>  [Uninteresting stuff omitted...]
>
> @@ -1054,27 +1090,6 @@ Example:
>          [EXAMPLE_QAPI_EVENT_MY_EVENT] = "MY_EVENT",
>          [EXAMPLE_QAPI_EVENT__MAX] = NULL,
>      };
> -    $ cat qapi-generated/example-qapi-event.h
> -[Uninteresting stuff omitted...]
> -
> -    #ifndef EXAMPLE_QAPI_EVENT_H
> -    #define EXAMPLE_QAPI_EVENT_H
> -
> -    #include "qapi/error.h"
> -    #include "qapi/qmp/qdict.h"
> -    #include "example-qapi-types.h"
> -
> -
> -    void qapi_event_send_my_event(Error **errp);
> -
> -    typedef enum example_QAPIEvent {
> -        EXAMPLE_QAPI_EVENT_MY_EVENT = 0,
> -        EXAMPLE_QAPI_EVENT__MAX = 1,
> -    } example_QAPIEvent;
> -
> -    extern const char *const example_QAPIEvent_lookup[];
> -
> -    #endif
>
>  === scripts/qapi-introspect.py ===
>
> @@ -1089,6 +1104,15 @@ Example:
>
>      $ python scripts/qapi-introspect.py --output-dir="qapi-generated"
>      --prefix="example-" example-schema.json
> +    $ cat qapi-generated/example-qmp-introspect.h
> +[Uninteresting stuff omitted...]
> +
> +    #ifndef EXAMPLE_QMP_INTROSPECT_H
> +    #define EXAMPLE_QMP_INTROSPECT_H
> +
> +    extern const char example_qmp_schema_json[];
> +
> +    #endif
>      $ cat qapi-generated/example-qmp-introspect.c
>  [Uninteresting stuff omitted...]
>
> @@ -1096,16 +1120,8 @@ Example:
>          "{\"arg-type\": \"0\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
>          "{\"arg-type\": \"1\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"2\"}, "
>          "{\"members\": [], \"meta-type\": \"object\", \"name\": \"0\"}, "
> -        "{\"members\": [{\"name\": \"arg1\", \"type\": \"2\"}], \"meta-type\": \"object\", \"name\": \"1\"}, "
> -        "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"2\"}, "
> +        "{\"members\": [{\"name\": \"arg1\", \"type\": \"[2]\"}], \"meta-type\": \"object\", \"name\": \"1\"}, "
> +        "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"default\": null, \"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"2\"}, "
> +        "{\"element-type\": \"2\", \"meta-type\": \"array\", \"name\": \"[2]\"}, "
>          "{\"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": \"int\"}, "
>          "{\"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": \"str\"}]";
> -    $ cat qapi-generated/example-qmp-introspect.h
> -[Uninteresting stuff omitted...]
> -
> -    #ifndef EXAMPLE_QMP_INTROSPECT_H
> -    #define EXAMPLE_QMP_INTROSPECT_H
> -
> -    extern const char example_qmp_schema_json[];
> -
> -    #endif

With my three remarks addressed:

Reviewed-by: Markus Armbruster <armbru@redhat.com>