* [PATCH v2 01/11] qemu-option: Use g_autofree when calling get_opt_name_value()
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
@ 2026-09-30 22:10 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-09-30 22:10 ` [PATCH v2 02/11] tests/unit/test-qemu-opts: Validate help=foo options Fabiano Rosas
` (9 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Daniel P . Berrangé
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.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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] 31+ messages in thread* [PATCH v2 02/11] tests/unit/test-qemu-opts: Validate help=foo options
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
2026-09-30 22:10 ` [PATCH v2 01/11] qemu-option: Use g_autofree when calling get_opt_name_value() Fabiano Rosas
@ 2026-09-30 22:10 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-09-30 22:10 ` [PATCH v2 03/11] qemu-option: Remove short form options support Fabiano Rosas
` (8 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Daniel P . Berrangé
The test_has_help_option currently tests two functions:
has_help_option() and qemu_opt_has_help_opt(). The former is only used
by qemu-img and has a significant difference from the latter, which is
used by QOM and qdev:
- has_help_option() REJECTS help=on and all variants that provide a
value, including invalid ones such as help=foo.
- qemu_opt_has_help_opt() ACCEPTS help=on and all variants that
provide a value, including invalid ones such as help=foo.
Add a new test that validates these cases separately.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
tests/unit/test-qemu-opts.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/tests/unit/test-qemu-opts.c b/tests/unit/test-qemu-opts.c
index 8d03a69f7c..4ccb79d406 100644
--- a/tests/unit/test-qemu-opts.c
+++ b/tests/unit/test-qemu-opts.c
@@ -750,6 +750,31 @@ static void test_has_help_option(void)
}
}
+static void test_has_help_option_with_value(void)
+{
+ static const struct {
+ const char *params;
+ } test[] = {
+ { "help=on" },
+ { "help=off" },
+ { "help=foo" },
+ { "help=" },
+ };
+ int i;
+ QemuOpts *opts;
+
+ for (i = 0; i < ARRAY_SIZE(test); i++) {
+ /* reject all */
+ g_assert_cmpint(has_help_option(test[i].params), ==, false);
+
+ /* accept all */
+ opts = qemu_opts_parse(&opts_list_03, test[i].params, false,
+ &error_abort);
+ g_assert_cmpint(qemu_opt_has_help_opt(opts), ==, true);
+ qemu_opts_del(opts);
+ }
+}
+
static void append_verify_list_01(QemuOptDesc *desc, bool with_overlapping)
{
int i = 0;
@@ -1012,6 +1037,8 @@ int main(int argc, char *argv[])
g_test_add_func("/qemu-opts/opts_parse/number", test_opts_parse_number);
g_test_add_func("/qemu-opts/opts_parse/size", test_opts_parse_size);
g_test_add_func("/qemu-opts/has_help_option", test_has_help_option);
+ g_test_add_func("/qemu-opts/has_help_option_with_value",
+ test_has_help_option_with_value);
g_test_add_func("/qemu-opts/append_to_null", test_opts_append_to_null);
g_test_add_func("/qemu-opts/append", test_opts_append);
g_test_add_func("/qemu-opts/to_qdict/basic", test_opts_to_qdict_basic);
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 02/11] tests/unit/test-qemu-opts: Validate help=foo options
2026-09-30 22:10 ` [PATCH v2 02/11] tests/unit/test-qemu-opts: Validate help=foo options Fabiano Rosas
@ 2026-10-01 9:42 ` marcandre.lureau
0 siblings, 0 replies; 31+ messages in thread
From: marcandre.lureau @ 2026-10-01 9:42 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé
On Wed, 30 Sep 2026 19:10:56 -0300, Fabiano Rosas <farosas@suse.de> wrote:
> The test_has_help_option currently tests two functions:
> has_help_option() and qemu_opt_has_help_opt(). The former is only used
> by qemu-img and has a significant difference from the latter, which is
> used by QOM and qdev:
>
> - has_help_option() REJECTS help=on and all variants that provide a
> value, including invalid ones such as help=foo.
>
> [...]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--
Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 03/11] qemu-option: Remove short form options support
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
2026-09-30 22:10 ` [PATCH v2 01/11] qemu-option: Use g_autofree when calling get_opt_name_value() Fabiano Rosas
2026-09-30 22:10 ` [PATCH v2 02/11] tests/unit/test-qemu-opts: Validate help=foo options Fabiano Rosas
@ 2026-09-30 22:10 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
` (2 more replies)
2026-09-30 22:10 ` [PATCH v2 04/11] qemu-option: Fix 'help' parameter parsing Fabiano Rosas
` (7 subsequent siblings)
10 siblings, 3 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:10 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Daniel P . Berrangé, Pierrick Bouvier,
Kevin Wolf, Hanna Reitz
Option parameters without a value (i.e. key vs. key=val) are
deprecated. The missing value is currently implied to be either "on"
or "off" depending on whether the parameter name starts with "no".
Remove the implied behavior and start rejecting option parameters
without values by emitting the error message:
"Parameter 'foo' without a value."
Two special cases remain:
1) The 'help' parameter is special and still supported. Keep a
positive value attached to it ("on").
Note that currently there is no validation for the value (if any)
attached to the help parameter. help, help=on, help=off, help=foo
all result in the help text being emitted. This is not changed by
this patch.
2) The empty value is allowed if the key is also empty. This can be
achieved in the command line by adding commas.
E.g.: share=on,, is parsed as:
key:"share" value:"on" and
key:"" value:"on"
One effect of this change that might not be obvious is that parameter
combinations of the form:
id=mem0,share,help
no longer produce the help output. The lack of value for the 'share'
parameter is reported with precedence. Users will need to either use a
valid value or omit the parameter entirely:
id=mem0,share=on,help
id=mem0,help
About testing:
A few strings from the test_has_help_option test were removed because
they now trigger the missing value error during parsing and therefore
cannot be used for testing the qemu_opt_has_help_opt() routine which
needs the parsed QemuOpts.
The two "Implied value" tests were replaced with 8 new tests.
The "Implied key" test has had the extra parameters with implied
values removed, otherwise the implied key cannot be tested due to the
rejected missing values.
The test_inet_parse_all_implicit_bool_good was removed entirely
because it only tested the use case that is being removed.
A few block tests were updated to use the key=value form.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
docs/about/deprecated.rst | 13 -------
tests/qemu-iotests/084 | 2 +-
tests/qemu-iotests/146 | 2 +-
tests/qemu-iotests/197 | 2 +-
tests/qemu-iotests/215 | 2 +-
tests/unit/test-qemu-opts.c | 67 +++++++++++++++++++++-------------
tests/unit/test-util-sockets.c | 33 -----------------
util/qemu-option.c | 57 ++++++++++++++---------------
8 files changed, 73 insertions(+), 105 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ccbc421949..f54905eba9 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -30,19 +30,6 @@ deprecated.
System emulator command line arguments
--------------------------------------
-Short-form boolean options (since 6.0)
-''''''''''''''''''''''''''''''''''''''
-
-Boolean options such as ``share=on``/``share=off`` could be written
-in short form as ``share`` and ``noshare``. This is now deprecated
-and will cause a warning.
-
-``delay`` option for socket character devices (since 6.0)
-'''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
-rather than ``delay=off``.
-
Plugin argument passing through ``arg=<string>`` (since 6.1)
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
diff --git a/tests/qemu-iotests/084 b/tests/qemu-iotests/084
index 1181cb7cd0..f905396e72 100755
--- a/tests/qemu-iotests/084
+++ b/tests/qemu-iotests/084
@@ -51,7 +51,7 @@ bii_offset=384 # block in image field offset
echo
echo "=== Statically allocated image creation ==="
echo
-_make_test_img $size -o static
+_make_test_img $size -o static=on
_img_info
stat -c"disk image file size in bytes: %s" "${TEST_IMG}"
_cleanup_test_img
diff --git a/tests/qemu-iotests/146 b/tests/qemu-iotests/146
index f63291d0ff..a7853e03c9 100755
--- a/tests/qemu-iotests/146
+++ b/tests/qemu-iotests/146
@@ -165,7 +165,7 @@ echo
echo === Testing Image create, force_size ===
echo
-_make_test_img -o force_size 4G
+_make_test_img -o force_size=on 4G
echo
echo === Read created image, default opts ====
diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
index 69849c800e..271d1283c5 100755
--- a/tests/qemu-iotests/197
+++ b/tests/qemu-iotests/197
@@ -63,7 +63,7 @@ echo
# Prep the images
# VPC rounds image sizes to a specific geometry, force a specific size.
if [ "$IMGFMT" = "vpc" ]; then
- IMGOPTS=$(_optstr_add "$IMGOPTS" "force_size")
+ IMGOPTS=$(_optstr_add "$IMGOPTS" "force_size=on")
fi
_make_test_img 4G
$QEMU_IO -c "write -P 55 3G 1k" "$TEST_IMG" | _filter_qemu_io
diff --git a/tests/qemu-iotests/215 b/tests/qemu-iotests/215
index 6babbcdc1f..6a518a9f57 100755
--- a/tests/qemu-iotests/215
+++ b/tests/qemu-iotests/215
@@ -60,7 +60,7 @@ echo
# Prep the images
# VPC rounds image sizes to a specific geometry, force a specific size.
if [ "$IMGFMT" = "vpc" ]; then
- IMGOPTS=$(_optstr_add "$IMGOPTS" "force_size")
+ IMGOPTS=$(_optstr_add "$IMGOPTS" "force_size=on")
fi
_make_test_img 4G
$QEMU_IO -c "write -P 55 3G 1k" "$TEST_IMG" | _filter_qemu_io
diff --git a/tests/unit/test-qemu-opts.c b/tests/unit/test-qemu-opts.c
index 4ccb79d406..07ede50920 100644
--- a/tests/unit/test-qemu-opts.c
+++ b/tests/unit/test-qemu-opts.c
@@ -481,26 +481,10 @@ static void test_opts_parse(void)
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);
- 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);
- 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);
- g_assert_cmpuint(opts_count(opts), ==, 3);
+ opts = qemu_opts_parse(&opts_list_03, "an", true, &error_abort);
+ g_assert_cmpuint(opts_count(opts), ==, 1);
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);
@@ -523,6 +507,45 @@ static void test_opts_parse(void)
error_free_or_abort(&err);
g_assert(!opts);
+ /* Implied value */
+ opts = qemu_opts_parse(&opts_list_03, "an", false, &err);
+ error_free_or_abort(&err);
+ g_assert(!opts);
+
+ /* Implied value, unknown key */
+ opts = qemu_opts_parse(&opts_list_01, "nonexistent", false, &err);
+ error_free_or_abort(&err);
+ g_assert(!opts);
+
+ /* Implied value, negated key */
+ opts = qemu_opts_parse(&opts_list_03, "noaus", false, &err);
+ error_free_or_abort(&err);
+ g_assert(!opts);
+
+ /* Implied value, negated empty key */
+ opts = qemu_opts_parse(&opts_list_03, "no", false, &err);
+ error_free_or_abort(&err);
+ g_assert(!opts);
+
+ /* Empty value */
+ opts = qemu_opts_parse(&opts_list_03, "aus=", false, &error_abort);
+ g_assert_cmpuint(opts_count(opts), ==, 1);
+
+ /* Implied key */
+ opts = qemu_opts_parse(&opts_list_03, "an", true, &error_abort);
+ g_assert_cmpuint(opts_count(opts), ==, 1);
+
+ /* Implied key, implied value */
+ opts = qemu_opts_parse(&opts_list_03, "an,noaus,noaus=", true, &err);
+ error_free_or_abort(&err);
+ g_assert(!opts);
+
+ /* Implied key, empty value */
+ opts = qemu_opts_parse(&opts_list_03, "an,noaus=", true, &error_abort);
+ g_assert_cmpuint(opts_count(opts), ==, 2);
+ g_assert_cmpstr(qemu_opt_get(opts, "implied"), ==, "an");
+ g_assert_cmpstr(qemu_opt_get(opts, "noaus"), ==, "");
+
qemu_opts_reset(&opts_list_01);
qemu_opts_reset(&opts_list_03);
}
@@ -720,16 +743,8 @@ static void test_has_help_option(void)
} test[] = {
{ "help", true, false },
{ "?", true, false },
- { "helpme", false, false },
- { "?me", false, false },
- { "a,help", true, true },
- { "a,?", true, true },
- { "a=0,help,b", true, true },
- { "a=0,?,b", true, true },
{ "help,b=1", true, false },
{ "?,b=1", true, false },
- { "a,b,,help", true, true },
- { "a,b,,?", true, true },
};
int i;
QemuOpts *opts;
diff --git a/tests/unit/test-util-sockets.c b/tests/unit/test-util-sockets.c
index 006f5e579c..2875c547a0 100644
--- a/tests/unit/test-util-sockets.c
+++ b/tests/unit/test-util-sockets.c
@@ -516,36 +516,6 @@ static void test_inet_parse_all_options_good(void)
, &exp_addr, true);
}
-static void test_inet_parse_all_implicit_bool_good(void)
-{
- char host[] = "::1";
- char port[] = "5000";
- InetSocketAddress exp_addr = {
- .host = host,
- .port = port,
- .has_numeric = true,
- .numeric = true,
- .has_to = true,
- .to = 5006,
- .has_ipv4 = true,
- .ipv4 = true,
- .has_ipv6 = true,
- .ipv6 = true,
- .has_keep_alive = true,
- .keep_alive = true,
-#ifdef HAVE_IPPROTO_MPTCP
- .has_mptcp = true,
- .mptcp = true,
-#endif
- };
- inet_parse_test_helper(
- "[::1]:5000,numeric,to=5006,ipv4,ipv6,keep-alive"
-#ifdef HAVE_IPPROTO_MPTCP
- ",mptcp"
-#endif
- , &exp_addr, true);
-}
-
int main(int argc, char **argv)
{
bool has_ipv4, has_ipv6;
@@ -613,9 +583,6 @@ int main(int argc, char **argv)
test_inet_parse_hostname_good);
g_test_add_func("/util/socket/inet-parse/all-options-good",
test_inet_parse_all_options_good);
- g_test_add_func("/util/socket/inet-parse/all-bare-bool-good",
- test_inet_parse_all_implicit_bool_good);
-
end:
return g_test_run();
}
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 557d5d929c..8ac848bbc0 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -509,6 +509,11 @@ static bool opt_validate(QemuOpt *opt, Error **errp)
const QemuOptDesc *desc;
const QemuOptsList *list = opt->opts->list;
+ if (!opt->str) {
+ error_setg(errp, "Parameter '%s' without a value", opt->name);
+ return false;
+ }
+
desc = find_desc_by_name(list->desc, opt->name);
if (!desc && !opts_accepts_any(list)) {
error_setg(errp, "Invalid parameter '%s'", opt->name);
@@ -755,12 +760,10 @@ 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)
{
const char *p;
- const char *prefix = "";
size_t len;
bool is_help = false;
@@ -772,23 +775,22 @@ static const char *get_opt_name_value(const char *params,
*name = g_strdup(firstname);
p = get_opt_value(params, value);
} else {
- /* option without value, must be a flag */
p = get_opt_name(params, name, len);
- if (strncmp(*name, "no", 2) == 0) {
- memmove(*name, *name + 2, strlen(*name + 2) + 1);
- *value = g_strdup("off");
- prefix = "no";
- } else {
+
+ /*
+ * Short-form flags (i.e. without any value) are not
+ * supported, except for two cases:
+ */
+
+ /* the 'help' or '?' flag */
+ is_help = is_help_option(*name);
+ if (is_help) {
*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 (g_str_equal(*name, "delay")) {
- error_printf("Please use nodelay=%s instead\n", prefix[0] ? "on" : "off");
- } else {
- error_printf("Please use %s=%s instead\n", *name, *value);
- }
+
+ /* a missing, non-implicit key, i.e. a single comma ',' */
+ if (g_str_equal(*name, "") && *p == ',') {
+ *value = g_strdup("on");
}
}
} else {
@@ -811,7 +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)
+ bool *help_wanted, Error **errp)
{
const char *p;
QemuOpt *opt;
@@ -820,7 +822,7 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
g_autofree char *option = NULL;
g_autofree char *value = NULL;
- p = get_opt_name_value(p, firstname, warn_on_flag, help_wanted, &option, &value);
+ p = get_opt_name_value(p, firstname, help_wanted, &option, &value);
if (help_wanted && *help_wanted) {
return false;
}
@@ -848,7 +850,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);
if (!strcmp(name, "id")) {
return g_steal_pointer(&value);
}
@@ -866,7 +868,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);
if (ret) {
return true;
}
@@ -884,12 +886,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, NULL, errp);
+ return opts_do_parse(opts, params, firstname, NULL, 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 *help_wanted, Error **errp)
{
const char *firstname;
char *id = opts_parse_id(params);
@@ -904,8 +905,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
return NULL;
}
- if (!opts_do_parse(opts, params, firstname,
- warn_on_flag, help_wanted, errp)) {
+ if (!opts_do_parse(opts, params, firstname, help_wanted, errp)) {
qemu_opts_del(opts);
return NULL;
}
@@ -923,7 +923,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
QemuOpts *qemu_opts_parse(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, NULL, errp);
}
/**
@@ -941,9 +941,8 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
QemuOpts *opts;
bool help_wanted = false;
- opts = opts_parse(list, params, permit_abbrev, true,
- opts_accepts_any(list) ? NULL : &help_wanted,
- &err);
+ opts = opts_parse(list, params, permit_abbrev,
+ opts_accepts_any(list) ? NULL : &help_wanted, &err);
if (!opts) {
assert(!!err + !!help_wanted == 1);
if (help_wanted) {
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 03/11] qemu-option: Remove short form options support
2026-09-30 22:10 ` [PATCH v2 03/11] qemu-option: Remove short form options support Fabiano Rosas
@ 2026-10-01 9:42 ` marcandre.lureau
2026-10-07 7:08 ` Markus Armbruster
2026-10-07 8:16 ` Markus Armbruster
2 siblings, 0 replies; 31+ messages in thread
From: marcandre.lureau @ 2026-10-01 9:42 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé,
Pierrick Bouvier, Kevin Wolf, Hanna Reitz
On Wed, 30 Sep 2026 19:10:57 -0300, Fabiano Rosas <farosas@suse.de> wrote:
> Option parameters without a value (i.e. key vs. key=val) are
> deprecated. The missing value is currently implied to be either "on"
> or "off" depending on whether the parameter name starts with "no".
>
> Remove the implied behavior and start rejecting option parameters
> without values by emitting the error message:
>
> [...]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--
Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 03/11] qemu-option: Remove short form options support
2026-09-30 22:10 ` [PATCH v2 03/11] qemu-option: Remove short form options support Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
@ 2026-10-07 7:08 ` Markus Armbruster
2026-10-07 12:43 ` Fabiano Rosas
2026-10-07 8:16 ` Markus Armbruster
2 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2026-10-07 7:08 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Daniel P . Berrangé, Pierrick Bouvier,
Kevin Wolf, Hanna Reitz
Fabiano Rosas <farosas@suse.de> writes:
> Option parameters without a value (i.e. key vs. key=val) are
> deprecated. The missing value is currently implied to be either "on"
> or "off" depending on whether the parameter name starts with "no".
Commit ccd3b3b811 (qemu-option: warn for short-form boolean options,
2020-11-09). Six years of warnings should suffice.
> Remove the implied behavior and start rejecting option parameters
> without values by emitting the error message:
>
> "Parameter 'foo' without a value."
>
> Two special cases remain:
>
> 1) The 'help' parameter is special and still supported. Keep a
> positive value attached to it ("on").
>
> Note that currently there is no validation for the value (if any)
> attached to the help parameter. help, help=on, help=off, help=foo
> all result in the help text being emitted. This is not changed by
> this patch.
Really?
$ qemu-system-x86_64 -chardev help=foo
qemu-system-x86_64: -chardev help=foo: Invalid parameter 'help'
> 2) The empty value is allowed if the key is also empty. This can be
> achieved in the command line by adding commas.
> E.g.: share=on,, is parsed as:
> key:"share" value:"on" and
> key:"" value:"on"
I don't think so:
$ qemu-system-x86_64 -object memory-backend-ram,id=mem0,share=on,,
qemu-system-x86_64: -object memory-backend-ram,id=mem0,share=on,,: Parameter 'share' expects 'on' or 'off'
Set a breakpoint on qapi_bool_parse() to see the actual value. It's
"on,". In QemuOpts syntax, double comma is an escape, so you can put
comma in values.
> One effect of this change that might not be obvious is that parameter
> combinations of the form:
>
> id=mem0,share,help
>
> no longer produce the help output. The lack of value for the 'share'
> parameter is reported with precedence. Users will need to either use a
> valid value or omit the parameter entirely:
>
> id=mem0,share=on,help
> id=mem0,help
That's unfortunate.
[...]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 03/11] qemu-option: Remove short form options support
2026-10-07 7:08 ` Markus Armbruster
@ 2026-10-07 12:43 ` Fabiano Rosas
2026-10-08 4:42 ` Markus Armbruster
0 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-10-07 12:43 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Daniel P . Berrangé, Pierrick Bouvier,
Kevin Wolf, Hanna Reitz
Markus Armbruster <armbru@redhat.com> writes:
> Fabiano Rosas <farosas@suse.de> writes:
>
>> Option parameters without a value (i.e. key vs. key=val) are
>> deprecated. The missing value is currently implied to be either "on"
>> or "off" depending on whether the parameter name starts with "no".
>
> Commit ccd3b3b811 (qemu-option: warn for short-form boolean options,
> 2020-11-09). Six years of warnings should suffice.
>
>> Remove the implied behavior and start rejecting option parameters
>> without values by emitting the error message:
>>
>> "Parameter 'foo' without a value."
>>
>> Two special cases remain:
>>
>> 1) The 'help' parameter is special and still supported. Keep a
>> positive value attached to it ("on").
>>
>> Note that currently there is no validation for the value (if any)
>> attached to the help parameter. help, help=on, help=off, help=foo
>> all result in the help text being emitted. This is not changed by
>> this patch.
>
> Really?
>
> $ qemu-system-x86_64 -chardev help=foo
> qemu-system-x86_64: -chardev help=foo: Invalid parameter 'help'
>
Ah, ok, it depends on whether the option accepts any parameter
(i.e. opts_accepts_any()). Here's -object failing to reject the bogus
value:
$ qemu-system-x86_64 -object memory-backend-ram,id=m1,help=foo
memory-backend-ram options:
dump=<bool> - Set to 'off' to exclude from core dump
host-nodes=<[uint16]> - Binds memory to the list of NUMA host nodes
merge=<bool> - Mark memory as mergeable
policy=<HostMemPolicy> - Set the NUMA policy
prealloc-context=<link<thread-context>> - Context to use for creating CPU threads for preallocation
prealloc-threads=<int> - Number of CPU threads to use for prealloc
prealloc=<bool> - Preallocate memory
reserve=<bool> - Reserve swap space (or huge pages) if applicable
share=<bool> - Mark the memory as private to QEMU or shared
size=<size> - Size of the memory region (ex: 500M)
x-use-canonical-path-for-ramblock-id=<bool>
>> 2) The empty value is allowed if the key is also empty. This can be
>> achieved in the command line by adding commas.
>> E.g.: share=on,, is parsed as:
>> key:"share" value:"on" and
>> key:"" value:"on"
>
> I don't think so:
>
> $ qemu-system-x86_64 -object memory-backend-ram,id=mem0,share=on,,
> qemu-system-x86_64: -object memory-backend-ram,id=mem0,share=on,,: Parameter 'share' expects 'on' or 'off'
>
Bah, I wrote the example for the commit message but haven't tried
it. I'm trying to refer to the single leading comma scenario, we have a
test for it:
/* Except when it isn't */
opts = qemu_opts_parse(&opts_list_03, ",", false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpstr(qemu_opt_get(opts, ""), ==, "on");
The following are parsed as valid and only reject the empty key further
down the line:
$ qemu-system-x86_64 -drive ,
qemu-system-x86_64: -drive ,: warning: short-form boolean option '' deprecated
Please use =on instead
qemu-system-x86_64: -drive ,: Must specify either driver or file
$ qemu-system-x86_64 -drive ,file=dummy
qemu-system-x86_64: -drive ,file=dummy: warning: short-form boolean option '' deprecated
Please use =on instead
WARNING: Image format was not specified for 'dummy' and probing guessed raw.
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
qemu-system-x86_64: -drive ,file=dummy: Block format 'raw' does not support the option ''
> Set a breakpoint on qapi_bool_parse() to see the actual value. It's
> "on,". In QemuOpts syntax, double comma is an escape, so you can put
> comma in values.
>
>> One effect of this change that might not be obvious is that parameter
>> combinations of the form:
>>
>> id=mem0,share,help
>>
>> no longer produce the help output. The lack of value for the 'share'
>> parameter is reported with precedence. Users will need to either use a
>> valid value or omit the parameter entirely:
>>
>> id=mem0,share=on,help
>> id=mem0,help
>
> That's unfortunate.
>
> [...]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 03/11] qemu-option: Remove short form options support
2026-10-07 12:43 ` Fabiano Rosas
@ 2026-10-08 4:42 ` Markus Armbruster
2026-10-10 0:04 ` Fabiano Rosas
0 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2026-10-08 4:42 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Daniel P . Berrangé, Pierrick Bouvier,
Kevin Wolf, Hanna Reitz
Fabiano Rosas <farosas@suse.de> writes:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> Fabiano Rosas <farosas@suse.de> writes:
>>
>>> Option parameters without a value (i.e. key vs. key=val) are
>>> deprecated. The missing value is currently implied to be either "on"
>>> or "off" depending on whether the parameter name starts with "no".
>>
>> Commit ccd3b3b811 (qemu-option: warn for short-form boolean options,
>> 2020-11-09). Six years of warnings should suffice.
>>
>>> Remove the implied behavior and start rejecting option parameters
>>> without values by emitting the error message:
>>>
>>> "Parameter 'foo' without a value."
>>>
>>> Two special cases remain:
>>>
>>> 1) The 'help' parameter is special and still supported. Keep a
>>> positive value attached to it ("on").
>>>
>>> Note that currently there is no validation for the value (if any)
>>> attached to the help parameter. help, help=on, help=off, help=foo
>>> all result in the help text being emitted. This is not changed by
>>> this patch.
>>
>> Really?
>>
>> $ qemu-system-x86_64 -chardev help=foo
>> qemu-system-x86_64: -chardev help=foo: Invalid parameter 'help'
>>
>
> Ah, ok, it depends on whether the option accepts any parameter
> (i.e. opts_accepts_any()). Here's -object failing to reject the bogus
> value:
>
> $ qemu-system-x86_64 -object memory-backend-ram,id=m1,help=foo
> memory-backend-ram options:
> dump=<bool> - Set to 'off' to exclude from core dump
> host-nodes=<[uint16]> - Binds memory to the list of NUMA host nodes
> merge=<bool> - Mark memory as mergeable
> policy=<HostMemPolicy> - Set the NUMA policy
> prealloc-context=<link<thread-context>> - Context to use for creating CPU threads for preallocation
> prealloc-threads=<int> - Number of CPU threads to use for prealloc
> prealloc=<bool> - Preallocate memory
> reserve=<bool> - Reserve swap space (or huge pages) if applicable
> share=<bool> - Mark the memory as private to QEMU or shared
> size=<size> - Size of the memory region (ex: 500M)
> x-use-canonical-path-for-ramblock-id=<bool>
Bizarre :)
>>> 2) The empty value is allowed if the key is also empty. This can be
>>> achieved in the command line by adding commas.
>>> E.g.: share=on,, is parsed as:
>>> key:"share" value:"on" and
>>> key:"" value:"on"
>>
>> I don't think so:
>>
>> $ qemu-system-x86_64 -object memory-backend-ram,id=mem0,share=on,,
>> qemu-system-x86_64: -object memory-backend-ram,id=mem0,share=on,,: Parameter 'share' expects 'on' or 'off'
>>
>
> Bah, I wrote the example for the commit message but haven't tried
> it. I'm trying to refer to the single leading comma scenario, we have a
> test for it:
>
> /* Except when it isn't */
> opts = qemu_opts_parse(&opts_list_03, ",", false, &error_abort);
> g_assert_cmpuint(opts_count(opts), ==, 1);
> g_assert_cmpstr(qemu_opt_get(opts, ""), ==, "on");
The "except when it isn't" comment refers to the test right above:
/* Trailing comma is ignored */
opts = qemu_opts_parse(&opts_list_03, "x=y,", false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpstr(qemu_opt_get(opts, "x"), ==, "y");
This is about trailing comma. Is the exception is only possible when
the trailing comma is also the leading comma?
> The following are parsed as valid and only reject the empty key further
> down the line:
>
> $ qemu-system-x86_64 -drive ,
> qemu-system-x86_64: -drive ,: warning: short-form boolean option '' deprecated
> Please use =on instead
> qemu-system-x86_64: -drive ,: Must specify either driver or file
>
> $ qemu-system-x86_64 -drive ,file=dummy
> qemu-system-x86_64: -drive ,file=dummy: warning: short-form boolean option '' deprecated
> Please use =on instead
Looks like the answer is no. The tests could be clearer there. Might
not matter after your series.
> WARNING: Image format was not specified for 'dummy' and probing guessed raw.
> Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
> Specify the 'raw' format explicitly to remove the restrictions.
> qemu-system-x86_64: -drive ,file=dummy: Block format 'raw' does not support the option ''
The entire parser should be burned with fire.
What are the remaining differences to the keyval.c parser after your
series? Can we deprecate them?
>> Set a breakpoint on qapi_bool_parse() to see the actual value. It's
>> "on,". In QemuOpts syntax, double comma is an escape, so you can put
>> comma in values.
>>
>>> One effect of this change that might not be obvious is that parameter
>>> combinations of the form:
>>>
>>> id=mem0,share,help
>>>
>>> no longer produce the help output. The lack of value for the 'share'
>>> parameter is reported with precedence. Users will need to either use a
>>> valid value or omit the parameter entirely:
>>>
>>> id=mem0,share=on,help
>>> id=mem0,help
>>
>> That's unfortunate.
>>
>> [...]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 03/11] qemu-option: Remove short form options support
2026-10-08 4:42 ` Markus Armbruster
@ 2026-10-10 0:04 ` Fabiano Rosas
0 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-10-10 0:04 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Daniel P . Berrangé, Pierrick Bouvier,
Kevin Wolf, Hanna Reitz
Markus Armbruster <armbru@redhat.com> writes:
> Fabiano Rosas <farosas@suse.de> writes:
>
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> Fabiano Rosas <farosas@suse.de> writes:
>>>
>>>> Option parameters without a value (i.e. key vs. key=val) are
>>>> deprecated. The missing value is currently implied to be either "on"
>>>> or "off" depending on whether the parameter name starts with "no".
>>>
>>> Commit ccd3b3b811 (qemu-option: warn for short-form boolean options,
>>> 2020-11-09). Six years of warnings should suffice.
>>>
>>>> Remove the implied behavior and start rejecting option parameters
>>>> without values by emitting the error message:
>>>>
>>>> "Parameter 'foo' without a value."
>>>>
>>>> Two special cases remain:
>>>>
>>>> 1) The 'help' parameter is special and still supported. Keep a
>>>> positive value attached to it ("on").
>>>>
>>>> Note that currently there is no validation for the value (if any)
>>>> attached to the help parameter. help, help=on, help=off, help=foo
>>>> all result in the help text being emitted. This is not changed by
>>>> this patch.
>>>
>>> Really?
>>>
>>> $ qemu-system-x86_64 -chardev help=foo
>>> qemu-system-x86_64: -chardev help=foo: Invalid parameter 'help'
>>>
>>
>> Ah, ok, it depends on whether the option accepts any parameter
>> (i.e. opts_accepts_any()). Here's -object failing to reject the bogus
>> value:
>>
>> $ qemu-system-x86_64 -object memory-backend-ram,id=m1,help=foo
>> memory-backend-ram options:
>> dump=<bool> - Set to 'off' to exclude from core dump
>> host-nodes=<[uint16]> - Binds memory to the list of NUMA host nodes
>> merge=<bool> - Mark memory as mergeable
>> policy=<HostMemPolicy> - Set the NUMA policy
>> prealloc-context=<link<thread-context>> - Context to use for creating CPU threads for preallocation
>> prealloc-threads=<int> - Number of CPU threads to use for prealloc
>> prealloc=<bool> - Preallocate memory
>> reserve=<bool> - Reserve swap space (or huge pages) if applicable
>> share=<bool> - Mark the memory as private to QEMU or shared
>> size=<size> - Size of the memory region (ex: 500M)
>> x-use-canonical-path-for-ramblock-id=<bool>
>
> Bizarre :)
>
>>>> 2) The empty value is allowed if the key is also empty. This can be
>>>> achieved in the command line by adding commas.
>>>> E.g.: share=on,, is parsed as:
>>>> key:"share" value:"on" and
>>>> key:"" value:"on"
>>>
>>> I don't think so:
>>>
>>> $ qemu-system-x86_64 -object memory-backend-ram,id=mem0,share=on,,
>>> qemu-system-x86_64: -object memory-backend-ram,id=mem0,share=on,,: Parameter 'share' expects 'on' or 'off'
>>>
>>
>> Bah, I wrote the example for the commit message but haven't tried
>> it. I'm trying to refer to the single leading comma scenario, we have a
>> test for it:
>>
>> /* Except when it isn't */
>> opts = qemu_opts_parse(&opts_list_03, ",", false, &error_abort);
>> g_assert_cmpuint(opts_count(opts), ==, 1);
>> g_assert_cmpstr(qemu_opt_get(opts, ""), ==, "on");
>
> The "except when it isn't" comment refers to the test right above:
>
> /* Trailing comma is ignored */
> opts = qemu_opts_parse(&opts_list_03, "x=y,", false, &error_abort);
> g_assert_cmpuint(opts_count(opts), ==, 1);
> g_assert_cmpstr(qemu_opt_get(opts, "x"), ==, "y");
>
> This is about trailing comma. Is the exception is only possible when
> the trailing comma is also the leading comma?
>
>> The following are parsed as valid and only reject the empty key further
>> down the line:
>>
>> $ qemu-system-x86_64 -drive ,
>> qemu-system-x86_64: -drive ,: warning: short-form boolean option '' deprecated
>> Please use =on instead
>> qemu-system-x86_64: -drive ,: Must specify either driver or file
>>
>> $ qemu-system-x86_64 -drive ,file=dummy
>> qemu-system-x86_64: -drive ,file=dummy: warning: short-form boolean option '' deprecated
>> Please use =on instead
>
> Looks like the answer is no. The tests could be clearer there. Might
> not matter after your series.
>
>> WARNING: Image format was not specified for 'dummy' and probing guessed raw.
>> Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
>> Specify the 'raw' format explicitly to remove the restrictions.
>> qemu-system-x86_64: -drive ,file=dummy: Block format 'raw' does not support the option ''
>
> The entire parser should be burned with fire.
>
> What are the remaining differences to the keyval.c parser after your
> series? Can we deprecate them?
>
I ran a few of the qemu-opts tests with the keyval code and it differs
mostly in the handling of empty (non-implied) key and implied key +
empty value. I'll take a close look next week but I think we should
deprecate them anyway.
It looks easy to add a flag to keyval_parse_one() to turn its errors
into deprecation warnings ("warning: madness deprecated"). We could then
gradually make the conversion without having to wait for the deprecated
parts to be removed. The child is almost 10 years old.
d454dbe0ee3 ("keyval: New keyval_parse()")
Author: Markus Armbruster <armbru@redhat.com>
Date: Tue Feb 28 22:26:49 2017 +0100
>>> Set a breakpoint on qapi_bool_parse() to see the actual value. It's
>>> "on,". In QemuOpts syntax, double comma is an escape, so you can put
>>> comma in values.
>>>
>>>> One effect of this change that might not be obvious is that parameter
>>>> combinations of the form:
>>>>
>>>> id=mem0,share,help
>>>>
>>>> no longer produce the help output. The lack of value for the 'share'
>>>> parameter is reported with precedence. Users will need to either use a
>>>> valid value or omit the parameter entirely:
>>>>
>>>> id=mem0,share=on,help
>>>> id=mem0,help
>>>
>>> That's unfortunate.
>>>
>>> [...]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 03/11] qemu-option: Remove short form options support
2026-09-30 22:10 ` [PATCH v2 03/11] qemu-option: Remove short form options support Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-10-07 7:08 ` Markus Armbruster
@ 2026-10-07 8:16 ` Markus Armbruster
2 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2026-10-07 8:16 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Daniel P . Berrangé, Pierrick Bouvier,
Kevin Wolf, Hanna Reitz
Fabiano Rosas <farosas@suse.de> writes:
> Option parameters without a value (i.e. key vs. key=val) are
> deprecated. The missing value is currently implied to be either "on"
> or "off" depending on whether the parameter name starts with "no".
>
> Remove the implied behavior and start rejecting option parameters
> without values by emitting the error message:
>
> "Parameter 'foo' without a value."
[...]
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index ccbc421949..f54905eba9 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -30,19 +30,6 @@ deprecated.
> System emulator command line arguments
> --------------------------------------
>
> -Short-form boolean options (since 6.0)
> -''''''''''''''''''''''''''''''''''''''
> -
> -Boolean options such as ``share=on``/``share=off`` could be written
> -in short form as ``share`` and ``noshare``. This is now deprecated
> -and will cause a warning.
> -
> -``delay`` option for socket character devices (since 6.0)
> -'''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> -
> -The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
> -rather than ``delay=off``.
> -
> Plugin argument passing through ``arg=<string>`` (since 6.1)
> ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>
No docs/about/removed-features.rst update?
[...]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 04/11] qemu-option: Fix 'help' parameter parsing
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
` (2 preceding siblings ...)
2026-09-30 22:10 ` [PATCH v2 03/11] qemu-option: Remove short form options support Fabiano Rosas
@ 2026-09-30 22:10 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-09-30 22:10 ` [PATCH v2 05/11] qemu-option: Add qemu_opts_parse_list Fabiano Rosas
` (6 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Daniel P . Berrangé
Reject invalid values such as help= and help=foo.
Reject valid, but not applicable values such has help=on and help=off.
To achieve the above:
Add an 'is_help' member to QemuOpt so that information can be stored
while creating the option. This allows moving the help handling out of
get_opt_name_value(), which is necessary because that function doesn't
take an Error.
At the caller, opts_do_parse(), let the QemuOpt be created with the
name and value fields as they come out of get_opt_name_value().
With a new routine, validate the new QemuOpt against the 'help'
parameter rules (to fix the bug), then let the existing opt_validate()
reject any NULL values. The two validations need to be separate
because in between there needs to be a setting of opt->str to "on"
whenever the special short-form parameters require it.
The "on" value is required because of the conversions of QemuOpts
from/to QDict since QDict cannot take a NULL value. The conversions
also cause opt_validate() to be reentrant via qemu_opt_set(), so the
initial validation of 'help' needs to be out of that function,
otherwise there's no way to distinguish an user-set help=on vs. the
help=on set for compatibility with QDict (the "in between" part from
the previous paragraph).
The passing around of help_wanted becomes redundant since opt->is_help
can be checked throughout. Remove that argument everywhere.
Stop returning false from opts_do_parse() when is_help=true. Only
return false when there's an error and let the callers decide what to
do when is_help=true. However, still break the loop when is_help=true
to keep the behavior of stopping parsing once the 'help' parameter is
seen.
At qemu_opts_parse_noisily(), due to the change of return value
mentioned above, it becomes clear when to report the error and when to
print the help text. Use qemu_opt_has_help_opt() to obtain the
is_help information from QemuOpt.
Since get_opt_name_value() can now return while value=NULL, the other
two callsites need to be updated. At opts_parse_id(), assert value !=
NULL. At has_help_option(), use the NULL value to keep the existing
behavior of rejecting anything but 'help'.
The unit tests are updated accordingly.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/qemu/option_int.h | 1 +
tests/unit/test-qemu-opts.c | 9 ++--
util/qemu-option.c | 104 ++++++++++++++++++++----------------
3 files changed, 63 insertions(+), 51 deletions(-)
diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h
index 5dd9a5162d..c31bd6e3ab 100644
--- a/include/qemu/option_int.h
+++ b/include/qemu/option_int.h
@@ -32,6 +32,7 @@
struct QemuOpt {
char *name;
char *str;
+ bool is_help;
const QemuOptDesc *desc;
union {
diff --git a/tests/unit/test-qemu-opts.c b/tests/unit/test-qemu-opts.c
index 07ede50920..7cedfba332 100644
--- a/tests/unit/test-qemu-opts.c
+++ b/tests/unit/test-qemu-opts.c
@@ -777,16 +777,15 @@ static void test_has_help_option_with_value(void)
};
int i;
QemuOpts *opts;
+ Error *err = NULL;
for (i = 0; i < ARRAY_SIZE(test); i++) {
/* reject all */
g_assert_cmpint(has_help_option(test[i].params), ==, false);
- /* accept all */
- opts = qemu_opts_parse(&opts_list_03, test[i].params, false,
- &error_abort);
- g_assert_cmpint(qemu_opt_has_help_opt(opts), ==, true);
- qemu_opts_del(opts);
+ opts = qemu_opts_parse(&opts_list_03, test[i].params, false, &err);
+ error_free_or_abort(&err);
+ g_assert(!opts);
}
}
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 8ac848bbc0..483e6cbf70 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -331,7 +331,7 @@ bool qemu_opt_has_help_opt(QemuOpts *opts)
QemuOpt *opt;
QTAILQ_FOREACH_REVERSE(opt, &opts->head, next) {
- if (is_help_option(opt->name)) {
+ if (opt->is_help) {
return true;
}
}
@@ -499,6 +499,7 @@ static QemuOpt *opt_create(QemuOpts *opts, const char *name, char *value)
opt->name = g_strdup(name);
opt->str = value;
opt->opts = opts;
+ opt->is_help = is_help_option(name);
QTAILQ_INSERT_TAIL(&opts->head, opt, next);
return opt;
@@ -760,12 +761,10 @@ void qemu_opts_print(QemuOpts *opts, const char *separator)
static const char *get_opt_name_value(const char *params,
const char *firstname,
- bool *help_wanted,
char **name, char **value)
{
const char *p;
size_t len;
- bool is_help = false;
len = strcspn(params, "=,");
if (params[len] != '=') {
@@ -776,22 +775,6 @@ static const char *get_opt_name_value(const char *params,
p = get_opt_value(params, value);
} else {
p = get_opt_name(params, name, len);
-
- /*
- * Short-form flags (i.e. without any value) are not
- * supported, except for two cases:
- */
-
- /* the 'help' or '?' flag */
- is_help = is_help_option(*name);
- if (is_help) {
- *value = g_strdup("on");
- }
-
- /* a missing, non-implicit key, i.e. a single comma ',' */
- if (g_str_equal(*name, "") && *p == ',') {
- *value = g_strdup("on");
- }
}
} else {
/* found "foo=bar,more" */
@@ -802,18 +785,46 @@ static const char *get_opt_name_value(const char *params,
}
assert(!*p || *p == ',');
- if (help_wanted && is_help) {
- *help_wanted = true;
- }
if (*p == ',') {
p++;
}
return p;
}
+/*
+ * Short-form parameters (i.e. without any value) are not
+ * supported, except for two cases:
+ *
+ * - the 'help' or '?' flag
+ * - the empty, non-implicit key combined with an empty
+ * non-implicit value, i.e. a single comma ','
+ *
+ * The rest of the code is not prepared to deal with empty values, set
+ * the special cases to "on".
+ */
+static bool opt_short_form_compat(QemuOpt *opt, Error **errp)
+{
+ if (opt->str) {
+ /*
+ * Validate the 'help' parameter at this point because the
+ * opt_validate() routine can be reentrant and this very check
+ * would fail once the "on" value is set below.
+ */
+ if (opt->is_help) {
+ error_setg(errp, "Parameter '%s' doesn't take any values",
+ opt->name);
+ return false;
+ }
+ } else {
+ if (opt->is_help || g_str_equal(opt->name, "")) {
+ opt->str = g_strdup("on");
+ }
+ }
+ return true;
+}
+
static bool opts_do_parse(QemuOpts *opts, const char *params,
- const char *firstname,
- bool *help_wanted, Error **errp)
+ const char *firstname, Error **errp)
{
const char *p;
QemuOpt *opt;
@@ -822,10 +833,7 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
g_autofree char *option = NULL;
g_autofree char *value = NULL;
- p = get_opt_name_value(p, firstname, help_wanted, &option, &value);
- if (help_wanted && *help_wanted) {
- return false;
- }
+ p = get_opt_name_value(p, firstname, &option, &value);
firstname = NULL;
if (!strcmp(option, "id")) {
@@ -833,10 +841,15 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
}
opt = opt_create(opts, option, g_steal_pointer(&value));
- if (!opt_validate(opt, errp)) {
+ if (!opt_short_form_compat(opt, errp) ||
+ !opt_validate(opt, errp)) {
qemu_opt_del(opt);
return false;
}
+
+ if (opt->is_help) {
+ return true;
+ }
}
return true;
@@ -850,8 +863,9 @@ 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, NULL, &name, &value);
+ p = get_opt_name_value(p, NULL, &name, &value);
if (!strcmp(name, "id")) {
+ assert(value);
return g_steal_pointer(&value);
}
}
@@ -862,14 +876,13 @@ static char *opts_parse_id(const char *params)
bool has_help_option(const char *params)
{
const char *p;
- bool ret = false;
for (p = params; *p;) {
g_autofree char *name = NULL;
g_autofree char *value = NULL;
- p = get_opt_name_value(p, NULL, &ret, &name, &value);
- if (ret) {
+ p = get_opt_name_value(p, NULL, &name, &value);
+ if (is_help_option(name) && !value) {
return true;
}
}
@@ -886,11 +899,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, NULL, errp);
+ return opts_do_parse(opts, params, firstname, errp);
}
static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
- bool permit_abbrev, bool *help_wanted, Error **errp)
+ bool permit_abbrev, Error **errp)
{
const char *firstname;
char *id = opts_parse_id(params);
@@ -905,7 +918,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
return NULL;
}
- if (!opts_do_parse(opts, params, firstname, help_wanted, errp)) {
+ if (!opts_do_parse(opts, params, firstname, errp)) {
qemu_opts_del(opts);
return NULL;
}
@@ -923,7 +936,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
bool permit_abbrev, Error **errp)
{
- return opts_parse(list, params, permit_abbrev, NULL, errp);
+ return opts_parse(list, params, permit_abbrev, errp);
}
/**
@@ -939,17 +952,16 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
{
Error *err = NULL;
QemuOpts *opts;
- bool help_wanted = false;
- opts = opts_parse(list, params, permit_abbrev,
- opts_accepts_any(list) ? NULL : &help_wanted, &err);
+ opts = opts_parse(list, params, permit_abbrev, &err);
if (!opts) {
- assert(!!err + !!help_wanted == 1);
- if (help_wanted) {
- qemu_opts_print_help(list, true);
- } else {
- error_report_err(err);
- }
+ error_report_err(err);
+ return NULL;
+ }
+
+ if (qemu_opt_has_help_opt(opts) && !opts_accepts_any(list)) {
+ qemu_opts_print_help(list, true);
+ return NULL;
}
return opts;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 04/11] qemu-option: Fix 'help' parameter parsing
2026-09-30 22:10 ` [PATCH v2 04/11] qemu-option: Fix 'help' parameter parsing Fabiano Rosas
@ 2026-10-01 9:42 ` marcandre.lureau
2026-10-01 15:43 ` Fabiano Rosas
0 siblings, 1 reply; 31+ messages in thread
From: marcandre.lureau @ 2026-10-01 9:42 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé
> Reject invalid values such as help= and help=foo.
>
> Reject valid, but not applicable values such has help=on and help=off.
>
> To achieve the above:
>
> Add an 'is_help' member to QemuOpt so that information can be stored
> while creating the option. This allows moving the help handling out of
> get_opt_name_value(), which is necessary because that function doesn't
> take an Error.
>
> At the caller, opts_do_parse(), let the QemuOpt be created with the
> name and value fields as they come out of get_opt_name_value().
>
> With a new routine, validate the new QemuOpt against the 'help'
> parameter rules (to fix the bug), then let the existing opt_validate()
> reject any NULL values. The two validations need to be separate
> because in between there needs to be a setting of opt->str to "on"
> whenever the special short-form parameters require it.
>
> The "on" value is required because of the conversions of QemuOpts
> from/to QDict since QDict cannot take a NULL value. The conversions
> also cause opt_validate() to be reentrant via qemu_opt_set(), so the
> initial validation of 'help' needs to be out of that function,
> otherwise there's no way to distinguish an user-set help=on vs. the
> help=on set for compatibility with QDict (the "in between" part from
> the previous paragraph).
>
> The passing around of help_wanted becomes redundant since opt->is_help
> can be checked throughout. Remove that argument everywhere.
>
> Stop returning false from opts_do_parse() when is_help=true. Only
> return false when there's an error and let the callers decide what to
> do when is_help=true. However, still break the loop when is_help=true
> to keep the behavior of stopping parsing once the 'help' parameter is
> seen.
>
> At qemu_opts_parse_noisily(), due to the change of return value
> mentioned above, it becomes clear when to report the error and when to
> print the help text. Use qemu_opt_has_help_opt() to obtain the
> is_help information from QemuOpt.
>
> Since get_opt_name_value() can now return while value=NULL, the other
> two callsites need to be updated. At opts_parse_id(), assert value !=
> NULL. At has_help_option(), use the NULL value to keep the existing
> behavior of rejecting anything but 'help'.
>
> The unit tests are updated accordingly.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> Message-ID: <20260930221105.2262063-5-farosas@suse.de>
>
> diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h
> index 5dd9a5162db9..c31bd6e3aba1 100644
> --- a/include/qemu/option_int.h
> +++ b/include/qemu/option_int.h
> @@ -32,6 +32,7 @@
> struct QemuOpt {
> char *name;
> char *str;
> + bool is_help;
>
> const QemuOptDesc *desc;
> union {
> diff --git a/tests/unit/test-qemu-opts.c b/tests/unit/test-qemu-opts.c
> index 07ede509204a..7cedfba33278 100644
> --- a/tests/unit/test-qemu-opts.c
> +++ b/tests/unit/test-qemu-opts.c
> @@ -777,16 +777,15 @@ static void test_has_help_option_with_value(void)
> };
> int i;
> QemuOpts *opts;
> + Error *err = NULL;
>
> for (i = 0; i < ARRAY_SIZE(test); i++) {
> /* reject all */
> g_assert_cmpint(has_help_option(test[i].params), ==, false);
>
> - /* accept all */
> - opts = qemu_opts_parse(&opts_list_03, test[i].params, false,
> - &error_abort);
> - g_assert_cmpint(qemu_opt_has_help_opt(opts), ==, true);
> - qemu_opts_del(opts);
> + opts = qemu_opts_parse(&opts_list_03, test[i].params, false, &err);
> + error_free_or_abort(&err);
> + g_assert(!opts);
> }
> }
>
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 8ac848bbc06f..483e6cbf70a1 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -331,7 +331,7 @@ bool qemu_opt_has_help_opt(QemuOpts *opts)
> QemuOpt *opt;
>
> QTAILQ_FOREACH_REVERSE(opt, &opts->head, next) {
> - if (is_help_option(opt->name)) {
> + if (opt->is_help) {
> return true;
> }
> }
> @@ -499,6 +499,7 @@ static QemuOpt *opt_create(QemuOpts *opts, const char *name, char *value)
> opt->name = g_strdup(name);
> opt->str = value;
> opt->opts = opts;
> + opt->is_help = is_help_option(name);
> QTAILQ_INSERT_TAIL(&opts->head, opt, next);
>
> return opt;
> @@ -760,12 +761,10 @@ void qemu_opts_print(QemuOpts *opts, const char *separator)
>
> static const char *get_opt_name_value(const char *params,
> const char *firstname,
> - bool *help_wanted,
> char **name, char **value)
> {
> const char *p;
> size_t len;
> - bool is_help = false;
>
> len = strcspn(params, "=,");
> if (params[len] != '=') {
> @@ -776,22 +775,6 @@ static const char *get_opt_name_value(const char *params,
> p = get_opt_value(params, value);
> } else {
> p = get_opt_name(params, name, len);
> -
> - /*
> - * Short-form flags (i.e. without any value) are not
> - * supported, except for two cases:
> - */
> -
> - /* the 'help' or '?' flag */
> - is_help = is_help_option(*name);
> - if (is_help) {
> - *value = g_strdup("on");
> - }
> -
> - /* a missing, non-implicit key, i.e. a single comma ',' */
> - if (g_str_equal(*name, "") && *p == ',') {
> - *value = g_strdup("on");
> - }
> }
> } else {
> /* found "foo=bar,more" */
> @@ -802,18 +785,46 @@ static const char *get_opt_name_value(const char *params,
> }
>
> assert(!*p || *p == ',');
> - if (help_wanted && is_help) {
> - *help_wanted = true;
> - }
> if (*p == ',') {
> p++;
> }
> return p;
> }
>
> +/*
> + * Short-form parameters (i.e. without any value) are not
> + * supported, except for two cases:
> + *
> + * - the 'help' or '?' flag
> + * - the empty, non-implicit key combined with an empty
> + * non-implicit value, i.e. a single comma ','
> + *
> + * The rest of the code is not prepared to deal with empty values, set
> + * the special cases to "on".
> + */
> +static bool opt_short_form_compat(QemuOpt *opt, Error **errp)
> +{
> + if (opt->str) {
> + /*
> + * Validate the 'help' parameter at this point because the
> + * opt_validate() routine can be reentrant and this very check
> + * would fail once the "on" value is set below.
> + */
> + if (opt->is_help) {
> + error_setg(errp, "Parameter '%s' doesn't take any values",
> + opt->name);
> + return false;
> + }
> + } else {
> + if (opt->is_help || g_str_equal(opt->name, "")) {
> + opt->str = g_strdup("on");
> + }
> + }
> + return true;
> +}
> +
> static bool opts_do_parse(QemuOpts *opts, const char *params,
> - const char *firstname,
> - bool *help_wanted, Error **errp)
> + const char *firstname, Error **errp)
> {
> const char *p;
> QemuOpt *opt;
> @@ -822,10 +833,7 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
> g_autofree char *option = NULL;
> g_autofree char *value = NULL;
>
> - p = get_opt_name_value(p, firstname, help_wanted, &option, &value);
> - if (help_wanted && *help_wanted) {
> - return false;
> - }
> + p = get_opt_name_value(p, firstname, &option, &value);
> firstname = NULL;
>
> if (!strcmp(option, "id")) {
> @@ -833,10 +841,15 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
> }
>
> opt = opt_create(opts, option, g_steal_pointer(&value));
> - if (!opt_validate(opt, errp)) {
> + if (!opt_short_form_compat(opt, errp) ||
> + !opt_validate(opt, errp)) {
opt_validate() will reject "help" as an invalid parameter beofre the is_help check below. Change it to?:
opt = opt_create(opts, option, g_steal_pointer(&value));
if (!opt_short_form_compat(opt, errp)) {
qemu_opt_del(opt);
return false;
}
if (opt->is_help) {
return true;
}
if (!opt_validate(opt, errp)) {
qemu_opt_del(opt);
return false;
}
> qemu_opt_del(opt);
> return false;
> }
> +
> + if (opt->is_help) {
> + return true;
> + }
> }
>
> return true;
> @@ -850,8 +863,9 @@ 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, NULL, &name, &value);
> + p = get_opt_name_value(p, NULL, &name, &value);
> if (!strcmp(name, "id")) {
> + assert(value);
> return g_steal_pointer(&value);
> }
> }
> @@ -862,14 +876,13 @@ static char *opts_parse_id(const char *params)
> bool has_help_option(const char *params)
> {
> const char *p;
> - bool ret = false;
>
> for (p = params; *p;) {
> g_autofree char *name = NULL;
> g_autofree char *value = NULL;
>
> - p = get_opt_name_value(p, NULL, &ret, &name, &value);
> - if (ret) {
> + p = get_opt_name_value(p, NULL, &name, &value);
> + if (is_help_option(name) && !value) {
> return true;
> }
> }
> @@ -886,11 +899,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, NULL, errp);
> + return opts_do_parse(opts, params, firstname, errp);
> }
>
> static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
> - bool permit_abbrev, bool *help_wanted, Error **errp)
> + bool permit_abbrev, Error **errp)
> {
> const char *firstname;
> char *id = opts_parse_id(params);
> @@ -905,7 +918,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
> return NULL;
> }
>
> - if (!opts_do_parse(opts, params, firstname, help_wanted, errp)) {
> + if (!opts_do_parse(opts, params, firstname, errp)) {
> qemu_opts_del(opts);
> return NULL;
> }
> @@ -923,7 +936,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
> QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
> bool permit_abbrev, Error **errp)
> {
> - return opts_parse(list, params, permit_abbrev, NULL, errp);
> + return opts_parse(list, params, permit_abbrev, errp);
> }
>
> /**
> @@ -939,17 +952,16 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
> {
> Error *err = NULL;
> QemuOpts *opts;
> - bool help_wanted = false;
>
> - opts = opts_parse(list, params, permit_abbrev,
> - opts_accepts_any(list) ? NULL : &help_wanted, &err);
> + opts = opts_parse(list, params, permit_abbrev, &err);
> if (!opts) {
> - assert(!!err + !!help_wanted == 1);
> - if (help_wanted) {
> - qemu_opts_print_help(list, true);
> - } else {
> - error_report_err(err);
> - }
> + error_report_err(err);
> + return NULL;
> + }
> +
> + if (qemu_opt_has_help_opt(opts) && !opts_accepts_any(list)) {
> + qemu_opts_print_help(list, true);
qemu_opts_del(opts) missing
--
Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 04/11] qemu-option: Fix 'help' parameter parsing
2026-10-01 9:42 ` marcandre.lureau
@ 2026-10-01 15:43 ` Fabiano Rosas
0 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-10-01 15:43 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé
marcandre.lureau@redhat.com writes:
>> Reject invalid values such as help= and help=foo.
>>
>> Reject valid, but not applicable values such has help=on and help=off.
>>
>> To achieve the above:
>>
>> Add an 'is_help' member to QemuOpt so that information can be stored
>> while creating the option. This allows moving the help handling out of
>> get_opt_name_value(), which is necessary because that function doesn't
>> take an Error.
>>
>> At the caller, opts_do_parse(), let the QemuOpt be created with the
>> name and value fields as they come out of get_opt_name_value().
>>
>> With a new routine, validate the new QemuOpt against the 'help'
>> parameter rules (to fix the bug), then let the existing opt_validate()
>> reject any NULL values. The two validations need to be separate
>> because in between there needs to be a setting of opt->str to "on"
>> whenever the special short-form parameters require it.
>>
>> The "on" value is required because of the conversions of QemuOpts
>> from/to QDict since QDict cannot take a NULL value. The conversions
>> also cause opt_validate() to be reentrant via qemu_opt_set(), so the
>> initial validation of 'help' needs to be out of that function,
>> otherwise there's no way to distinguish an user-set help=on vs. the
>> help=on set for compatibility with QDict (the "in between" part from
>> the previous paragraph).
>>
>> The passing around of help_wanted becomes redundant since opt->is_help
>> can be checked throughout. Remove that argument everywhere.
>>
>> Stop returning false from opts_do_parse() when is_help=true. Only
>> return false when there's an error and let the callers decide what to
>> do when is_help=true. However, still break the loop when is_help=true
>> to keep the behavior of stopping parsing once the 'help' parameter is
>> seen.
>>
>> At qemu_opts_parse_noisily(), due to the change of return value
>> mentioned above, it becomes clear when to report the error and when to
>> print the help text. Use qemu_opt_has_help_opt() to obtain the
>> is_help information from QemuOpt.
>>
>> Since get_opt_name_value() can now return while value=NULL, the other
>> two callsites need to be updated. At opts_parse_id(), assert value !=
>> NULL. At has_help_option(), use the NULL value to keep the existing
>> behavior of rejecting anything but 'help'.
>>
>> The unit tests are updated accordingly.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> Message-ID: <20260930221105.2262063-5-farosas@suse.de>
>>
>> diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h
>> index 5dd9a5162db9..c31bd6e3aba1 100644
>> --- a/include/qemu/option_int.h
>> +++ b/include/qemu/option_int.h
>> @@ -32,6 +32,7 @@
>> struct QemuOpt {
>> char *name;
>> char *str;
>> + bool is_help;
>>
>> const QemuOptDesc *desc;
>> union {
>> diff --git a/tests/unit/test-qemu-opts.c b/tests/unit/test-qemu-opts.c
>> index 07ede509204a..7cedfba33278 100644
>> --- a/tests/unit/test-qemu-opts.c
>> +++ b/tests/unit/test-qemu-opts.c
>> @@ -777,16 +777,15 @@ static void test_has_help_option_with_value(void)
>> };
>> int i;
>> QemuOpts *opts;
>> + Error *err = NULL;
>>
>> for (i = 0; i < ARRAY_SIZE(test); i++) {
>> /* reject all */
>> g_assert_cmpint(has_help_option(test[i].params), ==, false);
>>
>> - /* accept all */
>> - opts = qemu_opts_parse(&opts_list_03, test[i].params, false,
>> - &error_abort);
>> - g_assert_cmpint(qemu_opt_has_help_opt(opts), ==, true);
>> - qemu_opts_del(opts);
>> + opts = qemu_opts_parse(&opts_list_03, test[i].params, false, &err);
>> + error_free_or_abort(&err);
>> + g_assert(!opts);
>> }
>> }
>>
>> diff --git a/util/qemu-option.c b/util/qemu-option.c
>> index 8ac848bbc06f..483e6cbf70a1 100644
>> --- a/util/qemu-option.c
>> +++ b/util/qemu-option.c
>> @@ -331,7 +331,7 @@ bool qemu_opt_has_help_opt(QemuOpts *opts)
>> QemuOpt *opt;
>>
>> QTAILQ_FOREACH_REVERSE(opt, &opts->head, next) {
>> - if (is_help_option(opt->name)) {
>> + if (opt->is_help) {
>> return true;
>> }
>> }
>> @@ -499,6 +499,7 @@ static QemuOpt *opt_create(QemuOpts *opts, const char *name, char *value)
>> opt->name = g_strdup(name);
>> opt->str = value;
>> opt->opts = opts;
>> + opt->is_help = is_help_option(name);
>> QTAILQ_INSERT_TAIL(&opts->head, opt, next);
>>
>> return opt;
>> @@ -760,12 +761,10 @@ void qemu_opts_print(QemuOpts *opts, const char *separator)
>>
>> static const char *get_opt_name_value(const char *params,
>> const char *firstname,
>> - bool *help_wanted,
>> char **name, char **value)
>> {
>> const char *p;
>> size_t len;
>> - bool is_help = false;
>>
>> len = strcspn(params, "=,");
>> if (params[len] != '=') {
>> @@ -776,22 +775,6 @@ static const char *get_opt_name_value(const char *params,
>> p = get_opt_value(params, value);
>> } else {
>> p = get_opt_name(params, name, len);
>> -
>> - /*
>> - * Short-form flags (i.e. without any value) are not
>> - * supported, except for two cases:
>> - */
>> -
>> - /* the 'help' or '?' flag */
>> - is_help = is_help_option(*name);
>> - if (is_help) {
>> - *value = g_strdup("on");
>> - }
>> -
>> - /* a missing, non-implicit key, i.e. a single comma ',' */
>> - if (g_str_equal(*name, "") && *p == ',') {
>> - *value = g_strdup("on");
>> - }
>> }
>> } else {
>> /* found "foo=bar,more" */
>> @@ -802,18 +785,46 @@ static const char *get_opt_name_value(const char *params,
>> }
>>
>> assert(!*p || *p == ',');
>> - if (help_wanted && is_help) {
>> - *help_wanted = true;
>> - }
>> if (*p == ',') {
>> p++;
>> }
>> return p;
>> }
>>
>> +/*
>> + * Short-form parameters (i.e. without any value) are not
>> + * supported, except for two cases:
>> + *
>> + * - the 'help' or '?' flag
>> + * - the empty, non-implicit key combined with an empty
>> + * non-implicit value, i.e. a single comma ','
>> + *
>> + * The rest of the code is not prepared to deal with empty values, set
>> + * the special cases to "on".
>> + */
>> +static bool opt_short_form_compat(QemuOpt *opt, Error **errp)
>> +{
>> + if (opt->str) {
>> + /*
>> + * Validate the 'help' parameter at this point because the
>> + * opt_validate() routine can be reentrant and this very check
>> + * would fail once the "on" value is set below.
>> + */
>> + if (opt->is_help) {
>> + error_setg(errp, "Parameter '%s' doesn't take any values",
>> + opt->name);
>> + return false;
>> + }
>> + } else {
>> + if (opt->is_help || g_str_equal(opt->name, "")) {
>> + opt->str = g_strdup("on");
>> + }
>> + }
>> + return true;
>> +}
>> +
>> static bool opts_do_parse(QemuOpts *opts, const char *params,
>> - const char *firstname,
>> - bool *help_wanted, Error **errp)
>> + const char *firstname, Error **errp)
>> {
>> const char *p;
>> QemuOpt *opt;
>> @@ -822,10 +833,7 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
>> g_autofree char *option = NULL;
>> g_autofree char *value = NULL;
>>
>> - p = get_opt_name_value(p, firstname, help_wanted, &option, &value);
>> - if (help_wanted && *help_wanted) {
>> - return false;
>> - }
>> + p = get_opt_name_value(p, firstname, &option, &value);
>> firstname = NULL;
>>
>> if (!strcmp(option, "id")) {
>> @@ -833,10 +841,15 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
>> }
>>
>> opt = opt_create(opts, option, g_steal_pointer(&value));
>> - if (!opt_validate(opt, errp)) {
>> + if (!opt_short_form_compat(opt, errp) ||
>> + !opt_validate(opt, errp)) {
>
> opt_validate() will reject "help" as an invalid parameter beofre the is_help check below. Change it to?:
>
It does already happen before the series, I'll add a test case for that.
Is it something we want to fix? The situation is that 'help' was present
but the QemuOptsList declares a set of parameters of which 'help' is a
part of. I'm not sure if this is an oversight in that it wants to reject
unlisted parameters, but 'help' should have been special-cased or if the
API is that options that list a set of parameters must also list the
'help'.
> opt = opt_create(opts, option, g_steal_pointer(&value));
> if (!opt_short_form_compat(opt, errp)) {
> qemu_opt_del(opt);
> return false;
> }
> if (opt->is_help) {
> return true;
> }
> if (!opt_validate(opt, errp)) {
> qemu_opt_del(opt);
> return false;
> }
>
>> qemu_opt_del(opt);
>> return false;
>> }
>> +
>> + if (opt->is_help) {
>> + return true;
>> + }
>> }
>>
>> return true;
>> @@ -850,8 +863,9 @@ 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, NULL, &name, &value);
>> + p = get_opt_name_value(p, NULL, &name, &value);
>> if (!strcmp(name, "id")) {
>> + assert(value);
>> return g_steal_pointer(&value);
>> }
>> }
>> @@ -862,14 +876,13 @@ static char *opts_parse_id(const char *params)
>> bool has_help_option(const char *params)
>> {
>> const char *p;
>> - bool ret = false;
>>
>> for (p = params; *p;) {
>> g_autofree char *name = NULL;
>> g_autofree char *value = NULL;
>>
>> - p = get_opt_name_value(p, NULL, &ret, &name, &value);
>> - if (ret) {
>> + p = get_opt_name_value(p, NULL, &name, &value);
>> + if (is_help_option(name) && !value) {
>> return true;
>> }
>> }
>> @@ -886,11 +899,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, NULL, errp);
>> + return opts_do_parse(opts, params, firstname, errp);
>> }
>>
>> static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
>> - bool permit_abbrev, bool *help_wanted, Error **errp)
>> + bool permit_abbrev, Error **errp)
>> {
>> const char *firstname;
>> char *id = opts_parse_id(params);
>> @@ -905,7 +918,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
>> return NULL;
>> }
>>
>> - if (!opts_do_parse(opts, params, firstname, help_wanted, errp)) {
>> + if (!opts_do_parse(opts, params, firstname, errp)) {
>> qemu_opts_del(opts);
>> return NULL;
>> }
>> @@ -923,7 +936,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
>> QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
>> bool permit_abbrev, Error **errp)
>> {
>> - return opts_parse(list, params, permit_abbrev, NULL, errp);
>> + return opts_parse(list, params, permit_abbrev, errp);
>> }
>>
>> /**
>> @@ -939,17 +952,16 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
>> {
>> Error *err = NULL;
>> QemuOpts *opts;
>> - bool help_wanted = false;
>>
>> - opts = opts_parse(list, params, permit_abbrev,
>> - opts_accepts_any(list) ? NULL : &help_wanted, &err);
>> + opts = opts_parse(list, params, permit_abbrev, &err);
>> if (!opts) {
>> - assert(!!err + !!help_wanted == 1);
>> - if (help_wanted) {
>> - qemu_opts_print_help(list, true);
>> - } else {
>> - error_report_err(err);
>> - }
>> + error_report_err(err);
>> + return NULL;
>> + }
>> +
>> + if (qemu_opt_has_help_opt(opts) && !opts_accepts_any(list)) {
>> + qemu_opts_print_help(list, true);
>
> qemu_opts_del(opts) missing
Indeed. Thanks!
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 05/11] qemu-option: Add qemu_opts_parse_list
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
` (3 preceding siblings ...)
2026-09-30 22:10 ` [PATCH v2 04/11] qemu-option: Fix 'help' parameter parsing Fabiano Rosas
@ 2026-09-30 22:10 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-09-30 22:11 ` [PATCH v2 06/11] qemu-option: Change qemu_parse_opts() to take the group name Fabiano Rosas
` (5 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:10 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.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/qemu/option.h | 2 +
tests/unit/test-qemu-opts.c | 179 +++++++++++++++++++-----------------
util/qemu-option.c | 13 +++
util/qemu-sockets.c | 2 +-
4 files changed, 109 insertions(+), 87 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 7cedfba332..c4b6be312e 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,117 +431,118 @@ 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 key */
- opts = qemu_opts_parse(&opts_list_03, "an", true, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "an", true, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
g_assert_cmpstr(qemu_opt_get(opts, "implied"), ==, "an");
/* 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);
/* Implied value */
- opts = qemu_opts_parse(&opts_list_03, "an", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_03, "an", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Implied value, 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);
/* Implied value, negated key */
- opts = qemu_opts_parse(&opts_list_03, "noaus", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_03, "noaus", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Implied value, negated empty key */
- opts = qemu_opts_parse(&opts_list_03, "no", false, &err);
+ opts = qemu_opts_parse_list(&opts_list_03, "no", false, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Empty value */
- opts = qemu_opts_parse(&opts_list_03, "aus=", false, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "aus=", false, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
/* Implied key */
- opts = qemu_opts_parse(&opts_list_03, "an", true, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "an", true, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 1);
/* Implied key, implied value */
- opts = qemu_opts_parse(&opts_list_03, "an,noaus,noaus=", true, &err);
+ opts = qemu_opts_parse_list(&opts_list_03, "an,noaus,noaus=", true, &err);
error_free_or_abort(&err);
g_assert(!opts);
/* Implied key, empty value */
- opts = qemu_opts_parse(&opts_list_03, "an,noaus=", true, &error_abort);
+ opts = qemu_opts_parse_list(&opts_list_03, "an,noaus=", true, &error_abort);
g_assert_cmpuint(opts_count(opts), ==, 2);
g_assert_cmpstr(qemu_opt_get(opts, "implied"), ==, "an");
g_assert_cmpstr(qemu_opt_get(opts, "noaus"), ==, "");
@@ -555,13 +556,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);
@@ -574,59 +575,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);
@@ -639,18 +641,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);
@@ -660,11 +662,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);
@@ -674,10 +677,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);
@@ -685,47 +689,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);
@@ -752,13 +757,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);
@@ -783,7 +788,7 @@ static void test_has_help_option_with_value(void)
/* reject all */
g_assert_cmpint(has_help_option(test[i].params), ==, false);
- opts = qemu_opts_parse(&opts_list_03, test[i].params, false, &err);
+ opts = qemu_opts_parse_list(&opts_list_03, test[i].params, false, &err);
error_free_or_abort(&err);
g_assert(!opts);
}
@@ -916,8 +921,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);
@@ -942,9 +948,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 */
@@ -1001,7 +1007,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 483e6cbf70..218bb8ae58 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -939,6 +939,19 @@ QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
return opts_parse(list, params, permit_abbrev, 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] 31+ messages in thread* Re: [PATCH v2 05/11] qemu-option: Add qemu_opts_parse_list
2026-09-30 22:10 ` [PATCH v2 05/11] qemu-option: Add qemu_opts_parse_list Fabiano Rosas
@ 2026-10-01 9:42 ` marcandre.lureau
0 siblings, 0 replies; 31+ messages in thread
From: marcandre.lureau @ 2026-10-01 9:42 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé
On Wed, 30 Sep 2026 19:10:59 -0300, Fabiano Rosas <farosas@suse.de> 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.
>
> [...]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--
Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 06/11] qemu-option: Change qemu_parse_opts() to take the group name
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
` (4 preceding siblings ...)
2026-09-30 22:10 ` [PATCH v2 05/11] qemu-option: Add qemu_opts_parse_list Fabiano Rosas
@ 2026-09-30 22:11 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-09-30 22:11 ` [PATCH v2 07/11] qemu-option: Add qemu_opts_parse_list_noisily Fabiano Rosas
` (4 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:11 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Daniel P . Berrangé, 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.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 468a9fc247..87398cc8d0 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 218bb8ae58..7cb45f0710 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"
@@ -927,16 +928,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, errp);
+ QemuOptsList *list = qemu_find_opts_err(group, errp);
+
+ return qemu_opts_parse_list(list, params, permit_abbrev, errp);
}
/**
@@ -949,7 +952,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, errp);
}
/**
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 07/11] qemu-option: Add qemu_opts_parse_list_noisily
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
` (5 preceding siblings ...)
2026-09-30 22:11 ` [PATCH v2 06/11] qemu-option: Change qemu_parse_opts() to take the group name Fabiano Rosas
@ 2026-09-30 22:11 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-10-07 12:54 ` Eric Blake
2026-09-30 22:11 ` [PATCH v2 08/11] qemu-option: Use qemu_opts_parse_list_noisily where appropriate Fabiano Rosas
` (3 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Daniel P . Berrangé
Add a new function that takes a QemuOptsList pointer and expects
informative messages to be printed instead of returned in an Error
object. This makes the _noisily version simmetric with the
"non-noisily". The next patch will convert the callers to use the
appropriate version.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/qemu/option.h | 2 ++
util/qemu-option.c | 14 ++++++++++++++
2 files changed, 16 insertions(+)
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/util/qemu-option.c b/util/qemu-option.c
index 7cb45f0710..be9404ab7f 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -965,6 +965,20 @@ QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params,
*/
QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
bool permit_abbrev)
+{
+ return qemu_opts_parse_list_noisily(list, params, permit_abbrev);
+}
+
+/**
+ * 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)
{
Error *err = NULL;
QemuOpts *opts;
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 07/11] qemu-option: Add qemu_opts_parse_list_noisily
2026-09-30 22:11 ` [PATCH v2 07/11] qemu-option: Add qemu_opts_parse_list_noisily Fabiano Rosas
@ 2026-10-01 9:42 ` marcandre.lureau
2026-10-07 12:54 ` Eric Blake
1 sibling, 0 replies; 31+ messages in thread
From: marcandre.lureau @ 2026-10-01 9:42 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé
On Wed, 30 Sep 2026 19:11:01 -0300, Fabiano Rosas <farosas@suse.de> wrote:
> Add a new function that takes a QemuOptsList pointer and expects
> informative messages to be printed instead of returned in an Error
> object. This makes the _noisily version simmetric with the
> "non-noisily". The next patch will convert the callers to use the
> appropriate version.
>
>
> [...]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--
Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 07/11] qemu-option: Add qemu_opts_parse_list_noisily
2026-09-30 22:11 ` [PATCH v2 07/11] qemu-option: Add qemu_opts_parse_list_noisily Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
@ 2026-10-07 12:54 ` Eric Blake
1 sibling, 0 replies; 31+ messages in thread
From: Eric Blake @ 2026-10-07 12:54 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé
On Wed, Sep 30, 2026 at 07:11:01PM -0300, Fabiano Rosas wrote:
> Add a new function that takes a QemuOptsList pointer and expects
> informative messages to be printed instead of returned in an Error
> object. This makes the _noisily version simmetric with the
symmetric
> "non-noisily". The next patch will convert the callers to use the
> appropriate version.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 08/11] qemu-option: Use qemu_opts_parse_list_noisily where appropriate
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
` (6 preceding siblings ...)
2026-09-30 22:11 ` [PATCH v2 07/11] qemu-option: Add qemu_opts_parse_list_noisily Fabiano Rosas
@ 2026-09-30 22:11 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-09-30 22:11 ` [PATCH v2 09/11] qemu-option: Change qemu_opts_parse_noisily() to take the group Fabiano Rosas
` (2 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:11 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Daniel P . Berrangé, Kevin Wolf,
Hanna Reitz, Dr. David Alan Gilbert, Jason Wang, Eric Blake,
Vladimir Sementsov-Ogievskiy, Paolo Bonzini, Stefan Berger,
Lukas Straub, Marc-André Lureau
Change the callers of qemu_opts_parse_noisily() that currently take a
QemuOptsList pointer to call qemu_opts_parse_list_noisily().
This is done to free up the qemu_opts_parse_noisily() version to be
used to take the group name instead.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
block/monitor/block-hmp-cmds.c | 2 +-
monitor/hmp.c | 2 +-
net/net.c | 2 +-
qemu-img.c | 8 ++++----
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 +-
13 files changed, 26 insertions(+), 25 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/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..c645de6462 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -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 8185822daa..dc2d9e76bc 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -1204,7 +1204,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 87398cc8d0..bf5146d1c0 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -3277,7 +3277,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;
@@ -3292,7 +3292,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);
}
@@ -3644,7 +3644,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;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 08/11] qemu-option: Use qemu_opts_parse_list_noisily where appropriate
2026-09-30 22:11 ` [PATCH v2 08/11] qemu-option: Use qemu_opts_parse_list_noisily where appropriate Fabiano Rosas
@ 2026-10-01 9:42 ` marcandre.lureau
0 siblings, 0 replies; 31+ messages in thread
From: marcandre.lureau @ 2026-10-01 9:42 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé,
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 Wed, 30 Sep 2026 19:11:02 -0300, Fabiano Rosas <farosas@suse.de> wrote:
> Change the callers of qemu_opts_parse_noisily() that currently take a
> QemuOptsList pointer to call qemu_opts_parse_list_noisily().
>
> This is done to free up the qemu_opts_parse_noisily() version to be
> used to take the group name instead.
>
>
> [...]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--
Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 09/11] qemu-option: Change qemu_opts_parse_noisily() to take the group
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
` (7 preceding siblings ...)
2026-09-30 22:11 ` [PATCH v2 08/11] qemu-option: Use qemu_opts_parse_list_noisily where appropriate Fabiano Rosas
@ 2026-09-30 22:11 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-10-07 12:54 ` Eric Blake
2026-09-30 22:11 ` [PATCH v2 10/11] qemu-option: Check for NULL list at qemu_opts_parse() Fabiano Rosas
2026-09-30 22:11 ` [PATCH v2 11/11] qemu-option: Remove a few instances of noisily parsing Fabiano Rosas
10 siblings, 2 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:11 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Daniel P . Berrangé, Stefan Hajnoczi,
Kevin Wolf, Hanna Reitz, Marc-André Lureau, Paolo Bonzini,
Alex Bennée, Pierrick Bouvier, Alexandre Iooss
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.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 | 12 +++++-
13 files changed, 52 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 c645de6462..5e58ccc587 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 bf5146d1c0..4a3fa80c5c 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1871,8 +1871,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);
}
@@ -1898,8 +1897,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);
}
@@ -2490,7 +2488,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);
@@ -2990,8 +2988,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;
@@ -3016,8 +3013,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;
@@ -3076,8 +3072,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;
@@ -3130,8 +3125,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;
@@ -3259,15 +3253,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;
@@ -3371,9 +3363,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: {
@@ -3405,24 +3396,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;
@@ -3445,8 +3433,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");
@@ -3488,8 +3475,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);
}
}
@@ -3505,12 +3491,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) {
@@ -3524,8 +3508,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);
}
@@ -3547,8 +3530,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);
}
@@ -3564,15 +3546,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);
}
@@ -3621,7 +3601,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);
}
@@ -3651,8 +3631,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);
}
@@ -3684,8 +3663,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);
}
@@ -3715,8 +3693,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 be9404ab7f..7c42b4feda 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -956,16 +956,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_noisily(list, params, permit_abbrev);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 09/11] qemu-option: Change qemu_opts_parse_noisily() to take the group
2026-09-30 22:11 ` [PATCH v2 09/11] qemu-option: Change qemu_opts_parse_noisily() to take the group Fabiano Rosas
@ 2026-10-01 9:42 ` marcandre.lureau
2026-10-07 12:54 ` Eric Blake
1 sibling, 0 replies; 31+ messages in thread
From: marcandre.lureau @ 2026-10-01 9:42 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé,
Stefan Hajnoczi, Kevin Wolf, Hanna Reitz, Marc-André Lureau,
Paolo Bonzini, Alex Bennée, Pierrick Bouvier,
Alexandre Iooss
On Wed, 30 Sep 2026 19:11:03 -0300, Fabiano Rosas <farosas@suse.de> 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.
>
> [...]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--
Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 09/11] qemu-option: Change qemu_opts_parse_noisily() to take the group
2026-09-30 22:11 ` [PATCH v2 09/11] qemu-option: Change qemu_opts_parse_noisily() to take the group Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
@ 2026-10-07 12:54 ` Eric Blake
1 sibling, 0 replies; 31+ messages in thread
From: Eric Blake @ 2026-10-07 12:54 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé,
Stefan Hajnoczi, Kevin Wolf, Hanna Reitz, Marc-André Lureau,
Paolo Bonzini, Alex Bennée, Pierrick Bouvier,
Alexandre Iooss
On Wed, Sep 30, 2026 at 07:11:03PM -0300, Fabiano Rosas wrote:
> Pass the group name as argument to qemu_opts_parse_noisily() to make
> it simmetric with qemu_opts_parse().
symmetric
>
> 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.
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 10/11] qemu-option: Check for NULL list at qemu_opts_parse()
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
` (8 preceding siblings ...)
2026-09-30 22:11 ` [PATCH v2 09/11] qemu-option: Change qemu_opts_parse_noisily() to take the group Fabiano Rosas
@ 2026-09-30 22:11 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
2026-09-30 22:11 ` [PATCH v2 11/11] qemu-option: Remove a few instances of noisily parsing Fabiano Rosas
10 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Daniel P . Berrangé
Now that qemu_opts_parse_noisily() is protected against NULL list, do
the same for the non noisy version.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 7c42b4feda..83fa2399d3 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -939,6 +939,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] 31+ messages in thread* [PATCH v2 11/11] qemu-option: Remove a few instances of noisily parsing
2026-09-30 22:10 [PATCH v2 00/11] qemu-options: Spring cleanup Fabiano Rosas
` (9 preceding siblings ...)
2026-09-30 22:11 ` [PATCH v2 10/11] qemu-option: Check for NULL list at qemu_opts_parse() Fabiano Rosas
@ 2026-09-30 22:11 ` Fabiano Rosas
2026-10-01 9:42 ` marcandre.lureau
10 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-09-30 22:11 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Daniel P . Berrangé, Kevin Wolf,
Hanna Reitz, Stefan Berger, Dr. David Alan Gilbert, Jason Wang,
Alex Bennée, Pierrick Bouvier, Alexandre Iooss,
Paolo Bonzini, Lukas Straub
With the removal of the support for short-form flags, the only other
source of "noise" in the qemu_parse_*noisily() functions aside from
errors is the printing of the help text.
Therefore, the only difference from the _noisily versions to the
normal ones is this block:
if (qemu_opt_has_help_opt(opts) && !opts_accepts_any(list)) {
qemu_opts_print_help(list, true);
return NULL;
}
Stop calling qemu_parse_*noisily() in any case where it's certain that
opts_accepts_any(list) returns true, i.e. when list->desc is empty.
This creates some instances of the pattern: error_report ->
exit(1). Use &error_fatal instead.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
block/monitor/block-hmp-cmds.c | 4 ++--
include/system/tpm.h | 3 ++-
monitor/hmp.c | 4 +++-
net/net.c | 4 +---
plugins/loader.c | 5 +---
qemu-img.c | 5 +++-
qemu-io-cmds.c | 2 +-
qemu-io.c | 7 ++----
system/tpm.c | 8 +++----
system/vl.c | 42 +++++++++++++++-------------------
tests/unit/test-replication.c | 17 +++++++++++---
util/qemu-option.c | 3 +++
12 files changed, 55 insertions(+), 49 deletions(-)
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index d94a07314f..93b622227c 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -63,9 +63,9 @@ static void hmp_drive_add_node(MonitorHMP *hmp, const char *optstr)
QDict *qdict;
Error *err = NULL;
- opts = qemu_opts_parse_list_noisily(&qemu_drive_opts, optstr, false);
+ opts = qemu_opts_parse_list(&qemu_drive_opts, optstr, false, &err);
if (!opts) {
- return;
+ goto out;
}
qdict = qemu_opts_to_qdict(opts, NULL);
diff --git a/include/system/tpm.h b/include/system/tpm.h
index 874068d19a..4132a3a641 100644
--- a/include/system/tpm.h
+++ b/include/system/tpm.h
@@ -17,7 +17,8 @@
#ifdef CONFIG_TPM
-int tpm_config_parse(QemuOptsList *opts_list, const char *optstr);
+bool tpm_config_parse(QemuOptsList *opts_list, const char *optstr,
+ Error **errp);
int tpm_init(void);
void tpm_cleanup(void);
diff --git a/monitor/hmp.c b/monitor/hmp.c
index fc32cfd1ff..6a6be8f12f 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -883,6 +883,7 @@ static QDict *monitor_parse_arguments(MonitorHMP *mon,
{
QemuOptsList *opts_list;
QemuOpts *opts;
+ Error *err = NULL;
opts_list = qemu_find_opts(key);
if (!opts_list || opts_list->desc->name) {
@@ -897,8 +898,9 @@ static QDict *monitor_parse_arguments(MonitorHMP *mon,
if (get_str(buf, sizeof(buf), &p) < 0) {
goto fail;
}
- opts = qemu_opts_parse_list_noisily(opts_list, buf, true);
+ opts = qemu_opts_parse_list(opts_list, buf, true, &err);
if (!opts) {
+ error_report_err(err);
goto fail;
}
qemu_opts_to_qdict(opts, qdict);
diff --git a/net/net.c b/net/net.c
index 37401ab2b4..09ed34557d 100644
--- a/net/net.c
+++ b/net/net.c
@@ -2029,9 +2029,7 @@ void netdev_parse_modern(const char *optstr)
void net_client_parse(QemuOptsList *opts_list, const char *optstr)
{
- if (!qemu_opts_parse_list_noisily(opts_list, optstr, true)) {
- exit(1);
- }
+ qemu_opts_parse_list(opts_list, optstr, true, &error_fatal);
}
/* From FreeBSD */
diff --git a/plugins/loader.c b/plugins/loader.c
index 1808679c82..db6ae6d1f4 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -147,10 +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("plugin", optstr, true);
- if (opts == NULL) {
- exit(1);
- }
+ opts = qemu_opts_parse("plugin", optstr, true, &error_fatal);
arg.head = head;
arg.curr = NULL;
qemu_opt_foreach(opts, plugin_add, &arg, &error_fatal);
diff --git a/qemu-img.c b/qemu-img.c
index 5e58ccc587..18501f7fb9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -354,12 +354,15 @@ static BlockBackend *img_open(bool image_opts,
BlockBackend *blk;
if (image_opts) {
QemuOpts *opts;
+ Error *err = NULL;
+
if (fmt) {
error_report("--image-opts and --format are mutually exclusive");
return NULL;
}
- opts = qemu_opts_parse_noisily("source", filename, true);
+ opts = qemu_opts_parse("source", filename, true, &err);
if (!opts) {
+ error_report_err(err);
return NULL;
}
blk = img_open_opts(filename, opts, flags, writethrough, quiet,
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 6c8c4c9540..fa52fbb900 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_list_noisily(&reopen_opts, optarg, 0)) {
+ if (!qemu_opts_parse_list(&reopen_opts, optarg, 0, errp)) {
qemu_opts_reset(&reopen_opts);
return -EINVAL;
}
diff --git a/qemu-io.c b/qemu-io.c
index db0ee499e4..837e5048ba 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -659,11 +659,8 @@ int main(int argc, char **argv)
if ((argc - optind) == 1) {
if (imageOpts) {
QemuOpts *qopts = NULL;
- qopts = qemu_opts_parse_list_noisily(&file_opts, argv[optind],
- false);
- if (!qopts) {
- exit(1);
- }
+ qopts = qemu_opts_parse_list(&file_opts, argv[optind], false,
+ &error_fatal);
opts = qemu_opts_to_qdict(qopts, NULL);
if (openfile(NULL, flags, writethrough, force_share, opts)) {
exit(1);
diff --git a/system/tpm.c b/system/tpm.c
index f0f5a64d9a..31aeaa5cc6 100644
--- a/system/tpm.c
+++ b/system/tpm.c
@@ -176,7 +176,7 @@ int tpm_init(void)
* Parse the TPM configuration options.
* To display all available TPM backends the user may use '-tpmdev help'
*/
-int tpm_config_parse(QemuOptsList *opts_list, const char *optstr)
+bool tpm_config_parse(QemuOptsList *opts_list, const char *optstr, Error **errp)
{
QemuOpts *opts;
@@ -184,11 +184,11 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optstr)
tpm_display_backend_drivers();
exit(EXIT_SUCCESS);
}
- opts = qemu_opts_parse_list_noisily(opts_list, optstr, true);
+ opts = qemu_opts_parse_list(opts_list, optstr, true, errp);
if (!opts) {
- return -1;
+ return false;
}
- return 0;
+ return true;
}
/*
diff --git a/system/vl.c b/system/vl.c
index 4a3fa80c5c..8382bb902b 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1871,11 +1871,7 @@ static void object_option_parse(const char *str)
v = qobject_input_visitor_new(obj);
qobject_unref(obj);
} else {
- opts = qemu_opts_parse_noisily("object", str, true);
- if (!opts) {
- exit(1);
- }
-
+ opts = qemu_opts_parse("object", str, true, &error_fatal);
type = qemu_opt_get(opts, "qom-type");
if (!type) {
error_report(QERR_MISSING_PARAMETER, "qom-type");
@@ -2483,12 +2479,15 @@ static void configure_accelerators(const char *progname)
accel_list = g_strsplit(accelerators, ":", 0);
for (tmp = accel_list; *tmp; tmp++) {
+ Error *err = NULL;
/*
* Filter invalid accelerators here, to prevent obscenities
* such as "-machine accel=tcg,,thread=single".
*/
if (accel_find(*tmp)) {
- qemu_opts_parse_noisily("accel", *tmp, true);
+ if (!qemu_opts_parse("accel", *tmp, true, &err)) {
+ error_report_err(err);
+ }
} else {
init_failed = true;
error_report("invalid accelerator %s", *tmp);
@@ -3013,9 +3012,7 @@ void qemu_init(int argc, char **argv)
replay_add_blocker("-snapshot");
break;
case QEMU_OPTION_numa:
- if (!qemu_opts_parse_noisily("numa", optarg, true)) {
- exit(1);
- }
+ qemu_opts_parse("numa", optarg, true, &error_fatal);
break;
case QEMU_OPTION_display:
parse_display(optarg);
@@ -3131,9 +3128,7 @@ void qemu_init(int argc, char **argv)
break;
#ifdef CONFIG_TPM
case QEMU_OPTION_tpmdev:
- if (tpm_config_parse(qemu_find_opts("tpmdev"), optarg) < 0) {
- exit(1);
- }
+ tpm_config_parse(qemu_find_opts("tpmdev"), optarg, &error_fatal);
break;
#endif
case QEMU_OPTION_mempath:
@@ -3396,17 +3391,11 @@ 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("acpi", optarg, true);
- if (!opts) {
- exit(1);
- }
+ opts = qemu_opts_parse("acpi", optarg, true, &error_fatal);
acpi_table_add(opts, &error_fatal);
break;
case QEMU_OPTION_smbios:
- opts = qemu_opts_parse_noisily("smbios", optarg, false);
- if (!opts) {
- exit(1);
- }
+ opts = qemu_opts_parse("smbios", optarg, false, &error_fatal);
smbios_entry_add(opts, &error_fatal);
break;
case QEMU_OPTION_fwcfg:
@@ -3433,7 +3422,13 @@ void qemu_init(int argc, char **argv)
break;
}
case QEMU_OPTION_accel:
- accel_opts = qemu_opts_parse_noisily("accel", optarg, true);
+ {
+ Error *err = NULL;
+
+ accel_opts = qemu_opts_parse("accel", optarg, true, &err);
+ if (!accel_opts) {
+ error_report_err(err);
+ }
optarg = qemu_opt_get(accel_opts, "accel");
if (!optarg || is_help_option(optarg)) {
printf("Accelerators supported in QEMU binary:\n");
@@ -3459,6 +3454,7 @@ void qemu_init(int argc, char **argv)
exit(0);
}
break;
+ }
case QEMU_OPTION_usb:
qdict_put_str(machine_opts_dict, "usb", "on");
break;
@@ -3475,9 +3471,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("device", optarg, true)) {
- exit(1);
- }
+ qemu_opts_parse("device", optarg, true, &error_fatal);
}
break;
case QEMU_OPTION_smp:
diff --git a/tests/unit/test-replication.c b/tests/unit/test-replication.c
index 101f72acdc..dbed415f87 100644
--- a/tests/unit/test-replication.c
+++ b/tests/unit/test-replication.c
@@ -174,12 +174,16 @@ static BlockBackend *start_primary(void)
QemuOpts *opts;
QDict *qdict;
char *cmdline;
+ Error *err = NULL;
cmdline = g_strdup_printf("driver=replication,mode=primary,node-name=xxx,"
"file.driver=qcow2,file.file.filename=%s,"
"file.file.locking=off"
, p_local_disk);
- opts = qemu_opts_parse_list_noisily(&qemu_drive_opts, cmdline, false);
+ opts = qemu_opts_parse_list(&qemu_drive_opts, cmdline, false, &err);
+ if (!opts) {
+ error_report_err(err);
+ }
g_free(cmdline);
qdict = qemu_opts_to_qdict(opts, NULL);
@@ -290,12 +294,16 @@ static BlockBackend *start_secondary(void)
QDict *qdict;
BlockBackend *blk;
char *cmdline;
+ Error *err = NULL;
/* add s_local_disk and forge S_LOCAL_DISK_ID */
cmdline = g_strdup_printf("file.filename=%s,driver=qcow2,"
"file.locking=off",
s_local_disk);
- opts = qemu_opts_parse_list_noisily(&qemu_drive_opts, cmdline, false);
+ opts = qemu_opts_parse_list(&qemu_drive_opts, cmdline, false, &err);
+ if (!opts) {
+ error_report_err(err);
+ }
g_free(cmdline);
qdict = qemu_opts_to_qdict(opts, NULL);
@@ -321,7 +329,10 @@ 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_list_noisily(&qemu_drive_opts, cmdline, false);
+ opts = qemu_opts_parse_list(&qemu_drive_opts, cmdline, false, &err);
+ if (!opts) {
+ error_report_err(err);
+ }
g_free(cmdline);
qdict = qemu_opts_to_qdict(opts, NULL);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 83fa2399d3..833c69f6a5 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -995,6 +995,9 @@ QemuOpts *qemu_opts_parse_list_noisily(QemuOptsList *list, const char *params,
Error *err = NULL;
QemuOpts *opts;
+ assert(g_str_equal("drive", list->name) ||
+ !opts_accepts_any(list));
+
opts = opts_parse(list, params, permit_abbrev, &err);
if (!opts) {
error_report_err(err);
--
2.53.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 11/11] qemu-option: Remove a few instances of noisily parsing
2026-09-30 22:11 ` [PATCH v2 11/11] qemu-option: Remove a few instances of noisily parsing Fabiano Rosas
@ 2026-10-01 9:42 ` marcandre.lureau
0 siblings, 0 replies; 31+ messages in thread
From: marcandre.lureau @ 2026-10-01 9:42 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Markus Armbruster, Daniel P . Berrangé,
Kevin Wolf, Hanna Reitz, Stefan Berger, Dr. David Alan Gilbert,
Jason Wang, Alex Bennée, Pierrick Bouvier, Alexandre Iooss,
Paolo Bonzini, Lukas Straub
On Wed, 30 Sep 2026 19:11:05 -0300, Fabiano Rosas <farosas@suse.de> wrote:
> With the removal of the support for short-form flags, the only other
> source of "noise" in the qemu_parse_*noisily() functions aside from
> errors is the printing of the help text.
>
> Therefore, the only difference from the _noisily versions to the
> normal ones is this block:
>
> [...]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--
Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread