* [RFC PATCH 01/14] meson: Fix meson suites invocation from make check
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 12:14 ` Fabiano Rosas
2026-09-18 22:29 ` [RFC PATCH 02/14] qemu-options: Use g_autofree when calling get_opt_name_value() Fabiano Rosas
` (12 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Paolo Bonzini, Alex Bennée, John Snow,
Cleber Rosa, Kevin Wolf, Hanna Reitz
make check is currently by default running tests tagged
"slow". Running make check with no SPEED variable set should be
equivalent to SPEED=quick and run the meson 'quick' test setup, which
explicitly excludes slow tests:
add_test_setup('quick', exclude_suites: ['slow', 'thorough'] ...
The problem is that the mtest2make.py script is effectively overriding
the "meson test --setup" setting by using --suite which takes
precedence over --setup according to meson documentation.
Fix by excluding suites directly with the "--no-suite" option while
generating Makefile.mtest.
Although the 'exclude_suites' in meson.build are useless for make
check, they still help when running 'meson test' from the
command-line, so keep them around.
For the configure line I'm using:
make check goes from 1100 to 996 tests
make SPEED=slow check stays at 2158 tests
make SPEED=thorough check stays at 2279 tests
(the rest of the test targets look sane, but maintainers please
double-check)
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
- With this change we obviously lose coverage in make check. I suggest
we use this opportunity to review what gets put in the quick
suite.
For migration, I want to keep only the smoke tests in make
check. I'll send a patch adding them back if this change is
accepted.
- Some of the suffixes from functional tests could probably be removed
and rely on having the tests in the proper suite instead. I.e. use
'slow' instead of 'foobar_slow'.
---
scripts/mtest2make.py | 11 +++++++++--
tests/qemu-iotests/meson.build | 2 +-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
index 383ea68b16..a61a83f775 100644
--- a/scripts/mtest2make.py
+++ b/scripts/mtest2make.py
@@ -26,6 +26,11 @@ def names(self, base):
.speed.slow = $(sort $(filter-out %-thorough, $1))
.speed.thorough = $(sort $1)
+.speed_exclude_suites.quick = --no-suite slow --no-suite thorough \
+ --no-suite optional
+.speed_exclude_suites.slow = --no-suite thorough
+.speed_exclude_suites.thorough =
+
TIMEOUT_MULTIPLIER ?= 1
.mtestargs = --no-rebuild -t $(TIMEOUT_MULTIPLIER)
ifneq ($(SPEED), quick)
@@ -34,7 +39,8 @@ def names(self, base):
.mtestargs += $(subst -j,--num-processes , $(filter-out -j, $(lastword -j1 $(filter -j%, $(MAKEFLAGS)))))
.check.mtestargs = $(MTESTARGS) $(.mtestargs) $(if $(V),--verbose,--print-errorlogs) \
- $(foreach s, $(sort $(.check.mtest-suites)), --suite $s)
+ $(foreach s, $(sort $(.check.mtest-suites)), --suite $s) \
+ $(.speed_exclude_suites.$(SPEED))
.bench.mtestargs = $(MTESTARGS) $(.mtestargs) --benchmark --verbose \
$(foreach s, $(sort $(.bench.mtest-suites)), --suite $s)''')
@@ -102,7 +108,8 @@ def emit_suite(name, suite, prefix):
targets += f' {prefix} {prefix}-report.junit.xml'
print(f'ifneq ($(filter {targets}, $(MAKECMDGOALS)),)')
# for the "base" suite possibly add FOO-slow and FOO-thorough
- print(f".{prefix}.mtest-suites += {name} $(call .speed.$(SPEED), {names})")
+ print(f".{prefix}.mtest-suites += {name} $(call .speed.$(SPEED), {names}) "
+ ".speed_exclude_suites.$(SPEED)")
print(f'endif')
targets = {t['id']: [os.path.relpath(f) for f in t['filename']]
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index 8857f25f29..34cfcdc53b 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -68,7 +68,7 @@ foreach driver, speed: qemu_iotests_drivers
endif
# Every driver gets put in the driver specific suite
- suites = ['block-' + driver + '-optional']
+ suites = ['block-' + driver + '-optional', 'optional']
# Any driver tagged quick or slow also gets added to slow
# otherwise its tagged thorough
if speed != 'thorough'
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH 01/14] meson: Fix meson suites invocation from make check
2026-09-18 22:29 ` [RFC PATCH 01/14] meson: Fix meson suites invocation from make check Fabiano Rosas
@ 2026-09-25 12:14 ` Fabiano Rosas
0 siblings, 0 replies; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-25 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Paolo Bonzini, Alex Bennée, John Snow,
Cleber Rosa, Kevin Wolf, Hanna Reitz
Fabiano Rosas <farosas@suse.de> writes:
> make check is currently by default running tests tagged
> "slow". Running make check with no SPEED variable set should be
> equivalent to SPEED=quick and run the meson 'quick' test setup, which
> explicitly excludes slow tests:
>
> add_test_setup('quick', exclude_suites: ['slow', 'thorough'] ...
>
> The problem is that the mtest2make.py script is effectively overriding
> the "meson test --setup" setting by using --suite which takes
> precedence over --setup according to meson documentation.
>
> Fix by excluding suites directly with the "--no-suite" option while
> generating Makefile.mtest.
>
> Although the 'exclude_suites' in meson.build are useless for make
> check, they still help when running 'meson test' from the
> command-line, so keep them around.
>
> For the configure line I'm using:
>
> make check goes from 1100 to 996 tests
> make SPEED=slow check stays at 2158 tests
> make SPEED=thorough check stays at 2279 tests
>
> (the rest of the test targets look sane, but maintainers please
> double-check)
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> - With this change we obviously lose coverage in make check. I suggest
> we use this opportunity to review what gets put in the quick
> suite.
>
> For migration, I want to keep only the smoke tests in make
> check. I'll send a patch adding them back if this change is
> accepted.
>
> - Some of the suffixes from functional tests could probably be removed
> and rely on having the tests in the proper suite instead. I.e. use
> 'slow' instead of 'foobar_slow'.
> ---
> scripts/mtest2make.py | 11 +++++++++--
> tests/qemu-iotests/meson.build | 2 +-
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
> index 383ea68b16..a61a83f775 100644
> --- a/scripts/mtest2make.py
> +++ b/scripts/mtest2make.py
> @@ -26,6 +26,11 @@ def names(self, base):
> .speed.slow = $(sort $(filter-out %-thorough, $1))
> .speed.thorough = $(sort $1)
>
> +.speed_exclude_suites.quick = --no-suite slow --no-suite thorough \
> + --no-suite optional
> +.speed_exclude_suites.slow = --no-suite thorough
> +.speed_exclude_suites.thorough =
> +
> TIMEOUT_MULTIPLIER ?= 1
> .mtestargs = --no-rebuild -t $(TIMEOUT_MULTIPLIER)
> ifneq ($(SPEED), quick)
> @@ -34,7 +39,8 @@ def names(self, base):
> .mtestargs += $(subst -j,--num-processes , $(filter-out -j, $(lastword -j1 $(filter -j%, $(MAKEFLAGS)))))
>
> .check.mtestargs = $(MTESTARGS) $(.mtestargs) $(if $(V),--verbose,--print-errorlogs) \
> - $(foreach s, $(sort $(.check.mtest-suites)), --suite $s)
> + $(foreach s, $(sort $(.check.mtest-suites)), --suite $s) \
> + $(.speed_exclude_suites.$(SPEED))
> .bench.mtestargs = $(MTESTARGS) $(.mtestargs) --benchmark --verbose \
> $(foreach s, $(sort $(.bench.mtest-suites)), --suite $s)''')
>
> @@ -102,7 +108,8 @@ def emit_suite(name, suite, prefix):
> targets += f' {prefix} {prefix}-report.junit.xml'
> print(f'ifneq ($(filter {targets}, $(MAKECMDGOALS)),)')
> # for the "base" suite possibly add FOO-slow and FOO-thorough
> - print(f".{prefix}.mtest-suites += {name} $(call .speed.$(SPEED), {names})")
> + print(f".{prefix}.mtest-suites += {name} $(call .speed.$(SPEED), {names}) "
> + ".speed_exclude_suites.$(SPEED)")
> print(f'endif')
>
> targets = {t['id']: [os.path.relpath(f) for f in t['filename']]
> diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
> index 8857f25f29..34cfcdc53b 100644
> --- a/tests/qemu-iotests/meson.build
> +++ b/tests/qemu-iotests/meson.build
> @@ -68,7 +68,7 @@ foreach driver, speed: qemu_iotests_drivers
> endif
>
> # Every driver gets put in the driver specific suite
> - suites = ['block-' + driver + '-optional']
> + suites = ['block-' + driver + '-optional', 'optional']
> # Any driver tagged quick or slow also gets added to slow
> # otherwise its tagged thorough
> if speed != 'thorough'
Oops, this one sneaked in, please ignore it. I sent it as a separate
patch already.
https://lore.kernel.org/r/20260918203153.3874488-1-farosas@suse.de
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC PATCH 02/14] qemu-options: Use g_autofree when calling get_opt_name_value()
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
2026-09-18 22:29 ` [RFC PATCH 01/14] meson: Fix meson suites invocation from make check Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 10:05 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 03/14] qemu-options: Consolidate branches in opts_parse Fabiano Rosas
` (11 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
The get_opt_name_value function takes two pointer arguments that must
be freed by the callers when not used. This is a good situation to use
g_autofree. Steal the pointers that need to be passed forward.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
util/qemu-option.c | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 9fbf425f86..557d5d929c 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -813,27 +813,24 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
const char *firstname,
bool warn_on_flag, bool *help_wanted, Error **errp)
{
- char *option, *value;
const char *p;
QemuOpt *opt;
for (p = params; *p;) {
+ g_autofree char *option = NULL;
+ g_autofree char *value = NULL;
+
p = get_opt_name_value(p, firstname, warn_on_flag, help_wanted, &option, &value);
if (help_wanted && *help_wanted) {
- g_free(option);
- g_free(value);
return false;
}
firstname = NULL;
if (!strcmp(option, "id")) {
- g_free(option);
- g_free(value);
continue;
}
- opt = opt_create(opts, option, value);
- g_free(option);
+ opt = opt_create(opts, option, g_steal_pointer(&value));
if (!opt_validate(opt, errp)) {
qemu_opt_del(opt);
return false;
@@ -846,16 +843,15 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
static char *opts_parse_id(const char *params)
{
const char *p;
- char *name, *value;
for (p = params; *p;) {
+ g_autofree char *name = NULL;
+ g_autofree char *value = NULL;
+
p = get_opt_name_value(p, NULL, false, NULL, &name, &value);
if (!strcmp(name, "id")) {
- g_free(name);
- return value;
+ return g_steal_pointer(&value);
}
- g_free(name);
- g_free(value);
}
return NULL;
@@ -864,13 +860,13 @@ static char *opts_parse_id(const char *params)
bool has_help_option(const char *params)
{
const char *p;
- char *name, *value;
bool ret = false;
for (p = params; *p;) {
+ g_autofree char *name = NULL;
+ g_autofree char *value = NULL;
+
p = get_opt_name_value(p, NULL, false, &ret, &name, &value);
- g_free(name);
- g_free(value);
if (ret) {
return true;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH 03/14] qemu-options: Consolidate branches in opts_parse
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
2026-09-18 22:29 ` [RFC PATCH 01/14] meson: Fix meson suites invocation from make check Fabiano Rosas
2026-09-18 22:29 ` [RFC PATCH 02/14] qemu-options: Use g_autofree when calling get_opt_name_value() Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 10:07 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 04/14] qemu-options: Add qemu_opts_parse_list Fabiano Rosas
` (10 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
The two branches that return NULL can be put under a single branch
because the qemu_opts_del function can take a NULL opt. The next patch
will add common cleanup code to the error path so change this now to
avoid code duplication.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
util/qemu-option.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 557d5d929c..e115e1acef 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -900,12 +900,8 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
opts = qemu_opts_create(list, id, !list->merge_lists, errp);
g_free(id);
- if (opts == NULL) {
- return NULL;
- }
-
- if (!opts_do_parse(opts, params, firstname,
- warn_on_flag, help_wanted, errp)) {
+ if (!opts || !opts_do_parse(opts, params, firstname,
+ warn_on_flag, help_wanted, errp)) {
qemu_opts_del(opts);
return NULL;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH 04/14] qemu-options: Add qemu_opts_parse_list
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (2 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 03/14] qemu-options: Consolidate branches in opts_parse Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 10:14 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 05/14] qemu-options: Change qemu_parse_opts() to take the group name Fabiano Rosas
` (9 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Daniel P. Berrangé
Currently the qemu_opts_parse and qemu_opts_parse_noisily functions
are called in one two ways:
1) qemu_opts_parse|_noisily(&some_group_list, ...)
2) qemu_opts_parse|_noisily(qemu_find_opts("some_opt"), ...)
The second form is problematic. Having the result of qemu_find_opts()
going directly into the first argument makes it difficult to handle a
possible NULL pointer retuned by it.
Next patches will change qemu_opts_parse() and
qemu_opts_parse_noisily() to receive the option name and call
qemu_find_opts() in their body so the return of the function can be
handled properly.
Add a new qemu_opts_parse_list() and change all callsites that pass
the list directly to use it. In the next patches this new _list
version will become the inner function, but at this point, simply make
it defer to qemu_opts_parse() to make the refactoring easier.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/qemu/option.h | 2 +
tests/unit/test-qemu-opts.c | 169 +++++++++++++++++++-----------------
util/qemu-option.c | 13 +++
util/qemu-sockets.c | 2 +-
4 files changed, 104 insertions(+), 82 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 9a00ac0a35..f96d9fae42 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -132,6 +132,8 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
bool permit_abbrev);
QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
bool permit_abbrev, Error **errp);
+QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
+ bool permit_abbrev, Error **errp);
QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
Error **errp);
QDict *qemu_opts_to_qdict_filtered(QemuOpts *opts, QDict *qdict,
diff --git a/tests/unit/test-qemu-opts.c b/tests/unit/test-qemu-opts.c
index 8d03a69f7c..1257ef750b 100644
--- a/tests/unit/test-qemu-opts.c
+++ b/tests/unit/test-qemu-opts.c
@@ -349,7 +349,7 @@ static void test_qemu_opt_unset(void)
int ret;
/* dynamically initialized (parsed) opts */
- opts = qemu_opts_parse(&opts_list_03, "key=value", false, NULL);
+ opts = qemu_opts_parse_list(&opts_list_03, "key=value", false, NULL);
g_assert(opts != NULL);
/* check default/parsed value */
@@ -431,95 +431,96 @@ static void test_opts_parse(void)
QemuOpts *opts;
/* Nothing */
- opts = qemu_opts_parse(&opts_list_03, "", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "", false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 0);
/* Empty key */
- opts = qemu_opts_parse(&opts_list_03, "=val", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "=val", false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpstr(qemu_opt_get(opts, ""), ==, "val");
/* Multiple keys, last one wins */
- opts = qemu_opts_parse(&opts_list_03, "a=1,b=2,,x,a=3",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "a=1,b=2,,x,a=3",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 3);
g_assert_cmpstr(qemu_opt_get(opts, "a"), ==, "3");
g_assert_cmpstr(qemu_opt_get(opts, "b"), ==, "2,x");
/* Except when it doesn't */
- opts = qemu_opts_parse(&opts_list_03, "id=foo,id=bar",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "id=foo,id=bar",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 0);
g_assert_cmpstr(qemu_opts_id(opts), ==, "foo");
/* TODO Cover low-level access to repeated keys */
/* Trailing comma is ignored */
- opts = qemu_opts_parse(&opts_list_03, "x=y,", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "x=y,", false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpstr(qemu_opt_get(opts, "x"), ==, "y");
/* Except when it isn't */
- opts = qemu_opts_parse(&opts_list_03, ",", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, ",", false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpstr(qemu_opt_get(opts, ""), ==, "on");
/* Duplicate ID */
- opts = qemu_opts_parse(&opts_list_03, "x=y,id=foo", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_03, "x=y,id=foo", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* TODO Cover .merge_lists = true */
/* Buggy ID recognition (fixed) */
- opts = qemu_opts_parse(&opts_list_03, "x=,,id=bar", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "x=,,id=bar", false,
+ &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert(!qemu_opts_id(opts));
g_assert_cmpstr(qemu_opt_get(opts, "x"), ==, ",id=bar");
/* Anti-social ID */
- opts = qemu_opts_parse(&opts_list_01, "id=666", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_01, "id=666", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Implied value (qemu_opts_parse warns but accepts it) */
- opts = qemu_opts_parse(&opts_list_03, "an,noaus,noaus=",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "an,noaus,noaus=",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 3);
g_assert_cmpstr(qemu_opt_get(opts, "an"), ==, "on");
g_assert_cmpstr(qemu_opt_get(opts, "aus"), ==, "off");
g_assert_cmpstr(qemu_opt_get(opts, "noaus"), ==, "");
/* Implied value, negated empty key */
- opts = qemu_opts_parse(&opts_list_03, "no", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "no", false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpstr(qemu_opt_get(opts, ""), ==, "off");
/* Implied key */
- opts = qemu_opts_parse(&opts_list_03, "an,noaus,noaus=", true,
- &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "an,noaus,noaus=", true,
+ &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 3);
g_assert_cmpstr(qemu_opt_get(opts, "implied"), ==, "an");
g_assert_cmpstr(qemu_opt_get(opts, "aus"), ==, "off");
g_assert_cmpstr(qemu_opt_get(opts, "noaus"), ==, "");
/* Implied key with empty value */
- opts = qemu_opts_parse(&opts_list_03, ",", true, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, ",", true, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpstr(qemu_opt_get(opts, "implied"), ==, "");
/* Implied key with comma value */
- opts = qemu_opts_parse(&opts_list_03, ",,,a=1", true, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, ",,,a=1", true, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 2);
g_assert_cmpstr(qemu_opt_get(opts, "implied"), ==, ",");
g_assert_cmpstr(qemu_opt_get(opts, "a"), ==, "1");
/* Empty key is not an implied key */
- opts = qemu_opts_parse(&opts_list_03, "=val", true, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "=val", true, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpstr(qemu_opt_get(opts, ""), ==, "val");
/* Unknown key */
- opts = qemu_opts_parse(&opts_list_01, "nonexistent=", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_01, "nonexistent=", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
@@ -532,13 +533,13 @@ static void test_opts_parse_bool(void)
Error *err = NULL;
QemuOpts *opts;
- opts = qemu_opts_parse(&opts_list_02, "bool1=on,bool2=off",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_02, "bool1=on,bool2=off",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 2);
g_assert(qemu_opt_get_bool(opts, "bool1", false));
g_assert(!qemu_opt_get_bool(opts, "bool2", true));
- opts = qemu_opts_parse(&opts_list_02, "bool1=offer", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_02, "bool1=offer", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
@@ -551,59 +552,60 @@ static void test_opts_parse_number(void)
QemuOpts *opts;
/* Lower limit zero */
- opts = qemu_opts_parse(&opts_list_01, "number1=0", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1=0", false,
+ &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpuint(qemu_opt_get_number(opts, "number1", 1), ==, 0);
/* Upper limit 2^64-1 */
- opts = qemu_opts_parse(&opts_list_01,
- "number1=18446744073709551615,number2=-1",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_01,
+ "number1=18446744073709551615,number2=-1",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 2);
g_assert_cmphex(qemu_opt_get_number(opts, "number1", 1), ==, UINT64_MAX);
g_assert_cmphex(qemu_opt_get_number(opts, "number2", 0), ==, UINT64_MAX);
/* Above upper limit */
- opts = qemu_opts_parse(&opts_list_01, "number1=18446744073709551616",
- false, &err);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1=18446744073709551616",
+ false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Below lower limit */
- opts = qemu_opts_parse(&opts_list_01, "number1=-18446744073709551616",
- false, &err);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1=-18446744073709551616",
+ false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Hex and octal */
- opts = qemu_opts_parse(&opts_list_01, "number1=0x2a,number2=052",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1=0x2a,number2=052",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 2);
g_assert_cmpuint(qemu_opt_get_number(opts, "number1", 1), ==, 42);
g_assert_cmpuint(qemu_opt_get_number(opts, "number2", 0), ==, 42);
/* Invalid */
- opts = qemu_opts_parse(&opts_list_01, "number1=", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1=", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
- opts = qemu_opts_parse(&opts_list_01, "number1=eins", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1=eins", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Leading whitespace */
- opts = qemu_opts_parse(&opts_list_01, "number1= \t42",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1= \t42",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpuint(qemu_opt_get_number(opts, "number1", 1), ==, 42);
/* Trailing crap */
- opts = qemu_opts_parse(&opts_list_01, "number1=3.14", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1=3.14", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
- opts = qemu_opts_parse(&opts_list_01, "number1=08", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1=08", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
- opts = qemu_opts_parse(&opts_list_01, "number1=0 ", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_01, "number1=0 ", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
@@ -616,18 +618,18 @@ static void test_opts_parse_size(void)
QemuOpts *opts;
/* Lower limit zero */
- opts = qemu_opts_parse(&opts_list_02, "size1=0", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_02, "size1=0", false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpuint(qemu_opt_get_size(opts, "size1", 1), ==, 0);
/* Note: full 64 bits of precision */
/* Around double limit of precision: 2^53-1, 2^53, 2^53+1 */
- opts = qemu_opts_parse(&opts_list_02,
- "size1=9007199254740991,"
- "size2=9007199254740992,"
- "size3=9007199254740993",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_02,
+ "size1=9007199254740991,"
+ "size2=9007199254740992,"
+ "size3=9007199254740993",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 3);
g_assert_cmphex(qemu_opt_get_size(opts, "size1", 1),
==, 0x1fffffffffffff);
@@ -637,11 +639,12 @@ static void test_opts_parse_size(void)
==, 0x20000000000001);
/* Close to signed int limit: 2^63-1, 2^63, 2^63+1 */
- opts = qemu_opts_parse(&opts_list_02,
- "size1=9223372036854775807," /* 7fffffffffffffff */
- "size2=9223372036854775808," /* 8000000000000000 */
- "size3=9223372036854775809", /* 8000000000000001 */
- false, &error_abort);
+ opts = qemu_opts_parse_list(
+ &opts_list_02,
+ "size1=9223372036854775807," /* 7fffffffffffffff */
+ "size2=9223372036854775808," /* 8000000000000000 */
+ "size3=9223372036854775809", /* 8000000000000001 */
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 3);
g_assert_cmphex(qemu_opt_get_size(opts, "size1", 1),
==, 0x7fffffffffffffff);
@@ -651,10 +654,11 @@ static void test_opts_parse_size(void)
==, 0x8000000000000001);
/* Close to actual upper limit 0xfffffffffffff800 (53 msbs set) */
- opts = qemu_opts_parse(&opts_list_02,
- "size1=18446744073709549568," /* fffffffffffff800 */
- "size2=18446744073709550591", /* fffffffffffffbff */
- false, &error_abort);
+ opts = qemu_opts_parse_list(
+ &opts_list_02,
+ "size1=18446744073709549568," /* fffffffffffff800 */
+ "size2=18446744073709550591", /* fffffffffffffbff */
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 2);
g_assert_cmphex(qemu_opt_get_size(opts, "size1", 1),
==, 0xfffffffffffff800);
@@ -662,47 +666,48 @@ static void test_opts_parse_size(void)
==, 0xfffffffffffffbff);
/* Actual limit, 2^64-1 */
- opts = qemu_opts_parse(&opts_list_02,
- "size1=18446744073709551615", /* ffffffffffffffff */
- false, &error_abort);
+ opts = qemu_opts_parse_list(
+ &opts_list_02,
+ "size1=18446744073709551615", /* ffffffffffffffff */
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmphex(qemu_opt_get_size(opts, "size1", 1),
==, 0xffffffffffffffff);
/* Beyond limits */
- opts = qemu_opts_parse(&opts_list_02, "size1=-1", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_02, "size1=-1", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
- opts = qemu_opts_parse(&opts_list_02,
- "size1=18446744073709551616", /* 2^64 */
- false, &err);
+ opts = qemu_opts_parse_list(&opts_list_02,
+ "size1=18446744073709551616", /* 2^64 */
+ false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Suffixes */
- opts = qemu_opts_parse(&opts_list_02, "size1=8b,size2=1.5k,size3=2M",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_02, "size1=8b,size2=1.5k,size3=2M",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 3);
g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, 8);
g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 1536);
g_assert_cmphex(qemu_opt_get_size(opts, "size3", 0), ==, 2 * MiB);
- opts = qemu_opts_parse(&opts_list_02, "size1=0.1G,size2=16777215T",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_02, "size1=0.1G,size2=16777215T",
+ false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 2);
g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, GiB / 10);
g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 16777215ULL * TiB);
/* Beyond limit with suffix */
- opts = qemu_opts_parse(&opts_list_02, "size1=16777216T",
- false, &err);
+ opts = qemu_opts_parse_list(&opts_list_02, "size1=16777216T",
+ false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Trailing crap */
- opts = qemu_opts_parse(&opts_list_02, "size1=16E", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_02, "size1=16E", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
- opts = qemu_opts_parse(&opts_list_02, "size1=16Gi", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_02, "size1=16Gi", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
@@ -737,13 +742,13 @@ static void test_has_help_option(void)
for (i = 0; i < ARRAY_SIZE(test); i++) {
g_assert_cmpint(has_help_option(test[i].params),
==, test[i].expect);
- opts = qemu_opts_parse(&opts_list_03, test[i].params, false,
- &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, test[i].params, false,
+ &error_abort);
g_assert_cmpint(qemu_opt_has_help_opt(opts),
==, test[i].expect);
qemu_opts_del(opts);
- opts = qemu_opts_parse(&opts_list_03, test[i].params, true,
- &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, test[i].params, true,
+ &error_abort);
g_assert_cmpint(qemu_opt_has_help_opt(opts),
==, test[i].expect_implied);
qemu_opts_del(opts);
@@ -877,8 +882,9 @@ static void test_opts_to_qdict_basic(void)
QemuOpts *opts;
QDict *dict;
- opts = qemu_opts_parse(&opts_list_01, "str1=foo,str2=,str3=bar,number1=42",
- false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_01,
+ "str1=foo,str2=,str3=bar,number1=42",
+ false, &error_abort);
g_assert(opts != NULL);
dict = qemu_opts_to_qdict(opts, NULL);
@@ -903,9 +909,9 @@ static void test_opts_to_qdict_filtered(void)
first = qemu_opts_append(NULL, &opts_list_02);
merged = qemu_opts_append(first, &opts_list_01);
- opts = qemu_opts_parse(merged,
- "str1=foo,str2=,str3=bar,bool1=off,number1=42",
- false, &error_abort);
+ opts = qemu_opts_parse_list(
+ merged, "str1=foo,str2=,str3=bar,bool1=off,number1=42",
+ false, &error_abort);
g_assert(opts != NULL);
/* Convert to QDict without deleting from opts */
@@ -962,7 +968,8 @@ static void test_opts_to_qdict_duplicates(void)
QemuOpt *opt;
QDict *dict;
- opts = qemu_opts_parse(&opts_list_03, "foo=a,foo=b", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "foo=a,foo=b", false,
+ &error_abort);
g_assert(opts != NULL);
/* Verify that opts has two options with the same name */
diff --git a/util/qemu-option.c b/util/qemu-option.c
index e115e1acef..aa879b78f8 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -922,6 +922,19 @@ QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
return opts_parse(list, params, permit_abbrev, false, NULL, errp);
}
+/**
+ * Create a QemuOpts from @list with options parsed from @params. If
+ * @permit_abbrev, the first key=value in @params may omit key= and is
+ * treated as if key was @list->implied_opt_name. On error, store an
+ * error object through @errp if non-null. Return the new QemuOpts on
+ * success, null pointer on error.
+ */
+QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
+ bool permit_abbrev, Error **errp)
+{
+ return qemu_opts_parse(list, params, permit_abbrev, errp);
+}
+
/**
* Create a QemuOpts in @list and with options parsed from @params.
* If @permit_abbrev, the first key=value in @params may omit key=,
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 4773755fd5..30226671a9 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -704,7 +704,7 @@ static QemuOptsList inet_opts = {
int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
{
- QemuOpts *opts = qemu_opts_parse(&inet_opts, str, true, errp);
+ QemuOpts *opts = qemu_opts_parse_list(&inet_opts, str, true, errp);
if (!opts) {
return -1;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH 04/14] qemu-options: Add qemu_opts_parse_list
2026-09-18 22:29 ` [RFC PATCH 04/14] qemu-options: Add qemu_opts_parse_list Fabiano Rosas
@ 2026-09-25 10:14 ` Daniel P. Berrangé
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrangé @ 2026-09-25 10:14 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster
On Fri, Sep 18, 2026 at 07:29:51PM -0300, Fabiano Rosas wrote:
> Currently the qemu_opts_parse and qemu_opts_parse_noisily functions
> are called in one two ways:
>
> 1) qemu_opts_parse|_noisily(&some_group_list, ...)
> 2) qemu_opts_parse|_noisily(qemu_find_opts("some_opt"), ...)
>
> The second form is problematic. Having the result of qemu_find_opts()
> going directly into the first argument makes it difficult to handle a
> possible NULL pointer retuned by it.
>
> Next patches will change qemu_opts_parse() and
> qemu_opts_parse_noisily() to receive the option name and call
> qemu_find_opts() in their body so the return of the function can be
> handled properly.
>
> Add a new qemu_opts_parse_list() and change all callsites that pass
> the list directly to use it. In the next patches this new _list
> version will become the inner function, but at this point, simply make
> it defer to qemu_opts_parse() to make the refactoring easier.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> include/qemu/option.h | 2 +
> tests/unit/test-qemu-opts.c | 169 +++++++++++++++++++-----------------
> util/qemu-option.c | 13 +++
> util/qemu-sockets.c | 2 +-
> 4 files changed, 104 insertions(+), 82 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC PATCH 05/14] qemu-options: Change qemu_parse_opts() to take the group name
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (3 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 04/14] qemu-options: Add qemu_opts_parse_list Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 10:11 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 06/14] qemu-options: Call qemu_opts_print_help() one level down Fabiano Rosas
` (8 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Paolo Bonzini, Michael Roth
Get rid of the pattern qemu_parse_opts(qemu_find_opts("some-opt")) by
passing the string directly to qemu_parse_opts().
Don't check for NULL list at this time to preserve the old behavior. A
subsequent patch will correct that.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/qemu/option.h | 2 +-
system/vl.c | 5 ++---
tests/unit/test-opts-visitor.c | 12 ++++--------
util/qemu-option.c | 19 +++++++++++--------
4 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index f96d9fae42..dfe0d9b893 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -130,7 +130,7 @@ bool qemu_opts_do_parse(QemuOpts *opts, const char *params,
const char *firstname, Error **errp);
QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
bool permit_abbrev);
-QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
+QemuOpts *qemu_opts_parse(const char *group, const char *params,
bool permit_abbrev, Error **errp);
QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
bool permit_abbrev, Error **errp);
diff --git a/system/vl.c b/system/vl.c
index 9bd7664b85..239c537e45 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1441,10 +1441,9 @@ static void qemu_create_default_devices(void)
}
if (default_net) {
- QemuOptsList *net = qemu_find_opts("net");
- qemu_opts_parse(net, "nic", true, &error_abort);
+ qemu_opts_parse("net", "nic", true, &error_abort);
#ifdef CONFIG_SLIRP
- qemu_opts_parse(net, "user", true, &error_abort);
+ qemu_opts_parse("net", "user", true, &error_abort);
#endif
}
diff --git a/tests/unit/test-opts-visitor.c b/tests/unit/test-opts-visitor.c
index 23e897061c..a9e45c0251 100644
--- a/tests/unit/test-opts-visitor.c
+++ b/tests/unit/test-opts-visitor.c
@@ -39,8 +39,7 @@ setup_fixture(OptsVisitorFixture *f, gconstpointer test_data)
QemuOpts *opts;
Visitor *v;
- opts = qemu_opts_parse(qemu_find_opts("userdef"), opts_string, false,
- NULL);
+ opts = qemu_opts_parse("userdef", opts_string, false, NULL);
g_assert(opts != NULL);
v = opts_visitor_new(opts);
@@ -181,8 +180,7 @@ test_opts_range_unvisited(void)
QemuOpts *opts;
Visitor *v;
- opts = qemu_opts_parse(qemu_find_opts("userdef"), "ilist=0-2", false,
- &error_abort);
+ opts = qemu_opts_parse("userdef", "ilist=0-2", false, &error_abort);
v = opts_visitor_new(opts);
@@ -222,8 +220,7 @@ test_opts_range_beyond(void)
Visitor *v;
int64_t val;
- opts = qemu_opts_parse(qemu_find_opts("userdef"), "ilist=0", false,
- &error_abort);
+ opts = qemu_opts_parse("userdef", "ilist=0", false, &error_abort);
v = opts_visitor_new(opts);
@@ -257,8 +254,7 @@ test_opts_dict_unvisited(void)
Visitor *v;
UserDefOptions *userdef;
- opts = qemu_opts_parse(qemu_find_opts("userdef"), "i64x=0,bogus=1", false,
- &error_abort);
+ opts = qemu_opts_parse("userdef", "i64x=0,bogus=1", false, &error_abort);
v = opts_visitor_new(opts);
visit_type_UserDefOptions(v, NULL, &userdef, &err);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index aa879b78f8..2c23841376 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "qemu/config-file.h"
#include "qemu/error-report.h"
#include "qobject/qbool.h"
#include "qobject/qdict.h"
@@ -910,16 +911,18 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
}
/**
- * Create a QemuOpts in @list and with options parsed from @params.
- * If @permit_abbrev, the first key=value in @params may omit key=,
- * and is treated as if key was @list->implied_opt_name.
- * On error, store an error object through @errp if non-null.
- * Return the new QemuOpts on success, null pointer on error.
+ * Find the @group and create a QemuOpts with options parsed from
+ * @params. If @permit_abbrev, the first key=value in @params may
+ * omit key=. On error, store an error object through @errp if
+ * non-null. Return the new QemuOpts on success, null pointer on
+ * error.
*/
-QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
+QemuOpts *qemu_opts_parse(const char *group, const char *params,
bool permit_abbrev, Error **errp)
{
- return opts_parse(list, params, permit_abbrev, false, NULL, errp);
+ QemuOptsList *list = qemu_find_opts_err(group, errp);
+
+ return qemu_opts_parse_list(list, params, permit_abbrev, errp);
}
/**
@@ -932,7 +935,7 @@ QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
bool permit_abbrev, Error **errp)
{
- return qemu_opts_parse(list, params, permit_abbrev, errp);
+ return opts_parse(list, params, permit_abbrev, false, NULL, errp);
}
/**
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH 05/14] qemu-options: Change qemu_parse_opts() to take the group name
2026-09-18 22:29 ` [RFC PATCH 05/14] qemu-options: Change qemu_parse_opts() to take the group name Fabiano Rosas
@ 2026-09-25 10:11 ` Daniel P. Berrangé
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrangé @ 2026-09-25 10:11 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster, Paolo Bonzini, Michael Roth
On Fri, Sep 18, 2026 at 07:29:52PM -0300, Fabiano Rosas wrote:
> Get rid of the pattern qemu_parse_opts(qemu_find_opts("some-opt")) by
> passing the string directly to qemu_parse_opts().
>
> Don't check for NULL list at this time to preserve the old behavior. A
> subsequent patch will correct that.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> include/qemu/option.h | 2 +-
> system/vl.c | 5 ++---
> tests/unit/test-opts-visitor.c | 12 ++++--------
> util/qemu-option.c | 19 +++++++++++--------
> 4 files changed, 18 insertions(+), 20 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC PATCH 06/14] qemu-options: Call qemu_opts_print_help() one level down
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (4 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 05/14] qemu-options: Change qemu_parse_opts() to take the group name Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 10:15 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 07/14] qemu-options: Consolidate qemu_opts_parse* functions Fabiano Rosas
` (7 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
When calling opts_parse(), there is no need to pass in &help_wanted
only to call qemu_opts_print_help() in case it's true. The opts_parse
function itself can call qemu_opts_print_help() and avoid the
roundtrip.
qemu_opts_print_help() takes a QemuOptsList pointer. opts_parse() can
access the list via opts->list.
Note that this change doesn't avoid the need for opts_parse() to pass
a pointer into get_opt_name_value().
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
util/qemu-option.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 2c23841376..bb38d305c9 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -812,7 +812,7 @@ static const char *get_opt_name_value(const char *params,
static bool opts_do_parse(QemuOpts *opts, const char *params,
const char *firstname,
- bool warn_on_flag, bool *help_wanted, Error **errp)
+ bool warn_on_flag, bool help_wanted, Error **errp)
{
const char *p;
QemuOpt *opt;
@@ -820,9 +820,12 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
for (p = params; *p;) {
g_autofree char *option = NULL;
g_autofree char *value = NULL;
+ bool is_help = false;
- p = get_opt_name_value(p, firstname, warn_on_flag, help_wanted, &option, &value);
- if (help_wanted && *help_wanted) {
+ p = get_opt_name_value(p, firstname, warn_on_flag, &is_help, &option,
+ &value);
+ if (help_wanted && is_help) {
+ qemu_opts_print_help(opts->list, true);
return false;
}
firstname = NULL;
@@ -885,12 +888,12 @@ bool has_help_option(const char *params)
bool qemu_opts_do_parse(QemuOpts *opts, const char *params,
const char *firstname, Error **errp)
{
- return opts_do_parse(opts, params, firstname, false, NULL, errp);
+ return opts_do_parse(opts, params, firstname, false, false, errp);
}
static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
bool permit_abbrev,
- bool warn_on_flag, bool *help_wanted, Error **errp)
+ bool warn_on_flag, bool help_wanted, Error **errp)
{
const char *firstname;
char *id = opts_parse_id(params);
@@ -935,7 +938,7 @@ 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, NULL, errp);
+ return opts_parse(list, params, permit_abbrev, false, false, errp);
}
/**
@@ -951,15 +954,12 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
{
Error *err = NULL;
QemuOpts *opts;
- bool help_wanted = false;
+ bool help_wanted = !opts_accepts_any(list);
- opts = opts_parse(list, params, permit_abbrev, true,
- opts_accepts_any(list) ? NULL : &help_wanted,
- &err);
+ opts = opts_parse(list, params, permit_abbrev, true, help_wanted, &err);
if (!opts) {
- assert(!!err + !!help_wanted == 1);
if (help_wanted) {
- qemu_opts_print_help(list, true);
+ assert(!err);
} else {
error_report_err(err);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH 07/14] qemu-options: Consolidate qemu_opts_parse* functions
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (5 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 06/14] qemu-options: Call qemu_opts_print_help() one level down Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 10:21 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 08/14] qemu-options: Add qemu_opts_parse_list_noisily Fabiano Rosas
` (6 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Michael Roth
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.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
tests/unit/test-opts-visitor.c | 2 +-
util/qemu-option.c | 32 ++++++++++++++++++--------------
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/tests/unit/test-opts-visitor.c b/tests/unit/test-opts-visitor.c
index a9e45c0251..dd9a8faf12 100644
--- a/tests/unit/test-opts-visitor.c
+++ b/tests/unit/test-opts-visitor.c
@@ -39,7 +39,7 @@ setup_fixture(OptsVisitorFixture *f, gconstpointer test_data)
QemuOpts *opts;
Visitor *v;
- opts = qemu_opts_parse("userdef", opts_string, false, NULL);
+ opts = qemu_opts_parse("userdef", opts_string, false, &error_abort);
g_assert(opts != NULL);
v = opts_visitor_new(opts);
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
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH 07/14] qemu-options: Consolidate qemu_opts_parse* functions
2026-09-18 22:29 ` [RFC PATCH 07/14] qemu-options: Consolidate qemu_opts_parse* functions Fabiano Rosas
@ 2026-09-25 10:21 ` Daniel P. Berrangé
2026-09-25 12:21 ` Fabiano Rosas
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2026-09-25 10:21 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster, Michael Roth
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.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> 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
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH 07/14] qemu-options: Consolidate qemu_opts_parse* functions
2026-09-25 10:21 ` Daniel P. Berrangé
@ 2026-09-25 12:21 ` Fabiano Rosas
2026-09-28 10:41 ` Daniel P. Berrangé
0 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-25 12:21 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, Markus Armbruster, Michael Roth
Daniel P. Berrangé <berrange@redhat.com> 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'.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>> 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
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH 07/14] qemu-options: Consolidate qemu_opts_parse* functions
2026-09-25 12:21 ` Fabiano Rosas
@ 2026-09-28 10:41 ` Daniel P. Berrangé
2026-09-28 14:52 ` Fabiano Rosas
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2026-09-28 10:41 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster, Michael Roth
On Fri, Sep 25, 2026 at 09:21:20AM -0300, Fabiano Rosas wrote:
> Daniel P. Berrangé <berrange@redhat.com> 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 <farosas@suse.de>
> >> ---
> >> 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 :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC PATCH 07/14] qemu-options: Consolidate qemu_opts_parse* functions
2026-09-28 10:41 ` Daniel P. Berrangé
@ 2026-09-28 14:52 ` Fabiano Rosas
0 siblings, 0 replies; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-28 14:52 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, Markus Armbruster, Michael Roth
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Fri, Sep 25, 2026 at 09:21:20AM -0300, Fabiano Rosas wrote:
>> Daniel P. Berrangé <berrange@redhat.com> 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 <farosas@suse.de>
>> >> ---
>> >> 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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC PATCH 08/14] qemu-options: Add qemu_opts_parse_list_noisily
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (6 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 07/14] qemu-options: Consolidate qemu_opts_parse* functions Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 10:25 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 09/14] qemu-options: Change qemu_opts_parse_noisily() to take the group Fabiano Rosas
` (5 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Kevin Wolf, Hanna Reitz,
Dr. David Alan Gilbert, Jason Wang, Eric Blake,
Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
Daniel P. Berrangé, Stefan Berger, Lukas Straub,
Marc-André Lureau
Add a wrapper similar to qemu_opts_parse_list() with the same
objective, to free the non _list version,
i.e. qemu_opts_parse_noisily() to be converted to receive the group
name.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
block/monitor/block-hmp-cmds.c | 2 +-
include/qemu/option.h | 2 ++
monitor/hmp.c | 2 +-
net/net.c | 2 +-
qemu-img.c | 10 +++++-----
qemu-io-cmds.c | 2 +-
qemu-io.c | 7 ++++---
qemu-nbd.c | 6 +++---
storage-daemon/qemu-storage-daemon.c | 4 ++--
system/qdev-monitor.c | 2 +-
system/tpm.c | 2 +-
system/vl.c | 6 +++---
tests/unit/test-replication.c | 6 +++---
tools/qemu-vnc/qemu-vnc.c | 2 +-
util/qemu-option.c | 14 ++++++++++++++
15 files changed, 43 insertions(+), 26 deletions(-)
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 7bae4d425c..cbd64166ec 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -63,7 +63,7 @@ static void hmp_drive_add_node(MonitorHMP *hmp, const char *optstr)
QDict *qdict;
Error *err = NULL;
- opts = qemu_opts_parse_noisily(&qemu_drive_opts, optstr, false);
+ opts = qemu_opts_parse_list_noisily(&qemu_drive_opts, optstr, false);
if (!opts) {
return;
}
diff --git a/include/qemu/option.h b/include/qemu/option.h
index dfe0d9b893..df68da9aa8 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -134,6 +134,8 @@ QemuOpts *qemu_opts_parse(const char *group, const char *params,
bool permit_abbrev, Error **errp);
QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
bool permit_abbrev, Error **errp);
+QemuOpts *qemu_opts_parse_list_noisily(QemuOptsList *list, const char *params,
+ bool permit_abbrev);
QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
Error **errp);
QDict *qemu_opts_to_qdict_filtered(QemuOpts *opts, QDict *qdict,
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 488ec23937..fc32cfd1ff 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -897,7 +897,7 @@ static QDict *monitor_parse_arguments(MonitorHMP *mon,
if (get_str(buf, sizeof(buf), &p) < 0) {
goto fail;
}
- opts = qemu_opts_parse_noisily(opts_list, buf, true);
+ opts = qemu_opts_parse_list_noisily(opts_list, buf, true);
if (!opts) {
goto fail;
}
diff --git a/net/net.c b/net/net.c
index d7fa637ce5..37401ab2b4 100644
--- a/net/net.c
+++ b/net/net.c
@@ -2029,7 +2029,7 @@ void netdev_parse_modern(const char *optstr)
void net_client_parse(QemuOptsList *opts_list, const char *optstr)
{
- if (!qemu_opts_parse_noisily(opts_list, optstr, true)) {
+ if (!qemu_opts_parse_list_noisily(opts_list, optstr, true)) {
exit(1);
}
}
diff --git a/qemu-img.c b/qemu-img.c
index 2f63d31141..4996618a4e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -161,7 +161,7 @@ static OutputFormat parse_output_format(const char *argv0, const char *arg)
* an odd number of ',' (or else a separating ',' following it gets
* escaped), or be empty (or else a separating ',' preceding it can
* escape a separating ',' following it).
- *
+ *
*/
static bool is_valid_option_list(const char *list)
{
@@ -2383,8 +2383,8 @@ static int img_convert(const img_cmd_t *ccmd, int argc, char **argv)
break;
case 'l':
if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
- sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
- optarg, false);
+ sn_opts = qemu_opts_parse_list_noisily(&internal_snapshot_opts,
+ optarg, false);
if (!sn_opts) {
error_report("Failed in parsing snapshot param '%s'",
optarg);
@@ -5768,8 +5768,8 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
break;
case 'l':
if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
- sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
- optarg, false);
+ sn_opts = qemu_opts_parse_list_noisily(&internal_snapshot_opts,
+ optarg, false);
if (!sn_opts) {
error_report("Failed in parsing snapshot param '%s'",
optarg);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index aa795fd87a..6c8c4c9540 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2540,7 +2540,7 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv, Error **errp)
has_cache_option = true;
break;
case 'o':
- if (!qemu_opts_parse_noisily(&reopen_opts, optarg, 0)) {
+ if (!qemu_opts_parse_list_noisily(&reopen_opts, optarg, 0)) {
qemu_opts_reset(&reopen_opts);
return -EINVAL;
}
diff --git a/qemu-io.c b/qemu-io.c
index 598d5b1c9c..db0ee499e4 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -220,7 +220,7 @@ static int open_f(BlockBackend *blk, int argc, char **argv, Error **errp)
qemu_opts_reset(&empty_opts);
return -EINVAL;
}
- if (!qemu_opts_parse_noisily(&empty_opts, optarg, false)) {
+ if (!qemu_opts_parse_list_noisily(&empty_opts, optarg, false)) {
qemu_opts_reset(&empty_opts);
return -EINVAL;
}
@@ -240,7 +240,7 @@ static int open_f(BlockBackend *blk, int argc, char **argv, Error **errp)
}
if (imageOpts && (optind == argc - 1)) {
- if (!qemu_opts_parse_noisily(&empty_opts, argv[optind], false)) {
+ if (!qemu_opts_parse_list_noisily(&empty_opts, argv[optind], false)) {
qemu_opts_reset(&empty_opts);
return -EINVAL;
}
@@ -659,7 +659,8 @@ int main(int argc, char **argv)
if ((argc - optind) == 1) {
if (imageOpts) {
QemuOpts *qopts = NULL;
- qopts = qemu_opts_parse_noisily(&file_opts, argv[optind], false);
+ qopts = qemu_opts_parse_list_noisily(&file_opts, argv[optind],
+ false);
if (!qopts) {
exit(1);
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index ed5895861b..749c915b58 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -706,8 +706,8 @@ int main(int argc, char **argv)
break;
case 'l':
if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
- sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
- optarg, false);
+ sn_opts = qemu_opts_parse_list_noisily(&internal_snapshot_opts,
+ optarg, false);
if (!sn_opts) {
error_report("Failed in parsing snapshot param `%s'",
optarg);
@@ -1117,7 +1117,7 @@ int main(int argc, char **argv)
error_report("--image-opts and -f are mutually exclusive");
exit(EXIT_FAILURE);
}
- o = qemu_opts_parse_noisily(&file_opts, opts.srcpath, true);
+ o = qemu_opts_parse_list_noisily(&file_opts, opts.srcpath, true);
if (!o) {
qemu_opts_reset(&file_opts);
exit(EXIT_FAILURE);
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 50dbfbd97a..5d63ef2efc 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -283,8 +283,8 @@ static void process_options(int argc, char *argv[], bool pre_init_pass)
case OPTION_CHARDEV:
{
/* TODO This interface is not stable until we QAPIfy it */
- QemuOpts *opts = qemu_opts_parse_noisily(&qemu_chardev_opts,
- optarg, true);
+ QemuOpts *opts = qemu_opts_parse_list_noisily(
+ &qemu_chardev_opts, optarg, true);
if (opts == NULL) {
exit(EXIT_FAILURE);
}
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index a62ad23ecf..8ba85a59d7 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -1197,7 +1197,7 @@ int qemu_global_option(const char *str)
return 0;
}
- opts = qemu_opts_parse_noisily(&qemu_global_opts, str, false);
+ opts = qemu_opts_parse_list_noisily(&qemu_global_opts, str, false);
if (!opts) {
return -1;
}
diff --git a/system/tpm.c b/system/tpm.c
index 903b29c043..f0f5a64d9a 100644
--- a/system/tpm.c
+++ b/system/tpm.c
@@ -184,7 +184,7 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optstr)
tpm_display_backend_drivers();
exit(EXIT_SUCCESS);
}
- opts = qemu_opts_parse_noisily(opts_list, optstr, true);
+ opts = qemu_opts_parse_list_noisily(opts_list, optstr, true);
if (!opts) {
return -1;
}
diff --git a/system/vl.c b/system/vl.c
index 239c537e45..adda7b0f70 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -3262,7 +3262,7 @@ void qemu_init(int argc, char **argv)
error_report("fsdev support is disabled");
exit(1);
}
- if (!qemu_opts_parse_noisily(olist, optarg, true)) {
+ if (!qemu_opts_parse_list_noisily(olist, optarg, true)) {
exit(1);
}
break;
@@ -3277,7 +3277,7 @@ void qemu_init(int argc, char **argv)
error_report("virtfs support is disabled");
exit(1);
}
- opts = qemu_opts_parse_noisily(olist, optarg, true);
+ opts = qemu_opts_parse_list_noisily(olist, optarg, true);
if (!opts) {
exit(1);
}
@@ -3626,7 +3626,7 @@ void qemu_init(int argc, char **argv)
exit(1);
}
- opts = qemu_opts_parse_noisily(olist, optarg, true);
+ opts = qemu_opts_parse_list_noisily(olist, optarg, true);
if (!opts) {
exit(1);
}
diff --git a/tests/unit/test-replication.c b/tests/unit/test-replication.c
index 3aa98e6f56..101f72acdc 100644
--- a/tests/unit/test-replication.c
+++ b/tests/unit/test-replication.c
@@ -179,7 +179,7 @@ static BlockBackend *start_primary(void)
"file.driver=qcow2,file.file.filename=%s,"
"file.file.locking=off"
, p_local_disk);
- opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
+ opts = qemu_opts_parse_list_noisily(&qemu_drive_opts, cmdline, false);
g_free(cmdline);
qdict = qemu_opts_to_qdict(opts, NULL);
@@ -295,7 +295,7 @@ static BlockBackend *start_secondary(void)
cmdline = g_strdup_printf("file.filename=%s,driver=qcow2,"
"file.locking=off",
s_local_disk);
- opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
+ opts = qemu_opts_parse_list_noisily(&qemu_drive_opts, cmdline, false);
g_free(cmdline);
qdict = qemu_opts_to_qdict(opts, NULL);
@@ -321,7 +321,7 @@ static BlockBackend *start_secondary(void)
"file.backing.backing=%s"
, S_ID, s_active_disk, s_hidden_disk
, S_LOCAL_DISK_ID);
- opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
+ opts = qemu_opts_parse_list_noisily(&qemu_drive_opts, cmdline, false);
g_free(cmdline);
qdict = qemu_opts_to_qdict(opts, NULL);
diff --git a/tools/qemu-vnc/qemu-vnc.c b/tools/qemu-vnc/qemu-vnc.c
index 5c2ba3b7a5..10ac76298a 100644
--- a/tools/qemu-vnc/qemu-vnc.c
+++ b/tools/qemu-vnc/qemu-vnc.c
@@ -377,7 +377,7 @@ setup_vnc_opts(const char *vnc_addr, const char *tls_creds_dir,
g_string_append(opts_str, ",non-adaptive=on");
}
- opts = qemu_opts_parse_noisily(olist, opts_str->str, true);
+ opts = qemu_opts_parse_list_noisily(olist, opts_str->str, true);
if (!opts) {
return false;
}
diff --git a/util/qemu-option.c b/util/qemu-option.c
index c8e552823b..c762eb20a3 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -971,6 +971,20 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
return qemu_opts_parse_list(list, params, permit_abbrev, NULL);
}
+/**
+ * Create a QemuOpts in @list and with options parsed from @params.
+ * If @permit_abbrev, the first key=value in @params may omit key=,
+ * and is treated as if key was @list->implied_opt_name.
+ * Report errors with error_report_err(). This is inappropriate in
+ * QMP context. Do not use this function there!
+ * Return the new QemuOpts on success, null pointer on error.
+ */
+QemuOpts *qemu_opts_parse_list_noisily(QemuOptsList *list, const char *params,
+ bool permit_abbrev)
+{
+ return qemu_opts_parse_list(list, params, permit_abbrev, NULL);
+}
+
static bool qemu_opts_from_qdict_entry(QemuOpts *opts,
const QDictEntry *entry,
Error **errp)
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH 08/14] qemu-options: Add qemu_opts_parse_list_noisily
2026-09-18 22:29 ` [RFC PATCH 08/14] qemu-options: Add qemu_opts_parse_list_noisily Fabiano Rosas
@ 2026-09-25 10:25 ` Daniel P. Berrangé
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrangé @ 2026-09-25 10:25 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Markus Armbruster, Kevin Wolf, Hanna Reitz,
Dr. David Alan Gilbert, Jason Wang, Eric Blake,
Vladimir Sementsov-Ogievskiy, Paolo Bonzini, Stefan Berger,
Lukas Straub, Marc-André Lureau
On Fri, Sep 18, 2026 at 07:29:55PM -0300, Fabiano Rosas wrote:
> Add a wrapper similar to qemu_opts_parse_list() with the same
> objective, to free the non _list version,
> i.e. qemu_opts_parse_noisily() to be converted to receive the group
> name.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> block/monitor/block-hmp-cmds.c | 2 +-
> include/qemu/option.h | 2 ++
> monitor/hmp.c | 2 +-
> net/net.c | 2 +-
> qemu-img.c | 10 +++++-----
> qemu-io-cmds.c | 2 +-
> qemu-io.c | 7 ++++---
> qemu-nbd.c | 6 +++---
> storage-daemon/qemu-storage-daemon.c | 4 ++--
> system/qdev-monitor.c | 2 +-
> system/tpm.c | 2 +-
> system/vl.c | 6 +++---
> tests/unit/test-replication.c | 6 +++---
> tools/qemu-vnc/qemu-vnc.c | 2 +-
> util/qemu-option.c | 14 ++++++++++++++
> 15 files changed, 43 insertions(+), 26 deletions(-)
>
> diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> index 7bae4d425c..cbd64166ec 100644
> --- a/block/monitor/block-hmp-cmds.c
> +++ b/block/monitor/block-hmp-cmds.c
> @@ -63,7 +63,7 @@ static void hmp_drive_add_node(MonitorHMP *hmp, const char *optstr)
> QDict *qdict;
> Error *err = NULL;
>
> - opts = qemu_opts_parse_noisily(&qemu_drive_opts, optstr, false);
> + opts = qemu_opts_parse_list_noisily(&qemu_drive_opts, optstr, false);
> if (!opts) {
> return;
> }
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index dfe0d9b893..df68da9aa8 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -134,6 +134,8 @@ QemuOpts *qemu_opts_parse(const char *group, const char *params,
> bool permit_abbrev, Error **errp);
> QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
> bool permit_abbrev, Error **errp);
> +QemuOpts *qemu_opts_parse_list_noisily(QemuOptsList *list, const char *params,
> + bool permit_abbrev);
> QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
> Error **errp);
> QDict *qemu_opts_to_qdict_filtered(QemuOpts *opts, QDict *qdict,
This is adding a new function and not removing an existnig function, but....
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 488ec23937..fc32cfd1ff 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -897,7 +897,7 @@ static QDict *monitor_parse_arguments(MonitorHMP *mon,
> if (get_str(buf, sizeof(buf), &p) < 0) {
> goto fail;
> }
> - opts = qemu_opts_parse_noisily(opts_list, buf, true);
> + opts = qemu_opts_parse_list_noisily(opts_list, buf, true);
> if (!opts) {
> goto fail;
> }
...then the patch changes alot of callers to the new
function, which wasn't mentioned in the commit message.
Did you accidentally squash two patches together ? I'd
suggest the conversion of callers should be separate.
> diff --git a/qemu-img.c b/qemu-img.c
> index 2f63d31141..4996618a4e 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -161,7 +161,7 @@ static OutputFormat parse_output_format(const char *argv0, const char *arg)
> * an odd number of ',' (or else a separating ',' following it gets
> * escaped), or be empty (or else a separating ',' preceding it can
> * escape a separating ',' following it).
> - *
> + *
> */
> static bool is_valid_option_list(const char *list)
> {
Extraneous change
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index c8e552823b..c762eb20a3 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -971,6 +971,20 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
> return qemu_opts_parse_list(list, params, permit_abbrev, NULL);
> }
>
> +/**
> + * Create a QemuOpts in @list and with options parsed from @params.
> + * If @permit_abbrev, the first key=value in @params may omit key=,
> + * and is treated as if key was @list->implied_opt_name.
> + * Report errors with error_report_err(). This is inappropriate in
> + * QMP context. Do not use this function there!
> + * Return the new QemuOpts on success, null pointer on error.
> + */
> +QemuOpts *qemu_opts_parse_list_noisily(QemuOptsList *list, const char *params,
> + bool permit_abbrev)
> +{
> + return qemu_opts_parse_list(list, params, permit_abbrev, NULL);
> +}
I guess this is why you moved the error reporting in the previous
patch. Personally I'd skip the previous patch and accept the
small duplicate code pattern between the two "noisily" methods.
> +
> static bool qemu_opts_from_qdict_entry(QemuOpts *opts,
> const QDictEntry *entry,
> Error **errp)
> --
> 2.53.0
>
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 :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC PATCH 09/14] qemu-options: Change qemu_opts_parse_noisily() to take the group
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (7 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 08/14] qemu-options: Add qemu_opts_parse_list_noisily Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-21 13:51 ` Stefan Hajnoczi
2026-09-25 10:27 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 10/14] qemu-options: Check for NULL list at qemu_opts_parse() Fabiano Rosas
` (4 subsequent siblings)
13 siblings, 2 replies; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Kevin Wolf, Hanna Reitz,
Marc-André Lureau, Paolo Bonzini, Alex Bennée,
Pierrick Bouvier, Alexandre Iooss, Daniel P. Berrangé,
Stefan Hajnoczi
Pass the group name as argument to qemu_opts_parse_noisily() to make
it simmetric with qemu_opts_parse().
This change solves the issue of qemu_opts_parse_noisily() crashing on
a NULL return from qemu_find_opts(), which can happen when an option
is invoked for code in a module that is not loaded.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
block/monitor/block-hmp-cmds.c | 2 +-
blockdev.c | 2 +-
chardev/char-hmp-cmds.c | 5 +--
include/qemu/option.h | 2 +-
plugins/loader.c | 2 +-
qemu-img.c | 3 +-
semihosting/config.c | 4 +-
system/vl.c | 75 ++++++++++++----------------------
tests/unit/test-char.c | 12 ++----
tests/unit/test-seccomp.c | 6 +--
trace/control.c | 3 +-
ui/vnc.c | 3 +-
util/qemu-option.c | 14 ++++++-
13 files changed, 54 insertions(+), 79 deletions(-)
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index cbd64166ec..d94a07314f 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -101,7 +101,7 @@ void hmp_drive_add(MonitorHMP *hmp, const QDict *qdict)
return;
}
- opts = qemu_opts_parse_noisily(qemu_find_opts("drive"), optstr, false);
+ opts = qemu_opts_parse_noisily("drive", optstr, false);
if (!opts)
return;
diff --git a/blockdev.c b/blockdev.c
index 6e86c6262f..21554500dc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -203,7 +203,7 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
GLOBAL_STATE_CODE();
- opts = qemu_opts_parse_noisily(qemu_find_opts("drive"), optstr, false);
+ opts = qemu_opts_parse_noisily("drive", optstr, false);
if (!opts) {
return NULL;
}
diff --git a/chardev/char-hmp-cmds.c b/chardev/char-hmp-cmds.c
index fb0560054b..938dbd3ba2 100644
--- a/chardev/char-hmp-cmds.c
+++ b/chardev/char-hmp-cmds.c
@@ -83,7 +83,7 @@ void hmp_chardev_add(MonitorHMP *hmp, const QDict *qdict)
Error *err = NULL;
QemuOpts *opts;
- opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"), args, true);
+ opts = qemu_opts_parse_noisily("chardev", args, true);
if (opts == NULL) {
error_setg(&err, "Parsing chardev args failed");
} else {
@@ -100,8 +100,7 @@ void hmp_chardev_change(MonitorHMP *hmp, const QDict *qdict)
Error *err = NULL;
ChardevBackend *backend = NULL;
ChardevReturn *ret = NULL;
- QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"), args,
- true);
+ QemuOpts *opts = qemu_opts_parse_noisily("chardev", args, true);
if (!opts) {
error_setg(&err, "Parsing chardev args failed");
goto end;
diff --git a/include/qemu/option.h b/include/qemu/option.h
index df68da9aa8..2955ed3b2d 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -128,7 +128,7 @@ void qemu_opts_del(QemuOpts *opts);
bool qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp);
bool qemu_opts_do_parse(QemuOpts *opts, const char *params,
const char *firstname, Error **errp);
-QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
+QemuOpts *qemu_opts_parse_noisily(const char *group, const char *params,
bool permit_abbrev);
QemuOpts *qemu_opts_parse(const char *group, const char *params,
bool permit_abbrev, Error **errp);
diff --git a/plugins/loader.c b/plugins/loader.c
index b10ebe8cc0..1808679c82 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -147,7 +147,7 @@ void qemu_plugin_opt_parse(const char *optstr, QemuPluginList *head)
struct qemu_plugin_parse_arg arg;
QemuOpts *opts;
- opts = qemu_opts_parse_noisily(qemu_find_opts("plugin"), optstr, true);
+ opts = qemu_opts_parse_noisily("plugin", optstr, true);
if (opts == NULL) {
exit(1);
}
diff --git a/qemu-img.c b/qemu-img.c
index 4996618a4e..0459faac5f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -358,8 +358,7 @@ static BlockBackend *img_open(bool image_opts,
error_report("--image-opts and --format are mutually exclusive");
return NULL;
}
- opts = qemu_opts_parse_noisily(qemu_find_opts("source"),
- filename, true);
+ opts = qemu_opts_parse_noisily("source", filename, true);
if (!opts) {
return NULL;
}
diff --git a/semihosting/config.c b/semihosting/config.c
index 56283b5c3c..c8c865ebae 100644
--- a/semihosting/config.c
+++ b/semihosting/config.c
@@ -134,8 +134,8 @@ void qemu_semihosting_enable(void)
int qemu_semihosting_config_options(const char *optstr)
{
- QemuOptsList *opt_list = qemu_find_opts("semihosting-config");
- QemuOpts *opts = qemu_opts_parse_noisily(opt_list, optstr, false);
+ QemuOpts *opts = qemu_opts_parse_noisily("semihosting-config", optstr,
+ false);
semihosting.enabled = true;
diff --git a/system/vl.c b/system/vl.c
index adda7b0f70..4a6c3e5582 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1870,8 +1870,7 @@ static void object_option_parse(const char *str)
v = qobject_input_visitor_new(obj);
qobject_unref(obj);
} else {
- opts = qemu_opts_parse_noisily(qemu_find_opts("object"),
- str, true);
+ opts = qemu_opts_parse_noisily("object", str, true);
if (!opts) {
exit(1);
}
@@ -1897,8 +1896,7 @@ static void overcommit_parse(const char *str)
QemuOpts *opts;
const char *mem_lock_opt;
- opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
- str, false);
+ opts = qemu_opts_parse_noisily("overcommit", str, false);
if (!opts) {
exit(1);
}
@@ -2475,7 +2473,7 @@ static void configure_accelerators(const char *progname)
* such as "-machine accel=tcg,,thread=single".
*/
if (accel_find(*tmp)) {
- qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true);
+ qemu_opts_parse_noisily("accel", *tmp, true);
} else {
init_failed = true;
error_report("invalid accelerator %s", *tmp);
@@ -2975,8 +2973,7 @@ void qemu_init(int argc, char **argv)
break;
}
case QEMU_OPTION_drive:
- if (!qemu_opts_parse_noisily(qemu_find_opts("drive"),
- optarg, false)) {
+ if (!qemu_opts_parse_noisily("drive", optarg, false)) {
exit(1);
}
break;
@@ -3001,8 +2998,7 @@ void qemu_init(int argc, char **argv)
replay_add_blocker("-snapshot");
break;
case QEMU_OPTION_numa:
- if (!qemu_opts_parse_noisily(qemu_find_opts("numa"),
- optarg, true)) {
+ if (!qemu_opts_parse_noisily("numa", optarg, true)) {
exit(1);
}
break;
@@ -3061,8 +3057,7 @@ void qemu_init(int argc, char **argv)
break;
#ifdef CONFIG_LIBISCSI
case QEMU_OPTION_iscsi:
- if (!qemu_opts_parse_noisily(qemu_find_opts("iscsi"),
- optarg, false)) {
+ if (!qemu_opts_parse_noisily("iscsi", optarg, false)) {
exit(1);
}
break;
@@ -3115,8 +3110,7 @@ void qemu_init(int argc, char **argv)
exit(0);
break;
case QEMU_OPTION_m:
- if (!qemu_opts_parse_noisily(qemu_find_opts("memory"),
- optarg, true)) {
+ if (!qemu_opts_parse_noisily("memory", optarg, true)) {
exit(1);
}
break;
@@ -3244,15 +3238,13 @@ void qemu_init(int argc, char **argv)
"See '-object' docs in the QEMU manual for further "
"configuration guidance: "
"https://www.qemu.org/docs/master/system/invocation.html");
- if (!qemu_opts_parse_noisily(qemu_find_opts("mon"), optarg,
- true)) {
+ if (!qemu_opts_parse_noisily("mon", optarg, true)) {
exit(1);
}
default_monitor = 0;
break;
case QEMU_OPTION_chardev:
- if (!qemu_opts_parse_noisily(qemu_find_opts("chardev"),
- optarg, true)) {
+ if (!qemu_opts_parse_noisily("chardev", optarg, true)) {
exit(1);
}
break;
@@ -3356,9 +3348,8 @@ void qemu_init(int argc, char **argv)
}
break;
case QEMU_OPTION_action:
- olist = qemu_find_opts("action");
- if (!qemu_opts_parse_noisily(olist, optarg, false)) {
- exit(1);
+ if (!qemu_opts_parse_noisily("action", optarg, false)) {
+ exit(1);
}
break;
case QEMU_OPTION_watchdog_action: {
@@ -3390,24 +3381,21 @@ void qemu_init(int argc, char **argv)
object_register_sugar_prop("ide-device", "win2k-install-hack", "true", true);
break;
case QEMU_OPTION_acpitable:
- opts = qemu_opts_parse_noisily(qemu_find_opts("acpi"),
- optarg, true);
+ opts = qemu_opts_parse_noisily("acpi", optarg, true);
if (!opts) {
exit(1);
}
acpi_table_add(opts, &error_fatal);
break;
case QEMU_OPTION_smbios:
- opts = qemu_opts_parse_noisily(qemu_find_opts("smbios"),
- optarg, false);
+ opts = qemu_opts_parse_noisily("smbios", optarg, false);
if (!opts) {
exit(1);
}
smbios_entry_add(opts, &error_fatal);
break;
case QEMU_OPTION_fwcfg:
- if (!qemu_opts_parse_noisily(qemu_find_opts("fw_cfg"),
- optarg, true)) {
+ if (!qemu_opts_parse_noisily("fw_cfg", optarg, true)) {
exit(1);
}
break;
@@ -3430,8 +3418,7 @@ void qemu_init(int argc, char **argv)
break;
}
case QEMU_OPTION_accel:
- accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"),
- optarg, true);
+ accel_opts = qemu_opts_parse_noisily("accel", optarg, true);
optarg = qemu_opt_get(accel_opts, "accel");
if (!optarg || is_help_option(optarg)) {
printf("Accelerators supported in QEMU binary:\n");
@@ -3470,8 +3457,7 @@ void qemu_init(int argc, char **argv)
assert(opt->opts != NULL);
QTAILQ_INSERT_TAIL(&device_opts, opt, next);
} else {
- if (!qemu_opts_parse_noisily(qemu_find_opts("device"),
- optarg, true)) {
+ if (!qemu_opts_parse_noisily("device", optarg, true)) {
exit(1);
}
}
@@ -3487,12 +3473,10 @@ void qemu_init(int argc, char **argv)
break;
#endif
case QEMU_OPTION_no_reboot:
- olist = qemu_find_opts("action");
- qemu_opts_parse_noisily(olist, "reboot=shutdown", false);
+ qemu_opts_parse_noisily("action", "reboot=shutdown", false);
break;
case QEMU_OPTION_no_shutdown:
- olist = qemu_find_opts("action");
- qemu_opts_parse_noisily(olist, "shutdown=pause", false);
+ qemu_opts_parse_noisily("action", "shutdown=pause", false);
break;
case QEMU_OPTION_uuid:
if (qemu_uuid_parse(optarg, &qemu_uuid) < 0) {
@@ -3506,8 +3490,7 @@ void qemu_init(int argc, char **argv)
error_report("too many option ROMs");
exit(1);
}
- opts = qemu_opts_parse_noisily(qemu_find_opts("option-rom"),
- optarg, true);
+ opts = qemu_opts_parse_noisily("option-rom", optarg, true);
if (!opts) {
exit(1);
}
@@ -3529,8 +3512,7 @@ void qemu_init(int argc, char **argv)
}
break;
case QEMU_OPTION_name:
- opts = qemu_opts_parse_noisily(qemu_find_opts("name"),
- optarg, true);
+ opts = qemu_opts_parse_noisily("name", optarg, true);
if (!opts) {
exit(1);
}
@@ -3546,15 +3528,13 @@ void qemu_init(int argc, char **argv)
nb_prom_envs++;
break;
case QEMU_OPTION_rtc:
- opts = qemu_opts_parse_noisily(qemu_find_opts("rtc"), optarg,
- false);
+ opts = qemu_opts_parse_noisily("rtc", optarg, false);
if (!opts) {
exit(1);
}
break;
case QEMU_OPTION_icount:
- icount_opts = qemu_opts_parse_noisily(qemu_find_opts("icount"),
- optarg, true);
+ icount_opts = qemu_opts_parse_noisily("icount", optarg, true);
if (!icount_opts) {
exit(1);
}
@@ -3603,7 +3583,7 @@ void qemu_init(int argc, char **argv)
break;
#ifdef CONFIG_SPICE
case QEMU_OPTION_spice:
- opts = qemu_opts_parse_noisily(qemu_find_opts("spice"), optarg, false);
+ opts = qemu_opts_parse_noisily("spice", optarg, false);
if (!opts) {
exit(1);
}
@@ -3633,8 +3613,7 @@ void qemu_init(int argc, char **argv)
break;
case QEMU_OPTION_add_fd:
#ifndef _WIN32
- opts = qemu_opts_parse_noisily(qemu_find_opts("add-fd"),
- optarg, false);
+ opts = qemu_opts_parse_noisily("add-fd", optarg, false);
if (!opts) {
exit(1);
}
@@ -3666,8 +3645,7 @@ void qemu_init(int argc, char **argv)
break;
}
case QEMU_OPTION_msg:
- opts = qemu_opts_parse_noisily(qemu_find_opts("msg"), optarg,
- false);
+ opts = qemu_opts_parse_noisily("msg", optarg, false);
if (!opts) {
exit(1);
}
@@ -3697,8 +3675,7 @@ void qemu_init(int argc, char **argv)
break;
case QEMU_OPTION_run_with: {
const char *str;
- opts = qemu_opts_parse_noisily(qemu_find_opts("run-with"),
- optarg, false);
+ opts = qemu_opts_parse_noisily("run-with", optarg, false);
if (!opts) {
exit(1);
}
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index b88b557133..bbcdf9a9ce 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -1210,8 +1210,7 @@ static void char_socket_server_test(gconstpointer opaque)
config->fd_pass,
NULL,
true);
- opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"),
- optstr, true);
+ opts = qemu_opts_parse_noisily("chardev", optstr, true);
g_assert_nonnull(opts);
chr = qemu_chr_new_from_opts(opts, NULL, &error_abort);
qemu_opts_del(opts);
@@ -1352,8 +1351,7 @@ static void char_socket_client_dupid_test(gconstpointer opaque)
config->reconnect,
false);
- opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"),
- optstr, true);
+ opts = qemu_opts_parse_noisily("chardev", optstr, true);
g_assert_nonnull(opts);
chr1 = qemu_chr_new_from_opts(opts, NULL, &error_abort);
g_assert_nonnull(chr1);
@@ -1412,8 +1410,7 @@ static void char_socket_client_test(gconstpointer opaque)
config->reconnect,
false);
- opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"),
- optstr, true);
+ opts = qemu_opts_parse_noisily("chardev", optstr, true);
g_assert_nonnull(opts);
chr = qemu_chr_new_from_opts(opts, NULL, &error_abort);
qemu_opts_del(opts);
@@ -1544,8 +1541,7 @@ static void char_socket_server_two_clients_test(gconstpointer opaque)
false,
NULL,
true);
- opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"),
- optstr, true);
+ opts = qemu_opts_parse_noisily("chardev", optstr, true);
g_assert_nonnull(opts);
chr = qemu_chr_new_from_opts(opts, NULL, &error_abort);
qemu_opts_del(opts);
diff --git a/tests/unit/test-seccomp.c b/tests/unit/test-seccomp.c
index 71d4083439..4438012989 100644
--- a/tests/unit/test-seccomp.c
+++ b/tests/unit/test-seccomp.c
@@ -31,15 +31,11 @@ static void test_seccomp_helper(const char *args, bool killed,
int errnum, int (*doit)(void))
{
if (g_test_subprocess()) {
- QemuOptsList *olist;
QemuOpts *opts;
int ret;
module_call_init(MODULE_INIT_OPTS);
- olist = qemu_find_opts("sandbox");
- g_assert(olist != NULL);
-
- opts = qemu_opts_parse_noisily(olist, args, true);
+ opts = qemu_opts_parse_noisily("sandbox", args, true);
g_assert(opts != NULL);
parse_sandbox(NULL, opts, &error_abort);
diff --git a/trace/control.c b/trace/control.c
index 49f0a4c5cd..51cecf5827 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -288,8 +288,7 @@ bool trace_init_backends(void)
void trace_opt_parse(const char *optstr)
{
- QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("trace"),
- optstr, true);
+ QemuOpts *opts = qemu_opts_parse_noisily("trace", optstr, true);
if (!opts) {
exit(1);
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 656768f9c9..7ddf660856 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4324,8 +4324,7 @@ static char *vnc_auto_assign_id(QemuOpts *opts)
void vnc_parse(const char *str)
{
- QemuOptsList *olist = qemu_find_opts("vnc");
- QemuOpts *opts = qemu_opts_parse_noisily(olist, str, !is_help_option(str));
+ QemuOpts *opts = qemu_opts_parse_noisily("vnc", str, !is_help_option(str));
if (!opts) {
exit(1);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index c762eb20a3..f8494c8842 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -941,6 +941,8 @@ QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
bool noisily = !errp;
QemuOpts *opts = NULL;
+ assert(list);
+
if (noisily) {
Error *err = NULL;
bool help_wanted = !opts_accepts_any(list);
@@ -958,16 +960,24 @@ QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
}
/**
- * Create a QemuOpts in @list and with options parsed from @params.
+ * Find the @group and create a QemuOpts with options parsed from @params.
* If @permit_abbrev, the first key=value in @params may omit key=,
* and is treated as if key was @list->implied_opt_name.
* Report errors with error_report_err(). This is inappropriate in
* QMP context. Do not use this function there!
* Return the new QemuOpts on success, null pointer on error.
*/
-QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
+QemuOpts *qemu_opts_parse_noisily(const char *group, const char *params,
bool permit_abbrev)
{
+ Error *err = NULL;
+ QemuOptsList *list = qemu_find_opts_err(group, &err);
+
+ if (!list) {
+ error_report_err(err);
+ return NULL;
+ }
+
return qemu_opts_parse_list(list, params, permit_abbrev, NULL);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH 09/14] qemu-options: Change qemu_opts_parse_noisily() to take the group
2026-09-18 22:29 ` [RFC PATCH 09/14] qemu-options: Change qemu_opts_parse_noisily() to take the group Fabiano Rosas
@ 2026-09-21 13:51 ` Stefan Hajnoczi
2026-09-25 10:27 ` Daniel P. Berrangé
1 sibling, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2026-09-21 13:51 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Markus Armbruster, Kevin Wolf, Hanna Reitz,
Marc-André Lureau, Paolo Bonzini, Alex Bennée,
Pierrick Bouvier, Alexandre Iooss, Daniel P. Berrangé
[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]
On Fri, Sep 18, 2026 at 07:29:56PM -0300, Fabiano Rosas wrote:
> Pass the group name as argument to qemu_opts_parse_noisily() to make
> it simmetric with qemu_opts_parse().
>
> This change solves the issue of qemu_opts_parse_noisily() crashing on
> a NULL return from qemu_find_opts(), which can happen when an option
> is invoked for code in a module that is not loaded.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> block/monitor/block-hmp-cmds.c | 2 +-
> blockdev.c | 2 +-
> chardev/char-hmp-cmds.c | 5 +--
> include/qemu/option.h | 2 +-
> plugins/loader.c | 2 +-
> qemu-img.c | 3 +-
> semihosting/config.c | 4 +-
> system/vl.c | 75 ++++++++++++----------------------
> tests/unit/test-char.c | 12 ++----
> tests/unit/test-seccomp.c | 6 +--
> trace/control.c | 3 +-
> ui/vnc.c | 3 +-
> util/qemu-option.c | 14 ++++++-
> 13 files changed, 54 insertions(+), 79 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC PATCH 09/14] qemu-options: Change qemu_opts_parse_noisily() to take the group
2026-09-18 22:29 ` [RFC PATCH 09/14] qemu-options: Change qemu_opts_parse_noisily() to take the group Fabiano Rosas
2026-09-21 13:51 ` Stefan Hajnoczi
@ 2026-09-25 10:27 ` Daniel P. Berrangé
1 sibling, 0 replies; 30+ messages in thread
From: Daniel P. Berrangé @ 2026-09-25 10:27 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Markus Armbruster, Kevin Wolf, Hanna Reitz,
Marc-André Lureau, Paolo Bonzini, Alex Bennée,
Pierrick Bouvier, Alexandre Iooss, Stefan Hajnoczi
On Fri, Sep 18, 2026 at 07:29:56PM -0300, Fabiano Rosas wrote:
> Pass the group name as argument to qemu_opts_parse_noisily() to make
> it simmetric with qemu_opts_parse().
>
> This change solves the issue of qemu_opts_parse_noisily() crashing on
> a NULL return from qemu_find_opts(), which can happen when an option
> is invoked for code in a module that is not loaded.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> block/monitor/block-hmp-cmds.c | 2 +-
> blockdev.c | 2 +-
> chardev/char-hmp-cmds.c | 5 +--
> include/qemu/option.h | 2 +-
> plugins/loader.c | 2 +-
> qemu-img.c | 3 +-
> semihosting/config.c | 4 +-
> system/vl.c | 75 ++++++++++++----------------------
> tests/unit/test-char.c | 12 ++----
> tests/unit/test-seccomp.c | 6 +--
> trace/control.c | 3 +-
> ui/vnc.c | 3 +-
> util/qemu-option.c | 14 ++++++-
> 13 files changed, 54 insertions(+), 79 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC PATCH 10/14] qemu-options: Check for NULL list at qemu_opts_parse()
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (8 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 09/14] qemu-options: Change qemu_opts_parse_noisily() to take the group Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 10:28 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 11/14] qemu-options: pass an Error into get_opt_name_value Fabiano Rosas
` (3 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
Now that qemu_opts_parse_noisily() is protected against NULL list, do
the same for the non noisy version.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
util/qemu-option.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index f8494c8842..37e6d68f02 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -925,6 +925,10 @@ QemuOpts *qemu_opts_parse(const char *group, const char *params,
{
QemuOptsList *list = qemu_find_opts_err(group, errp);
+ if (!list) {
+ return NULL;
+ }
+
return qemu_opts_parse_list(list, params, permit_abbrev, errp);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH 11/14] qemu-options: pass an Error into get_opt_name_value
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (9 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 10/14] qemu-options: Check for NULL list at qemu_opts_parse() Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-25 10:35 ` Daniel P. Berrangé
2026-09-18 22:29 ` [RFC PATCH 12/14] qemu-options: merge warn and help flags Fabiano Rosas
` (2 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
Passing a boolean into get_opt_name_value() is to put too much
responsibility into this low-level routine. Have it just fill an Error
object and let the caller decide what to do with it. Setting an Error
is an easily recognizable pattern so it saves the reader from mentally
parsing the boolean logic.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
util/qemu-option.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 37e6d68f02..94e06b194d 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -756,9 +756,9 @@ void qemu_opts_print(QemuOpts *opts, const char *separator)
static const char *get_opt_name_value(const char *params,
const char *firstname,
- bool warn_on_flag,
bool *help_wanted,
- char **name, char **value)
+ char **name, char **value,
+ Error **errp)
{
const char *p;
const char *prefix = "";
@@ -783,13 +783,15 @@ static const char *get_opt_name_value(const char *params,
*value = g_strdup("on");
is_help = is_help_option(*name);
}
- if (!is_help && warn_on_flag) {
- warn_report("short-form boolean option '%s%s' deprecated", prefix, *name);
+ if (!is_help) {
if (g_str_equal(*name, "delay")) {
- error_printf("Please use nodelay=%s instead\n", prefix[0] ? "on" : "off");
+ error_setg(errp, "Please use nodelay=%s instead",
+ prefix[0] ? "on" : "off");
} else {
- error_printf("Please use %s=%s instead\n", *name, *value);
+ error_setg(errp, "Please use %s=%s instead", *name, *value);
}
+ error_prepend(errp, "short-form boolean option '%s%s' "
+ "deprecated", prefix, *name);
}
}
} else {
@@ -821,9 +823,14 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
g_autofree char *option = NULL;
g_autofree char *value = NULL;
bool is_help = false;
+ Error *warn = NULL;
+
+ p = get_opt_name_value(p, firstname, &is_help, &option, &value, &warn);
+
+ if (warn_on_flag && warn) {
+ warn_report_err(warn);
+ }
- p = get_opt_name_value(p, firstname, warn_on_flag, &is_help, &option,
- &value);
if (help_wanted && is_help) {
qemu_opts_print_help(opts->list, true);
return false;
@@ -852,7 +859,7 @@ static char *opts_parse_id(const char *params)
g_autofree char *name = NULL;
g_autofree char *value = NULL;
- p = get_opt_name_value(p, NULL, false, NULL, &name, &value);
+ p = get_opt_name_value(p, NULL, NULL, &name, &value, NULL);
if (!strcmp(name, "id")) {
return g_steal_pointer(&value);
}
@@ -870,7 +877,7 @@ bool has_help_option(const char *params)
g_autofree char *name = NULL;
g_autofree char *value = NULL;
- p = get_opt_name_value(p, NULL, false, &ret, &name, &value);
+ p = get_opt_name_value(p, NULL, &ret, &name, &value, NULL);
if (ret) {
return true;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC PATCH 11/14] qemu-options: pass an Error into get_opt_name_value
2026-09-18 22:29 ` [RFC PATCH 11/14] qemu-options: pass an Error into get_opt_name_value Fabiano Rosas
@ 2026-09-25 10:35 ` Daniel P. Berrangé
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrangé @ 2026-09-25 10:35 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster
On Fri, Sep 18, 2026 at 07:29:58PM -0300, Fabiano Rosas wrote:
> Passing a boolean into get_opt_name_value() is to put too much
> responsibility into this low-level routine. Have it just fill an Error
> object and let the caller decide what to do with it. Setting an Error
> is an easily recognizable pattern so it saves the reader from mentally
> parsing the boolean logic.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> util/qemu-option.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 37e6d68f02..94e06b194d 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -756,9 +756,9 @@ void qemu_opts_print(QemuOpts *opts, const char *separator)
>
> static const char *get_opt_name_value(const char *params,
> const char *firstname,
> - bool warn_on_flag,
> bool *help_wanted,
> - char **name, char **value)
> + char **name, char **value,
> + Error **errp)
> {
> const char *p;
> const char *prefix = "";
> @@ -783,13 +783,15 @@ static const char *get_opt_name_value(const char *params,
> *value = g_strdup("on");
> is_help = is_help_option(*name);
> }
> - if (!is_help && warn_on_flag) {
> - warn_report("short-form boolean option '%s%s' deprecated", prefix, *name);
> + if (!is_help) {
> if (g_str_equal(*name, "delay")) {
> - error_printf("Please use nodelay=%s instead\n", prefix[0] ? "on" : "off");
> + error_setg(errp, "Please use nodelay=%s instead",
> + prefix[0] ? "on" : "off");
> } else {
> - error_printf("Please use %s=%s instead\n", *name, *value);
> + error_setg(errp, "Please use %s=%s instead", *name, *value);
> }
> + error_prepend(errp, "short-form boolean option '%s%s' "
> + "deprecated", prefix, *name);
> }
> }
We've had short form bool options deprecated for years now.
I'd be inclined to follow through on the deprecation and
delete support for short form bools now.
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 :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC PATCH 12/14] qemu-options: merge warn and help flags
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (10 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 11/14] qemu-options: pass an Error into get_opt_name_value Fabiano Rosas
@ 2026-09-18 22:29 ` Fabiano Rosas
2026-09-18 22:30 ` [RFC PATCH 13/14] qemu-options: Further simplify qemu_opts_parse_list() Fabiano Rosas
2026-09-18 22:30 ` [RFC PATCH 14/14] qemu-options: Remove opts_parse() Fabiano Rosas
13 siblings, 0 replies; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
There's no need to propagate two separate flags to perform the "noisy"
parsing. The catch is that the opposite scenario, the "silent" parsing
never sets the flags and always uses the Error object, so we can just
push the opts_accepts_any() check that is exclusive to the noisy case
down the stack.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
util/qemu-option.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 94e06b194d..8dadb47170 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -813,8 +813,7 @@ static const char *get_opt_name_value(const char *params,
}
static bool opts_do_parse(QemuOpts *opts, const char *params,
- const char *firstname,
- bool warn_on_flag, bool help_wanted, Error **errp)
+ const char *firstname, bool noisily, Error **errp)
{
const char *p;
QemuOpt *opt;
@@ -827,13 +826,15 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
p = get_opt_name_value(p, firstname, &is_help, &option, &value, &warn);
- if (warn_on_flag && warn) {
- warn_report_err(warn);
- }
+ if (noisily) {
+ if (warn) {
+ warn_report_err(warn);
+ }
- if (help_wanted && is_help) {
- qemu_opts_print_help(opts->list, true);
- return false;
+ if (is_help && !opts_accepts_any(opts->list)) {
+ qemu_opts_print_help(opts->list, true);
+ return false;
+ }
}
firstname = NULL;
@@ -895,12 +896,11 @@ bool has_help_option(const char *params)
bool qemu_opts_do_parse(QemuOpts *opts, const char *params,
const char *firstname, Error **errp)
{
- return opts_do_parse(opts, params, firstname, false, false, errp);
+ return opts_do_parse(opts, params, firstname, false, errp);
}
static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
- bool permit_abbrev,
- bool warn_on_flag, bool help_wanted, Error **errp)
+ bool permit_abbrev, bool noisily, Error **errp)
{
const char *firstname;
char *id = opts_parse_id(params);
@@ -911,8 +911,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
opts = qemu_opts_create(list, id, !list->merge_lists, errp);
g_free(id);
- if (!opts || !opts_do_parse(opts, params, firstname,
- warn_on_flag, help_wanted, errp)) {
+ if (!opts || !opts_do_parse(opts, params, firstname, noisily, errp)) {
qemu_opts_del(opts);
return NULL;
}
@@ -956,15 +955,14 @@ QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
if (noisily) {
Error *err = NULL;
- bool help_wanted = !opts_accepts_any(list);
- opts = opts_parse(list, params, permit_abbrev, true, help_wanted, &err);
+ opts = opts_parse(list, params, permit_abbrev, noisily, &err);
if (!opts && err) {
error_report_err(err);
}
} else {
- opts = opts_parse(list, params, permit_abbrev, false, false, errp);
+ opts = opts_parse(list, params, permit_abbrev, noisily, errp);
}
return opts;
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH 13/14] qemu-options: Further simplify qemu_opts_parse_list()
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (11 preceding siblings ...)
2026-09-18 22:29 ` [RFC PATCH 12/14] qemu-options: merge warn and help flags Fabiano Rosas
@ 2026-09-18 22:30 ` Fabiano Rosas
2026-09-18 22:30 ` [RFC PATCH 14/14] qemu-options: Remove opts_parse() Fabiano Rosas
13 siblings, 0 replies; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
Now that the two branches of this function are similar, turn it into a
single call to opts_parse().
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
util/qemu-option.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 8dadb47170..1e8042a1bd 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -950,19 +950,18 @@ QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
{
bool noisily = !errp;
QemuOpts *opts = NULL;
+ Error *err = NULL;
assert(list);
+ opts = opts_parse(list, params, permit_abbrev, noisily, &err);
- if (noisily) {
- Error *err = NULL;
-
- opts = opts_parse(list, params, permit_abbrev, noisily, &err);
- if (!opts && err) {
+ if (!opts) {
+ assert(err);
+ if (noisily) {
error_report_err(err);
+ } else {
+ error_propagate(errp, err);
}
-
- } else {
- opts = opts_parse(list, params, permit_abbrev, noisily, errp);
}
return opts;
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC PATCH 14/14] qemu-options: Remove opts_parse()
2026-09-18 22:29 [RFC PATCH 00/14] qemu-options: Remove some complexity Fabiano Rosas
` (12 preceding siblings ...)
2026-09-18 22:30 ` [RFC PATCH 13/14] qemu-options: Further simplify qemu_opts_parse_list() Fabiano Rosas
@ 2026-09-18 22:30 ` Fabiano Rosas
13 siblings, 0 replies; 30+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
There's only one caller of opts_parse(): qemu_opts_parse_list().
Merge the two functions.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
util/qemu-option.c | 33 ++++++++++-----------------------
1 file changed, 10 insertions(+), 23 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 1e8042a1bd..167c81c17a 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -899,26 +899,6 @@ bool qemu_opts_do_parse(QemuOpts *opts, const char *params,
return opts_do_parse(opts, params, firstname, false, errp);
}
-static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
- bool permit_abbrev, bool noisily, Error **errp)
-{
- const char *firstname;
- char *id = opts_parse_id(params);
- QemuOpts *opts;
-
- assert(!permit_abbrev || list->implied_opt_name);
- firstname = permit_abbrev ? list->implied_opt_name : NULL;
-
- opts = qemu_opts_create(list, id, !list->merge_lists, errp);
- g_free(id);
- if (!opts || !opts_do_parse(opts, params, firstname, noisily, errp)) {
- qemu_opts_del(opts);
- return NULL;
- }
-
- return opts;
-}
-
/**
* Find the @group and create a QemuOpts with options parsed from
* @params. If @permit_abbrev, the first key=value in @params may
@@ -948,20 +928,27 @@ 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)
{
+ const char *firstname;
+ g_autofree char *id = opts_parse_id(params);
bool noisily = !errp;
QemuOpts *opts = NULL;
Error *err = NULL;
assert(list);
- opts = opts_parse(list, params, permit_abbrev, noisily, &err);
+ assert(!permit_abbrev || list->implied_opt_name);
+
+ firstname = permit_abbrev ? list->implied_opt_name : NULL;
+
+ opts = qemu_opts_create(list, id, !list->merge_lists, &err);
+ if (!opts || !opts_do_parse(opts, params, firstname, noisily, &err)) {
+ qemu_opts_del(opts);
- if (!opts) {
- assert(err);
if (noisily) {
error_report_err(err);
} else {
error_propagate(errp, err);
}
+ return NULL;
}
return opts;
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread