[Qemu-devel] [PATCH v2 0/9] Miscellaneous error reporting improvements

[Qemu-devel] [PATCH v2 0/9] Miscellaneous error reporting improvements

[Markus Armbruster]

Touches vl.c, which gives me pretext to ask Paolo: would you be
willing to take this through your tree?  Or should I take it through
mine?

v2:
* Straightforward rebase, R-bys retained.
* PATCH 1,3,4: Commit message improvements [Eric]
* PATCH 5,8: Use NULL instead of &error_abort [Kevin]
* PATCH 5,8: Assert we got no error from callback when it returned
  success [Eric]

Markus Armbruster (9):
  vl: Report failure to sandbox at most once
  vl: Print -device help at most once
  vl: Fail right after first bad -object
  QemuOpts: Drop qemu_opts_foreach() parameter abort_on_failure
  QemuOpts: Convert qemu_opts_foreach() to Error
  blkdebug: Simplify passing of Error through qemu_opts_foreach()
  QemuOpts: Drop qemu_opt_foreach() parameter abort_on_failure
  QemuOpts: Convert qemu_opt_foreach() to Error
  vhost-user: Improve -netdev/netdev_add/-net/... error reporting

 block/blkdebug.c                 | 12 +++---
 hw/core/qdev-properties-system.c |  5 ++-
 include/qemu/option.h            | 12 +++---
 include/ui/console.h             |  2 +-
 net/net.c                        | 10 +++--
 net/vhost-user.c                 | 32 +++++++--------
 numa.c                           |  5 +--
 qdev-monitor.c                   |  5 ++-
 tpm.c                            |  5 +--
 ui/spice-core.c                  |  5 ++-
 ui/vnc.c                         |  2 +-
 util/qemu-config.c               |  9 +++--
 util/qemu-option.c               | 43 +++++++++++++-------
 vl.c                             | 84 ++++++++++++++++++++++++----------------
 14 files changed, 134 insertions(+), 97 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v2 1/9] vl: Report failure to sandbox at most once

[Markus Armbruster]

It's reported once per -sandbox on.  Stop on the first failure, like
we do for other options.

Not fixed: "-sandbox on -sandbox off" should leave the sandbox off.
It doesn't.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 1d4c089..04a6066 100644
--- a/vl.c
+++ b/vl.c
@@ -3780,7 +3780,7 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 0)) {
+    if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 1)) {
         exit(1);
     }
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 2/9] vl: Print -device help at most once

[Markus Armbruster]

We print it once for each -device help.  Not helpful.  Stop after the
first one.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 04a6066..2194e5d 100644
--- a/vl.c
+++ b/vl.c
@@ -4038,7 +4038,7 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("device"), device_help_func, NULL, 0)
+    if (qemu_opts_foreach(qemu_find_opts("device"), device_help_func, NULL, 1)
         != 0) {
         exit(0);
     }
-- 
1.9.3

[Qemu-devel] [PATCH v2 3/9] vl: Fail right after first bad -object

[Markus Armbruster]

Failure to create an object with -object is a fatal error.  However,
we delay the actual exit until all -object are processed.  On the one
hand, this permits detection of genuine additional errors.  On the
other hand, it can muddy the waters with uninteresting additional
errors, e.g. when a later -object tries to reference a prior one that
failed.

We generally stop right on the first bad option, so do that for
-object as well.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 2194e5d..bc89100 100644
--- a/vl.c
+++ b/vl.c
@@ -4044,7 +4044,7 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (qemu_opts_foreach(qemu_find_opts("object"),
-                          object_create, NULL, 0) != 0) {
+                          object_create, NULL, 1) != 0) {
         exit(1);
     }
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 4/9] QemuOpts: Drop qemu_opts_foreach() parameter abort_on_failure

[Markus Armbruster]

When the argument is non-zero, qemu_opts_foreach() stops on callback
returning non-zero, and returns that value.

When the argument is zero, it doesn't stop, and returns the bit-wise
inclusive or of all the return values.  Funky :)

The callers that pass zero could just as well pass one, because their
callbacks can't return anything but zero:

* qemu_add_globals()'s callback qdev_add_one_global()

* qemu_config_write()'s callback config_write_opts()

* main()'s callbacks default_driver_check(), drive_enable_snapshot(),
  vnc_init_func()

Drop the parameter, and always stop.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
---
 block/blkdebug.c                 |  4 ++--
 hw/core/qdev-properties-system.c |  2 +-
 include/qemu/option.h            |  4 ++--
 net/net.c                        |  5 +++--
 net/vhost-user.c                 |  2 +-
 numa.c                           |  3 +--
 tpm.c                            |  3 +--
 util/qemu-config.c               |  2 +-
 util/qemu-option.c               | 21 ++++++++++++++-------
 vl.c                             | 35 ++++++++++++++++++-----------------
 10 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 3c30edb..58f5105 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -320,7 +320,7 @@ static int read_config(BDRVBlkdebugState *s, const char *filename,
     d.s = s;
     d.action = ACTION_INJECT_ERROR;
     d.errp = &local_err;
-    qemu_opts_foreach(&inject_error_opts, add_rule, &d, 1);
+    qemu_opts_foreach(&inject_error_opts, add_rule, &d);
     if (local_err) {
         error_propagate(errp, local_err);
         ret = -EINVAL;
@@ -328,7 +328,7 @@ static int read_config(BDRVBlkdebugState *s, const char *filename,
     }
 
     d.action = ACTION_SET_STATE;
-    qemu_opts_foreach(&set_state_opts, add_rule, &d, 1);
+    qemu_opts_foreach(&set_state_opts, add_rule, &d);
     if (local_err) {
         error_propagate(errp, local_err);
         ret = -EINVAL;
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index c413226..93daeb0 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -404,5 +404,5 @@ static int qdev_add_one_global(QemuOpts *opts, void *opaque)
 
 void qemu_add_globals(void)
 {
-    qemu_opts_foreach(qemu_find_opts("global"), qdev_add_one_global, NULL, 0);
+    qemu_opts_foreach(qemu_find_opts("global"), qdev_add_one_global, NULL);
 }
diff --git a/include/qemu/option.h b/include/qemu/option.h
index f88b545..2edf58f 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -126,9 +126,9 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
 void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp);
 
 typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
+int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func,
+                      void *opaque);
 void qemu_opts_print(QemuOpts *opts, const char *sep);
-int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
-                      int abort_on_failure);
 void qemu_opts_print_help(QemuOptsList *list);
 void qemu_opts_free(QemuOptsList *list);
 QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
