[v2] blockjob: Fix error message for negative speed

[PATCH v2] blockjob: Fix error message for negative speed

Kevin Wolf posted 1 patch 4 years, 4 months ago

Diff against v1
Download mbox

Test asan failed

Test checkpatch failed

Test FreeBSD failed

Test docker-mingw@fedora failed

Test docker-clang@ubuntu failed

Test docker-quick@centos7 failed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191217150458.10288-1-kwolf@redhat.com

Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>

blockjob.c             | 3 ++-
tests/qemu-iotests/030 | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)

Expand all Fold all

[PATCH v2] blockjob: Fix error message for negative speed

Posted by Kevin Wolf 4 years, 4 months ago

The error message for a negative speed uses QERR_INVALID_PARAMETER,
which implies that the 'speed' option doesn't even exist:

    {"error": {"class": "GenericError", "desc": "Invalid parameter 'speed'"}}

Make it use QERR_INVALID_PARAMETER_VALUE instead:

    {"error": {"class": "GenericError", "desc": "Parameter 'speed' expects a non-negative value"}}

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---

v2:
- Update reference output for 030 [Max]

 blockjob.c             | 3 ++-
 tests/qemu-iotests/030 | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index c6e20e2fcd..5d63b1e89d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -261,7 +261,8 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
         return;
     }
     if (speed < 0) {
-        error_setg(errp, QERR_INVALID_PARAMETER, "speed");
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "speed",
+                   "a non-negative value");
         return;
     }
 
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index f3766f2a81..be35bde06f 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -943,7 +943,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('block-stream', device='drive0', speed=-1)
-        self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'")
+        self.assert_qmp(result, 'error/desc', "Parameter 'speed' expects a non-negative value")
 
         self.assert_no_active_block_jobs()
 
@@ -952,7 +952,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
-        self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'")
+        self.assert_qmp(result, 'error/desc', "Parameter 'speed' expects a non-negative value")
 
         self.cancel_and_wait(resume=True)
 
-- 
2.20.1

Re: [PATCH v2] blockjob: Fix error message for negative speed

Posted by Max Reitz 4 years, 4 months ago

On 17.12.19 16:04, Kevin Wolf wrote:
> The error message for a negative speed uses QERR_INVALID_PARAMETER,
> which implies that the 'speed' option doesn't even exist:
> 
>     {"error": {"class": "GenericError", "desc": "Invalid parameter 'speed'"}}
> 
> Make it use QERR_INVALID_PARAMETER_VALUE instead:
> 
>     {"error": {"class": "GenericError", "desc": "Parameter 'speed' expects a non-negative value"}}
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> ---
> 
> v2:
> - Update reference output for 030 [Max]
> 
>  blockjob.c             | 3 ++-
>  tests/qemu-iotests/030 | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>