diff mbox

[15/15] blockjob: Add 'id' field to 'BlockJobInfo' and all BLOCK_JOB_* events

Message ID de472afd02ddd58a88fd663b52fae4c9bdd74ee8.1465459496.git.berto@igalia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alberto Garcia June 9, 2016, 8:20 a.m. UTC 
Now that all jobs have an ID and all QMP commands that create and
operate on block jobs can specify one we can finally expose the ID in
all BLOCK_JOB_* events and the BlockJobInfo structure.

This modifies the output of several iotests, but now we have full
control of the job IDs so this patch updates the affected ones.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 blockjob.c                 |  6 +++++-
 docs/qmp-events.txt        |  4 ++++
 qapi/block-core.json       | 18 ++++++++++++++++--
 tests/qemu-iotests/095     |  2 +-
 tests/qemu-iotests/095.out |  2 +-
 tests/qemu-iotests/124     |  3 ++-
 tests/qemu-iotests/141     |  6 +++++-
 tests/qemu-iotests/141.out | 14 +++++++-------
 tests/qemu-iotests/144     |  1 +
 tests/qemu-iotests/144.out |  4 ++--
 10 files changed, 44 insertions(+), 16 deletions(-)

Comments

Max Reitz June 20, 2016, 7:14 p.m. UTC | #1 
On 09.06.2016 10:20, Alberto Garcia wrote:
> Now that all jobs have an ID and all QMP commands that create and
> operate on block jobs can specify one we can finally expose the ID in
> all BLOCK_JOB_* events and the BlockJobInfo structure.
> 
> This modifies the output of several iotests, but now we have full
> control of the job IDs so this patch updates the affected ones.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  blockjob.c                 |  6 +++++-
>  docs/qmp-events.txt        |  4 ++++
>  qapi/block-core.json       | 18 ++++++++++++++++--
>  tests/qemu-iotests/095     |  2 +-
>  tests/qemu-iotests/095.out |  2 +-
>  tests/qemu-iotests/124     |  3 ++-
>  tests/qemu-iotests/141     |  6 +++++-
>  tests/qemu-iotests/141.out | 14 +++++++-------
>  tests/qemu-iotests/144     |  1 +
>  tests/qemu-iotests/144.out |  4 ++--
>  10 files changed, 44 insertions(+), 16 deletions(-)

Looks good, but in the meantime iotest 156 has been added which needs
the same treatment as the rest.

Also, I personally wouldn't mind making use of the IDs where trivial.
For example, you can just replace the "'device': 'virtio0'" by "'id':
'commit1'" in the block-job-complete invocation in 144. Optional, of
course, but would make sense.

Max
Alberto Garcia June 21, 2016, 2:23 p.m. UTC | #2 
On Mon 20 Jun 2016 09:14:04 PM CEST, Max Reitz wrote:

> Also, I personally wouldn't mind making use of the IDs where trivial.
> For example, you can just replace the "'device': 'virtio0'" by "'id':
> 'commit1'" in the block-job-complete invocation in 144. Optional, of
> course, but would make sense.

My plan was to leave this patch with the minimum necessary changes and
then write additional tests for the job ID with all possible scenarios.

Berto
diff mbox

Patch