diff --git a/net/net.c b/net/net.c
index db6be12..011de59 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1373,10 +1373,11 @@ int net_init_clients(void)
 
     QTAILQ_INIT(&net_clients);
 
-    if (qemu_opts_foreach(qemu_find_opts("netdev"), net_init_netdev, NULL, 1) == -1)
+    if (qemu_opts_foreach(qemu_find_opts("netdev"), net_init_netdev, NULL)) {
         return -1;
+    }
 
-    if (qemu_opts_foreach(net, net_init_client, NULL, 1) == -1) {
+    if (qemu_opts_foreach(net, net_init_client, NULL)) {
         return -1;
     }
 
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 8d26728..f1a15ce 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -245,7 +245,7 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
 
     /* verify net frontend */
     if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
-                          (char *)name, true) == -1) {
+                          (char *)name)) {
         return -1;
     }
 
diff --git a/numa.c b/numa.c
index c975fb2..360a595 100644
--- a/numa.c
+++ b/numa.c
@@ -216,8 +216,7 @@ void parse_numa_opts(MachineClass *mc)
 {
     int i;
 
-    if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa,
-                          NULL, 1) != 0) {
+    if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, NULL)) {
         exit(1);
     }
 
diff --git a/tpm.c b/tpm.c
index 963b7ee..bca3b3a 100644
--- a/tpm.c
+++ b/tpm.c
@@ -207,8 +207,7 @@ void tpm_cleanup(void)
  */
 int tpm_init(void)
 {
-    if (qemu_opts_foreach(qemu_find_opts("tpmdev"),
-                          tpm_init_tpmdev, NULL, 1) != 0) {
+    if (qemu_opts_foreach(qemu_find_opts("tpmdev"), tpm_init_tpmdev, NULL)) {
         return -1;
     }
 
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 30d6dcf..b38927a 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -367,7 +367,7 @@ void qemu_config_write(FILE *fp)
     fprintf(fp, "# qemu config file\n\n");
     for (i = 0; lists[i] != NULL; i++) {
         data.list = lists[i];
-        qemu_opts_foreach(data.list, config_write_opts, &data, 0);
+        qemu_opts_foreach(data.list, config_write_opts, &data);
     }
 }
 
diff --git a/util/qemu-option.c b/util/qemu-option.c
index fda4e5f..7672aae 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1046,22 +1046,29 @@ void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp)
     }
 }
 
-int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
-                      int abort_on_failure)
+/**
+ * For each member of @list, call @func(member, @opaque).
+ * Call it with the current location temporarily set to the member's.
+ * When @func() returns non-zero, break the loop and return that value.
+ * Return zero when the loop completes.
+ */
+int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func,
+                      void *opaque)
 {
     Location loc;
     QemuOpts *opts;
-    int rc = 0;
+    int rc;
 
     loc_push_none(&loc);
     QTAILQ_FOREACH(opts, &list->head, next) {
         loc_restore(&opts->loc);
-        rc |= func(opts, opaque);
-        if (abort_on_failure  &&  rc != 0)
-            break;
+        rc = func(opts, opaque);
+        if (rc) {
+            return rc;
+        }
     }
     loc_pop(&loc);
-    return rc;
+    return 0;
 }
 
 static size_t count_opts_list(QemuOptsList *list)
diff --git a/vl.c b/vl.c
index bc89100..190f2f0 100644
--- a/vl.c
+++ b/vl.c
@@ -3780,20 +3780,20 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 1)) {
+    if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL)) {
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("name"), parse_name, NULL, 1)) {
+    if (qemu_opts_foreach(qemu_find_opts("name"), parse_name, NULL)) {
         exit(1);
     }
 
 #ifndef _WIN32
-    if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL, 1)) {
+    if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL)) {
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("add-fd"), cleanup_add_fd, NULL, 1)) {
+    if (qemu_opts_foreach(qemu_find_opts("add-fd"), cleanup_add_fd, NULL)) {
         exit(1);
     }
 #endif
@@ -3886,8 +3886,8 @@ int main(int argc, char **argv, char **envp)
                                machine_class->default_machine_opts, 0);
     }
 
-    qemu_opts_foreach(qemu_find_opts("device"), default_driver_check, NULL, 0);
-    qemu_opts_foreach(qemu_find_opts("global"), default_driver_check, NULL, 0);
+    qemu_opts_foreach(qemu_find_opts("device"), default_driver_check, NULL);
+    qemu_opts_foreach(qemu_find_opts("global"), default_driver_check, NULL);
 
     if (!vga_model && !default_vga) {
         vga_interface_type = VGA_DEVICE;
@@ -4025,10 +4025,12 @@ int main(int argc, char **argv, char **envp)
 
     socket_init();
 
-    if (qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, NULL, 1) != 0)
+    if (qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, NULL)) {
         exit(1);
+    }
+
 #ifdef CONFIG_VIRTFS
