On Fri, Sep 25, 2026 at 09:21:20AM -0300, Fabiano Rosas wrote: > Daniel P. Berrangé <[email protected]> writes: > > > On Fri, Sep 18, 2026 at 07:29:54PM -0300, Fabiano Rosas wrote: > >> The qemu_opts_parse, qemu_opts_parse_list and qemu_opts_parse_noisily > >> functions all call opts_parse(), they can reuse code. > >> > >> Consolidate them at qemu_opts_parse_list(). Use the presence of the > >> Error pointer to determine whether the caller wants to be noisy: if > >> there is an Error pointer, then use it, otherwise print the message. > >> > >> This makes the code more opaque, so keep the qemu_opts_parse and > >> qemu_opts_parse_noisily functions as wrappers to make sure the intent > >> at the callers is still clear. > > > > It is also somewhat differnt from the normal semantics in QEMU. > > > > The behaviour of passing NULL for an "Error **errp" parameter > > indicates that the errors are to be ignored/discarded. > > > > This patch introduces atypical semantics, that the errors must > > be printed, so I'm not entirely convinced this patch is a win. > > > > I understand. This is the main reason why this series is RFC. I could > exchange the implicit "pass NULL to mean noisily" with an explicit > parameter 'bool noisily'.
I think is to better to have full separation between code that propagates an Error **errp back to the caller, vs code that reports the error to the user. A "bool noisily" would be mixing the two. I'd stil go for some duplication of error reporting logic in the callers, together with the main APIs always using the errp parameter. > > >> > >> Signed-off-by: Fabiano Rosas <[email protected]> > >> --- > >> tests/unit/test-opts-visitor.c | 2 +- > >> util/qemu-option.c | 32 ++++++++++++++++++-------------- > >> 2 files changed, 19 insertions(+), 15 deletions(-) > > > >> diff --git a/util/qemu-option.c b/util/qemu-option.c > >> index bb38d305c9..c8e552823b 100644 > >> --- a/util/qemu-option.c > >> +++ b/util/qemu-option.c > >> @@ -938,7 +938,23 @@ QemuOpts *qemu_opts_parse(const char *group, const > >> char *params, > >> QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params, > >> bool permit_abbrev, Error **errp) > >> { > >> - return opts_parse(list, params, permit_abbrev, false, false, errp); > >> + bool noisily = !errp; > >> + QemuOpts *opts = NULL; > >> + > >> + if (noisily) { > >> + Error *err = NULL; > >> + bool help_wanted = !opts_accepts_any(list); > >> + > >> + opts = opts_parse(list, params, permit_abbrev, true, help_wanted, > >> &err); > >> + if (!opts && err) { > >> + error_report_err(err); > >> + } > >> + > >> + } else { > >> + opts = opts_parse(list, params, permit_abbrev, false, false, > >> errp); > >> + } > >> + > >> + return opts; > >> } > >> > >> /** > >> @@ -952,19 +968,7 @@ QemuOpts *qemu_opts_parse_list(QemuOptsList *list, > >> const char *params, > >> QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params, > >> bool permit_abbrev) > >> { > >> - Error *err = NULL; > >> - QemuOpts *opts; > >> - bool help_wanted = !opts_accepts_any(list); > >> - > >> - opts = opts_parse(list, params, permit_abbrev, true, help_wanted, > >> &err); > >> - if (!opts) { > >> - if (help_wanted) { > >> - assert(!err); > >> - } else { > >> - error_report_err(err); > >> - } > >> - } > >> - return opts; > >> + return qemu_opts_parse_list(list, params, permit_abbrev, NULL); > >> } > >> > >> static bool qemu_opts_from_qdict_entry(QemuOpts *opts, > >> -- > >> 2.53.0 > >> > >> > > > > With regards, > > Daniel > With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
