qapi: Only input visitors can actually fail

The previous few commits have made this more obvious, and removed the
one exception.  Time to clarify the documentation, and drop dead error
checking.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200424084338.26803-13-armbru@redhat.com>

7 files changed, 35 insertions, 48 deletions

diff --git a/block.c b/block.c
index 2e3905c99e..c11385ae05 100644
--- a/block.c
+++ b/block.c
@@ -2982,7 +2982,6 @@ BdrvChild *bdrv_open_child(const char *filename,
 BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)
 {
     BlockDriverState *bs = NULL;
-    Error *local_err = NULL;
     QObject *obj = NULL;
     QDict *qdict = NULL;
     const char *reference = NULL;
@@ -2995,11 +2994,7 @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)
         assert(ref->type == QTYPE_QDICT);
 
         v = qobject_output_visitor_new(&obj);
-        visit_type_BlockdevOptions(v, NULL, &options, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-            goto fail;
-        }
+        visit_type_BlockdevOptions(v, NULL, &options, &error_abort);
         visit_complete(v, &obj);
 
         qdict = qobject_to(QDict, obj);
@@ -3017,8 +3012,6 @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)
 
     bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, errp);
     obj = NULL;
-
-fail:
     qobject_unref(obj);
     visit_free(v);
     return bs;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 59f7ebb171..5f3aead038 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1854,19 +1854,12 @@ static int sd_create_prealloc(BlockdevOptionsSheepdog *location, int64_t size,
     Visitor *v;
     QObject *obj = NULL;
     QDict *qdict;
-    Error *local_err = NULL;
     int ret;
 
     v = qobject_output_visitor_new(&obj);
-    visit_type_BlockdevOptionsSheepdog(v, NULL, &location, &local_err);
+    visit_type_BlockdevOptionsSheepdog(v, NULL, &location, &error_abort);
     visit_free(v);
 
-    if (local_err) {
-        error_propagate(errp, local_err);
-        qobject_unref(obj);
-        return -EINVAL;
-    }
-
     qdict = qobject_to(QDict, obj);
     qdict_flatten(qdict);
 
diff --git a/blockdev.c b/blockdev.c
index 5faddaa705..9da960b1e7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3725,14 +3725,8 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
     QObject *obj;
     Visitor *v = qobject_output_visitor_new(&obj);
     QDict *qdict;
-    Error *local_err = NULL;
-
-    visit_type_BlockdevOptions(v, NULL, &options, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        goto fail;
-    }
 
+    visit_type_BlockdevOptions(v, NULL, &options, &error_abort);
     visit_complete(v, &obj);
     qdict = qobject_to(QDict, obj);
 
@@ -3760,7 +3754,6 @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp)
     AioContext *ctx;
     QObject *obj;
     Visitor *v = qobject_output_visitor_new(&obj);
-    Error *local_err = NULL;
     BlockReopenQueue *queue;
     QDict *qdict;
 
@@ -3777,12 +3770,7 @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp)
     }
 
     /* Put all options in a QDict and flatten it */
-    visit_type_BlockdevOptions(v, NULL, &options, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        goto fail;
-    }
-
+    visit_type_BlockdevOptions(v, NULL, &options, &error_abort);
     visit_complete(v, &obj);
     qdict = qobject_to(QDict, obj);
 
diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index b76f7223af..39999c47c5 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -113,7 +113,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
 
     while (m) {
         v = string_output_visitor_new(false, &str);
-        visit_type_uint16List(v, NULL, &m->value->host_nodes, NULL);
+        visit_type_uint16List(v, NULL, &m->value->host_nodes, &error_abort);
         monitor_printf(mon, "memory backend: %s\n", m->value->id);
         monitor_printf(mon, "  size:  %" PRId64 "\n", m->value->size);
         monitor_printf(mon, "  merge: %s\n",
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 252206dc0d..98dc533d39 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -43,6 +43,10 @@ typedef enum VisitorType {
 
 struct Visitor
 {
+    /*
+     * Only input visitors may fail!
+     */
+
     /* Must be set to visit structs */
     void (*start_struct)(Visitor *v, const char *name, void **obj,
                          size_t size, Error **errp);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 2d40d2fe0f..5573906966 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -82,7 +82,7 @@
  * Each function also takes the customary @errp argument (see
  * qapi/error.h for details), for reporting any errors (such as if a
  * member @name is not present, or is present but not the specified
- * type).
+ * type).  Only input visitors can fail.
  *
  * If an error is detected during visit_type_FOO() with an input
  * visitor, then *@obj will be set to NULL for pointer types, and left
@@ -164,19 +164,14 @@
  *
  * <example>
  *  Foo *f = ...obtain populated object...
- *  Error *err = NULL;
  *  Visitor *v;
  *  Type *result;
  *
  *  v = FOO_visitor_new(..., &result);
- *  visit_type_Foo(v, NULL, &f, &err);
- *  if (err) {
- *      ...handle error...
- *  } else {
- *      visit_complete(v, &result);
- *      ...use result...
- *  }
+ *  visit_type_Foo(v, NULL, &f, &error_abort);
+ *  visit_complete(v, &result);
  *  visit_free(v);
+ *  ...use result...
  * </example>
  *
  * It is also possible to use the visitors to do a virtual walk, where
@@ -289,6 +284,7 @@ void visit_free(Visitor *v);
  * case @size is ignored.
  *
  * On failure, set *@obj to NULL and store an error through @errp.
+ * Can happen only when @v is an input visitor.
  *
  * After visit_start_struct() succeeds, the caller may visit its
  * members one after the other, passing the member's name and address
@@ -305,7 +301,8 @@ void visit_start_struct(Visitor *v, const char *name, void **obj,
 /*
  * Prepare for completing an object visit.
  *
- * On failure, store an error through @errp.
+ * On failure, store an error through @errp.  Can happen only when @v
+ * is an input visitor.
  *
  * Should be called prior to visit_end_struct() if all other
  * intermediate visit steps were successful, to allow the visitor one
@@ -342,6 +339,7 @@ void visit_end_struct(Visitor *v, void **obj);
  * ignored.
  *
  * On failure, set *@list to NULL and store an error through @errp.
+ * Can happen only when @v is an input visitor.
  *
  * After visit_start_list() succeeds, the caller may visit its members
  * one after the other.  A real visit (where @list is non-NULL) uses
@@ -375,7 +373,8 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size);
 /*
  * Prepare for completing a list visit.
  *
- * On failure, store an error through @errp.
+ * On failure, store an error through @errp.  Can happen only when @v
+ * is an input visitor.
  *
  * Should be called prior to visit_end_list() if all other
  * intermediate visit steps were successful, to allow the visitor one
@@ -411,6 +410,7 @@ void visit_end_list(Visitor *v, void **list);
  * (*@obj)->type.  Other visitors leave @obj unchanged.
  *
  * On failure, set *@obj to NULL and store an error through @errp.
+ * Can happen only when @v is an input visitor.
  *
  * If successful, this must be paired with visit_end_alternate() with
  * the same @obj to clean up, even if visiting the contents of the
@@ -465,11 +465,13 @@ bool visit_optional(Visitor *v, const char *name, bool *present);
  * visitors produce text output.  The mapping between enumeration
  * values and strings is done by the visitor core, using @lookup.
  *
- * On failure, store an error through @errp.
+ * On failure, store an error through @errp.  Can happen only when @v
+ * is an input visitor.
  *
  * May call visit_type_str() under the hood, and the enum visit may
  * fail even if the corresponding string visit succeeded; this implies
- * that visit_type_str() must have no unwelcome side effects.
+ * that an input visitor's visit_type_str() must have no unwelcome
+ * side effects.
  */
 void visit_type_enum(Visitor *v, const char *name, int *obj,
                      const QEnumLookup *lookup, Error **errp);
@@ -495,7 +497,8 @@ bool visit_is_dealloc(Visitor *v);
  * @obj must be non-NULL.  Input visitors set *@obj to the value;
  * other visitors will leave *@obj unchanged.
  *
- * On failure, store an error through @errp.
+ * On failure, store an error through @errp.  Can happen only when @v
+ * is an input visitor.
  */
 void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp);
 
@@ -573,7 +576,8 @@ void visit_type_size(Visitor *v, const char *name, uint64_t *obj,
  * @obj must be non-NULL.  Input visitors set *@obj to the value;
  * other visitors will leave *@obj unchanged.
  *
- * On failure, store an error through @errp.
+ * On failure, store an error through @errp.  Can happen only when @v
+ * is an input visitor.
  */
 void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp);
 
@@ -592,6 +596,7 @@ void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp);
  * into @obj for use by an output visitor.
  *
  * On failure, set *@obj to NULL and store an error through @errp.
+ * Can happen only when @v is an input visitor.
  *
  * FIXME: Callers that try to output NULL *obj should not be allowed.
  */
@@ -607,7 +612,8 @@ void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp);
  * other visitors will leave *@obj unchanged.  Visitors should
  * document if infinity or NaN are not permitted.
  *
- * On failure, store an error through @errp.
+ * On failure, store an error through @errp.  Can happen only when @v
+ * is an input visitor.
  */
 void visit_type_number(Visitor *v, const char *name, double *obj,
                        Error **errp);
@@ -623,6 +629,7 @@ void visit_type_number(Visitor *v, const char *name, double *obj,
  * for output visitors.
  *
  * On failure, set *@obj to NULL and store an error through @errp.
+ * Can happen only when @v is an input visitor.
  *
  * Note that some kinds of input can't express arbitrary QObject.
  * E.g. the visitor returned by qobject_input_visitor_new_keyval()
@@ -640,6 +647,7 @@ void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp);
  * other visitors ignore *@obj.
  *
  * On failure, set *@obj to NULL and store an error through @errp.
+ * Can happen only when @v is an input visitor.
  */
 void visit_type_null(Visitor *v, const char *name, QNull **obj,
                      Error **errp);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 9b94e67879..7f6e982dc8 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -334,7 +334,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
         Visitor *v;
         char *str;
         v = string_output_visitor_new(false, &str);
-        visit_type_uint32List(v, NULL, &info->postcopy_vcpu_blocktime, NULL);
+        visit_type_uint32List(v, NULL, &info->postcopy_vcpu_blocktime,
+                              &error_abort);
         visit_complete(v, &str);
         monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
         g_free(str);