-    if (qemu_opts_foreach(qemu_find_opts("fsdev"), fsdev_init_func, NULL, 1) != 0) {
+    if (qemu_opts_foreach(qemu_find_opts("fsdev"), fsdev_init_func, NULL)) {
         exit(1);
     }
 #endif
@@ -4038,13 +4040,11 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("device"), device_help_func, NULL, 1)
-        != 0) {
+    if (qemu_opts_foreach(qemu_find_opts("device"), device_help_func, NULL)) {
         exit(0);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("object"),
-                          object_create, NULL, 1) != 0) {
+    if (qemu_opts_foreach(qemu_find_opts("object"), object_create, NULL)) {
         exit(1);
     }
 
@@ -4178,9 +4178,9 @@ int main(int argc, char **argv, char **envp)
 
     /* open the virtual block devices */
     if (snapshot)
-        qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0);
+        qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL);
     if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
-                          &machine_class->block_default_type, 1) != 0) {
+                          &machine_class->block_default_type)) {
         exit(1);
     }
 
@@ -4191,7 +4191,7 @@ int main(int argc, char **argv, char **envp)
 
     parse_numa_opts(machine_class);
 
-    if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) {
+    if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL)) {
         exit(1);
     }
 
@@ -4257,8 +4257,9 @@ int main(int argc, char **argv, char **envp)
     }
 
     /* init generic devices */
-    if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1) != 0)
+    if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL)) {
         exit(1);
+    }
 
     /* Did we create any drives that we failed to create a device for? */
     drive_check_orphaned();
@@ -4310,7 +4311,7 @@ int main(int argc, char **argv, char **envp)
 
 #ifdef CONFIG_VNC
     /* init remote displays */
-    qemu_opts_foreach(qemu_find_opts("vnc"), vnc_init_func, NULL, 0);
+    qemu_opts_foreach(qemu_find_opts("vnc"), vnc_init_func, NULL);
     if (show_vnc_port) {
         printf("VNC server running on `%s'\n",
                vnc_display_local_addr("default"));
-- 
1.9.3

[Qemu-devel] [PATCH v2 5/9] QemuOpts: Convert qemu_opts_foreach() to Error

[Markus Armbruster]

Retain the function value for now, to permit selective conversion of
its callers.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
---
 block/blkdebug.c                 |  6 ++--
 hw/core/qdev-properties-system.c |  5 +--
 include/qemu/option.h            |  4 +--
 include/ui/console.h             |  2 +-
 net/net.c                        |  9 ++---
 net/vhost-user.c                 |  4 +--
 numa.c                           |  4 +--
 tpm.c                            |  6 ++--
 ui/vnc.c                         |  2 +-
 util/qemu-config.c               |  4 +--
 util/qemu-option.c               |  8 +++--
 vl.c                             | 74 ++++++++++++++++++++++++----------------
 12 files changed, 73 insertions(+), 55 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 58f5105..be0d273 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -219,7 +219,7 @@ struct add_rule_data {
     Error **errp;
 };
 
-static int add_rule(QemuOpts *opts, void *opaque)
+static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
 {
     struct add_rule_data *d = opaque;
     BDRVBlkdebugState *s = d->s;
@@ -320,7 +320,7 @@ static int read_config(BDRVBlkdebugState *s, const char *filename,
     d.s = s;
     d.action = ACTION_INJECT_ERROR;
     d.errp = &local_err;
-    qemu_opts_foreach(&inject_error_opts, add_rule, &d);
+    qemu_opts_foreach(&inject_error_opts, add_rule, &d, NULL);
     if (local_err) {
         error_propagate(errp, local_err);
         ret = -EINVAL;
@@ -328,7 +328,7 @@ static int read_config(BDRVBlkdebugState *s, const char *filename,
     }
 
     d.action = ACTION_SET_STATE;
-    qemu_opts_foreach(&set_state_opts, add_rule, &d);
+    qemu_opts_foreach(&set_state_opts, add_rule, &d, NULL);
     if (local_err) {
         error_propagate(errp, local_err);
         ret = -EINVAL;
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 93daeb0..0309fe5 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -389,7 +389,7 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
     nd->instantiated = 1;
 }
 
-static int qdev_add_one_global(QemuOpts *opts, void *opaque)
+static int qdev_add_one_global(void *opaque, QemuOpts *opts, Error **errp)
 {
     GlobalProperty *g;
 
@@ -404,5 +404,6 @@ static int qdev_add_one_global(QemuOpts *opts, void *opaque)
 
 void qemu_add_globals(void)
 {
-    qemu_opts_foreach(qemu_find_opts("global"), qdev_add_one_global, NULL);
+    qemu_opts_foreach(qemu_find_opts("global"),
+                      qdev_add_one_global, NULL, NULL);
 }
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 2edf58f..a3850b2 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -125,9 +125,9 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
 QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
 void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp);
 
-typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
+typedef int (*qemu_opts_loopfunc)(void *opaque, QemuOpts *opts, Error **errp);
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func,
-                      void *opaque);
+                      void *opaque, Error **errp);
 void qemu_opts_print(QemuOpts *opts, const char *sep);
 void qemu_opts_print_help(QemuOptsList *list);
 void qemu_opts_free(QemuOptsList *list);
diff --git a/include/ui/console.h b/include/ui/console.h
index 6f7550e..401e738 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -374,7 +374,7 @@ char *vnc_display_local_addr(const char *id);
 int vnc_display_password(const char *id, const char *password);
 int vnc_display_pw_expire(const char *id, time_t expires);
 QemuOpts *vnc_parse_func(const char *str);
-int vnc_init_func(QemuOpts *opts, void *opaque);
+int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
 #else
 static inline int vnc_display_password(const char *id, const char *password)
 {
diff --git a/net/net.c b/net/net.c
index 011de59..25c2ef3 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1329,7 +1329,7 @@ void net_check_clients(void)
     }
 }
 
-static int net_init_client(QemuOpts *opts, void *dummy)
+static int net_init_client(void *dummy, QemuOpts *opts, Error **errp)
 {
     Error *local_err = NULL;
 
@@ -1342,7 +1342,7 @@ static int net_init_client(QemuOpts *opts, void *dummy)
     return 0;
 }
 
-static int net_init_netdev(QemuOpts *opts, void *dummy)
+static int net_init_netdev(void *dummy, QemuOpts *opts, Error **errp)
 {
     Error *local_err = NULL;
     int ret;
@@ -1373,11 +1373,12 @@ int net_init_clients(void)
 
     QTAILQ_INIT(&net_clients);
 
-    if (qemu_opts_foreach(qemu_find_opts("netdev"), net_init_netdev, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("netdev"),
+                          net_init_netdev, NULL, NULL)) {
         return -1;
     }
 
-    if (qemu_opts_foreach(net, net_init_client, NULL)) {
+    if (qemu_opts_foreach(net, net_init_client, NULL, NULL)) {
         return -1;
     }
 
diff --git a/net/vhost-user.c b/net/vhost-user.c
index f1a15ce..cce168a 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -204,7 +204,7 @@ static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *op
     return chr;
 }
 
-static int net_vhost_check_net(QemuOpts *opts, void *opaque)
+static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
 {
     const char *name = opaque;
     const char *driver, *netdev;
@@ -245,7 +245,7 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
 
     /* verify net frontend */
     if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
-                          (char *)name)) {
+                          (char *)name, NULL)) {
         return -1;
     }
 
