[Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()

Markus Armbruster posted 31 patches 7 years ago
There is a newer version of this series
[Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
Posted by Markus Armbruster 7 years ago
Calling error_report() in a function that takes an Error ** argument
is suspicious.  user_creatable_add_opts_foreach() does that, and then
fails without setting an error.  Its caller main(), via
qemu_opts_foreach(), is fine with it, but clean it up anyway.

Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qemu-io.c               |  8 +++-----
 qemu-nbd.c              |  8 +++-----
 qom/object_interfaces.c |  4 +---
 vl.c                    | 16 ++++++----------
 4 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index 13829f5e21..6df7731af4 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -620,11 +620,9 @@ int main(int argc, char **argv)
         exit(1);
     }
 
-    if (qemu_opts_foreach(&qemu_object_opts,
-                          user_creatable_add_opts_foreach,
-                          NULL, NULL)) {
-        exit(1);
-    }
+    qemu_opts_foreach(&qemu_object_opts,
+                      user_creatable_add_opts_foreach,
+                      NULL, &error_fatal);
 
     if (!trace_init_backends()) {
         exit(1);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 7874bc973c..ca7109652e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -766,11 +766,9 @@ int main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }
 
-    if (qemu_opts_foreach(&qemu_object_opts,
-                          user_creatable_add_opts_foreach,
-                          NULL, NULL)) {
-        exit(EXIT_FAILURE);
-    }
+    qemu_opts_foreach(&qemu_object_opts,
+                      user_creatable_add_opts_foreach,
+                      NULL, &error_fatal);
 
     if (!trace_init_backends()) {
         exit(1);
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 72b97a8bed..4052d6c4a7 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -143,7 +143,6 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
 {
     bool (*type_predicate)(const char *) = opaque;
     Object *obj = NULL;
-    Error *err = NULL;
     const char *type;
 
     type = qemu_opt_get(opts, "qom-type");
@@ -152,9 +151,8 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
         return 0;
     }
 
-    obj = user_creatable_add_opts(opts, &err);
+    obj = user_creatable_add_opts(opts, errp);
     if (!obj) {
-        error_report_err(err);
         return -1;
     }
     object_unref(obj);
diff --git a/vl.c b/vl.c
index 7ce8299d9d..b8576f8f10 100644
--- a/vl.c
+++ b/vl.c
@@ -4181,11 +4181,9 @@ int main(int argc, char **argv, char **envp)
     page_size_init();
     socket_init();
 
-    if (qemu_opts_foreach(qemu_find_opts("object"),
-                          user_creatable_add_opts_foreach,
-                          object_create_initial, NULL)) {
-        exit(1);
-    }
+    qemu_opts_foreach(qemu_find_opts("object"),
+                      user_creatable_add_opts_foreach,
+                      object_create_initial, &error_fatal);
 
     if (qemu_opts_foreach(qemu_find_opts("chardev"),
                           chardev_init_func, NULL, NULL)) {
@@ -4316,11 +4314,9 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (qemu_opts_foreach(qemu_find_opts("object"),
-                          user_creatable_add_opts_foreach,
-                          object_create_delayed, NULL)) {
-        exit(1);
-    }
+    qemu_opts_foreach(qemu_find_opts("object"),
+                      user_creatable_add_opts_foreach,
+                      object_create_delayed, &error_fatal);
 
     if (tpm_init() < 0) {
         exit(1);
-- 
2.17.1


Re: [Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
Posted by Eric Blake 7 years ago
On 10/8/18 12:31 PM, Markus Armbruster wrote:
> Calling error_report() in a function that takes an Error ** argument
> is suspicious.  user_creatable_add_opts_foreach() does that, and then
> fails without setting an error.  Its caller main(), via
> qemu_opts_foreach(), is fine with it, but clean it up anyway.
> 
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   qemu-io.c               |  8 +++-----
>   qemu-nbd.c              |  8 +++-----
>   qom/object_interfaces.c |  4 +---
>   vl.c                    | 16 ++++++----------
>   4 files changed, 13 insertions(+), 23 deletions(-)

While I could take this through my NBD tree, I think it makes more sense 
to go through your error tree with the rest of the series.

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
Posted by Marc-André Lureau 7 years ago
Hi

On Mon, Oct 8, 2018 at 9:39 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Calling error_report() in a function that takes an Error ** argument
> is suspicious.  user_creatable_add_opts_foreach() does that, and then
> fails without setting an error.  Its caller main(), via
> qemu_opts_foreach(), is fine with it, but clean it up anyway.
>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  qemu-io.c               |  8 +++-----
>  qemu-nbd.c              |  8 +++-----
>  qom/object_interfaces.c |  4 +---
>  vl.c                    | 16 ++++++----------
>  4 files changed, 13 insertions(+), 23 deletions(-)
>
> diff --git a/qemu-io.c b/qemu-io.c
> index 13829f5e21..6df7731af4 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -620,11 +620,9 @@ int main(int argc, char **argv)
>          exit(1);
>      }
>
> -    if (qemu_opts_foreach(&qemu_object_opts,
> -                          user_creatable_add_opts_foreach,
> -                          NULL, NULL)) {
> -        exit(1);
> -    }
> +    qemu_opts_foreach(&qemu_object_opts,
> +                      user_creatable_add_opts_foreach,
> +                      NULL, &error_fatal);
>
>      if (!trace_init_backends()) {
>          exit(1);
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 7874bc973c..ca7109652e 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -766,11 +766,9 @@ int main(int argc, char **argv)
>          exit(EXIT_FAILURE);
>      }
>
> -    if (qemu_opts_foreach(&qemu_object_opts,
> -                          user_creatable_add_opts_foreach,
> -                          NULL, NULL)) {
> -        exit(EXIT_FAILURE);
> -    }
> +    qemu_opts_foreach(&qemu_object_opts,
> +                      user_creatable_add_opts_foreach,
> +                      NULL, &error_fatal);
>
>      if (!trace_init_backends()) {
>          exit(1);
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 72b97a8bed..4052d6c4a7 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -143,7 +143,6 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
>  {
>      bool (*type_predicate)(const char *) = opaque;
>      Object *obj = NULL;
> -    Error *err = NULL;
>      const char *type;
>
>      type = qemu_opt_get(opts, "qom-type");
> @@ -152,9 +151,8 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
>          return 0;
>      }
>
> -    obj = user_creatable_add_opts(opts, &err);
> +    obj = user_creatable_add_opts(opts, errp);
>      if (!obj) {
> -        error_report_err(err);
>          return -1;
>      }
>      object_unref(obj);
> diff --git a/vl.c b/vl.c
> index 7ce8299d9d..b8576f8f10 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4181,11 +4181,9 @@ int main(int argc, char **argv, char **envp)
>      page_size_init();
>      socket_init();
>
> -    if (qemu_opts_foreach(qemu_find_opts("object"),
> -                          user_creatable_add_opts_foreach,
> -                          object_create_initial, NULL)) {
> -        exit(1);
> -    }
> +    qemu_opts_foreach(qemu_find_opts("object"),
> +                      user_creatable_add_opts_foreach,
> +                      object_create_initial, &error_fatal);
>
>      if (qemu_opts_foreach(qemu_find_opts("chardev"),
>                            chardev_init_func, NULL, NULL)) {
> @@ -4316,11 +4314,9 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>
> -    if (qemu_opts_foreach(qemu_find_opts("object"),
> -                          user_creatable_add_opts_foreach,
> -                          object_create_delayed, NULL)) {
> -        exit(1);
> -    }
> +    qemu_opts_foreach(qemu_find_opts("object"),
> +                      user_creatable_add_opts_foreach,
> +                      object_create_delayed, &error_fatal);
>
>      if (tpm_init() < 0) {
>          exit(1);
> --
> 2.17.1
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
Posted by Daniel P. Berrangé 7 years ago
On Mon, Oct 08, 2018 at 07:31:12PM +0200, Markus Armbruster wrote:
> Calling error_report() in a function that takes an Error ** argument
> is suspicious.  user_creatable_add_opts_foreach() does that, and then
> fails without setting an error.  Its caller main(), via
> qemu_opts_foreach(), is fine with it, but clean it up anyway.
> 
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qemu-io.c               |  8 +++-----
>  qemu-nbd.c              |  8 +++-----
>  qom/object_interfaces.c |  4 +---
>  vl.c                    | 16 ++++++----------
>  4 files changed, 13 insertions(+), 23 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|