Merge remote-tracking branch 'remotes/qmp-unstable/queue/qmp' into staging

* remotes/qmp-unstable/queue/qmp:
  qapi: skip redundant includes
  monitor: Add netdev_del id argument completion.
  monitor: Add netdev_add type argument completion.
  monitor: Add set_link arguments completion.
  monitor: Add chardev-add backend argument completion.
  monitor: Add chardev-remove command completion.
  monitor: Convert sendkey to use command_completion.
  qapi: Show qapi-commands.py invocation in qapi-code-gen.txt
  qapi: Replace uncommon use of the error API by the common one
  tests: Don't call visit_end_struct() after visit_start_struct() fails
  hw: Don't call visit_end_struct() after visit_start_struct() fails
  hmp: Call visit_end_struct() after visit_start_struct() succeeds
  qapi: Un-inline visit of implicit struct
  qapi-visit.py: Clean up a sloppy use of field prefix
  qapi: Clean up shadowing of parameters and locals in inner scopes
  qapi-visit.py: Clean up confusing push_indent() / pop_indent() use
  qapi: Replace start_optional()/end_optional() by optional()
  qapi: Remove unused Visitor callbacks start_handle(), end_handle()
  qapi: Normalize marshalling's visitor initialization and cleanup
  qapi: Update qapi-code-gen.txt example to match current code

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

27 files changed, 752 insertions, 445 deletions

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 26312d84e8..dea0d505a7 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -48,7 +48,7 @@ The QAPI schema definitions can be modularized using the 'include' directive:
  { 'include': 'path/to/file.json'}
 
 The directive is evaluated recursively, and include paths are relative to the
-file using the directive.
+file using the directive. Multiple includes of the same file are safe.
 
 
 === Complex types ===
@@ -230,14 +230,13 @@ 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:
 
-    mdroth@illuin:~/w/qemu2.git$ cat example-schema.json
+    $ cat example-schema.json
     { 'type': 'UserDefOne',
       'data': { 'integer': 'int', 'string': 'str' } }
 
     { 'command': 'my-command',
       'data':    {'arg1': 'UserDefOne'},
       'returns': 'UserDefOne' }
-    mdroth@illuin:~/w/qemu2.git$
 
 === scripts/qapi-types.py ===
 
@@ -255,14 +254,25 @@ created code.
 
 Example:
 
-    mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \
-      --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.c
-    /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
+    $ python scripts/qapi-types.py --output-dir="qapi-generated" \
+    --prefix="example-" --input-file=example-schema.json
+    $ cat qapi-generated/example-qapi-types.c
+[Uninteresting stuff omitted...]
 
-    #include "qapi/qapi-dealloc-visitor.h"
-    #include "example-qapi-types.h"
-    #include "example-qapi-visit.h"
+    void qapi_free_UserDefOneList(UserDefOneList * obj)
+    {
+        QapiDeallocVisitor *md;
+        Visitor *v;
+
+        if (!obj) {
+            return;
+        }
+
+        md = qapi_dealloc_visitor_new();
+        v = qapi_dealloc_get_visitor(md);
+        visit_type_UserDefOneList(v, &obj, NULL, NULL);
+        qapi_dealloc_visitor_cleanup(md);
+    }
 
     void qapi_free_UserDefOne(UserDefOne * obj)
     {
@@ -279,32 +289,38 @@ Example:
         qapi_dealloc_visitor_cleanup(md);
     }
 
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.h
-    /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
-    #ifndef QAPI_GENERATED_EXAMPLE_QAPI_TYPES
-    #define QAPI_GENERATED_EXAMPLE_QAPI_TYPES
+    $ cat qapi-generated/example-qapi-types.h
+[Uninteresting stuff omitted...]
+
+    #ifndef EXAMPLE_QAPI_TYPES_H
+    #define EXAMPLE_QAPI_TYPES_H
 
-    #include "qapi/qapi-types-core.h"
+[Builtin types omitted...]
 
     typedef struct UserDefOne UserDefOne;
 
     typedef struct UserDefOneList
     {
-        UserDefOne *value;
+        union {
+            UserDefOne *value;
+            uint64_t padding;
+        };
         struct UserDefOneList *next;
     } UserDefOneList;
 
+[Functions on builtin types omitted...]
+
     struct UserDefOne
     {
         int64_t integer;
         char * string;
     };
 
+    void qapi_free_UserDefOneList(UserDefOneList * obj);
     void qapi_free_UserDefOne(UserDefOne * obj);
 
     #endif
 
-
 === scripts/qapi-visit.py ===
 
 Used to generate the visitor functions used to walk through and convert
@@ -325,51 +341,78 @@ $(prefix)qapi-visit.h: declarations for previously mentioned visitor
 
 Example:
 
-    mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-visit.py \
-        --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.c
-    /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+    $ python scripts/qapi-visit.py --output-dir="qapi-generated"
+    --prefix="example-" --input-file=example-schema.json
+    $ cat qapi-generated/example-qapi-visit.c
+[Uninteresting stuff omitted...]
 
-    #include "example-qapi-visit.h"
+    static void visit_type_UserDefOne_fields(Visitor *m, UserDefOne ** obj, Error **errp)
+    {
+        Error *err = NULL;
+        visit_type_int(m, &(*obj)->integer, "integer", &err);
+        if (err) {
+            goto out;
+        }
+        visit_type_str(m, &(*obj)->string, "string", &err);
+        if (err) {
+            goto out;
+        }
+
+    out:
+        error_propagate(errp, err);
+    }
 
     void visit_type_UserDefOne(Visitor *m, UserDefOne ** obj, const char *name, Error **errp)
     {
-        visit_start_struct(m, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), errp);
-        visit_type_int(m, (obj && *obj) ? &(*obj)->integer : NULL, "integer", errp);
-        visit_type_str(m, (obj && *obj) ? &(*obj)->string : NULL, "string", errp);
-        visit_end_struct(m, errp);
+        Error *err = NULL;
+
+        visit_start_struct(m, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), &err);
+        if (!err) {
+            if (*obj) {
+                visit_type_UserDefOne_fields(m, obj, errp);
+            }
+            visit_end_struct(m, &err);
+        }
+        error_propagate(errp, err);
     }
 
     void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const char *name, Error **errp)
     {
-        GenericList *i, **prev = (GenericList **)obj;
+        Error *err = NULL;
+        GenericList *i, **prev;
 
-        visit_start_list(m, name, errp);
+        visit_start_list(m, name, &err);
+        if (err) {
+            goto out;
+        }
 
-        for (; (i = visit_next_list(m, prev, errp)) != NULL; prev = &i) {
+        for (prev = (GenericList **)obj;
+             !err && (i = visit_next_list(m, prev, &err)) != NULL;
+             prev = &i) {
             UserDefOneList *native_i = (UserDefOneList *)i;
-            visit_type_UserDefOne(m, &native_i->value, NULL, errp);
+            visit_type_UserDefOne(m, &native_i->value, NULL, &err);
         }
 
-        visit_end_list(m, errp);
+        error_propagate(errp, err);
+        err = NULL;
+        visit_end_list(m, &err);
+    out:
+        error_propagate(errp, err);
     }
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.h
-    /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+    $ python scripts/qapi-commands.py --output-dir="qapi-generated" \
+    --prefix="example-" --input-file=example-schema.json
+    $ cat qapi-generated/example-qapi-visit.h
+[Uninteresting stuff omitted...]
 
-    #ifndef QAPI_GENERATED_EXAMPLE_QAPI_VISIT
-    #define QAPI_GENERATED_EXAMPLE_QAPI_VISIT
+    #ifndef EXAMPLE_QAPI_VISIT_H
+    #define EXAMPLE_QAPI_VISIT_H
 
-    #include "qapi/qapi-visit-core.h"
-    #include "example-qapi-types.h"
+[Visitors for builtin types omitted...]
 
     void visit_type_UserDefOne(Visitor *m, UserDefOne ** obj, const char *name, Error **errp);
     void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const char *name, Error **errp);
 
     #endif
-    mdroth@illuin:~/w/qemu2.git$
-
-(The actual structure of the visit_type_* functions is a bit more complex
-in order to propagate errors correctly and avoid leaking memory).
 
 === scripts/qapi-commands.py ===
 
@@ -390,77 +433,80 @@ $(prefix)qmp-commands.h: Function prototypes for the QMP commands
 
 Example:
 
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qmp-marshal.c
-    /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
-
-    #include "qemu-objects.h"
-    #include "qapi/qmp-core.h"
-    #include "qapi/qapi-visit-core.h"
-    #include "qapi/qmp-output-visitor.h"
-    #include "qapi/qmp-input-visitor.h"
-    #include "qapi/qapi-dealloc-visitor.h"
-    #include "example-qapi-types.h"
-    #include "example-qapi-visit.h"
+    $ cat qapi-generated/example-qmp-marshal.c
+[Uninteresting stuff omitted...]
 
-    #include "example-qmp-commands.h"
     static void qmp_marshal_output_my_command(UserDefOne * ret_in, QObject **ret_out, Error **errp)
     {
-        QapiDeallocVisitor *md = qapi_dealloc_visitor_new();
+        Error *local_err = NULL;
         QmpOutputVisitor *mo = qmp_output_visitor_new();
+        QapiDeallocVisitor *md;
         Visitor *v;
 
         v = qmp_output_get_visitor(mo);
-        visit_type_UserDefOne(v, &ret_in, "unused", errp);
+        visit_type_UserDefOne(v, &ret_in, "unused", &local_err);
+        if (local_err) {
+            goto out;
+        }
+        *ret_out = qmp_output_get_qobject(mo);
+
+    out:
+        error_propagate(errp, local_err);
+        qmp_output_visitor_cleanup(mo);
+        md = qapi_dealloc_visitor_new();
         v = qapi_dealloc_get_visitor(md);
-        visit_type_UserDefOne(v, &ret_in, "unused", errp);
+        visit_type_UserDefOne(v, &ret_in, "unused", NULL);
         qapi_dealloc_visitor_cleanup(md);
-
-
-        *ret_out = qmp_output_get_qobject(mo);
     }
 
-    static void qmp_marshal_input_my_command(QmpState *qmp__sess, QDict *args, QObject **ret, Error **errp)
+    static void qmp_marshal_input_my_command(QDict *args, QObject **ret, Error **errp)
     {
+        Error *local_err = NULL;
         UserDefOne * retval = NULL;
-        QmpInputVisitor *mi;
+        QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
         QapiDeallocVisitor *md;
         Visitor *v;
         UserDefOne * arg1 = NULL;
 
-        mi = qmp_input_visitor_new(QOBJECT(args));
         v = qmp_input_get_visitor(mi);
-        visit_type_UserDefOne(v, &arg1, "arg1", errp);
+        visit_type_UserDefOne(v, &arg1, "arg1", &local_err);
+        if (local_err) {
+            goto out;
+        }
 
-        if (error_is_set(errp)) {
+        retval = qmp_my_command(arg1, &local_err);
+        if (local_err) {
             goto out;
         }
-        retval = qmp_my_command(arg1, errp);
-        qmp_marshal_output_my_command(retval, ret, errp);
+
+        qmp_marshal_output_my_command(retval, ret, &local_err);
 
     out:
+        error_propagate(errp, local_err);
+        qmp_input_visitor_cleanup(mi);
         md = qapi_dealloc_visitor_new();
         v = qapi_dealloc_get_visitor(md);
-        visit_type_UserDefOne(v, &arg1, "arg1", errp);
+        visit_type_UserDefOne(v, &arg1, "arg1", NULL);
         qapi_dealloc_visitor_cleanup(md);
         return;
     }
 
     static void qmp_init_marshal(void)
     {
-        qmp_register_command("my-command", qmp_marshal_input_my_command);
+        qmp_register_command("my-command", qmp_marshal_input_my_command, QCO_NO_OPTIONS);
     }
 
     qapi_init(qmp_init_marshal);
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qmp-commands.h
-    /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+    $ cat qapi-generated/example-qmp-commands.h
+[Uninteresting stuff omitted...]
 
-    #ifndef QAPI_GENERATED_EXAMPLE_QMP_COMMANDS
-    #define QAPI_GENERATED_EXAMPLE_QMP_COMMANDS
+    #ifndef EXAMPLE_QMP_COMMANDS_H
+    #define EXAMPLE_QMP_COMMANDS_H
 
     #include "example-qapi-types.h"
-    #include "error.h"
+    #include "qapi/qmp/qdict.h"
+    #include "qapi/error.h"
 
     UserDefOne * qmp_my_command(UserDefOne * arg1, Error **errp);
 
     #endif
-    mdroth@illuin:~/w/qemu2.git$
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8971f1b153..2e462c04aa 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -556,6 +556,7 @@ ETEXI
         .params     = "keys [hold_ms]",
         .help       = "send keys to the VM (e.g. 'sendkey ctrl-alt-f1', default hold time=100 ms)",
         .mhandler.cmd = hmp_send_key,
+        .command_completion = sendkey_completion,
     },
 
 STEXI
@@ -1233,9 +1234,10 @@ ETEXI
     {
         .name       = "netdev_add",
         .args_type  = "netdev:O",
-        .params     = "[user|tap|socket|hubport|netmap],id=str[,prop=value][,...]",
+        .params     = "[user|tap|socket|vde|bridge|hubport|netmap],id=str[,prop=value][,...]",
         .help       = "add host network device",
         .mhandler.cmd = hmp_netdev_add,
+        .command_completion = netdev_add_completion,
     },
 
 STEXI
@@ -1250,6 +1252,7 @@ ETEXI
         .params     = "id",
         .help       = "remove host network device",
         .mhandler.cmd = hmp_netdev_del,
+        .command_completion = netdev_del_completion,
     },
 
 STEXI
@@ -1339,6 +1342,7 @@ ETEXI
         .params     = "name on|off",
         .help       = "change the link status of a network adapter",
         .mhandler.cmd = hmp_set_link,
+        .command_completion = set_link_completion,
     },
 
 STEXI
@@ -1622,6 +1626,7 @@ ETEXI
         .params     = "args",
         .help       = "add chardev",
         .mhandler.cmd = hmp_chardev_add,
+        .command_completion = chardev_add_completion,
     },
 
 STEXI
@@ -1638,6 +1643,7 @@ ETEXI
         .params     = "id",
         .help       = "remove chardev",
         .mhandler.cmd = hmp_chardev_remove,
