[v2,05/15] blockjob: Add 'job_id' parameter to block_job_create()

Commit Message

Alberto Garcia June 22, 2016, 12:25 p.m. UTC

Job IDs are generated automatically when a new job is created. This
patch adds a new 'job_id' parameter to let the caller provide one
instead. In this case the ID is verified to be unique and well-formed.

Signed-off-by: Alberto Garcia <[e-mail]>
Reviewed-by: Max Reitz <[e-mail]>
---
 block/backup.c            |  3 ++-
 block/commit.c            |  2 +-
 block/mirror.c            |  2 +-
 block/stream.c            |  2 +-
 blockjob.c                | 19 +++++++++++++++----
 include/block/blockjob.h  |  8 +++++---
 tests/test-blockjob-txn.c |  2 +-
 7 files changed, 26 insertions(+), 12 deletions(-)

Comments

Kevin Wolf June 22, 2016, 1:10 p.m. UTC | #1

Am 22.06.2016 um 14:25 hat Alberto Garcia geschrieben:
> Job IDs are generated automatically when a new job is created. This
> patch adds a new 'job_id' parameter to let the caller provide one
> instead. In this case the ID is verified to be unique and well-formed.
> 
> Signed-off-by: Alberto Garcia <[e-mail]>
> Reviewed-by: Max Reitz <[e-mail]>

> @@ -140,7 +151,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
>  
>      job->driver        = driver;
>      job->device        = g_strdup(bdrv_get_device_name(bs));
> -    job->id            = id_generate(ID_JOB);
> +    job->id            = job_id ? g_strdup(job_id) : id_generate(ID_JOB);
>      job->blk           = blk;
>      job->cb            = cb;
>      job->opaque        = opaque;

This hunk will trivially conflict with removing device names. You can
keep my R-b while resolving that.

Reviewed-by: Kevin Wolf <[e-mail]>

Patch

diff --git a/block/backup.c b/block/backup.c
index 581269b..33bb2ce 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -542,7 +542,8 @@  void backup_start(BlockDriverState *bs, BlockDriverState *target,
         goto error;
     }
 
-    job = block_job_create(&backup_job_driver, bs, speed, cb, opaque, errp);
+    job = block_job_create(NULL, &backup_job_driver, bs, speed,
+                           cb, opaque, errp);
     if (!job) {
         goto error;
     }
diff --git a/block/commit.c b/block/commit.c
index 444333b..3535ba7 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -236,7 +236,7 @@  void commit_start(BlockDriverState *bs, BlockDriverState *base,
         return;
     }
 
-    s = block_job_create(&commit_job_driver, bs, speed, cb, opaque, errp);
+    s = block_job_create(NULL, &commit_job_driver, bs, speed, cb, opaque, errp);
     if (!s) {
         return;
     }
diff --git a/block/mirror.c b/block/mirror.c
index bcb1999..48253fb 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -867,7 +867,7 @@  static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
         buf_size = DEFAULT_MIRROR_BUF_SIZE;
     }
 
-    s = block_job_create(driver, bs, speed, cb, opaque, errp);
+    s = block_job_create(NULL, driver, bs, speed, cb, opaque, errp);
     if (!s) {
         return;
     }
diff --git a/block/stream.c b/block/stream.c
index c0efbda..e4319d3 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -226,7 +226,7 @@  void stream_start(BlockDriverState *bs, BlockDriverState *base,
 {
     StreamBlockJob *s;
 
-    s = block_job_create(&stream_job_driver, bs, speed, cb, opaque, errp);
+    s = block_job_create(NULL, &stream_job_driver, bs, speed, cb, opaque, errp);
     if (!s) {
         return;
     }
diff --git a/blockjob.c b/blockjob.c
index 51c6402..c7f6992 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -117,9 +117,9 @@  static void block_job_detach_aio_context(void *opaque)
     block_job_unref(job);
 }
 
-void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
-                       int64_t speed, BlockCompletionFunc *cb,
-                       void *opaque, Error **errp)
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
+                       BlockDriverState *bs, int64_t speed,
+                       BlockCompletionFunc *cb, void *opaque, Error **errp)
 {
     BlockBackend *blk;
     BlockJob *job;
@@ -129,6 +129,17 @@  void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
         return NULL;
     }
 
+    if (job_id) {
+        if (!id_wellformed(job_id)) {
+            error_setg(errp, "Invalid job ID '%s'", job_id);
+            return NULL;
+        }
+        if (block_job_get(job_id)) {
+            error_setg(errp, "Job ID '%s' already in use", job_id);
+            return NULL;
+        }
+    }
+
     blk = blk_new();
     blk_insert_bs(blk, bs);
 
@@ -140,7 +151,7 @@  void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
 
     job->driver        = driver;
     job->device        = g_strdup(bdrv_get_device_name(bs));
-    job->id            = id_generate(ID_JOB);
+    job->id            = job_id ? g_strdup(job_id) : id_generate(ID_JOB);
     job->blk           = blk;
     job->cb            = cb;
     job->opaque        = opaque;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 9f28230..0fd6eaa 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -229,6 +229,8 @@  BlockJob *block_job_get(const char *id);
 
 /**
  * block_job_create:
+ * @job_id: The id of the newly-created job, or %NULL to have one
+ * generated automatically.
  * @job_type: The class object for the newly-created job.
  * @bs: The block
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
@@ -245,9 +247,9 @@  BlockJob *block_job_get(const char *id);
  * This function is not part of the public job interface; it should be
  * 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,
-                       void *opaque, Error **errp);
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
+                       BlockDriverState *bs, int64_t speed,
+                       BlockCompletionFunc *cb, void *opaque, Error **errp);
 
 /**
  * block_job_sleep_ns:
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index d3030f1..84e2af5 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -94,7 +94,7 @@  static BlockJob *test_block_job_start(unsigned int iterations,
 
     data = g_new0(TestBlockJobCBData, 1);
     bs = bdrv_new();
-    s = block_job_create(&test_block_job_driver, bs, 0, test_block_job_cb,
+    s = block_job_create(NULL, &test_block_job_driver, bs, 0, test_block_job_cb,
                          data, &error_abort);
     s->iterations = iterations;
     s->use_timer = use_timer;