diff --git a/numa.c b/numa.c
index 360a595..d227ccc 100644
--- a/numa.c
+++ b/numa.c
@@ -125,7 +125,7 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp)
     max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
 }
 
-static int parse_numa(QemuOpts *opts, void *opaque)
+static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
 {
     NumaOptions *object = NULL;
     Error *err = NULL;
@@ -216,7 +216,7 @@ void parse_numa_opts(MachineClass *mc)
 {
     int i;
 
-    if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, NULL, NULL)) {
         exit(1);
     }
 
diff --git a/tpm.c b/tpm.c
index bca3b3a..a3f3b7f 100644
--- a/tpm.c
+++ b/tpm.c
@@ -182,7 +182,7 @@ static int configure_tpm(QemuOpts *opts)
     return 0;
 }
 
-static int tpm_init_tpmdev(QemuOpts *opts, void *dummy)
+static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
 {
     return configure_tpm(opts);
 }
@@ -207,12 +207,12 @@ void tpm_cleanup(void)
  */
 int tpm_init(void)
 {
-    if (qemu_opts_foreach(qemu_find_opts("tpmdev"), tpm_init_tpmdev, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("tpmdev"),
+                          tpm_init_tpmdev, NULL, NULL)) {
         return -1;
     }
 
     atexit(tpm_cleanup);
-
     return 0;
 }
 
diff --git a/ui/vnc.c b/ui/vnc.c
index 1013ea5..0c6b5e3 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3770,7 +3770,7 @@ QemuOpts *vnc_parse_func(const char *str)
     return opts;
 }
 
-int vnc_init_func(QemuOpts *opts, void *opaque)
+int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     Error *local_err = NULL;
     char *id = (char *)qemu_opts_id(opts);
diff --git a/util/qemu-config.c b/util/qemu-config.c
index b38927a..a88461f 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -343,7 +343,7 @@ static int config_write_opt(const char *name, const char *value, void *opaque)
     return 0;
 }
 
-static int config_write_opts(QemuOpts *opts, void *opaque)
+static int config_write_opts(void *opaque, QemuOpts *opts, Error **errp)
 {
     struct ConfigWriteData *data = opaque;
     const char *id = qemu_opts_id(opts);
@@ -367,7 +367,7 @@ void qemu_config_write(FILE *fp)
     fprintf(fp, "# qemu config file\n\n");
     for (i = 0; lists[i] != NULL; i++) {
         data.list = lists[i];
-        qemu_opts_foreach(data.list, config_write_opts, &data);
+        qemu_opts_foreach(data.list, config_write_opts, &data, NULL);
     }
 }
 
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 7672aae..07b03e3 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1047,13 +1047,14 @@ void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp)
 }
 
 /**
- * For each member of @list, call @func(member, @opaque).
+ * For each member of @list, call @func(@opaque, member, @errp).
  * Call it with the current location temporarily set to the member's.
+ * @func() may store an Error through @errp, but must return non-zero then.
  * When @func() returns non-zero, break the loop and return that value.
  * Return zero when the loop completes.
  */
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func,
-                      void *opaque)
+                      void *opaque, Error **errp)
 {
     Location loc;
     QemuOpts *opts;
@@ -1062,10 +1063,11 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func,
     loc_push_none(&loc);
     QTAILQ_FOREACH(opts, &list->head, next) {
         loc_restore(&opts->loc);
-        rc = func(opts, opaque);
+        rc = func(opaque, opts, errp);
         if (rc) {
             return rc;
         }
+        assert(!errp || !*errp);
     }
     loc_pop(&loc);
     return 0;
diff --git a/vl.c b/vl.c
index 190f2f0..125f026 100644
--- a/vl.c
+++ b/vl.c
@@ -514,7 +514,7 @@ static void res_free(void)
     }
 }
 
-static int default_driver_check(QemuOpts *opts, void *opaque)
+static int default_driver_check(void *opaque, QemuOpts *opts, Error **errp)
 {
     const char *driver = qemu_opt_get(opts, "driver");
     int i;
@@ -960,7 +960,7 @@ static int bt_parse(const char *opt)
     return 1;
 }
 
-static int parse_sandbox(QemuOpts *opts, void *opaque)
+static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
 {
     /* FIXME: change this to true for 1.3 */
     if (qemu_opt_get_bool(opts, "enable", false)) {
@@ -980,7 +980,7 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
     return 0;
 }
 
-static int parse_name(QemuOpts *opts, void *opaque)
+static int parse_name(void *opaque, QemuOpts *opts, Error **errp)
 {
     const char *proc_name;
 
@@ -1008,7 +1008,7 @@ bool usb_enabled(void)
 }
 
 #ifndef _WIN32
-static int parse_add_fd(QemuOpts *opts, void *opaque)
+static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
 {
     int fd, dupfd, flags;
     int64_t fdset_id;
@@ -1070,7 +1070,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
     return 0;
 }
 
-static int cleanup_add_fd(QemuOpts *opts, void *opaque)
+static int cleanup_add_fd(void *opaque, QemuOpts *opts, Error **errp)
 {
     int fd;
 
@@ -1091,14 +1091,14 @@ static int cleanup_add_fd(QemuOpts *opts, void *opaque)
 #define MTD_OPTS ""
 #define SD_OPTS ""
 
-static int drive_init_func(QemuOpts *opts, void *opaque)
+static int drive_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     BlockInterfaceType *block_default_type = opaque;
 
     return drive_new(opts, *block_default_type) == NULL;
 }
 
-static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
+static int drive_enable_snapshot(void *opaque, QemuOpts *opts, Error **errp)
 {
     if (qemu_opt_get(opts, "snapshot") == NULL) {
         qemu_opt_set(opts, "snapshot", "on", &error_abort);
@@ -1118,7 +1118,7 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type,
 
     opts = drive_add(type, index, NULL, optstr);
     if (snapshot) {
-        drive_enable_snapshot(opts, NULL);
+        drive_enable_snapshot(NULL, opts, NULL);
     }
 
     dinfo = drive_new(opts, type);
@@ -2122,12 +2122,12 @@ char *qemu_find_file(int type, const char *name)
     return NULL;
 }
 
-static int device_help_func(QemuOpts *opts, void *opaque)
+static int device_help_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     return qdev_device_help(opts);
 }
 
-static int device_init_func(QemuOpts *opts, void *opaque)
+static int device_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     DeviceState *dev;
 
@@ -2138,7 +2138,7 @@ static int device_init_func(QemuOpts *opts, void *opaque)
     return 0;
 }
 
-static int chardev_init_func(QemuOpts *opts, void *opaque)
+static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     Error *local_err = NULL;
 
@@ -2151,7 +2151,7 @@ static int chardev_init_func(QemuOpts *opts, void *opaque)
 }
 
 #ifdef CONFIG_VIRTFS
-static int fsdev_init_func(QemuOpts *opts, void *opaque)
+static int fsdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     int ret;
     ret = qemu_fsdev_add(opts);
@@ -2160,7 +2160,7 @@ static int fsdev_init_func(QemuOpts *opts, void *opaque)
 }
 #endif
 
-static int mon_init_func(QemuOpts *opts, void *opaque)
+static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     CharDriverState *chr;
     const char *chardev;
@@ -2595,7 +2595,7 @@ static int machine_set_property(const char *name, const char *value,
     return 0;
 }
 
-static int object_create(QemuOpts *opts, void *opaque)
+static int object_create(void *opaque, QemuOpts *opts, Error **errp)
 {
     Error *err = NULL;
     char *type = NULL;
@@ -3780,20 +3780,24 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("sandbox"),
+                          parse_sandbox, NULL, NULL)) {
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("name"), parse_name, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("name"),
+                          parse_name, NULL, NULL)) {
         exit(1);
     }
 
-#ifndef _WIN32
-    if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL)) {
+#ifndef _win32
+    if (qemu_opts_foreach(qemu_find_opts("add-fd"),
+                          parse_add_fd, NULL, NULL)) {
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("add-fd"), cleanup_add_fd, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("add-fd"),
+                          cleanup_add_fd, NULL, NULL)) {
         exit(1);
     }
 #endif
@@ -3886,8 +3890,10 @@ int main(int argc, char **argv, char **envp)
                                machine_class->default_machine_opts, 0);
     }
 
-    qemu_opts_foreach(qemu_find_opts("device"), default_driver_check, NULL);
-    qemu_opts_foreach(qemu_find_opts("global"), default_driver_check, NULL);
+    qemu_opts_foreach(qemu_find_opts("device"),
+                      default_driver_check, NULL, NULL);
+    qemu_opts_foreach(qemu_find_opts("global"),
+                      default_driver_check, NULL, NULL);
 
     if (!vga_model && !default_vga) {
         vga_interface_type = VGA_DEVICE;
@@ -4025,12 +4031,14 @@ int main(int argc, char **argv, char **envp)
 
     socket_init();
 
-    if (qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("chardev"),
+                          chardev_init_func, NULL, NULL)) {
         exit(1);
     }
 
 #ifdef CONFIG_VIRTFS
