Re: [Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
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é > Signed-off-by: Markus Armbruster > --- > 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é 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 :|
Re: [Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
Hi On Mon, Oct 8, 2018 at 9:39 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é > Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau > --- > 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(_object_opts, > - user_creatable_add_opts_foreach, > - NULL, NULL)) { > -exit(1); > -} > +qemu_opts_foreach(_object_opts, > + user_creatable_add_opts_foreach, > + NULL, _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(_object_opts, > - user_creatable_add_opts_foreach, > - NULL, NULL)) { > -exit(EXIT_FAILURE); > -} > +qemu_opts_foreach(_object_opts, > + user_creatable_add_opts_foreach, > + NULL, _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, ); > +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, _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, _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()
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é Signed-off-by: Markus Armbruster --- 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 -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
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é Signed-off-by: Markus Armbruster --- 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(_object_opts, - user_creatable_add_opts_foreach, - NULL, NULL)) { -exit(1); -} +qemu_opts_foreach(_object_opts, + user_creatable_add_opts_foreach, + NULL, _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(_object_opts, - user_creatable_add_opts_foreach, - NULL, NULL)) { -exit(EXIT_FAILURE); -} +qemu_opts_foreach(_object_opts, + user_creatable_add_opts_foreach, + NULL, _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, ); +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, _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, _fatal); if (tpm_init() < 0) { exit(1); -- 2.17.1