blockjob: Add 'job_id' parameter to block_job_create()

When a new job is created, the job ID is taken from the device name of
the BDS. This patch adds a new 'job_id' parameter to let the caller
provide one instead.

This patch also verifies that the ID is always unique and well-formed.
This causes problems in a couple of places where no ID is being set,
because the BDS does not have a device name.

In the case of test_block_job_start() (from test-blockjob-txn.c) we
can simply use this new 'job_id' parameter to set the missing ID.

In the case of img_commit() (from qemu-img.c) we still don't have the
API to make commit_active_start() set the job ID, so we solve it by
setting a default value. We'll get rid of this as soon as we extend
the API.

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

block/backup.c

@@ -541,7 +541,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;
     }

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;
     }

block/mirror.c

@@ -873,7 +873,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;
     }

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;
     }

blockjob.c

@@ -33,6 +33,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
 #include "qemu/coroutine.h"
+#include "qemu/id.h"
 #include "qmp-commands.h"
 #include "qemu/timer.h"
 #include "qapi-event.h"
@@ -116,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,
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
-                       int64_t speed, BlockCompletionFunc *cb,
+                       BlockDriverState *bs, int64_t speed,
-                       void *opaque, Error **errp)
+                       BlockCompletionFunc *cb, void *opaque, Error **errp)
 {
     BlockBackend *blk;
     BlockJob *job;
@@ -129,6 +130,26 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
         return NULL;
     }
 
+    if (job_id == NULL) {
+        job_id = bdrv_get_device_name(bs);
+        /* Assign a default ID if the BDS does not have a device
+         * name. We'll get rid of this soon when we finish extending
+         * the API of all commands that create block jobs. */
+        if (job_id[0] == '\0') {
+            job_id = "default_job";
+        }
+    }
+
+    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);
 
@@ -139,7 +160,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
     bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
 
     job->driver        = driver;
-    job->id            = g_strdup(bdrv_get_device_name(bs));
+    job->id            = g_strdup(job_id);
     job->blk           = blk;
     job->cb            = cb;
     job->opaque        = opaque;

include/block/blockjob.h

@@ -223,6 +223,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.
@@ -239,9 +241,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,
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
-                       int64_t speed, BlockCompletionFunc *cb,
+                       BlockDriverState *bs, int64_t speed,
-                       void *opaque, Error **errp);
+                       BlockCompletionFunc *cb, void *opaque, Error **errp);
 
 /**
  * block_job_sleep_ns:

tests/test-blockjob-txn.c

@@ -91,11 +91,14 @@ static BlockJob *test_block_job_start(unsigned int iterations,
     BlockDriverState *bs;
     TestBlockJob *s;
     TestBlockJobCBData *data;
+    static unsigned counter;
+    char job_id[24];
 
     data = g_new0(TestBlockJobCBData, 1);
     bs = bdrv_new();
-    s = block_job_create(&test_block_job_driver, bs, 0, test_block_job_cb,
+    snprintf(job_id, sizeof(job_id), "job%u", counter++);
-                         data, &error_abort);
+    s = block_job_create(job_id, &test_block_job_driver, bs, 0,
+                         test_block_job_cb, data, &error_abort);
     s->iterations = iterations;
     s->use_timer = use_timer;
     s->rc = rc;