-    if (qemu_opts_foreach(qemu_find_opts("fsdev"), fsdev_init_func, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("fsdev"),
+                          fsdev_init_func, NULL, NULL)) {
         exit(1);
     }
 #endif
@@ -4040,11 +4048,13 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("device"), device_help_func, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("device"),
+                          device_help_func, NULL, NULL)) {
         exit(0);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("object"), object_create, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("object"),
+                          object_create, NULL, NULL)) {
         exit(1);
     }
 
@@ -4178,9 +4188,10 @@ int main(int argc, char **argv, char **envp)
 
     /* open the virtual block devices */
     if (snapshot)
-        qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL);
+        qemu_opts_foreach(qemu_find_opts("drive"),
+                          drive_enable_snapshot, NULL, NULL);
     if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
-                          &machine_class->block_default_type)) {
+                          &machine_class->block_default_type, NULL)) {
         exit(1);
     }
 
@@ -4191,7 +4202,8 @@ int main(int argc, char **argv, char **envp)
 
     parse_numa_opts(machine_class);
 
-    if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("mon"),
+                          mon_init_func, NULL, NULL)) {
         exit(1);
     }
 
@@ -4257,7 +4269,8 @@ int main(int argc, char **argv, char **envp)
     }
 
     /* init generic devices */
-    if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL)) {
+    if (qemu_opts_foreach(qemu_find_opts("device"),
+                          device_init_func, NULL, NULL)) {
         exit(1);
     }
 
@@ -4311,7 +4324,8 @@ int main(int argc, char **argv, char **envp)
 
 #ifdef CONFIG_VNC
     /* init remote displays */
-    qemu_opts_foreach(qemu_find_opts("vnc"), vnc_init_func, NULL);
+    qemu_opts_foreach(qemu_find_opts("vnc"),
+                      vnc_init_func, NULL, NULL);
     if (show_vnc_port) {
         printf("VNC server running on `%s'\n",
                vnc_display_local_addr("default"));
-- 
1.9.3

[Qemu-devel] [PATCH v2 6/9] blkdebug: Simplify passing of Error through qemu_opts_foreach()

[Markus Armbruster]

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
---
 block/blkdebug.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index be0d273..1e92607 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -216,7 +216,6 @@ static int get_event_by_name(const char *name, BlkDebugEvent *event)
 struct add_rule_data {
     BDRVBlkdebugState *s;
     int action;
-    Error **errp;
 };
 
 static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
@@ -230,10 +229,10 @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
     /* Find the right event for the rule */
     event_name = qemu_opt_get(opts, "event");
     if (!event_name) {
-        error_setg(d->errp, "Missing event name for rule");
+        error_setg(errp, "Missing event name for rule");
         return -1;
     } else if (get_event_by_name(event_name, &event) < 0) {
-        error_setg(d->errp, "Invalid event name \"%s\"", event_name);
+        error_setg(errp, "Invalid event name \"%s\"", event_name);
         return -1;
     }
 
@@ -319,8 +318,7 @@ static int read_config(BDRVBlkdebugState *s, const char *filename,
 
     d.s = s;
     d.action = ACTION_INJECT_ERROR;
-    d.errp = &local_err;
-    qemu_opts_foreach(&inject_error_opts, add_rule, &d, NULL);
+    qemu_opts_foreach(&inject_error_opts, add_rule, &d, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         ret = -EINVAL;
@@ -328,7 +326,7 @@ static int read_config(BDRVBlkdebugState *s, const char *filename,
     }
 
     d.action = ACTION_SET_STATE;
-    qemu_opts_foreach(&set_state_opts, add_rule, &d, NULL);
+    qemu_opts_foreach(&set_state_opts, add_rule, &d, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         ret = -EINVAL;
-- 
1.9.3

[Qemu-devel] [PATCH v2 7/9] QemuOpts: Drop qemu_opt_foreach() parameter abort_on_failure

[Markus Armbruster]

When the argument is non-zero, qemu_opt_foreach() stops on callback
returning non-zero, and returns that value.

When the argument is zero, it doesn't stop, and returns the callback's
value from the last iteration.

The two callers that pass zero could just as well pass one:

* qemu_spice_init()'s callback add_channel() either returns zero or
  exit()s.

* config_write_opts()'s callback config_write_opt() always returns
  zero.

Drop the parameter, and always stop.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/qemu/option.h |  3 +--
 net/vhost-user.c      |  2 +-
 qdev-monitor.c        |  2 +-
 ui/spice-core.c       |  2 +-
 util/qemu-config.c    |  2 +-
 util/qemu-option.c    | 17 +++++++++++------
 vl.c                  |  4 ++--
 7 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index a3850b2..a3cf4c1 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -101,8 +101,7 @@ void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val,
 void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val,
                          Error **errp);
 typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque);
-int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
-                     int abort_on_failure);
+int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque);
 
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
diff --git a/net/vhost-user.c b/net/vhost-user.c
index cce168a..167082e 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -189,7 +189,7 @@ static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *op
 
     /* inspect chardev opts */
     memset(&props, 0, sizeof(props));
-    if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, true) != 0) {
+    if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props)) {
         return NULL;
     }
 
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 1d87f57..da2cbc9 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -564,7 +564,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     }
 
     /* set properties */
