[Qemu-devel] [PATCH v2 5/5] blockjob: add Job parameter to BlockCompletionFunc

[John Snow <jsnow@redhat.com>]

It will no longer be sufficient to rely on the opaque parameter
containing a BDS, and there's no way to reliably include a
self-reference to the job we're creating, so always pass the Job
object forward to any callbacks.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/backup.c            |  2 +-
 block/commit.c            |  2 +-
 block/mirror.c            |  6 +++---
 block/stream.c            |  2 +-
 blockdev.c                | 14 +++++---------
 blockjob.c                | 13 +++++++++++--
 include/block/block.h     |  2 ++
 include/block/block_int.h | 10 +++++-----
 include/block/blockjob.h  |  6 ++++--
 qemu-img.c                |  4 ++--
 tests/test-blockjob-txn.c |  4 ++--
 11 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 735cbe6..dd7e532 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -492,7 +492,7 @@ BlockJob *backup_start(BlockDriverState *bs, BlockDriverState *target,
                        BdrvDirtyBitmap *sync_bitmap,
                        BlockdevOnError on_source_error,
                        BlockdevOnError on_target_error,
-                       BlockCompletionFunc *cb, void *opaque,
+                       BlockJobCompletionFunc *cb, void *opaque,
                        BlockJobTxn *txn, Error **errp)
 {
     int64_t len;
diff --git a/block/commit.c b/block/commit.c
index 446a3ae..aca4b84 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -203,7 +203,7 @@ static const BlockJobDriver commit_job_driver = {
 
 void commit_start(BlockDriverState *bs, BlockDriverState *base,
                   BlockDriverState *top, int64_t speed,
-                  BlockdevOnError on_error, BlockCompletionFunc *cb,
+                  BlockdevOnError on_error, BlockJobCompletionFunc *cb,
                   void *opaque, const char *backing_file_str, Error **errp)
 {
     CommitBlockJob *s;
diff --git a/block/mirror.c b/block/mirror.c
index b193e36..8016834 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -715,7 +715,7 @@ static BlockJob *mirror_start_job(BlockDriverState *bs,
                                   BlockdevOnError on_source_error,
                                   BlockdevOnError on_target_error,
                                   bool unmap,
-                                  BlockCompletionFunc *cb,
+                                  BlockJobCompletionFunc *cb,
                                   void *opaque, Error **errp,
                                   const BlockJobDriver *driver,
                                   bool is_none_mode, BlockDriverState *base)
@@ -802,7 +802,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
                   MirrorSyncMode mode, BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   bool unmap,
-                  BlockCompletionFunc *cb,
+                  BlockJobCompletionFunc *cb,
                   void *opaque, Error **errp)
 {
     bool is_none_mode;
@@ -827,7 +827,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
 BlockJob *commit_active_start(BlockDriverState *bs, BlockDriverState *base,
                               int64_t speed,
                               BlockdevOnError on_error,
-                              BlockCompletionFunc *cb,
+                              BlockJobCompletionFunc *cb,
                               void *opaque, Error **errp)
 {
     int64_t length, base_length;
diff --git a/block/stream.c b/block/stream.c
index 26a2990..0c8ae20 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -217,7 +217,7 @@ static const BlockJobDriver stream_job_driver = {
 BlockJob *stream_start(BlockDriverState *bs, BlockDriverState *base,
                   const char *backing_file_str, int64_t speed,
                   BlockdevOnError on_error,
-                  BlockCompletionFunc *cb,
+                  BlockJobCompletionFunc *cb,
                   void *opaque, Error **errp)
 {
     StreamBlockJob *s;
diff --git a/blockdev.c b/blockdev.c
index ad46aa8..9851405 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2854,28 +2854,24 @@ out:
     aio_context_release(aio_context);
 }
 
-static void block_job_cb(void *opaque, int ret)
+static void block_job_cb(BlockJob *job, int ret)
 {
     /* Note that this function may be executed from another AioContext besides
      * the QEMU main loop.  If you need to access anything that assumes the
      * QEMU global mutex, use a BH or introduce a mutex.
      */
-
-    BlockDriverState *bs = opaque;
     const char *msg = NULL;
 
-    trace_block_job_cb(bs, bs->job, ret);
-
-    assert(bs->job);
+    trace_block_job_cb(job->bs, job, ret);
 
     if (ret < 0) {
         msg = strerror(-ret);
     }
 
-    if (block_job_is_cancelled(bs->job)) {
-        block_job_event_cancelled(bs->job);
+    if (block_job_is_cancelled(job)) {
+        block_job_event_cancelled(job);
     } else {
-        block_job_event_completed(bs->job, msg);
+        block_job_event_completed(job, msg);
     }
 }
 
diff --git a/blockjob.c b/blockjob.c
index 80adb9d..fe6873c 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -51,7 +51,7 @@ struct BlockJobTxn {
 };
 
 void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
-                       int64_t speed, BlockCompletionFunc *cb,
+                       int64_t speed, BlockJobCompletionFunc *cb,
                        void *opaque, Error **errp)
 {
     BlockJob *job;
@@ -90,6 +90,15 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
     return job;
 }
 
+/**
+ * For generic callbacks to retrieve the data they submitted
+ * during callback registration.
+ */
+void *block_job_data(BlockJob *job)
+{
+    return job->opaque;
+}
+
 void block_job_ref(BlockJob *job)
 {
     ++job->refcnt;
@@ -118,7 +127,7 @@ static void block_job_completed_single(BlockJob *job)
             job->driver->abort(job);
         }
     }
-    job->cb(job->opaque, job->ret);
+    job->cb(job, job->ret);
     if (job->txn) {
         block_job_txn_unref(job->txn);
     }
diff --git a/include/block/block.h b/include/block/block.h
index 25f36dc..ed6f2ee 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -16,6 +16,8 @@ typedef struct BdrvChild BdrvChild;
 typedef struct BdrvChildRole BdrvChildRole;
 typedef struct BlockJobTxn BlockJobTxn;
 
+typedef void BlockJobCompletionFunc(BlockJob *job, int ret);
+
 typedef struct BlockDriverInfo {
     /* in bytes, 0 if irrelevant */
     int cluster_size;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 9a5cc39..271e922 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -601,7 +601,7 @@ int is_windows_drive(const char *filename);
 BlockJob *stream_start(BlockDriverState *bs, BlockDriverState *base,
                        const char *base_id, int64_t speed,
                        BlockdevOnError on_error,
-                       BlockCompletionFunc *cb,
+                       BlockJobCompletionFunc *cb,
                        void *opaque, Error **errp);
 
 /**
@@ -619,7 +619,7 @@ BlockJob *stream_start(BlockDriverState *bs, BlockDriverState *base,
  */
 void commit_start(BlockDriverState *bs, BlockDriverState *base,
                  BlockDriverState *top, int64_t speed,
-                 BlockdevOnError on_error, BlockCompletionFunc *cb,
+                 BlockdevOnError on_error, BlockJobCompletionFunc *cb,
                  void *opaque, const char *backing_file_str, Error **errp);
 /**
  * commit_active_start:
@@ -635,7 +635,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
 BlockJob *commit_active_start(BlockDriverState *bs, BlockDriverState *base,
                               int64_t speed,
                               BlockdevOnError on_error,
-                              BlockCompletionFunc *cb,
+                              BlockJobCompletionFunc *cb,
                               void *opaque, Error **errp);
 /*
  * mirror_start:
@@ -665,7 +665,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
                   MirrorSyncMode mode, BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   bool unmap,
-                  BlockCompletionFunc *cb,
+                  BlockJobCompletionFunc *cb,
                   void *opaque, Error **errp);
 
 /*
@@ -689,7 +689,7 @@ BlockJob *backup_start(BlockDriverState *bs, BlockDriverState *target,
                   BdrvDirtyBitmap *sync_bitmap,
                   BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
-                  BlockCompletionFunc *cb, void *opaque,
+                  BlockJobCompletionFunc *cb, void *opaque,
                   BlockJobTxn *txn, Error **errp);
 
 void blk_set_bs(BlockBackend *blk, BlockDriverState *bs);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index d84ccd8..b233d27 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -143,7 +143,7 @@ struct BlockJob {
     int64_t speed;
 
     /** The completion function that will be called when the job completes.  */
-    BlockCompletionFunc *cb;
+    BlockJobCompletionFunc *cb;
 
     /** Block other operations when block job is running */
     Error *blocker;
@@ -186,9 +186,11 @@ struct BlockJob {
  * called from a wrapper that is specific to the job type.
  */
 void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
-                       int64_t speed, BlockCompletionFunc *cb,
+                       int64_t speed, BlockJobCompletionFunc *cb,
                        void *opaque, Error **errp);
 
+void *block_job_data(BlockJob *job);
+
 /**
  * block_job_sleep_ns:
  * @job: The job that calls the function.
diff --git a/qemu-img.c b/qemu-img.c
index 8d11708..cc96cc8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -635,9 +635,9 @@ typedef struct CommonBlockJobCBInfo {
     Error **errp;
 } CommonBlockJobCBInfo;
 
-static void common_block_job_cb(void *opaque, int ret)
+static void common_block_job_cb(BlockJob *job, int ret)
 {
-    CommonBlockJobCBInfo *cbi = opaque;
+    CommonBlockJobCBInfo *cbi = block_job_data(job);
 
     if (ret < 0) {
         error_setg_errno(cbi->errp, -ret, "Block job failed");
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index 34747e9..3e8d17d 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -66,9 +66,9 @@ typedef struct {
     int *result;
 } TestBlockJobCBData;
 
-static void test_block_job_cb(void *opaque, int ret)
+static void test_block_job_cb(BlockJob *job, int ret)
 {
-    TestBlockJobCBData *data = opaque;
+    TestBlockJobCBData *data = block_job_data(job);
     if (!ret && block_job_is_cancelled(&data->job->common)) {
         ret = -ECANCELED;
     }
-- 
2.4.3