| Message ID | 20211011131558.3273255-1-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi: Improve input_type_enum()'s error message | expand |
On 10/11/21 15:15, Markus Armbruster wrote: > The error message claims the parameter is invalid: > > $ qemu-system-x86_64 -object qom-type=nonexistent > qemu-system-x86_64: -object qom-type=nonexistent: Invalid parameter 'nonexistent' > > What's wrong is actually the *value* 'nonexistent'. Improve the > message to > > qemu-system-x86_64: -object qom-type=nonexistent: Parameter 'qom-type' does not accept value 'nonexistent' > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > qapi/qapi-visit-core.c | 3 ++- > tests/unit/check-qom-proplist.c | 2 +- > tests/qemu-iotests/049.out | 6 +++--- > tests/qemu-iotests/206.out | 2 +- > tests/qemu-iotests/237.out | 6 +++--- > tests/qemu-iotests/245 | 2 +- > 6 files changed, 11 insertions(+), 10 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Am 11.10.2021 um 15:15 hat Markus Armbruster geschrieben: > The error message claims the parameter is invalid: > > $ qemu-system-x86_64 -object qom-type=nonexistent > qemu-system-x86_64: -object qom-type=nonexistent: Invalid parameter 'nonexistent' > > What's wrong is actually the *value* 'nonexistent'. Improve the > message to > > qemu-system-x86_64: -object qom-type=nonexistent: Parameter 'qom-type' does not accept value 'nonexistent' > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > qapi/qapi-visit-core.c | 3 ++- > tests/unit/check-qom-proplist.c | 2 +- > tests/qemu-iotests/049.out | 6 +++--- > tests/qemu-iotests/206.out | 2 +- > tests/qemu-iotests/237.out | 6 +++--- > tests/qemu-iotests/245 | 2 +- Good that you remembered to update iotests cases. I'm afraid there are two more that need an update. 287 contains these lines: output=$(_make_test_img -o 'compression_type=zstd' 64M; _cleanup_test_img) if echo "$output" | grep -q "Invalid parameter 'zstd'"; then _notrun "ZSTD is disabled" fi Instead of skipping the test case when zstd support is not compiled in, this test fails now. 308 contains a similar check for FUSE support and fails now when FUSE support is disabled. Kevin
Kevin Wolf <kwolf@redhat.com> writes: > Am 11.10.2021 um 15:15 hat Markus Armbruster geschrieben: >> The error message claims the parameter is invalid: >> >> $ qemu-system-x86_64 -object qom-type=nonexistent >> qemu-system-x86_64: -object qom-type=nonexistent: Invalid parameter 'nonexistent' >> >> What's wrong is actually the *value* 'nonexistent'. Improve the >> message to >> >> qemu-system-x86_64: -object qom-type=nonexistent: Parameter 'qom-type' does not accept value 'nonexistent' >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> qapi/qapi-visit-core.c | 3 ++- >> tests/unit/check-qom-proplist.c | 2 +- >> tests/qemu-iotests/049.out | 6 +++--- >> tests/qemu-iotests/206.out | 2 +- >> tests/qemu-iotests/237.out | 6 +++--- >> tests/qemu-iotests/245 | 2 +- > > Good that you remembered to update iotests cases. I'm afraid there are > two more that need an update. > > 287 contains these lines: > > output=$(_make_test_img -o 'compression_type=zstd' 64M; _cleanup_test_img) > if echo "$output" | grep -q "Invalid parameter 'zstd'"; then > _notrun "ZSTD is disabled" > fi > > Instead of skipping the test case when zstd support is not compiled in, > this test fails now. > > 308 contains a similar check for FUSE support and fails now when FUSE > support is disabled. 287 passes for me. I figure that's because I built with CONFIG_ZSTD. 308 I simply missed. Thanks for your help!
On Tue, Oct 12, 2021 at 8:10 AM Markus Armbruster <armbru@redhat.com> wrote: > Kevin Wolf <kwolf@redhat.com> writes: > > > Am 11.10.2021 um 15:15 hat Markus Armbruster geschrieben: > >> The error message claims the parameter is invalid: > >> > >> $ qemu-system-x86_64 -object qom-type=nonexistent > >> qemu-system-x86_64: -object qom-type=nonexistent: Invalid parameter > 'nonexistent' > >> > >> What's wrong is actually the *value* 'nonexistent'. Improve the > >> message to > >> > >> qemu-system-x86_64: -object qom-type=nonexistent: Parameter > 'qom-type' does not accept value 'nonexistent' > >> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> --- > >> qapi/qapi-visit-core.c | 3 ++- > >> tests/unit/check-qom-proplist.c | 2 +- > >> tests/qemu-iotests/049.out | 6 +++--- > >> tests/qemu-iotests/206.out | 2 +- > >> tests/qemu-iotests/237.out | 6 +++--- > >> tests/qemu-iotests/245 | 2 +- > > > > Good that you remembered to update iotests cases. I'm afraid there are > > two more that need an update. > > > > 287 contains these lines: > > > > output=$(_make_test_img -o 'compression_type=zstd' 64M; > _cleanup_test_img) > > if echo "$output" | grep -q "Invalid parameter 'zstd'"; then > > _notrun "ZSTD is disabled" > > fi > > > > Instead of skipping the test case when zstd support is not compiled in, > > this test fails now. > > > > 308 contains a similar check for FUSE support and fails now when FUSE > > support is disabled. > > 287 passes for me. I figure that's because I built with CONFIG_ZSTD. > 308 I simply missed. > > Thanks for your help! > This likely fixes https://gitlab.com/qemu-project/qemu/-/issues/608 and you could include that in your commit message if it isn't too late. --js
John Snow <jsnow@redhat.com> writes: > This likely fixes https://gitlab.com/qemu-project/qemu/-/issues/608 and you > could include that in your commit message if it isn't too late. It's not. Have you verified it fixes the bug, or is it just an educated guess?
Markus Armbruster <armbru@redhat.com> writes: > John Snow <jsnow@redhat.com> writes: > >> This likely fixes https://gitlab.com/qemu-project/qemu/-/issues/608 and you >> could include that in your commit message if it isn't too late. > > It's not. > > Have you verified it fixes the bug, or is it just an educated guess? I did: it does fix it.
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index a641adec51..7310f0a0ca 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -392,7 +392,8 @@ static bool input_type_enum(Visitor *v, const char *name, int *obj, value = qapi_enum_parse(lookup, enum_str, -1, NULL); if (value < 0) { - error_setg(errp, QERR_INVALID_PARAMETER, enum_str); + error_setg(errp, "Parameter '%s' does not accept value '%s'", + name ? name : "null", enum_str); g_free(enum_str); return false; } diff --git a/tests/unit/check-qom-proplist.c b/tests/unit/check-qom-proplist.c index 48503e0dff..ed341088d3 100644 --- a/tests/unit/check-qom-proplist.c +++ b/tests/unit/check-qom-proplist.c @@ -488,7 +488,7 @@ static void test_dummy_badenum(void) g_assert(dobj == NULL); g_assert(err != NULL); g_assert_cmpstr(error_get_pretty(err), ==, - "Invalid parameter 'yeti'"); + "Parameter 'av' does not accept value 'yeti'"); g_assert(object_resolve_path_component(parent, "dummy0") == NULL); diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out index 01f7b1f240..8719c91b48 100644 --- a/tests/qemu-iotests/049.out +++ b/tests/qemu-iotests/049.out @@ -174,11 +174,11 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off comp qemu-img create -f qcow2 -o compat=0.42 TEST_DIR/t.qcow2 64M Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=67108864 compat=0.42 lazy_refcounts=off refcount_bits=16 -qemu-img: TEST_DIR/t.qcow2: Invalid parameter '0.42' +qemu-img: TEST_DIR/t.qcow2: Parameter 'version' does not accept value '0.42' qemu-img create -f qcow2 -o compat=foobar TEST_DIR/t.qcow2 64M Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=67108864 compat=foobar lazy_refcounts=off refcount_bits=16 -qemu-img: TEST_DIR/t.qcow2: Invalid parameter 'foobar' +qemu-img: TEST_DIR/t.qcow2: Parameter 'version' does not accept value 'foobar' == Check preallocation option == @@ -190,7 +190,7 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off prea qemu-img create -f qcow2 -o preallocation=1234 TEST_DIR/t.qcow2 64M Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off preallocation=1234 compression_type=zlib size=67108864 lazy_refcounts=off refcount_bits=16 -qemu-img: TEST_DIR/t.qcow2: Invalid parameter '1234' +qemu-img: TEST_DIR/t.qcow2: Parameter 'preallocation' does not accept value '1234' == Check encryption option == diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out index b68c443867..3593e8e9c2 100644 --- a/tests/qemu-iotests/206.out +++ b/tests/qemu-iotests/206.out @@ -192,7 +192,7 @@ Job failed: Could not resize image: Failed to grow the L1 table: File too large === Invalid version === {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "file": "node0", "size": 67108864, "version": "v1"}}} -{"error": {"class": "GenericError", "desc": "Invalid parameter 'v1'"}} +{"error": {"class": "GenericError", "desc": "Parameter 'version' does not accept value 'v1'"}} {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "file": "node0", "lazy-refcounts": true, "size": 67108864, "version": "v2"}}} {"return": {}} diff --git a/tests/qemu-iotests/237.out b/tests/qemu-iotests/237.out index aa94986803..2f09ff5512 100644 --- a/tests/qemu-iotests/237.out +++ b/tests/qemu-iotests/237.out @@ -116,13 +116,13 @@ Job failed: Cannot find device='this doesn't exist' nor node-name='this doesn't == Invalid adapter types == {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"adapter-type": "foo", "driver": "vmdk", "file": "node0", "size": 33554432}}} -{"error": {"class": "GenericError", "desc": "Invalid parameter 'foo'"}} +{"error": {"class": "GenericError", "desc": "Parameter 'adapter-type' does not accept value 'foo'"}} {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"adapter-type": "IDE", "driver": "vmdk", "file": "node0", "size": 33554432}}} -{"error": {"class": "GenericError", "desc": "Invalid parameter 'IDE'"}} +{"error": {"class": "GenericError", "desc": "Parameter 'adapter-type' does not accept value 'IDE'"}} {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"adapter-type": "legacyesx", "driver": "vmdk", "file": "node0", "size": 33554432}}} -{"error": {"class": "GenericError", "desc": "Invalid parameter 'legacyesx'"}} +{"error": {"class": "GenericError", "desc": "Parameter 'adapter-type' does not accept value 'legacyesx'"}} {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"adapter-type": 1, "driver": "vmdk", "file": "node0", "size": 33554432}}} {"error": {"class": "GenericError", "desc": "Invalid parameter type for 'options.adapter-type', expected: string"}} diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245 index bf8261eec0..1898807f25 100755 --- a/tests/qemu-iotests/245 +++ b/tests/qemu-iotests/245 @@ -149,7 +149,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): self.reopen(opts, {'node-name': ''}, "Failed to find node with node-name=''") self.reopen(opts, {'node-name': None}, "Invalid parameter type for 'options[0].node-name', expected: string") self.reopen(opts, {'driver': 'raw'}, "Cannot change the option 'driver'") - self.reopen(opts, {'driver': ''}, "Invalid parameter ''") + self.reopen(opts, {'driver': ''}, "Parameter 'driver' does not accept value ''") self.reopen(opts, {'driver': None}, "Invalid parameter type for 'options[0].driver', expected: string") self.reopen(opts, {'file': 'not-found'}, "Cannot find device='' nor node-name='not-found'") self.reopen(opts, {'file': ''}, "Cannot find device='' nor node-name=''")
The error message claims the parameter is invalid: $ qemu-system-x86_64 -object qom-type=nonexistent qemu-system-x86_64: -object qom-type=nonexistent: Invalid parameter 'nonexistent' What's wrong is actually the *value* 'nonexistent'. Improve the message to qemu-system-x86_64: -object qom-type=nonexistent: Parameter 'qom-type' does not accept value 'nonexistent' Signed-off-by: Markus Armbruster <armbru@redhat.com> --- qapi/qapi-visit-core.c | 3 ++- tests/unit/check-qom-proplist.c | 2 +- tests/qemu-iotests/049.out | 6 +++--- tests/qemu-iotests/206.out | 2 +- tests/qemu-iotests/237.out | 6 +++--- tests/qemu-iotests/245 | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-)