-    if (qemu_opt_foreach(opts, set_property, dev, 1) != 0) {
+    if (qemu_opt_foreach(opts, set_property, dev)) {
         object_unparent(OBJECT(dev));
         object_unref(OBJECT(dev));
         return NULL;
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 2e8384e..60818d9 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -782,7 +782,7 @@ void qemu_spice_init(void)
     spice_server_set_playback_compression
         (spice_server, qemu_opt_get_bool(opts, "playback-compression", 1));
 
-    qemu_opt_foreach(opts, add_channel, &tls_port, 0);
+    qemu_opt_foreach(opts, add_channel, &tls_port);
 
     spice_server_set_name(spice_server, qemu_name);
     spice_server_set_uuid(spice_server, qemu_uuid);
diff --git a/util/qemu-config.c b/util/qemu-config.c
index a88461f..aff4cb3 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -353,7 +353,7 @@ static int config_write_opts(void *opaque, QemuOpts *opts, Error **errp)
     } else {
         fprintf(data->fp, "[%s]\n", data->list->name);
     }
-    qemu_opt_foreach(opts, config_write_opt, data, 0);
+    qemu_opt_foreach(opts, config_write_opt, data);
     fprintf(data->fp, "\n");
     return 0;
 }
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 07b03e3..296e2b3 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -596,18 +596,23 @@ void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val,
     QTAILQ_INSERT_TAIL(&opts->head, opt, next);
 }
 
-int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
-                     int abort_on_failure)
+/**
+ * For each member of @opts, call @func(name, value, @opaque).
+ * When @func() returns non-zero, break the loop and return that value.
+ * Return zero when the loop completes.
+ */
+int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque)
 {
     QemuOpt *opt;
-    int rc = 0;
+    int rc;
 
     QTAILQ_FOREACH(opt, &opts->head, next) {
         rc = func(opt->name, opt->str, opaque);
-        if (abort_on_failure  &&  rc != 0)
-            break;
+        if (rc) {
+            return rc;
+        }
     }
-    return rc;
+    return 0;
 }
 
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id)
diff --git a/vl.c b/vl.c
index 125f026..2495f0e 100644
--- a/vl.c
+++ b/vl.c
@@ -4059,8 +4059,8 @@ int main(int argc, char **argv, char **envp)
     }
 
     machine_opts = qemu_get_machine_opts();
-    if (qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
-                         1) < 0) {
+    if (qemu_opt_foreach(machine_opts, machine_set_property,
+                         current_machine)) {
         object_unref(OBJECT(current_machine));
         exit(1);
     }
-- 
1.9.3

[Qemu-devel] [PATCH v2 8/9] QemuOpts: Convert qemu_opt_foreach() to Error

[Markus Armbruster]

Retain the function value for now, to permit selective conversion of
its callers.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/qemu/option.h | 7 +++++--
 net/vhost-user.c      | 7 ++++---
 qdev-monitor.c        | 5 +++--
 ui/spice-core.c       | 5 +++--
 util/qemu-config.c    | 5 +++--
 util/qemu-option.c    | 9 ++++++---
 vl.c                  | 9 +++++----
 7 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index a3cf4c1..ac0e43b 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -100,8 +100,11 @@ void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val,
                        Error **errp);
 void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val,
                          Error **errp);
-typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque);
-int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque);
+typedef int (*qemu_opt_loopfunc)(void *opaque,
+                                 const char *name, const char *value,
+                                 Error **errp);
+int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
+                     Error **errp);
 
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 167082e..7e8a9ec 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -157,8 +157,9 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
     return 0;
 }
 
-static int net_vhost_chardev_opts(const char *name, const char *value,
-                                  void *opaque)
+static int net_vhost_chardev_opts(void *opaque,
+                                  const char *name, const char *value,
+                                  Error **errp)
 {
     VhostUserChardevProps *props = opaque;
 
@@ -189,7 +190,7 @@ static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *op
 
     /* inspect chardev opts */
     memset(&props, 0, sizeof(props));
-    if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props)) {
+    if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, NULL)) {
         return NULL;
     }
 
diff --git a/qdev-monitor.c b/qdev-monitor.c
index da2cbc9..dc56082 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -143,7 +143,8 @@ static void qdev_print_devinfos(bool show_no_user)
     g_slist_free(list);
 }
 
-static int set_property(const char *name, const char *value, void *opaque)
+static int set_property(void *opaque, const char *name, const char *value,
+                        Error **errp)
 {
     Object *obj = opaque;
     Error *err = NULL;
@@ -564,7 +565,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     }
 
     /* set properties */
-    if (qemu_opt_foreach(opts, set_property, dev)) {
+    if (qemu_opt_foreach(opts, set_property, dev, NULL)) {
         object_unparent(OBJECT(dev));
         object_unref(OBJECT(dev));
         return NULL;
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 60818d9..a30da3c 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -583,7 +583,8 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
     return ret;
 }
 
-static int add_channel(const char *name, const char *value, void *opaque)
+static int add_channel(void *opaque, const char *name, const char *value,
+                       Error **errp)
 {
     int security = 0;
     int rc;
@@ -782,7 +783,7 @@ void qemu_spice_init(void)
     spice_server_set_playback_compression
         (spice_server, qemu_opt_get_bool(opts, "playback-compression", 1));
 
-    qemu_opt_foreach(opts, add_channel, &tls_port);
+    qemu_opt_foreach(opts, add_channel, &tls_port, NULL);
 
     spice_server_set_name(spice_server, qemu_name);
     spice_server_set_uuid(spice_server, qemu_uuid);
diff --git a/util/qemu-config.c b/util/qemu-config.c
index aff4cb3..35adfda 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -335,7 +335,8 @@ struct ConfigWriteData {
     FILE *fp;
 };
 
-static int config_write_opt(const char *name, const char *value, void *opaque)
+static int config_write_opt(void *opaque, const char *name, const char *value,
+                            Error **errp)
 {
     struct ConfigWriteData *data = opaque;
 
@@ -353,7 +354,7 @@ static int config_write_opts(void *opaque, QemuOpts *opts, Error **errp)
     } else {
         fprintf(data->fp, "[%s]\n", data->list->name);
     }
-    qemu_opt_foreach(opts, config_write_opt, data);
+    qemu_opt_foreach(opts, config_write_opt, data, NULL);
     fprintf(data->fp, "\n");
     return 0;
 }
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 296e2b3..840f5f7 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -597,20 +597,23 @@ void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val,
 }
 
 /**
- * For each member of @opts, call @func(name, value, @opaque).
+ * For each member of @opts, call @func(@opaque, name, value, @errp).
+ * @func() may store an Error through @errp, but must return non-zero then.
  * When @func() returns non-zero, break the loop and return that value.
  * Return zero when the loop completes.
  */
