[Qemu-devel] [PATCH 2/4] block: use Error mechanism instead of -errno for block_job_set_speed()

[Stefan Hajnoczi]

There are at least two different errors that can occur in
block_job_set_speed(): the job might not support setting speeds or the
value might be invalid.

Use the Error mechanism to report the error where it occurs.  This patch
adds the new BlockJobSpeedInvalid QError.  The error is specific to
block job speeds so we can add a speed argument to block-stream in the
future and clearly identify the invalid parameter.

Cc: Luiz Capitulino <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
---
 block.c          |   17 ++++++++++-------
 block/stream.c   |    6 +++---
 block_int.h      |    5 +++--
 blockdev.c       |    4 +---
 qapi-schema.json |    1 +
 qerror.c         |    4 ++++
 qerror.h         |    3 +++
 7 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index 528b696..7056d8c 100644
--- a/block.c
+++ b/block.c
@@ -4103,18 +4103,21 @@ void block_job_complete(BlockJob *job, int ret)
     bdrv_set_in_use(bs, 0);
 }
 
-int block_job_set_speed(BlockJob *job, int64_t value)
+void block_job_set_speed(BlockJob *job, int64_t value, Error **errp)
 {
-    int rc;
+    Error *local_error = NULL;
 
     if (!job->job_type->set_speed) {
-        return -ENOTSUP;
+        error_set(errp, QERR_NOT_SUPPORTED);
+        return;
     }
-    rc = job->job_type->set_speed(job, value);
-    if (rc == 0) {
-        job->speed = value;
+    job->job_type->set_speed(job, value, &local_error);
+    if (error_is_set(&local_error)) {
+        error_propagate(errp, local_error);
+        return;
     }
-    return rc;
+
+    job->speed = value;
 }
 
 void block_job_cancel(BlockJob *job)
diff --git a/block/stream.c b/block/stream.c
index 0aa7083..f0486a3 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -263,15 +263,15 @@ retry:
     block_job_complete(&s->common, ret);
 }
 
-static int stream_set_speed(BlockJob *job, int64_t value)
+static void stream_set_speed(BlockJob *job, int64_t value, Error **errp)
 {
     StreamBlockJob *s = container_of(job, StreamBlockJob, common);
 
     if (value < 0) {
-        return -EINVAL;
+        error_set(errp, QERR_BLOCK_JOB_SPEED_INVALID, value);
+        return;
     }
     ratelimit_set_speed(&s->limit, value / BDRV_SECTOR_SIZE);
-    return 0;
 }
 
 static BlockJobType stream_job_type = {
diff --git a/block_int.h b/block_int.h
index 1557d5c..6015c27 100644
--- a/block_int.h
+++ b/block_int.h
@@ -78,7 +78,7 @@ typedef struct BlockJobType {
     const char *job_type;
 
     /** Optional callback for job types that support setting a speed limit */
-    int (*set_speed)(BlockJob *job, int64_t value);
+    void (*set_speed)(BlockJob *job, int64_t value, Error **errp);
 } BlockJobType;
 
 /**
@@ -374,11 +374,12 @@ void block_job_complete(BlockJob *job, int ret);
  * block_job_set_speed:
  * @job: The job to set the speed for.
  * @speed: The new value
+ * @errp: A location to return NotSupported or BlockJobSpeedInvalid.
  *
  * Set a rate-limiting parameter for the job; the actual meaning may
  * vary depending on the job type.
  */
-int block_job_set_speed(BlockJob *job, int64_t value);
+void block_job_set_speed(BlockJob *job, int64_t value, Error **errp);
 
 /**
  * block_job_cancel:
diff --git a/blockdev.c b/blockdev.c
index 202c3ae..c484259 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1143,9 +1143,7 @@ void qmp_block_job_set_speed(const char *device, int64_t 
value, Error **errp)
         return;
     }
 
-    if (block_job_set_speed(job, value) < 0) {
-        error_set(errp, QERR_NOT_SUPPORTED);
-    }
+    block_job_set_speed(job, value, errp);
 }
 
 void qmp_block_job_cancel(const char *device, Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index 6499895..d0d4f693 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1596,6 +1596,7 @@
 #
 # Returns: Nothing on success
 #          If the job type does not support throttling, NotSupported
+#          If the speed value is invalid, BlockJobSpeedInvalid
 #          If streaming is not active on this device, DeviceNotActive
 #
 # Since: 1.1
diff --git a/qerror.c b/qerror.c
index 96fbe71..6aee606 100644
--- a/qerror.c
+++ b/qerror.c
@@ -64,6 +64,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Block format '%(format)' used by device '%(name)' does 
not support feature '%(feature)'",
     },
     {
+        .error_fmt = QERR_BLOCK_JOB_SPEED_INVALID,
+        .desc      = "Block job speed value '%(value)' is invalid",
+    },
+    {
         .error_fmt = QERR_BUS_NO_HOTPLUG,
         .desc      = "Bus '%(bus)' does not support hotplugging",
     },
diff --git a/qerror.h b/qerror.h
index 5c23c1f..e239ef1 100644
--- a/qerror.h
+++ b/qerror.h
@@ -67,6 +67,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
     "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 
'name': %s, 'feature': %s } }"
 
+#define QERR_BLOCK_JOB_SPEED_INVALID \
+    "{ 'class': 'BlockJobSpeedInvalid', 'data': { 'value': %"PRId64" } }"
+
 #define QERR_BUFFER_OVERRUN \
     "{ 'class': 'BufferOverrun', 'data': {} }"
 
-- 
1.7.9.5