+        .command_completion = chardev_remove_completion,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 5c4d612294..a9d0236cc3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1388,6 +1388,7 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
 void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
+    Error *err_end = NULL;
     QemuOpts *opts;
     char *type = NULL;
     char *id = NULL;
@@ -1411,24 +1412,23 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
     qdict_del(pdict, "qom-type");
     visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
     if (err) {
-        goto out_clean;
+        goto out_end;
     }
 
     qdict_del(pdict, "id");
     visit_type_str(opts_get_visitor(ov), &id, "id", &err);
     if (err) {
-        goto out_clean;
+        goto out_end;
     }
 
     object_add(type, id, pdict, opts_get_visitor(ov), &err);
-    if (err) {
-        goto out_clean;
-    }
-    visit_end_struct(opts_get_visitor(ov), &err);
-    if (err) {
+
+out_end:
+    visit_end_struct(opts_get_visitor(ov), &err_end);
+    if (!err && err_end) {
         qmp_object_del(id, NULL);
     }
-
+    error_propagate(&err, err_end);
 out_clean:
     opts_visitor_cleanup(ov);
 
diff --git a/hmp.h b/hmp.h
index 20ef454b4a..aba59e95f0 100644
--- a/hmp.h
+++ b/hmp.h
@@ -97,5 +97,11 @@ void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_del_completion(ReadLineState *rs, int nb_args, const char *str);
+void sendkey_completion(ReadLineState *rs, int nb_args, const char *str);
+void chardev_remove_completion(ReadLineState *rs, int nb_args, const char *str);
+void chardev_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void set_link_completion(ReadLineState *rs, int nb_args, const char *str);
+void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str);
 
 #endif
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 8509309fa7..df54546562 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -793,19 +793,46 @@ static const MemoryRegionOps cmos_ops = {
 static void rtc_get_date(Object *obj, Visitor *v, void *opaque,
                          const char *name, Error **errp)
 {
+    Error *err = NULL;
     RTCState *s = MC146818_RTC(obj);
     struct tm current_tm;
 
     rtc_update_time(s);
     rtc_get_time(s, &current_tm);
-    visit_start_struct(v, NULL, "struct tm", name, 0, errp);
-    visit_type_int32(v, &current_tm.tm_year, "tm_year", errp);
-    visit_type_int32(v, &current_tm.tm_mon, "tm_mon", errp);
-    visit_type_int32(v, &current_tm.tm_mday, "tm_mday", errp);
-    visit_type_int32(v, &current_tm.tm_hour, "tm_hour", errp);
-    visit_type_int32(v, &current_tm.tm_min, "tm_min", errp);
-    visit_type_int32(v, &current_tm.tm_sec, "tm_sec", errp);
+    visit_start_struct(v, NULL, "struct tm", name, 0, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_int32(v, &current_tm.tm_year, "tm_year", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_mon, "tm_mon", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_mday, "tm_mday", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_hour, "tm_hour", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_min, "tm_min", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_sec, "tm_sec", &err);
+    if (err) {
+        goto out_end;
+    }
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
     visit_end_struct(v, errp);
+out:
+    error_propagate(errp, err);
 }
 
 static void rtc_realizefn(DeviceState *dev, Error **errp)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 971a921777..bf2b588b24 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -108,6 +108,7 @@ static void balloon_stats_poll_cb(void *opaque)
 static void balloon_stats_get_all(Object *obj, struct Visitor *v,
                                   void *opaque, const char *name, Error **errp)
 {
+    Error *err = NULL;
     VirtIOBalloon *s = opaque;
     int i;
 
@@ -116,17 +117,33 @@ static void balloon_stats_get_all(Object *obj, struct Visitor *v,
         return;
     }
 
-    visit_start_struct(v, NULL, "guest-stats", name, 0, errp);
-    visit_type_int(v, &s->stats_last_update, "last-update", errp);
+    visit_start_struct(v, NULL, "guest-stats", name, 0, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_int(v, &s->stats_last_update, "last-update", &err);
+    if (err) {
+        goto out_end;
+    }
 
-    visit_start_struct(v, NULL, NULL, "stats", 0, errp);
-    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
+    visit_start_struct(v, NULL, NULL, "stats", 0, &err);
+    if (err) {
+        goto out_end;
+    }
+    for (i = 0; !err && i < VIRTIO_BALLOON_S_NR; i++) {
         visit_type_int64(v, (int64_t *) &s->stats[i], balloon_stat_names[i],
-                         errp);
+                         &err);
     }
-    visit_end_struct(v, errp);
-
-    visit_end_struct(v, errp);
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
+    error_propagate(errp, err);
 }
 
 static void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v,
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index f3fa420245..ecc0183196 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -42,13 +42,9 @@ struct Visitor
                         Error **errp);
 
     /* May be NULL */
-    void (*start_optional)(Visitor *v, bool *present, const char *name,
-                           Error **errp);
-    void (*end_optional)(Visitor *v, Error **errp);
+    void (*optional)(Visitor *v, bool *present, const char *name,
+                     Error **errp);
 
-    void (*start_handle)(Visitor *v, void **obj, const char *kind,
-                         const char *name, Error **errp);
-    void (*end_handle)(Visitor *v, Error **errp);
     void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
     void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
     void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 29da211b47..4a0178fa46 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -39,9 +39,8 @@ void visit_end_implicit_struct(Visitor *v, Error **errp);
 void visit_start_list(Visitor *v, const char *name, Error **errp);
 GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
 void visit_end_list(Visitor *v, Error **errp);
-void visit_start_optional(Visitor *v, bool *present, const char *name,
-                          Error **errp);
-void visit_end_optional(Visitor *v, Error **errp);
+void visit_optional(Visitor *v, bool *present, const char *name,
+                    Error **errp);
 void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
                          const char *name, Error **errp);
 void visit_type_enum(Visitor *v, int *obj, const char *strings[],
diff --git a/monitor.c b/monitor.c
index 9af6b0ad66..593679a17a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4269,6 +4269,55 @@ static const char *next_arg_type(const char *typestr)
     return (p != NULL ? ++p : typestr);
 }
 
+static void add_completion_option(ReadLineState *rs, const char *str,
+                                  const char *option)
+{
+    if (!str || !option) {
+        return;
+    }
+    if (!strncmp(option, str, strlen(str))) {
+        readline_add_completion(rs, option);
+    }
+}
+
+void chardev_add_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    size_t len;
+    ChardevBackendInfoList *list, *start;
+
+    if (nb_args != 2) {
+        return;
+    }
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+
+    start = list = qmp_query_chardev_backends(NULL);
+    while (list) {
+        const char *chr_name = list->value->name;
+
+        if (!strncmp(chr_name, str, len)) {
+            readline_add_completion(rs, chr_name);
+        }
+        list = list->next;
+    }
+    qapi_free_ChardevBackendInfoList(start);
+}
+
+void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    size_t len;
+    int i;
+
+    if (nb_args != 2) {
+        return;
+    }
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+    for (i = 0; NetClientOptionsKind_lookup[i]; i++) {
+        add_completion_option(rs, str, NetClientOptionsKind_lookup[i]);
+    }
+}
+
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str)
 {
     GSList *list, *elt;
@@ -4339,6 +4388,29 @@ static void device_del_bus_completion(ReadLineState *rs,  BusState *bus,
     }
 }
 
