block: use Error mechanism instead of -errno for block_job_create()

The block job API uses -errno return values internally and we convert
these to Error in the QMP functions.  This is ugly because the Error
should be created at the point where we still have all the relevant
information.  More importantly, it is hard to add new error cases to
this case since we quickly run out of -errno values without losing
information.

Go ahead and use Error directly and don't convert later.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>

diff --git a/block.c b/block.c
index fe74dddb134d240f2857625626a922232303d02d..2b72a0f3b066479cf1d09ba3261a3dc0b7a7f25f 100644 (file)
--- a/block.c
+++ b/block.c
@@ -4083,11 +4083,13 @@ out:
 }
 
 void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
-                       BlockDriverCompletionFunc *cb, void *opaque)
+                       BlockDriverCompletionFunc *cb, void *opaque,
+                       Error **errp)
 {
     BlockJob *job;
 
     if (bs->job || bdrv_in_use(bs)) {
+        error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
         return NULL;
     }
     bdrv_set_in_use(bs, 1);

diff --git a/block/stream.c b/block/stream.c
index 0efe1adfd5e1302dddf3378a48d4eafbe2560504..7002dc85734cccedf9fd682f168a02cc750d749b 100644 (file)
--- a/block/stream.c
+++ b/block/stream.c
@@ -280,16 +280,16 @@ static BlockJobType stream_job_type = {
     .set_speed     = stream_set_speed,
 };
 
-int stream_start(BlockDriverState *bs, BlockDriverState *base,
-                 const char *base_id, BlockDriverCompletionFunc *cb,
-                 void *opaque)
+void stream_start(BlockDriverState *bs, BlockDriverState *base,
+                  const char *base_id, BlockDriverCompletionFunc *cb,
+                  void *opaque, Error **errp)
 {
     StreamBlockJob *s;
     Coroutine *co;
 
-    s = block_job_create(&stream_job_type, bs, cb, opaque);
+    s = block_job_create(&stream_job_type, bs, cb, opaque, errp);
     if (!s) {
-        return -EBUSY; /* bs must already be in use */
+        return;
     }
 
     s->base = base;
@@ -300,5 +300,4 @@ int stream_start(BlockDriverState *bs, BlockDriverState *base,
     co = qemu_coroutine_create(stream_run);
     trace_stream_start(bs, base, s, co, opaque);
     qemu_coroutine_enter(co, s);
-    return 0;
 }

diff --git a/block_int.h b/block_int.h
index 0acb49f100e65219d2cf0af27ef3aefc994e7353..e70a33e0c2a24eb1bc5025a7c78361fcce747216 100644 (file)
--- a/block_int.h
+++ b/block_int.h
@@ -346,6 +346,7 @@ int is_windows_drive(const char *filename);
  * @bs: The block
  * @cb: Completion function for the job.
  * @opaque: Opaque pointer value passed to @cb.
+ * @errp: Error object.
  *
  * Create a new long-running block device job and return it.  The job
  * will call @cb asynchronously when the job completes.  Note that
@@ -357,7 +358,8 @@ int is_windows_drive(const char *filename);
  * called from a wrapper that is specific to the job type.
  */
 void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
-                       BlockDriverCompletionFunc *cb, void *opaque);
+                       BlockDriverCompletionFunc *cb, void *opaque,
+                       Error **errp);
 
 /**
  * block_job_complete:
@@ -417,6 +419,7 @@ void block_job_cancel_sync(BlockJob *job);
  * backing file if the job completes.  Ignored if @base is %NULL.
  * @cb: Completion function for the job.
  * @opaque: Opaque pointer value passed to @cb.
+ * @errp: Error object.
  *
  * Start a streaming operation on @bs.  Clusters that are unallocated
  * in @bs, but allocated in any image between @base and @bs (both
@@ -424,8 +427,8 @@ void block_job_cancel_sync(BlockJob *job);
  * streaming job, the backing file of @bs will be changed to
  * @base_id in the written image and to @base in the live BlockDriverState.
  */
-int stream_start(BlockDriverState *bs, BlockDriverState *base,
-                 const char *base_id, BlockDriverCompletionFunc *cb,
-                 void *opaque);
+void stream_start(BlockDriverState *bs, BlockDriverState *base,
+                  const char *base_id, BlockDriverCompletionFunc *cb,
+                  void *opaque, Error **errp);
 
 #endif /* BLOCK_INT_H */

diff --git a/blockdev.c b/blockdev.c
index 0c2440e249eb7c9eb9aa28d47f03cd7ed0e82711..a41147749a028067d549c9f6b300f2e007237d37 100644 (file)
--- a/blockdev.c
+++ b/blockdev.c
@@ -1095,7 +1095,7 @@ void qmp_block_stream(const char *device, bool has_base,
 {
     BlockDriverState *bs;
     BlockDriverState *base_bs = NULL;
-    int ret;
+    Error *local_err = NULL;
 
     bs = bdrv_find(device);
     if (!bs) {
@@ -1111,16 +1111,10 @@ void qmp_block_stream(const char *device, bool has_base,
         }
     }
 
-    ret = stream_start(bs, base_bs, base, block_stream_cb, bs);
-    if (ret < 0) {
-        switch (ret) {
-        case -EBUSY:
-            error_set(errp, QERR_DEVICE_IN_USE, device);
-            return;
-        default:
-            error_set(errp, QERR_NOT_SUPPORTED);
-            return;
-        }
+    stream_start(bs, base_bs, base, block_stream_cb, bs, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+        return;
     }
 
     /* Grab a reference so hotplug does not delete the BlockDriverState from