diff --git a/blockjob.c b/blockjob.c
index dd0eb7f..0b7fe50 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -416,6 +416,7 @@  BlockJobInfo *block_job_query(BlockJob *job)
 {
     BlockJobInfo *info = g_new0(BlockJobInfo, 1);
     info->type      = g_strdup(BlockJobType_lookup[job->driver->job_type]);
+    info->id        = g_strdup(job->id);
     info->device    = g_strdup(job->device);
     info->len       = job->len;
     info->busy      = job->busy;
@@ -438,6 +439,7 @@  static void block_job_iostatus_set_err(BlockJob *job, int error)
 void block_job_event_cancelled(BlockJob *job)
 {
     qapi_event_send_block_job_cancelled(job->driver->job_type,
+                                        job->id,
                                         job->device,
                                         job->len,
                                         job->offset,
@@ -448,6 +450,7 @@  void block_job_event_cancelled(BlockJob *job)
 void block_job_event_completed(BlockJob *job, const char *msg)
 {
     qapi_event_send_block_job_completed(job->driver->job_type,
+                                        job->id,
                                         job->device,
                                         job->len,
                                         job->offset,
@@ -462,6 +465,7 @@  void block_job_event_ready(BlockJob *job)
     job->ready = true;
 
     qapi_event_send_block_job_ready(job->driver->job_type,
+                                    job->id,
                                     job->device,
                                     job->len,
                                     job->offset,
@@ -490,7 +494,7 @@  BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,
     default:
         abort();
     }
-    qapi_event_send_block_job_error(job->device,
+    qapi_event_send_block_job_error(job->id, job->device,
                                     is_read ? IO_OPERATION_TYPE_READ :
                                     IO_OPERATION_TYPE_WRITE,
                                     action, &error_abort);
diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index fa7574d..2b2af0f 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -92,6 +92,7 @@  Data:
 
 - "type":     Job type (json-string; "stream" for image streaming
                                      "commit" for block commit)
+- "id":       Job identifier (json-string)
 - "device":   Device name (json-string)
 - "len":      Maximum progress value (json-int)
 - "offset":   Current progress value (json-int)
@@ -116,6 +117,7 @@  Data:
 
 - "type":     Job type (json-string; "stream" for image streaming
                                      "commit" for block commit)
+- "id":       Job identifier (json-string)
 - "device":   Device name (json-string)
 - "len":      Maximum progress value (json-int)
 - "offset":   Current progress value (json-int)
@@ -143,6 +145,7 @@  Emitted when a block job encounters an error.
 
 Data:
 
+- "id":     job identifier (json-string)
 - "device": device name (json-string)
 - "operation": I/O operation (json-string, "read" or "write")
 - "action": action that has been taken, it's one of the following (json-string):
@@ -167,6 +170,7 @@  Data:
 
 - "type":     Job type (json-string; "stream" for image streaming
                                      "commit" for block commit)
+- "id":       Job identifier (json-string)
 - "device":   Device name (json-string)
 - "len":      Maximum progress value (json-int)
 - "offset":   Current progress value (json-int)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f15e62f..b9544d1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -713,6 +713,8 @@ 
 #
 # @type: the job type ('stream' for image streaming)
 #
+# @id: the job identifier
+#
 # @device: the block device name
 #
 # @len: the maximum progress value
@@ -734,7 +736,7 @@ 
 # Since: 1.1
 ##
 { 'struct': 'BlockJobInfo',
-  'data': {'type': 'str', 'device': 'str', 'len': 'int',
+  'data': {'type': 'str', 'id': 'str', 'device': 'str', 'len': 'int',
            'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int',
            'io-status': 'BlockDeviceIoStatus', 'ready': 'bool'} }
 
@@ -2443,6 +2445,8 @@ 
 #
 # @type: job type
 #
+# @id: the job identifier
+#
 # @device: device name
 #
 # @len: maximum progress value
@@ -2461,6 +2465,7 @@ 
 ##
 { 'event': 'BLOCK_JOB_COMPLETED',
   'data': { 'type'  : 'BlockJobType',
+            'id'    : 'str',
             'device': 'str',
             'len'   : 'int',
             'offset': 'int',
@@ -2474,6 +2479,8 @@ 
 #
 # @type: job type
 #
+# @id: the job identifier
+#
 # @device: device name
 #
 # @len: maximum progress value
@@ -2487,6 +2494,7 @@ 
 ##
 { 'event': 'BLOCK_JOB_CANCELLED',
   'data': { 'type'  : 'BlockJobType',
+            'id'    : 'str',
             'device': 'str',
             'len'   : 'int',
             'offset': 'int',
@@ -2497,6 +2505,8 @@ 
 #
 # Emitted when a block job encounters an error
 #
+# @id: the job identifier
+#
 # @device: device name
 #
 # @operation: I/O operation
@@ -2506,7 +2516,8 @@ 
 # Since: 1.3
 ##
 { 'event': 'BLOCK_JOB_ERROR',
-  'data': { 'device'   : 'str',
+  'data': { 'id'       : 'str',
+            'device'   : 'str',
             'operation': 'IoOperationType',
             'action'   : 'BlockErrorAction' } }
 
@@ -2517,6 +2528,8 @@ 
 #
 # @type: job type
 #
+# @id: the job identifier
+#
 # @device: device name
 #
 # @len: maximum progress value
@@ -2533,6 +2546,7 @@ 
 ##
 { 'event': 'BLOCK_JOB_READY',
   'data': { 'type'  : 'BlockJobType',
+            'id'    : 'str',
             'device': 'str',
             'len'   : 'int',
             'offset': 'int',
diff --git a/tests/qemu-iotests/095 b/tests/qemu-iotests/095
index 030adb2..34617a7 100755
--- a/tests/qemu-iotests/095
+++ b/tests/qemu-iotests/095
@@ -71,7 +71,7 @@  h=$QEMU_HANDLE
 _send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return"
 
 _send_qemu_cmd $h "{ 'execute': 'block-commit',
-                                 'arguments': { 'device': 'test',
+                                 'arguments': { 'job-id': 'testjob', 'device': 'test',
                                  'top': '"${TEST_IMG}.snp1"' } }" "BLOCK_JOB_COMPLETED"
 
 _cleanup_qemu
diff --git a/tests/qemu-iotests/095.out b/tests/qemu-iotests/095.out
index 73875ca..c146c72 100644
--- a/tests/qemu-iotests/095.out
+++ b/tests/qemu-iotests/095.out
@@ -12,7 +12,7 @@  virtual size: 5.0M (5242880 bytes)
 
 {"return": {}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "test", "len": 104857600, "offset": 104857600, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "test", "len": 104857600, "offset": 104857600, "speed": 0, "type": "commit", "id": "testjob"}}
 
 === Base image info after commit and resize ===
 image: TEST_DIR/t.IMGFMT.base
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index de7cdbe..89d74cb 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -50,7 +50,7 @@  def transaction_bitmap_clear(node, name, **kwargs):
 
 def transaction_drive_backup(device, target, **kwargs):
     return transaction_action('drive-backup', device=device, target=target,
-                              **kwargs)
+                              job_id=device, **kwargs)
 
 
 class Bitmap:
@@ -479,6 +479,7 @@  class TestIncrementalBackup(TestIncrementalBackupBase):
         self.assertFalse(self.wait_qmp_backup(drive1['id']))
         error = self.vm.event_wait('BLOCK_JOB_ERROR')
         self.assert_qmp(error, 'data', {'device': drive1['id'],
+                                        'id': drive1['id'],
                                         'action': 'report',
                                         'operation': 'read'})
         self.assertFalse(self.vm.get_qmp_events(wait=False))
diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
index b2617e5..85d9d9d 100755
--- a/tests/qemu-iotests/141
+++ b/tests/qemu-iotests/141
@@ -102,6 +102,7 @@  echo
 test_blockjob \
     "{'execute': 'drive-backup',
       'arguments': {'device': 'drv0',
+                    'job-id': 'backup1',
                     'target': '$TEST_DIR/o.$IMGFMT',
                     'format': '$IMGFMT',
                     'sync': 'none'}}" \
@@ -118,6 +119,7 @@  echo
 test_blockjob \
     "{'execute': 'drive-mirror',
       'arguments': {'device': 'drv0',
+                    'job-id': 'mirror1',
                     'target': '$TEST_DIR/o.$IMGFMT',
                     'format': '$IMGFMT',
                     'sync': 'none'}}" \
@@ -134,7 +136,7 @@  echo
 
 test_blockjob \
     "{'execute': 'block-commit',
-      'arguments': {'device': 'drv0'}}" \
+      'arguments': {'device': 'drv0', 'job-id': 'commit1'}}" \
     'BLOCK_JOB_READY' \
     'BLOCK_JOB_COMPLETED'
 
@@ -151,6 +153,7 @@  $QEMU_IO -c 'write 0 1M' "$TEST_DIR/m.$IMGFMT" | _filter_qemu_io
 test_blockjob \
     "{'execute': 'block-commit',
       'arguments': {'device': 'drv0',
+                    'job-id': 'commit2',
                     'top':    '$TEST_DIR/m.$IMGFMT',
                     'speed':  1}}" \
     'return' \
@@ -173,6 +176,7 @@  $QEMU_IO -c 'write 0 1M' "$TEST_DIR/b.$IMGFMT" | _filter_qemu_io
 test_blockjob \
     "{'execute': 'block-stream',
       'arguments': {'device': 'drv0',
+                    'job-id': 'stream1',
                     'speed': 1}}" \
     'return' \
     'BLOCK_JOB_CANCELLED'
diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out
index adceac1..36c72bc 100644
--- a/tests/qemu-iotests/141.out
+++ b/tests/qemu-iotests/141.out
@@ -11,7 +11,7 @@  Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.
 {"return": {}}
 {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: backup"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 0, "speed": 0, "type": "backup"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 0, "speed": 0, "type": "backup", "id": "backup1"}}
 {"return": {}}
 
 === Testing drive-mirror ===
@@ -19,20 +19,20 @@  Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.
 {"return": {}}
 Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1"}}
 {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: mirror"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1"}}
 {"return": {}}
 
 === Testing active block-commit ===
 
 {"return": {}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit", "id": "commit1"}}
 {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit", "id": "commit1"}}
 {"return": {}}
 
 === Testing non-active block-commit ===
@@ -43,7 +43,7 @@  wrote 1048576/1048576 bytes at offset 0
 {"return": {}}
 {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "commit", "id": "commit2"}}
 {"return": {}}
 
 === Testing block-stream ===
@@ -54,6 +54,6 @@  wrote 1048576/1048576 bytes at offset 0
 {"return": {}}
 {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: stream"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "stream"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "stream", "id": "stream1"}}
 {"return": {}}
 *** done
diff --git a/tests/qemu-iotests/144 b/tests/qemu-iotests/144
index 00de3c3..09cebcc 100755
--- a/tests/qemu-iotests/144
+++ b/tests/qemu-iotests/144
@@ -85,6 +85,7 @@  echo
 # Block commit on active layer, push the new overlay into base
 _send_qemu_cmd $h "{ 'execute': 'block-commit',
                                 'arguments': {
+                                                 'job-id': 'commit1',
                                                  'device': 'virtio0'
                                               }
                     }" "READY"
diff --git a/tests/qemu-iotests/144.out b/tests/qemu-iotests/144.out
index 410d741..3208c1f 100644
--- a/tests/qemu-iotests/144.out
+++ b/tests/qemu-iotests/144.out
@@ -13,9 +13,9 @@  Formatting 'TEST_DIR/tmp.qcow2', fmt=qcow2 size=536870912 backing_file=TEST_DIR/
 === Performing block-commit on active layer ===
 
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit", "id": "commit1"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit", "id": "commit1"}}
 
 === Performing Live Snapshot 2 ===