* [PATCH] qapi, qemu-options: make all parsing visitors parse boolean options the same
@ 2020-11-04 11:49 Paolo Bonzini
2020-11-04 13:25 ` Markus Armbruster
0 siblings, 1 reply; 2+ messages in thread
From: Paolo Bonzini @ 2020-11-04 11:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P . Berrangé, Markus Armbruster
OptsVisitor, StringInputVisitor and the keyval visitor have
three different ideas of how a human could write the value of
a boolean option. Pay homage to the backwards-compatibility
gods and make the new common helper accept all four sets (on/off,
true/false, y/n and yes/no), but remove case-insensitivity.
Since OptsVisitor is supposed to match qemu-options, adjust
it as well.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201103161339.447118-1-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qapi/util.h | 2 ++
qapi/opts-visitor.c | 14 +-------------
qapi/qapi-util.c | 23 +++++++++++++++++++++++
qapi/qobject-input-visitor.c | 6 +-----
qapi/string-input-visitor.c | 17 +----------------
tests/qemu-iotests/051.out | 6 +++---
tests/qemu-iotests/051.pc.out | 6 +++---
tests/qemu-iotests/133.out | 8 ++++----
tests/qemu-iotests/137.out | 4 ++--
util/qemu-option.c | 20 ++------------------
10 files changed, 42 insertions(+), 64 deletions(-)
diff --git a/include/qapi/util.h b/include/qapi/util.h
index bc312e90aa..6178e98e97 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -19,6 +19,8 @@ typedef struct QEnumLookup {
const char *qapi_enum_lookup(const QEnumLookup *lookup, int val);
int qapi_enum_parse(const QEnumLookup *lookup, const char *buf,
int def, Error **errp);
+bool qapi_bool_parse(const char *name, const char *value, bool *obj,
+ Error **errp);
int parse_qapi_name(const char *name, bool complete);
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 7781c23a42..587f31baf6 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -368,7 +368,6 @@ opts_type_str(Visitor *v, const char *name, char **obj, Error **errp)
}
-/* mimics qemu-option.c::parse_option_bool() */
static bool
opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
{
@@ -379,19 +378,8 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
if (!opt) {
return false;
}
-
if (opt->str) {
- if (strcmp(opt->str, "on") == 0 ||
- strcmp(opt->str, "yes") == 0 ||
- strcmp(opt->str, "y") == 0) {
- *obj = true;
- } else if (strcmp(opt->str, "off") == 0 ||
- strcmp(opt->str, "no") == 0 ||
- strcmp(opt->str, "n") == 0) {
- *obj = false;
- } else {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
- "on|yes|y|off|no|n");
+ if (!qapi_bool_parse(opt->name, opt->str, obj, errp)) {
return false;
}
} else {
diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
index 29a6c98b53..dfc0bde497 100644
--- a/qapi/qapi-util.c
+++ b/qapi/qapi-util.c
@@ -13,6 +13,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qemu/ctype.h"
+#include "qapi/qmp/qerror.h"
const char *qapi_enum_lookup(const QEnumLookup *lookup, int val)
{
@@ -40,6 +41,28 @@ int qapi_enum_parse(const QEnumLookup *lookup, const char *buf,
return def;
}
+bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error **errp)
+{
+ if (g_str_equal(value, "on") ||
+ g_str_equal(value, "yes") ||
+ g_str_equal(value, "true") ||
+ g_str_equal(value, "y")) {
+ *obj = true;
+ return true;
+ }
+ if (g_str_equal(value, "off") ||
+ g_str_equal(value, "no") ||
+ g_str_equal(value, "false") ||
+ g_str_equal(value, "n")) {
+ *obj = false;
+ return true;
+ }
+
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
+ "boolean (on/off)");
+ return false;
+}
+
/*
* Parse a valid QAPI name from @str.
* A valid name consists of letters, digits, hyphen and underscore.
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 7b184b50a7..23843b242e 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -512,11 +512,7 @@ static bool qobject_input_type_bool_keyval(Visitor *v, const char *name,
return false;
}
- if (!strcmp(str, "on")) {
- *obj = true;
- } else if (!strcmp(str, "off")) {
- *obj = false;
- } else {
+ if (!qapi_bool_parse(name, str, obj, NULL)) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
full_name(qiv, name), "'on' or 'off'");
return false;
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 6e53396ea3..197139c1c0 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -332,22 +332,7 @@ static bool parse_type_bool(Visitor *v, const char *name, bool *obj,
StringInputVisitor *siv = to_siv(v);
assert(siv->lm == LM_NONE);
- if (!strcasecmp(siv->string, "on") ||
- !strcasecmp(siv->string, "yes") ||
- !strcasecmp(siv->string, "true")) {
- *obj = true;
- return true;
- }
- if (!strcasecmp(siv->string, "off") ||
- !strcasecmp(siv->string, "no") ||
- !strcasecmp(siv->string, "false")) {
- *obj = false;
- return true;
- }
-
- error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
- "boolean");
- return false;
+ return qapi_bool_parse(name ? name : "null", siv->string, obj, errp);
}
static bool parse_type_str(Visitor *v, const char *name, char **obj,
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index de4771bcb3..21d2dbec4c 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -110,13 +110,13 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) quit
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects boolean (on/off)
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects boolean (on/off)
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects boolean (on/off)
=== With version 2 images enabling lazy refcounts must fail ===
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index f707471fb0..d814b83871 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -110,13 +110,13 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) quit
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects boolean (on/off)
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects boolean (on/off)
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects boolean (on/off)
=== With version 2 images enabling lazy refcounts must fail ===
diff --git a/tests/qemu-iotests/133.out b/tests/qemu-iotests/133.out
index d70c2e8041..75431c2594 100644
--- a/tests/qemu-iotests/133.out
+++ b/tests/qemu-iotests/133.out
@@ -35,8 +35,8 @@ qemu-io: Cannot set both -c and the cache options
=== Check that invalid options are handled correctly ===
-qemu-io: Parameter 'read-only' expects 'on' or 'off'
-qemu-io: Parameter 'cache.no-flush' expects 'on' or 'off'
-qemu-io: Parameter 'cache.direct' expects 'on' or 'off'
-qemu-io: Parameter 'auto-read-only' expects 'on' or 'off'
+qemu-io: Parameter 'read-only' expects boolean (on/off)
+qemu-io: Parameter 'cache.no-flush' expects boolean (on/off)
+qemu-io: Parameter 'cache.direct' expects boolean (on/off)
+qemu-io: Parameter 'auto-read-only' expects boolean (on/off)
*** done
diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
index 86377c80cd..a420db61d5 100644
--- a/tests/qemu-iotests/137.out
+++ b/tests/qemu-iotests/137.out
@@ -15,7 +15,7 @@ read 33554432/33554432 bytes at offset 0
=== Try setting some invalid values ===
-qemu-io: Parameter 'lazy-refcounts' expects 'on' or 'off'
+qemu-io: Parameter 'lazy-refcounts' expects boolean (on/off)
qemu-io: cache-size, l2-cache-size and refcount-cache-size may not be set at the same time
qemu-io: l2-cache-size may not exceed cache-size
qemu-io: refcount-cache-size may not exceed cache-size
@@ -38,7 +38,7 @@ wrote 512/512 bytes at offset 0
./common.rc: Killed ( VALGRIND_QEMU="${VALGRIND_QEMU_IO}" _qemu_proc_exec "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
OK: Dirty bit not set
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: Parameter 'lazy-refcounts' expects 'on' or 'off'
+qemu-io: Parameter 'lazy-refcounts' expects boolean (on/off)
qcow2: Marking image as corrupt: Preventing invalid allocation of L2 table at offset 0; further corruption events will be suppressed
write failed: Input/output error
*** done
diff --git a/util/qemu-option.c b/util/qemu-option.c
index b9f93a7f8b..acefbc23fa 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -96,21 +96,6 @@ const char *get_opt_value(const char *p, char **value)
return offset;
}
-static bool parse_option_bool(const char *name, const char *value, bool *ret,
- Error **errp)
-{
- if (!strcmp(value, "on")) {
- *ret = 1;
- } else if (!strcmp(value, "off")) {
- *ret = 0;
- } else {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- name, "'on' or 'off'");
- return false;
- }
- return true;
-}
-
static bool parse_option_number(const char *name, const char *value,
uint64_t *ret, Error **errp)
{
@@ -363,7 +348,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name,
if (opt == NULL) {
def_val = find_default_by_name(opts, name);
if (def_val) {
- parse_option_bool(name, def_val, &ret, &error_abort);
+ qapi_bool_parse(name, def_val, &ret, &error_abort);
}
return ret;
}
@@ -471,8 +456,7 @@ static bool qemu_opt_parse(QemuOpt *opt, Error **errp)
/* nothing */
return true;
case QEMU_OPT_BOOL:
- return parse_option_bool(opt->name, opt->str, &opt->value.boolean,
- errp);
+ return qapi_bool_parse(opt->name, opt->str, &opt->value.boolean, errp);
case QEMU_OPT_NUMBER:
return parse_option_number(opt->name, opt->str, &opt->value.uint,
errp);
--
2.26.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] qapi, qemu-options: make all parsing visitors parse boolean options the same
2020-11-04 11:49 [PATCH] qapi, qemu-options: make all parsing visitors parse boolean options the same Paolo Bonzini
@ 2020-11-04 13:25 ` Markus Armbruster
0 siblings, 0 replies; 2+ messages in thread
From: Markus Armbruster @ 2020-11-04 13:25 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Daniel P . Berrangé, qemu-devel
This is v3.
Paolo Bonzini <pbonzini@redhat.com> writes:
> OptsVisitor, StringInputVisitor and the keyval visitor have
> three different ideas of how a human could write the value of
> a boolean option. Pay homage to the backwards-compatibility
> gods and make the new common helper accept all four sets (on/off,
> true/false, y/n and yes/no), but remove case-insensitivity.
>
> Since OptsVisitor is supposed to match qemu-options, adjust
> it as well.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Message-Id: <20201103161339.447118-1-pbonzini@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id and double-SoB are odd. Accident?
> ---
> include/qapi/util.h | 2 ++
> qapi/opts-visitor.c | 14 +-------------
> qapi/qapi-util.c | 23 +++++++++++++++++++++++
> qapi/qobject-input-visitor.c | 6 +-----
> qapi/string-input-visitor.c | 17 +----------------
> tests/qemu-iotests/051.out | 6 +++---
> tests/qemu-iotests/051.pc.out | 6 +++---
> tests/qemu-iotests/133.out | 8 ++++----
> tests/qemu-iotests/137.out | 4 ++--
> util/qemu-option.c | 20 ++------------------
> 10 files changed, 42 insertions(+), 64 deletions(-)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index bc312e90aa..6178e98e97 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -19,6 +19,8 @@ typedef struct QEnumLookup {
> const char *qapi_enum_lookup(const QEnumLookup *lookup, int val);
> int qapi_enum_parse(const QEnumLookup *lookup, const char *buf,
> int def, Error **errp);
> +bool qapi_bool_parse(const char *name, const char *value, bool *obj,
> + Error **errp);
>
> int parse_qapi_name(const char *name, bool complete);
>
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 7781c23a42..587f31baf6 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -368,7 +368,6 @@ opts_type_str(Visitor *v, const char *name, char **obj, Error **errp)
> }
>
>
> -/* mimics qemu-option.c::parse_option_bool() */
> static bool
> opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
> {
> @@ -379,19 +378,8 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
> if (!opt) {
> return false;
> }
> -
> if (opt->str) {
> - if (strcmp(opt->str, "on") == 0 ||
> - strcmp(opt->str, "yes") == 0 ||
> - strcmp(opt->str, "y") == 0) {
> - *obj = true;
> - } else if (strcmp(opt->str, "off") == 0 ||
> - strcmp(opt->str, "no") == 0 ||
> - strcmp(opt->str, "n") == 0) {
> - *obj = false;
> - } else {
> - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
> - "on|yes|y|off|no|n");
> + if (!qapi_bool_parse(opt->name, opt->str, obj, errp)) {
Error message changes from
$ qemu-system-x86_64 -netdev user,id=net0,ipv4=
qemu-system-x86_64: Parameter 'ipv4' expects on|yes|y|off|no|n
to
qemu-system-x86_64: Parameter 'ipv4' expects boolean (on/off)
Intentional (but see below).
> return false;
> }
> } else {
> diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
> index 29a6c98b53..dfc0bde497 100644
> --- a/qapi/qapi-util.c
> +++ b/qapi/qapi-util.c
> @@ -13,6 +13,7 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "qemu/ctype.h"
> +#include "qapi/qmp/qerror.h"
>
> const char *qapi_enum_lookup(const QEnumLookup *lookup, int val)
> {
> @@ -40,6 +41,28 @@ int qapi_enum_parse(const QEnumLookup *lookup, const char *buf,
> return def;
> }
>
> +bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error **errp)
> +{
> + if (g_str_equal(value, "on") ||
> + g_str_equal(value, "yes") ||
> + g_str_equal(value, "true") ||
> + g_str_equal(value, "y")) {
> + *obj = true;
> + return true;
> + }
> + if (g_str_equal(value, "off") ||
> + g_str_equal(value, "no") ||
> + g_str_equal(value, "false") ||
> + g_str_equal(value, "n")) {
> + *obj = false;
> + return true;
> + }
> +
> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> + "boolean (on/off)");
> + return false;
> +}
> +
> /*
> * Parse a valid QAPI name from @str.
> * A valid name consists of letters, digits, hyphen and underscore.
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 7b184b50a7..23843b242e 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -512,11 +512,7 @@ static bool qobject_input_type_bool_keyval(Visitor *v, const char *name,
> return false;
> }
>
> - if (!strcmp(str, "on")) {
> - *obj = true;
> - } else if (!strcmp(str, "off")) {
> - *obj = false;
> - } else {
> + if (!qapi_bool_parse(name, str, obj, NULL)) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> full_name(qiv, name), "'on' or 'off'");
> return false;
Error message stays the same:
$ qemu-system-x86_64 -display gtk,full-screen=
qemu-system-x86_64: -display gtk,full-screen=: Parameter 'full-screen' expects 'on' or 'off'
Good.
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 6e53396ea3..197139c1c0 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -332,22 +332,7 @@ static bool parse_type_bool(Visitor *v, const char *name, bool *obj,
> StringInputVisitor *siv = to_siv(v);
>
> assert(siv->lm == LM_NONE);
> - if (!strcasecmp(siv->string, "on") ||
> - !strcasecmp(siv->string, "yes") ||
> - !strcasecmp(siv->string, "true")) {
> - *obj = true;
> - return true;
> - }
> - if (!strcasecmp(siv->string, "off") ||
> - !strcasecmp(siv->string, "no") ||
> - !strcasecmp(siv->string, "false")) {
> - *obj = false;
> - return true;
> - }
> -
> - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> - "boolean");
> - return false;
> + return qapi_bool_parse(name ? name : "null", siv->string, obj, errp);
Error message changes from
$ qemu-system-x86_64 -nodefaults -global isa-serial.wakeup= -device isa-serial
qemu-system-x86_64: -device isa-serial: can't apply global isa-serial.wakeup=: Invalid parameter type for 'wakeup', expected: boolean
to
qemu-system-x86_64: -device isa-serial: can't apply global isa-serial.wakeup=: Parameter 'wakeup' expects boolean (on/off)
Okay (but see below).
> }
>
> static bool parse_type_str(Visitor *v, const char *name, char **obj,
> diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
> index de4771bcb3..21d2dbec4c 100644
> --- a/tests/qemu-iotests/051.out
> +++ b/tests/qemu-iotests/051.out
> @@ -110,13 +110,13 @@ QEMU X.Y.Z monitor - type 'help' for more information
> (qemu) quit
>
> Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off'
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects boolean (on/off)
>
> Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off'
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects boolean (on/off)
>
> Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off'
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects boolean (on/off)
>
>
> === With version 2 images enabling lazy refcounts must fail ===
> diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
> index f707471fb0..d814b83871 100644
> --- a/tests/qemu-iotests/051.pc.out
> +++ b/tests/qemu-iotests/051.pc.out
> @@ -110,13 +110,13 @@ QEMU X.Y.Z monitor - type 'help' for more information
> (qemu) quit
>
> Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off'
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects boolean (on/off)
>
> Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off'
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects boolean (on/off)
>
> Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off'
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects boolean (on/off)
>
>
> === With version 2 images enabling lazy refcounts must fail ===
> diff --git a/tests/qemu-iotests/133.out b/tests/qemu-iotests/133.out
> index d70c2e8041..75431c2594 100644
> --- a/tests/qemu-iotests/133.out
> +++ b/tests/qemu-iotests/133.out
> @@ -35,8 +35,8 @@ qemu-io: Cannot set both -c and the cache options
>
> === Check that invalid options are handled correctly ===
>
> -qemu-io: Parameter 'read-only' expects 'on' or 'off'
> -qemu-io: Parameter 'cache.no-flush' expects 'on' or 'off'
> -qemu-io: Parameter 'cache.direct' expects 'on' or 'off'
> -qemu-io: Parameter 'auto-read-only' expects 'on' or 'off'
> +qemu-io: Parameter 'read-only' expects boolean (on/off)
> +qemu-io: Parameter 'cache.no-flush' expects boolean (on/off)
> +qemu-io: Parameter 'cache.direct' expects boolean (on/off)
> +qemu-io: Parameter 'auto-read-only' expects boolean (on/off)
> *** done
> diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
> index 86377c80cd..a420db61d5 100644
> --- a/tests/qemu-iotests/137.out
> +++ b/tests/qemu-iotests/137.out
> @@ -15,7 +15,7 @@ read 33554432/33554432 bytes at offset 0
>
> === Try setting some invalid values ===
>
> -qemu-io: Parameter 'lazy-refcounts' expects 'on' or 'off'
> +qemu-io: Parameter 'lazy-refcounts' expects boolean (on/off)
> qemu-io: cache-size, l2-cache-size and refcount-cache-size may not be set at the same time
> qemu-io: l2-cache-size may not exceed cache-size
> qemu-io: refcount-cache-size may not exceed cache-size
> @@ -38,7 +38,7 @@ wrote 512/512 bytes at offset 0
> ./common.rc: Killed ( VALGRIND_QEMU="${VALGRIND_QEMU_IO}" _qemu_proc_exec "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
> OK: Dirty bit not set
> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
> -qemu-io: Parameter 'lazy-refcounts' expects 'on' or 'off'
> +qemu-io: Parameter 'lazy-refcounts' expects boolean (on/off)
> qcow2: Marking image as corrupt: Preventing invalid allocation of L2 table at offset 0; further corruption events will be suppressed
> write failed: Input/output error
> *** done
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index b9f93a7f8b..acefbc23fa 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -96,21 +96,6 @@ const char *get_opt_value(const char *p, char **value)
> return offset;
> }
>
> -static bool parse_option_bool(const char *name, const char *value, bool *ret,
> - Error **errp)
> -{
> - if (!strcmp(value, "on")) {
> - *ret = 1;
> - } else if (!strcmp(value, "off")) {
> - *ret = 0;
> - } else {
> - error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> - name, "'on' or 'off'");
> - return false;
> - }
> - return true;
> -}
> -
> static bool parse_option_number(const char *name, const char *value,
> uint64_t *ret, Error **errp)
> {
> @@ -363,7 +348,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name,
> if (opt == NULL) {
> def_val = find_default_by_name(opts, name);
> if (def_val) {
> - parse_option_bool(name, def_val, &ret, &error_abort);
> + qapi_bool_parse(name, def_val, &ret, &error_abort);
> }
> return ret;
> }
> @@ -471,8 +456,7 @@ static bool qemu_opt_parse(QemuOpt *opt, Error **errp)
> /* nothing */
> return true;
> case QEMU_OPT_BOOL:
> - return parse_option_bool(opt->name, opt->str, &opt->value.boolean,
> - errp);
> + return qapi_bool_parse(opt->name, opt->str, &opt->value.boolean, errp);
> case QEMU_OPT_NUMBER:
> return parse_option_number(opt->name, opt->str, &opt->value.uint,
> errp);
Error message changes from
$ qemu-system-x86_64 -smbios type=0,uefi=
qemu-system-x86_64: -smbios type=0,uefi=: Parameter 'uefi' expects 'on' or 'off'
to
qemu-system-x86_64: -smbios type=0,uefi=: Parameter 'uefi' expects boolean (on/off)
Not an improvement.
I recommend to refrain from messing with the error message. "expects
'on' or 'off'" is just fine in all cases.
Bonus: saves us the trouble of updating expected test output.
Tested-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-11-04 13:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-04 11:49 [PATCH] qapi, qemu-options: make all parsing visitors parse boolean options the same Paolo Bonzini
2020-11-04 13:25 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.