+void chardev_remove_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    size_t len;
+    ChardevInfoList *list, *start;
+
+    if (nb_args != 2) {
+        return;
+    }
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+
+    start = list = qmp_query_chardev(NULL);
+    while (list) {
+        ChardevInfo *chr = list->value;
+
+        if (!strncmp(chr->label, str, len)) {
+            readline_add_completion(rs, chr->label);
+        }
+        list = list->next;
+    }
+    qapi_free_ChardevInfoList(start);
+}
+
 void device_del_completion(ReadLineState *rs, int nb_args, const char *str)
 {
     size_t len;
@@ -4376,6 +4448,77 @@ void object_del_completion(ReadLineState *rs, int nb_args, const char *str)
     qapi_free_ObjectPropertyInfoList(start);
 }
 
+void sendkey_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    int i;
+    char *sep;
+    size_t len;
+
+    if (nb_args != 2) {
+        return;
+    }
+    sep = strrchr(str, '-');
+    if (sep) {
+        str = sep + 1;
+    }
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+    for (i = 0; i < Q_KEY_CODE_MAX; i++) {
+        if (!strncmp(str, QKeyCode_lookup[i], len)) {
+            readline_add_completion(rs, QKeyCode_lookup[i]);
+        }
+    }
+}
+
+void set_link_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    size_t len;
+
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+    if (nb_args == 2) {
+        NetClientState *ncs[255];
+        int count, i;
+        count = qemu_find_net_clients_except(NULL, ncs,
+                                             NET_CLIENT_OPTIONS_KIND_NONE, 255);
+        for (i = 0; i < count; i++) {
+            const char *name = ncs[i]->name;
+            if (!strncmp(str, name, len)) {
+                readline_add_completion(rs, name);
+            }
+        }
+    } else if (nb_args == 3) {
+        add_completion_option(rs, str, "on");
+        add_completion_option(rs, str, "off");
+    }
+}
+
+void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    int len, count, i;
+    NetClientState *ncs[255];
+
+    if (nb_args != 2) {
+        return;
+    }
+
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+    count = qemu_find_net_clients_except(NULL, ncs, NET_CLIENT_OPTIONS_KIND_NIC,
+                                         255);
+    for (i = 0; i < count; i++) {
+        QemuOpts *opts;
+        const char *name = ncs[i]->name;
+        if (strncmp(str, name, len)) {
+            continue;
+        }
+        opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
+        if (opts) {
+            readline_add_completion(rs, name);
+        }
+    }
+}
+
 static void monitor_find_completion_by_table(Monitor *mon,
                                              const mon_cmd_t *cmd_table,
                                              char **args,
@@ -4444,15 +4587,7 @@ static void monitor_find_completion_by_table(Monitor *mon,
             break;
         case 's':
         case 'S':
-            if (!strcmp(cmd->name, "sendkey")) {
-                char *sep = strrchr(str, '-');
-                if (sep)
-                    str = sep + 1;
-                readline_set_completion_index(mon->rs, strlen(str));
-                for (i = 0; i < Q_KEY_CODE_MAX; i++) {
-                    cmd_completion(mon, str, QKeyCode_lookup[i]);
-                }
-            } else if (!strcmp(cmd->name, "help|?")) {
+            if (!strcmp(cmd->name, "help|?")) {
                 monitor_find_completion_by_table(mon, cmd_table,
                                                  &args[1], nb_args - 1);
             }
diff --git a/net/net.c b/net/net.c
index 9db4dba769..0ff2e40f35 100644
--- a/net/net.c
+++ b/net/net.c
@@ -633,7 +633,7 @@ int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
         if (nc->info->type == type) {
             continue;
         }
-        if (!strcmp(nc->name, id)) {
+        if (!id || !strcmp(nc->name, id)) {
             if (ret < max) {
                 ncs[ret] = nc;
             }
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 87c1c789c9..16382e7a65 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -484,8 +484,7 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
 
 
 static void
-opts_start_optional(Visitor *v, bool *present, const char *name,
-                       Error **errp)
+opts_optional(Visitor *v, bool *present, const char *name, Error **errp)
 {
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
 
@@ -528,7 +527,7 @@ opts_visitor_new(const QemuOpts *opts)
     /* type_number() is not filled in, but this is not the first visitor to
      * skip some mandatory methods... */
 
-    ov->visitor.start_optional = &opts_start_optional;
+    ov->visitor.optional = &opts_optional;
 
     ov->opts_root = opts;
 
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 6451a21a28..55f8d4068c 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -17,46 +17,27 @@
 #include "qapi/visitor.h"
 #include "qapi/visitor-impl.h"
 
-void visit_start_handle(Visitor *v, void **obj, const char *kind,
-                        const char *name, Error **errp)
-{
-    if (!error_is_set(errp) && v->start_handle) {
-        v->start_handle(v, obj, kind, name, errp);
-    }
-}
-
-void visit_end_handle(Visitor *v, Error **errp)
-{
-    if (!error_is_set(errp) && v->end_handle) {
-        v->end_handle(v, errp);
-    }
-}
-
 void visit_start_struct(Visitor *v, void **obj, const char *kind,
                         const char *name, size_t size, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->start_struct(v, obj, kind, name, size, errp);
-    }
+    v->start_struct(v, obj, kind, name, size, errp);
 }
 
 void visit_end_struct(Visitor *v, Error **errp)
 {
-    assert(!error_is_set(errp));
     v->end_struct(v, errp);
 }
 
 void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
                                  Error **errp)
 {
-    if (!error_is_set(errp) && v->start_implicit_struct) {
+    if (v->start_implicit_struct) {
         v->start_implicit_struct(v, obj, size, errp);
     }
 }
 
 void visit_end_implicit_struct(Visitor *v, Error **errp)
 {
-    assert(!error_is_set(errp));
     if (v->end_implicit_struct) {
         v->end_implicit_struct(v, errp);
     }
@@ -64,45 +45,31 @@ void visit_end_implicit_struct(Visitor *v, Error **errp)
 
 void visit_start_list(Visitor *v, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->start_list(v, name, errp);
-    }
+    v->start_list(v, name, errp);
 }
 
 GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        return v->next_list(v, list, errp);
-    }
-
-    return 0;
+    return v->next_list(v, list, errp);
 }
 
 void visit_end_list(Visitor *v, Error **errp)
 {
-    assert(!error_is_set(errp));
     v->end_list(v, errp);
 }
 
-void visit_start_optional(Visitor *v, bool *present, const char *name,
-                          Error **errp)
+void visit_optional(Visitor *v, bool *present, const char *name,
+                    Error **errp)
 {
-    if (!error_is_set(errp) && v->start_optional) {
-        v->start_optional(v, present, name, errp);
-    }
-}
-
-void visit_end_optional(Visitor *v, Error **errp)
-{
-    if (!error_is_set(errp) && v->end_optional) {
-        v->end_optional(v, errp);
+    if (v->optional) {
+        v->optional(v, present, name, errp);
     }
 }
 
 void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
                          const char *name, Error **errp)
 {
-    if (!error_is_set(errp) && v->get_next_type) {
+    if (v->get_next_type) {
         v->get_next_type(v, obj, qtypes, name, errp);
     }
 }
@@ -110,192 +77,172 @@ void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
 void visit_type_enum(Visitor *v, int *obj, const char *strings[],
                      const char *kind, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_enum(v, obj, strings, kind, name, errp);
-    }
+    v->type_enum(v, obj, strings, kind, name, errp);
 }
 
 void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_int(v, obj, name, errp);
-    }
+    v->type_int(v, obj, name, errp);
 }
 
 void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint8) {
-            v->type_uint8(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < 0 || value > UINT8_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "uint8_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_uint8) {
+        v->type_uint8(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < 0 || value > UINT8_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "uint8_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint16) {
-            v->type_uint16(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < 0 || value > UINT16_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "uint16_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_uint16) {
+        v->type_uint16(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < 0 || value > UINT16_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "uint16_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint32) {
-            v->type_uint32(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < 0 || value > UINT32_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "uint32_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_uint32) {
+        v->type_uint32(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < 0 || value > UINT32_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "uint32_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint64) {
-            v->type_uint64(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            *obj = value;
-        }
+
+    if (v->type_uint64) {
+        v->type_uint64(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        *obj = value;
     }
 }
 
 void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_int8) {
-            v->type_int8(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < INT8_MIN || value > INT8_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "int8_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_int8) {
+        v->type_int8(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < INT8_MIN || value > INT8_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "int8_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_int16) {
-            v->type_int16(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < INT16_MIN || value > INT16_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "int16_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_int16) {
+        v->type_int16(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < INT16_MIN || value > INT16_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "int16_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_int32) {
-            v->type_int32(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < INT32_MIN || value > INT32_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "int32_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_int32) {
+        v->type_int32(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < INT32_MIN || value > INT32_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "int32_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        if (v->type_int64) {
-            v->type_int64(v, obj, name, errp);
-        } else {
-            v->type_int(v, obj, name, errp);
-        }
+    if (v->type_int64) {
+        v->type_int64(v, obj, name, errp);
+    } else {
+        v->type_int(v, obj, name, errp);
     }
 }
 
 void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_size) {
-            v->type_size(v, obj, name, errp);
-        } else if (v->type_uint64) {
-            v->type_uint64(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            *obj = value;
-        }
+
+    if (v->type_size) {
+        v->type_size(v, obj, name, errp);
+    } else if (v->type_uint64) {
+        v->type_uint64(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        *obj = value;
     }
 }
 
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_bool(v, obj, name, errp);
-    }
+    v->type_bool(v, obj, name, errp);
 }
 
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_str(v, obj, name, errp);
-    }
+    v->type_str(v, obj, name, errp);
 }
 
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_number(v, obj, name, errp);
-    }
+    v->type_number(v, obj, name, errp);
 }
 
 void output_type_enum(Visitor *v, int *obj, const char *strings[],
@@ -321,13 +268,15 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
                      const char *kind, const char *name,
                      Error **errp)
 {
+    Error *local_err = NULL;
     int64_t value = 0;
     char *enum_str;
 
     assert(strings);
 
-    visit_type_str(v, &enum_str, name, errp);
-    if (error_is_set(errp)) {
+    visit_type_str(v, &enum_str, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return;
     }
 
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index a2bed1ef10..d8612062f1 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -286,8 +286,8 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name,
     }
 }
 
-static void qmp_input_start_optional(Visitor *v, bool *present,
-                                     const char *name, Error **errp)
+static void qmp_input_optional(Visitor *v, bool *present, const char *name,
+                               Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
     QObject *qobj = qmp_input_get_object(qiv, name, true);
@@ -329,7 +329,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
     v->visitor.type_bool = qmp_input_type_bool;
     v->visitor.type_str = qmp_input_type_str;
     v->visitor.type_number = qmp_input_type_number;
-    v->visitor.start_optional = qmp_input_start_optional;
+    v->visitor.optional = qmp_input_optional;
     v->visitor.get_next_type = qmp_input_get_next_type;
 
     qmp_input_push(v, obj, NULL);
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 793548ae3a..5780944792 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -120,8 +120,8 @@ static void parse_type_number(Visitor *v, double *obj, const char *name,
     *obj = val;
 }
 
-static void parse_start_optional(Visitor *v, bool *present,
-                                 const char *name, Error **errp)
+static void parse_optional(Visitor *v, bool *present, const char *name,
+                           Error **errp)
 {
     StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
 
@@ -155,7 +155,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
     v->visitor.type_bool = parse_type_bool;
     v->visitor.type_str = parse_type_str;
     v->visitor.type_number = parse_type_number;
-    v->visitor.start_optional = parse_start_optional;
+    v->visitor.optional = parse_optional;
 
     v->string = str;
     return v;
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 8d9096f65c..386f17ef44 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -2,16 +2,19 @@
 # QAPI command marshaller generator
 #
 # Copyright IBM, Corp. 2011
+# Copyright (C) 2014 Red Hat, Inc.
 #
 # Authors:
 #  Anthony Liguori <aliguori@us.ibm.com>
 #  Michael Roth    <mdroth@linux.vnet.ibm.com>
+#  Markus Armbruster <armbru@redhat.com>
 #
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
 from ordereddict import OrderedDict
 from qapi import *
+import re
 import sys
 import os
 import getopt
@@ -37,6 +40,15 @@ def generate_command_decl(name, args, ret_type):
 ''',
                  ret_type=c_type(ret_type), name=c_fun(name), args=arglist).strip()
 
+def gen_err_check(errvar):
+    if errvar:
+        return mcgen('''
+if (local_err) {
+    goto out;
+}
+''')
+    return ''
+
 def gen_sync_call(name, args, ret_type, indent=0):
     ret = ""
     arglist=""
@@ -49,15 +61,14 @@ def gen_sync_call(name, args, ret_type, indent=0):
         arglist += "%s, " % (c_var(argname))
     push_indent(indent)
     ret = mcgen('''
-%(retval)sqmp_%(name)s(%(args)serrp);
+%(retval)sqmp_%(name)s(%(args)s&local_err);
 
 ''',
                 name=c_fun(name), args=arglist, retval=retval).rstrip()
     if ret_type:
+        ret += "\n" + gen_err_check('local_err')
         ret += "\n" + mcgen(''''
-if (!error_is_set(errp)) {
-    %(marshal_output_call)s
-}
+%(marshal_output_call)s
 ''',
                             marshal_output_call=gen_marshal_output_call(name, ret_type)).rstrip()
     pop_indent(indent)
@@ -67,18 +78,19 @@ if (!error_is_set(errp)) {
 def gen_marshal_output_call(name, ret_type):
     if not ret_type:
         return ""
-    return "qmp_marshal_output_%s(retval, ret, errp);" % c_fun(name)
+    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_fun(name)
 
-def gen_visitor_input_containers_decl(args):
+def gen_visitor_input_containers_decl(args, obj):
     ret = ""
 
     push_indent()
     if len(args) > 0:
         ret += mcgen('''
-QmpInputVisitor *mi;
+QmpInputVisitor *mi = qmp_input_visitor_new_strict(%(obj)s);
 QapiDeallocVisitor *md;
 Visitor *v;
-''')
+''',
+                     obj=obj)
     pop_indent()
 
     return ret.rstrip()
@@ -106,9 +118,10 @@ bool has_%(argname)s = false;
     pop_indent()
     return ret.rstrip()
 
-def gen_visitor_input_block(args, obj, dealloc=False):
+def gen_visitor_input_block(args, dealloc=False):
     ret = ""
-    errparg = 'errp'
+    errparg = '&local_err'
+    errarg = 'local_err'
 
     if len(args) == 0:
         return ret
@@ -117,45 +130,45 @@ def gen_visitor_input_block(args, obj, dealloc=False):
 
     if dealloc:
         errparg = 'NULL'
+        errarg = None;
         ret += mcgen('''
+qmp_input_visitor_cleanup(mi);
 md = qapi_dealloc_visitor_new();
 v = qapi_dealloc_get_visitor(md);
 ''')
     else:
         ret += mcgen('''
-mi = qmp_input_visitor_new_strict(%(obj)s);
 v = qmp_input_get_visitor(mi);
-''',
-                     obj=obj)
+''')
 
     for argname, argtype, optional, structured in parse_args(args):
         if optional:
             ret += mcgen('''
-visit_start_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
-if (has_%(c_name)s) {
+visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
 ''',
                          c_name=c_var(argname), name=argname, errp=errparg)
+            ret += gen_err_check(errarg)
+            ret += mcgen('''
+if (has_%(c_name)s) {
+''',
+                         c_name=c_var(argname))
             push_indent()
         ret += mcgen('''
 %(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
 ''',
                      c_name=c_var(argname), name=argname, argtype=argtype,
                      visitor=type_visitor(argtype), errp=errparg)
+        ret += gen_err_check(errarg)
         if optional:
             pop_indent()
             ret += mcgen('''
 }
-visit_end_optional(v, %(errp)s);
-''', errp=errparg)
+''')
 
     if dealloc:
         ret += mcgen('''
 qapi_dealloc_visitor_cleanup(md);
 ''')
-    else:
-        ret += mcgen('''
-qmp_input_visitor_cleanup(mi);
-''')
     pop_indent()
     return ret.rstrip()
 
@@ -166,16 +179,22 @@ def gen_marshal_output(name, args, ret_type, middle_mode):
     ret = mcgen('''
 static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_out, Error **errp)
 {
-    QapiDeallocVisitor *md = qapi_dealloc_visitor_new();
+    Error *local_err = NULL;
     QmpOutputVisitor *mo = qmp_output_visitor_new();
+    QapiDeallocVisitor *md;
     Visitor *v;
 
     v = qmp_output_get_visitor(mo);
-    %(visitor)s(v, &ret_in, "unused", errp);
-    if (!error_is_set(errp)) {
-        *ret_out = qmp_output_get_qobject(mo);
+    %(visitor)s(v, &ret_in, "unused", &local_err);
+    if (local_err) {
+        goto out;
     }
+    *ret_out = qmp_output_get_qobject(mo);
+
+out:
+    error_propagate(errp, local_err);
     qmp_output_visitor_cleanup(mo);
+    md = qapi_dealloc_visitor_new();
     v = qapi_dealloc_get_visitor(md);
     %(visitor)s(v, &ret_in, "unused", NULL);
     qapi_dealloc_visitor_cleanup(md);
@@ -200,13 +219,12 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
     ret = mcgen('''
 %(header)s
 {
+    Error *local_err = NULL;
 ''',
                 header=hdr)
 
     if middle_mode:
         ret += mcgen('''
-    Error *local_err = NULL;
-    Error **errp = &local_err;
     QDict *args = (QDict *)qdict;
 ''')
 
@@ -228,29 +246,32 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
 %(visitor_input_block)s
 
 ''',
-                     visitor_input_containers_decl=gen_visitor_input_containers_decl(args),
+                     visitor_input_containers_decl=gen_visitor_input_containers_decl(args, "QOBJECT(args)"),
                      visitor_input_vars_decl=gen_visitor_input_vars_decl(args),
-                     visitor_input_block=gen_visitor_input_block(args, "QOBJECT(args)"))
+                     visitor_input_block=gen_visitor_input_block(args))
     else:
         ret += mcgen('''
+
     (void)args;
 ''')
 
     ret += mcgen('''
-    if (error_is_set(errp)) {
-        goto out;
-    }
 %(sync_call)s
 ''',
                  sync_call=gen_sync_call(name, args, ret_type, indent=4))
-    ret += mcgen('''
+    if re.search('^ *goto out\\;', ret, re.MULTILINE):
+        ret += mcgen('''
 
 out:
 ''')
+    if not middle_mode:
+        ret += mcgen('''
+    error_propagate(errp, local_err);
+''')
     ret += mcgen('''
 %(visitor_input_block_cleanup)s
 ''',
-                 visitor_input_block_cleanup=gen_visitor_input_block(args, None,
+                 visitor_input_block_cleanup=gen_visitor_input_block(args,
                                                                      dealloc=True))
 
     if middle_mode:
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c6579beed5..06a79f1631 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -2,21 +2,47 @@
 # QAPI visitor generator
 #
 # Copyright IBM, Corp. 2011
+# Copyright (C) 2014 Red Hat, Inc.
 #
 # Authors:
 #  Anthony Liguori <aliguori@us.ibm.com>
 #  Michael Roth    <mdroth@linux.vnet.ibm.com>
+#  Markus Armbruster <armbru@redhat.com>
 #
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
 from ordereddict import OrderedDict
 from qapi import *
+import re
 import sys
 import os
 import getopt
 import errno
 
+implicit_structs = []
+
+def generate_visit_implicit_struct(type):
+    global implicit_structs
+    if type in implicit_structs:
+        return ''
+    implicit_structs.append(type)
+    return mcgen('''
+
+static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error **errp)
+{
+    Error *err = NULL;
+
+    visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err);
+    if (!err) {
+        visit_type_%(c_type)s_fields(m, obj, errp);
+        visit_end_implicit_struct(m, &err);
+    }
+    error_propagate(errp, err);
+}
+''',
+                 c_type=type_name(type))
+
 def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = None):
     substructs = []
     ret = ''
@@ -35,6 +61,19 @@ def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base =
             nested_field_prefix = "%s%s." % (field_prefix, argname)
             ret += generate_visit_struct_fields(name, nested_field_prefix,
                                                 nested_fn_prefix, argentry)
+            ret += mcgen('''
+
+static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj, Error **errp)
+{
+''',
+                         name=name, full_name=full_name, c_name=c_var(argname))
+            ret += generate_visit_struct_body(full_name, argname, argentry)
+            ret += mcgen('''
+}
+''')
+
+    if base:
+        ret += generate_visit_implicit_struct(base)
 
     ret += mcgen('''
 
@@ -47,12 +86,9 @@ static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error *
 
     if base:
         ret += mcgen('''
-visit_start_implicit_struct(m, (void**) &(*obj)->%(c_name)s, sizeof(%(type)s), &err);
-if (!err) {
-    visit_type_%(type)s_fields(m, &(*obj)->%(c_prefix)s%(c_name)s, &err);
-    error_propagate(errp, err);
-    err = NULL;
-    visit_end_implicit_struct(m, &err);
+visit_type_implicit_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, &err);
+if (err) {
+    goto out;
 }
 ''',
                      c_prefix=c_var(field_prefix),
@@ -61,15 +97,18 @@ if (!err) {
     for argname, argentry, optional, structured in parse_args(members):
         if optional:
             ret += mcgen('''
-visit_start_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err);
-if ((*obj)->%(prefix)shas_%(c_name)s) {
+visit_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err);
+if (!err && (*obj)->%(prefix)shas_%(c_name)s) {
 ''',
                          c_prefix=c_var(field_prefix), prefix=field_prefix,
                          c_name=c_var(argname), name=argname)
             push_indent()
 
         if structured:
-            ret += generate_visit_struct_body(full_name, argname, argentry)
+            ret += mcgen('''
+visit_type_%(full_name)s_field_%(c_name)s(m, obj, &err);
+''',
+                         full_name=full_name, c_name=c_var(argname))
         else:
             ret += mcgen('''
 visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
@@ -82,12 +121,20 @@ visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
             pop_indent()
             ret += mcgen('''
 }
-visit_end_optional(m, &err);
+''')
+        ret += mcgen('''
+if (err) {
+    goto out;
+}
 ''')
 
     pop_indent()
-    ret += mcgen('''
+    if re.search('^ *goto out\\;', ret, re.MULTILINE):
+        ret += mcgen('''
 
+out:
+''')
+    ret += mcgen('''
     error_propagate(errp, err);
 }
 ''')
@@ -96,9 +143,9 @@ visit_end_optional(m, &err);
 
 def generate_visit_struct_body(field_prefix, name, members):
     ret = mcgen('''
-if (!error_is_set(errp)) {
+    Error *err = NULL;
+
 ''')
-    push_indent()
 
     if not field_prefix:
         full_name = name
@@ -107,36 +154,26 @@ if (!error_is_set(errp)) {
 
     if len(field_prefix):
         ret += mcgen('''
-Error **errp = &err; /* from outer scope */
-Error *err = NULL;
-visit_start_struct(m, NULL, "", "%(name)s", 0, &err);
+    visit_start_struct(m, NULL, "", "%(name)s", 0, &err);
 ''',
                 name=name)
     else:
         ret += mcgen('''
-Error *err = NULL;
-visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
+    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
 ''',
                 name=name)
 
     ret += mcgen('''
-if (!err) {
-    if (*obj) {
-        visit_type_%(name)s_fields(m, obj, &err);
-        error_propagate(errp, err);
-        err = NULL;
+    if (!err) {
+        if (*obj) {
+            visit_type_%(name)s_fields(m, obj, errp);
+        }
+        visit_end_struct(m, &err);
     }
+    error_propagate(errp, err);
 ''',
         name=full_name)
 
-    pop_indent()
-    ret += mcgen('''
-        /* Always call end_struct if start_struct succeeded.  */
-        visit_end_struct(m, &err);
-    }
-    error_propagate(errp, err);
-}
-''')
     return ret
 
 def generate_visit_struct(expr):
@@ -154,9 +191,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 ''',
                 name=name)
 
-    push_indent()
     ret += generate_visit_struct_body("", name, members)
-    pop_indent()
 
     ret += mcgen('''
 }
@@ -168,24 +203,26 @@ def generate_visit_list(name, members):
 
 void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp)
 {
-    GenericList *i, **prev = (GenericList **)obj;
     Error *err = NULL;
+    GenericList *i, **prev;
 
-    if (!error_is_set(errp)) {
-        visit_start_list(m, name, &err);
-        if (!err) {
-            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
-                %(name)sList *native_i = (%(name)sList *)i;
-                visit_type_%(name)s(m, &native_i->value, NULL, &err);
-            }
-            error_propagate(errp, err);
-            err = NULL;
-
-            /* Always call end_list if start_list succeeded.  */
-            visit_end_list(m, &err);
-        }
-        error_propagate(errp, err);
+    visit_start_list(m, name, &err);
+    if (err) {
+        goto out;
     }
+
+    for (prev = (GenericList **)obj;
+         !err && (i = visit_next_list(m, prev, &err)) != NULL;
+         prev = &i) {
+        %(name)sList *native_i = (%(name)sList *)i;
+        visit_type_%(name)s(m, &native_i->value, NULL, &err);
+    }
+
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_list(m, &err);
+out:
+    error_propagate(errp, err);
 }
 ''',
                 name=name)
@@ -207,10 +244,15 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 {
     Error *err = NULL;
 
-    if (!error_is_set(errp)) {
-        visit_start_implicit_struct(m, (void**) obj, sizeof(%(name)s), &err);
-        visit_get_next_type(m, (int*) &(*obj)->kind, %(name)s_qtypes, name, &err);
-        switch ((*obj)->kind) {
+    visit_start_implicit_struct(m, (void**) obj, sizeof(%(name)s), &err);
+    if (err) {
+        goto out;
+    }
+    visit_get_next_type(m, (int*) &(*obj)->kind, %(name)s_qtypes, name, &err);
+    if (err) {
+        goto out_end;
+    }
+    switch ((*obj)->kind) {
 ''',
     name=name)
 
@@ -225,22 +267,24 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 
         enum_full_value = generate_enum_full_value(disc_type, key)
         ret += mcgen('''
-        case %(enum_full_value)s:
-            visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
-            break;
+    case %(enum_full_value)s:
+        visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
+        break;
 ''',
                 enum_full_value = enum_full_value,
                 c_type = type_name(members[key]),
                 c_name = c_fun(key))
 
     ret += mcgen('''
-        default:
-            abort();
-        }
-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_implicit_struct(m, &err);
+    default:
+        abort();
     }
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_implicit_struct(m, &err);
+out:
+    error_propagate(errp, err);
 }
 ''')
 
@@ -277,40 +321,43 @@ def generate_visit_union(expr):
             del base_fields[discriminator]
         ret += generate_visit_struct_fields(name, "", "", base_fields)
 
+    if discriminator:
+        for key in members:
+            ret += generate_visit_implicit_struct(members[key])
+
     ret += mcgen('''
 
 void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp)
 {
     Error *err = NULL;
 
-    if (!error_is_set(errp)) {
-        visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
-        if (!err) {
-            if (*obj) {
+    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
+    if (err) {
+        goto out;
+    }
+    if (*obj) {
 ''',
                  name=name)
 
-
-    push_indent()
-    push_indent()
-    push_indent()
-
     if base:
         ret += mcgen('''
-    visit_type_%(name)s_fields(m, obj, &err);
+        visit_type_%(name)s_fields(m, obj, &err);
+        if (err) {
+            goto out_obj;
+        }
 ''',
             name=name)
 
-    pop_indent()
-
     if not discriminator:
         disc_key = "type"
     else:
         disc_key = discriminator
     ret += mcgen('''
         visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
-        if (!err) {
-            switch ((*obj)->kind) {
+        if (err) {
+            goto out_obj;
+        }
+        switch ((*obj)->kind) {
 ''',
                  disc_type = disc_type,
                  disc_key = disc_key)
@@ -319,47 +366,32 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
         if not discriminator:
             fmt = 'visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);'
         else:
-            fmt = '''visit_start_implicit_struct(m, (void**) &(*obj)->%(c_name)s, sizeof(%(c_type)s), &err);
-                if (!err) {
-                    visit_type_%(c_type)s_fields(m, &(*obj)->%(c_name)s, &err);
-                    error_propagate(errp, err);
-                    err = NULL;
-                    visit_end_implicit_struct(m, &err);
-                }'''
+            fmt = 'visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);'
 
         enum_full_value = generate_enum_full_value(disc_type, key)
         ret += mcgen('''
-            case %(enum_full_value)s:
-                ''' + fmt + '''
-                break;
+        case %(enum_full_value)s:
+            ''' + fmt + '''
+            break;
 ''',
                 enum_full_value = enum_full_value,
                 c_type=type_name(members[key]),
                 c_name=c_fun(key))
 
     ret += mcgen('''
-            default:
-                abort();
-            }
+        default:
+            abort();
         }
+out_obj:
         error_propagate(errp, err);
         err = NULL;
     }
-''')
-    pop_indent()
-    ret += mcgen('''
-        /* Always call end_struct if start_struct succeeded.  */
-        visit_end_struct(m, &err);
-    }
+    visit_end_struct(m, &err);
+out:
     error_propagate(errp, err);
 }
 ''')
 
-    pop_indent();
-    ret += mcgen('''
-}
-''')
-
     return ret
 
 def generate_declaration(name, members, genlist=True, builtin_type=False):
@@ -476,7 +508,7 @@ fdecl.write(mcgen('''
 /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
 
 /*
- * schema-defined QAPI visitor function
+ * schema-defined QAPI visitor functions
  *
  * Copyright IBM, Corp. 2011
  *
diff --git a/scripts/qapi.py b/scripts/qapi.py
index ec806aabeb..0265b404dd 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -73,13 +73,18 @@ class QAPIExprError(Exception):
 
 class QAPISchema:
 
-    def __init__(self, fp, input_relname=None, include_hist=[], parent_info=None):
+    def __init__(self, fp, input_relname=None, include_hist=[],
+                 previously_included=[], parent_info=None):
+        """ include_hist is a stack used to detect inclusion cycles
+            previously_included is a global state used to avoid multiple
+                                inclusions of the same file"""
         input_fname = os.path.abspath(fp.name)
         if input_relname is None:
             input_relname = fp.name
         self.input_dir = os.path.dirname(input_fname)
         self.input_file = input_relname
         self.include_hist = include_hist + [(input_relname, input_fname)]
+        previously_included.append(input_fname)
         self.parent_info = parent_info
         self.src = fp.read()
         if self.src == '' or self.src[-1] != '\n':
@@ -106,13 +111,16 @@ class QAPISchema:
                        for elem in self.include_hist):
                     raise QAPIExprError(expr_info, "Inclusion loop for %s"
                                         % include)
+                # skip multiple include of the same file
+                if include_path in previously_included:
+                    continue
                 try:
                     fobj = open(include_path, 'r')
                 except IOError as e:
                     raise QAPIExprError(expr_info,
                                         '%s: %s' % (e.strerror, include))
-                exprs_include = QAPISchema(fobj, include,
-                                           self.include_hist, expr_info)
+                exprs_include = QAPISchema(fobj, include, self.include_hist,
+                                           previously_included, expr_info)
                 self.exprs.extend(exprs_include.exprs)
             else:
                 expr_elem = {'expr': expr,
diff --git a/tests/Makefile b/tests/Makefile
index 6b8b6f273a..9f7ca61ae3 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -193,7 +193,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
         flat-union-string-discriminator.json \
         include-simple.json include-relpath.json include-format-err.json \
         include-non-file.json include-no-file.json include-before-err.json \
-        include-nested-err.json include-self-cycle.json include-cycle.json)
+        include-nested-err.json include-self-cycle.json include-cycle.json \
+        include-repetition.json)
 
 GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
 
diff --git a/tests/qapi-schema/include-repetition-sub.json b/tests/qapi-schema/include-repetition-sub.json
new file mode 100644
index 0000000000..6bfffdfd55
--- /dev/null
+++ b/tests/qapi-schema/include-repetition-sub.json
@@ -0,0 +1,2 @@
+{ 'include': 'comments.json' }
+{ 'include': 'comments.json' }
diff --git a/tests/qapi-schema/include-repetition.err b/tests/qapi-schema/include-repetition.err
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.err
diff --git a/tests/qapi-schema/include-repetition.exit b/tests/qapi-schema/include-repetition.exit
new file mode 100644
index 0000000000..573541ac97
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/include-repetition.json b/tests/qapi-schema/include-repetition.json
new file mode 100644
index 0000000000..ec329dde58
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.json
@@ -0,0 +1,3 @@
+{ 'include': 'comments.json' }
+{ 'include': 'include-repetition-sub.json' }
+{ 'include': 'comments.json' }
diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out
new file mode 100644
index 0000000000..4ce3dcf12f
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
+[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
+[]
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 449d285e56..0f770034b1 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -72,14 +72,30 @@ typedef struct TestStruct
 static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
                                   const char *name, Error **errp)
 {
-    visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
-                       errp);
+    Error *err = NULL;
 
-    visit_type_int(v, &(*obj)->integer, "integer", errp);
-    visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
-    visit_type_str(v, &(*obj)->string, "string", errp);
+    visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
+                       &err);
+    if (err) {
+        goto out;
+    }
 
-    visit_end_struct(v, errp);
+    visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_str(v, &(*obj)->string, "string", &err);
+
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
+    error_propagate(errp, err);
 }
 
 static void test_validate_struct(TestInputVisitorData *data,
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index a58a3e6fdb..1c8e87295c 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -199,16 +199,24 @@ static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
 
     visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
                        &err);
-    if (!err) {
-        visit_type_int(v, &(*obj)->integer, "integer", &err);
-        visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
-        visit_type_str(v, &(*obj)->string, "string", &err);
-
-        /* Always call end_struct if start_struct succeeded.  */
-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_struct(v, &err);
+    if (err) {
+        goto out;
     }
+    visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_str(v, &(*obj)->string, "string", &err);
+
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
     error_propagate(errp, err);
 }
 
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 2580f3debf..9c154581d7 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -176,14 +176,30 @@ typedef struct TestStruct
 static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
                                   const char *name, Error **errp)
 {
-    visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
-                       errp);
+    Error *err = NULL;
 
-    visit_type_int(v, &(*obj)->integer, "integer", errp);
-    visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
-    visit_type_str(v, &(*obj)->string, "string", errp);
+    visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
+                       &err);
+    if (err) {
+        goto out;
+    }
 
-    visit_end_struct(v, errp);
+    visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_str(v, &(*obj)->string, "string", &err);
+
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
+    error_propagate(errp, err);
 }
 
 static void test_visitor_out_struct(TestOutputVisitorData *data,
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 8166cf1b05..74d6481992 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -195,13 +195,29 @@ typedef struct TestStruct
 static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
                                   const char *name, Error **errp)
 {
-    visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), errp);
+    Error *err = NULL;
 
-    visit_type_int(v, &(*obj)->integer, "integer", errp);
-    visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
-    visit_type_str(v, &(*obj)->string, "string", errp);
+    visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), &err);
+    if (err) {
+        goto out;
+    }
 
-    visit_end_struct(v, errp);
+    visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_str(v, &(*obj)->string, "string", &err);
+
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
+    error_propagate(errp, err);
 }
 
 static TestStruct *struct_create(void)