-int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque)
+int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
+                     Error **errp)
 {
     QemuOpt *opt;
     int rc;
 
     QTAILQ_FOREACH(opt, &opts->head, next) {
-        rc = func(opt->name, opt->str, opaque);
+        rc = func(opaque, opt->name, opt->str, errp);
         if (rc) {
             return rc;
         }
+        assert(!errp || !*errp);
     }
     return 0;
 }
diff --git a/vl.c b/vl.c
index 2495f0e..5e37602 100644
--- a/vl.c
+++ b/vl.c
@@ -2565,8 +2565,9 @@ static void free_and_trace(gpointer mem)
     free(mem);
 }
 
-static int machine_set_property(const char *name, const char *value,
-                                void *opaque)
+static int machine_set_property(void *opaque,
+                                const char *name, const char *value,
+                                Error **errp)
 {
     Object *obj = OBJECT(opaque);
     Error *local_err = NULL;
@@ -4059,8 +4060,8 @@ int main(int argc, char **argv, char **envp)
     }
 
     machine_opts = qemu_get_machine_opts();
-    if (qemu_opt_foreach(machine_opts, machine_set_property,
-                         current_machine)) {
+    if (qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
+                         NULL)) {
         object_unref(OBJECT(current_machine));
         exit(1);
     }
-- 
1.9.3

[Qemu-devel] [PATCH v2 9/9] vhost-user: Improve -netdev/netdev_add/-net/... error reporting

[Markus Armbruster]

When -netdev vhost-user fails, it first reports a specific error, then
one or more generic ones, like this:

    $ qemu-system-x86_64 -netdev vhost-user,id=foo,chardev=xxx
    qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: chardev "xxx" not found
    qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: No suitable chardev found
    qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: Device 'vhost-user' could not be initialized

With the command line, the messages go to stderr.  In HMP, they go to
the monitor.  In QMP, the last one becomes the error reply, and the
others go to stderr.

Convert net_init_vhost_user() and its helpers to Error.  This
suppresses the unwanted unspecific error messages, and makes the
specific error the QMP error reply.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/vhost-user.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 7e8a9ec..3930741 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -170,33 +170,34 @@ static int net_vhost_chardev_opts(void *opaque,
     } else if (strcmp(name, "server") == 0) {
         props->is_server = true;
     } else {
-        error_report("vhost-user does not support a chardev"
-                     " with the following option:\n %s = %s",
-                     name, value);
+        error_setg(errp,
+                   "vhost-user does not support a chardev with option %s=%s",
+                   name, value);
         return -1;
     }
     return 0;
 }
 
-static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *opts)
+static CharDriverState *net_vhost_parse_chardev(
+    const NetdevVhostUserOptions *opts, Error **errp)
 {
     CharDriverState *chr = qemu_chr_find(opts->chardev);
     VhostUserChardevProps props;
 
     if (chr == NULL) {
-        error_report("chardev \"%s\" not found", opts->chardev);
+        error_setg(errp, "chardev \"%s\" not found", opts->chardev);
         return NULL;
     }
 
     /* inspect chardev opts */
     memset(&props, 0, sizeof(props));
-    if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, NULL)) {
+    if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, errp)) {
         return NULL;
     }
 
     if (!props.is_socket || !props.is_unix) {
-        error_report("chardev \"%s\" is not a unix socket",
-                     opts->chardev);
+        error_setg(errp, "chardev \"%s\" is not a unix socket",
+                   opts->chardev);
         return NULL;
     }
 
@@ -220,7 +221,7 @@ static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
 
     if (strcmp(netdev, name) == 0 &&
         strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {
-        error_report("vhost-user requires frontend driver virtio-net-*");
+        error_setg(errp, "vhost-user requires frontend driver virtio-net-*");
         return -1;
     }
 
@@ -230,7 +231,6 @@ static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
 int net_init_vhost_user(const NetClientOptions *opts, const char *name,
                         NetClientState *peer, Error **errp)
 {
-    /* FIXME error_setg(errp, ...) on failure */
     uint32_t queues;
     const NetdevVhostUserOptions *vhost_user_opts;
     CharDriverState *chr;
@@ -238,15 +238,14 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
     assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
     vhost_user_opts = opts->vhost_user;
 
-    chr = net_vhost_parse_chardev(vhost_user_opts);
+    chr = net_vhost_parse_chardev(vhost_user_opts, errp);
     if (!chr) {
-        error_report("No suitable chardev found");
         return -1;
     }
 
     /* verify net frontend */
     if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
-                          (char *)name, NULL)) {
+                          (char *)name, errp)) {
         return -1;
     }
 
-- 
1.9.3