Daniel P. Berrangé <[email protected]> writes: > 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. >
Ok, no problem, I'll keep them separate. I'm very tempted to remove the _noisily versions and propagate the error up like usual. Probably too much work for now. > > >> >> >> >> >> 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
