diff mbox

[2.1,23/36] qapi event: convert other BLOCK_JOB events

Message ID 1403073840-32603-24-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini June 18, 2014, 6:43 a.m. UTC
From: Wenchao Xia <wenchaoqemu@gmail.com>

Since BLOCK_JOB_COMPLETED, BLOCK_JOB_CANCELLED, BLOCK_JOB_READY are
related, convert them in one patch. The block_job_event_* functions
are used to keep encapsulation of BlockJob structure.

Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/mirror.c           |  2 +-
 blockdev.c               | 12 ++++----
 blockjob.c               | 36 +++++++++++++-----------
 docs/qmp/qmp-events.txt  | 71 ------------------------------------------------
 include/block/blockjob.h | 17 +++++++++---
 qapi/block-core.json     | 71 ++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 110 insertions(+), 99 deletions(-)

Comments

Eric Blake June 19, 2014, 4:11 a.m. UTC | #1
On 06/18/2014 12:43 AM, Paolo Bonzini wrote:
> From: Wenchao Xia <wenchaoqemu@gmail.com>
> 
> Since BLOCK_JOB_COMPLETED, BLOCK_JOB_CANCELLED, BLOCK_JOB_READY are
> related, convert them in one patch. The block_job_event_* functions
> are used to keep encapsulation of BlockJob structure.
> 
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

> +++ b/include/block/blockjob.h
> @@ -217,12 +217,21 @@ void block_job_pause(BlockJob *job);
>  void block_job_resume(BlockJob *job);
>  
>  /**
> - * qobject_from_block_job:
> + * block_job_event_cancle:

s/cancle/cancel/

At this point, if you need to clean it up in a followup patch instead of
delaying the series, I can live with it.
Markus Armbruster June 26, 2014, 2:08 p.m. UTC | #2
Paolo Bonzini <pbonzini@redhat.com> writes:

> From: Wenchao Xia <wenchaoqemu@gmail.com>
>
> Since BLOCK_JOB_COMPLETED, BLOCK_JOB_CANCELLED, BLOCK_JOB_READY are
> related, convert them in one patch. The block_job_event_* functions
> are used to keep encapsulation of BlockJob structure.
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

This broke tests/qemu-iotests/040.  Apparently, the BLOCK_JOB_READY
event changed from

    {'timestamp': {'seconds': 1403791120, 'microseconds': 539716},
     'data': {'device': 'drive0',
               'type': 'commit',
               'speed': 0,
               'len': 1048576,
               'offset': 1048576},
     'event': 'BLOCK_JOB_READY'}

to just

    {'timestamp': {'seconds': 1403790920, 'microseconds': 123232},
     'data': {'device': 'drive0'},
     'event': 'BLOCK_JOB_READY'}

breaking qemu-iotests/040's tests of data/type and data/len.

Documentation of BLOCK_JOB_READY shows only data/device, before and
after this patch.

Suggests that

* Before this patch, code disagreed with docs, and qemu-iotests/040
  matched the code.

* After this patch, code matches docs, and qemu-iotests/040 matches
  neither.

Please advise.
Paolo Bonzini June 26, 2014, 2:19 p.m. UTC | #3
Il 26/06/2014 16:08, Markus Armbruster ha scritto:
>
> Suggests that
>
> * Before this patch, code disagreed with docs, and qemu-iotests/040
>   matched the code.
>
> * After this patch, code matches docs, and qemu-iotests/040 matches
>   neither.
>
> Please advise.

Code is always right. :)

Paolo
Luiz Capitulino June 26, 2014, 2:24 p.m. UTC | #4
On Thu, 26 Jun 2014 16:19:39 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 26/06/2014 16:08, Markus Armbruster ha scritto:
> >
> > Suggests that
> >
> > * Before this patch, code disagreed with docs, and qemu-iotests/040
> >   matched the code.
> >
> > * After this patch, code matches docs, and qemu-iotests/040 matches
> >   neither.
> >
> > Please advise.
> 
> Code is always right. :)

Agreed. We have to fix everything to match the code prior to the
conversion.

Are you going to take a stab at it yourself?
diff mbox

Patch

diff --git a/block/mirror.c b/block/mirror.c
index df58aea..301a04d 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -427,7 +427,7 @@  static void coroutine_fn mirror_run(void *opaque)
                  */
                 s->common.offset = end * BDRV_SECTOR_SIZE;
                 if (!s->synced) {
-                    block_job_ready(&s->common);
+                    block_job_event_ready(&s->common);
                     s->synced = true;
                 }
 
diff --git a/blockdev.c b/blockdev.c
index 9b0f8ac..03ab153 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1847,23 +1847,21 @@  void qmp_block_resize(bool has_device, const char *device,
 static void block_job_cb(void *opaque, int ret)
 {
     BlockDriverState *bs = opaque;
-    QObject *obj;
+    const char *msg = NULL;
 
     trace_block_job_cb(bs, bs->job, ret);
 
     assert(bs->job);
-    obj = qobject_from_block_job(bs->job);
+
     if (ret < 0) {
-        QDict *dict = qobject_to_qdict(obj);
-        qdict_put(dict, "error", qstring_from_str(strerror(-ret)));
+        msg = strerror(-ret);
     }
 
     if (block_job_is_cancelled(bs->job)) {
-        monitor_protocol_event(QEVENT_BLOCK_JOB_CANCELLED, obj);
+        block_job_event_cancelled(bs->job);
     } else {
-        monitor_protocol_event(QEVENT_BLOCK_JOB_COMPLETED, obj);
+        block_job_event_completed(bs->job, msg);
     }
-    qobject_decref(obj);
 
     bdrv_put_ref_bh_schedule(bs);
 }
diff --git a/blockjob.c b/blockjob.c
index ee2a6fb..4da86cd 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -26,7 +26,6 @@ 
 #include "config-host.h"
 #include "qemu-common.h"
 #include "trace.h"
-#include "monitor/monitor.h"
 #include "block/block.h"
 #include "block/blockjob.h"
 #include "block/block_int.h"
@@ -233,26 +232,31 @@  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,
+                                        bdrv_get_device_name(job->bs),
+                                        job->len,
+                                        job->offset,
+                                        job->speed,
+                                        &error_abort);
+}
 
-QObject *qobject_from_block_job(BlockJob *job)
+void block_job_event_completed(BlockJob *job, const char *msg)
 {
-    return qobject_from_jsonf("{ 'type': %s,"
-                              "'device': %s,"
-                              "'len': %" PRId64 ","
-                              "'offset': %" PRId64 ","
-                              "'speed': %" PRId64 " }",
-                              BlockJobType_lookup[job->driver->job_type],
-                              bdrv_get_device_name(job->bs),
-                              job->len,
-                              job->offset,
-                              job->speed);
+    qapi_event_send_block_job_completed(job->driver->job_type,
+                                        bdrv_get_device_name(job->bs),
+                                        job->len,
+                                        job->offset,
+                                        job->speed,
+                                        !!msg,
+                                        msg,
+                                        &error_abort);
 }
 
-void block_job_ready(BlockJob *job)
+void block_job_event_ready(BlockJob *job)
 {
-    QObject *data = qobject_from_block_job(job);
-    monitor_protocol_event(QEVENT_BLOCK_JOB_READY, data);
-    qobject_decref(data);
+    qapi_event_send_block_job_ready(bdrv_get_device_name(job->bs), &error_abort);
 }
 
 BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index eec3955..c57d5df 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -18,77 +18,6 @@  Example:
     "data": { "actual": 944766976 },
     "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
 
-BLOCK_JOB_CANCELLED
--------------------
-
-Emitted when a block job has been cancelled.
-
-Data:
-
-- "type":     Job type (json-string; "stream" for image streaming
-                                     "commit" for block commit)
-- "device":   Device name (json-string)
-- "len":      Maximum progress value (json-int)
-- "offset":   Current progress value (json-int)
-              On success this is equal to len.
-              On failure this is less than len.
-- "speed":    Rate limit, bytes per second (json-int)
-
-Example:
-
-{ "event": "BLOCK_JOB_CANCELLED",
-     "data": { "type": "stream", "device": "virtio-disk0",
-               "len": 10737418240, "offset": 134217728,
-               "speed": 0 },
-     "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
-
-BLOCK_JOB_COMPLETED
--------------------
-
-Emitted when a block job has completed.
-
-Data:
-
-- "type":     Job type (json-string; "stream" for image streaming
-                                     "commit" for block commit)
-- "device":   Device name (json-string)
-- "len":      Maximum progress value (json-int)
-- "offset":   Current progress value (json-int)
-              On success this is equal to len.
-              On failure this is less than len.
-- "speed":    Rate limit, bytes per second (json-int)
-- "error":    Error message (json-string, optional)
-              Only present on failure.  This field contains a human-readable
-              error message.  There are no semantics other than that streaming
-              has failed and clients should not try to interpret the error
-              string.
-
-Example:
-
-{ "event": "BLOCK_JOB_COMPLETED",
-     "data": { "type": "stream", "device": "virtio-disk0",
-               "len": 10737418240, "offset": 10737418240,
-               "speed": 0 },
-     "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
-
-BLOCK_JOB_READY
----------------
-
-Emitted when a block job is ready to complete.
-
-Data:
-
-- "device": device name (json-string)
-
-Example:
-
-{ "event": "BLOCK_JOB_READY",
-    "data": { "device": "ide0-hd1" },
-    "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
-
-Note: The "ready to complete" status is always reset by a BLOCK_JOB_ERROR
-event.
-
 GUEST_PANICKED
 --------------
 
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index c0a7875..e443987 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -217,12 +217,21 @@  void block_job_pause(BlockJob *job);
 void block_job_resume(BlockJob *job);
 
 /**
- * qobject_from_block_job:
+ * block_job_event_cancle:
  * @job: The job whose information is requested.
  *
- * Return a QDict corresponding to @job's query-block-jobs entry.
+ * Send a BLOCK_JOB_CANCELLED event for the specified job.
  */
-QObject *qobject_from_block_job(BlockJob *job);
+void block_job_event_cancelled(BlockJob *job);
+
+/**
+ * block_job_ready:
+ * @job: The job which is now ready to complete.
+ * @msg: Error message. Only present on failure.
+ *
+ * Send a BLOCK_JOB_COMPLETED event for the specified job.
+ */
+void block_job_event_completed(BlockJob *job, const char *msg);
 
 /**
  * block_job_ready:
@@ -230,7 +239,7 @@  QObject *qobject_from_block_job(BlockJob *job);
  *
  * Send a BLOCK_JOB_READY event for the specified job.
  */
-void block_job_ready(BlockJob *job);
+void block_job_event_ready(BlockJob *job);
 
 /**
  * block_job_is_paused:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2dbac42..af6b436 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1473,6 +1473,62 @@ 
             'action': 'BlockErrorAction' } }
 
 ##
+# @BLOCK_JOB_COMPLETED
+#
+# Emitted when a block job has completed
+#
+# @type: job type
+#
+# @device: device name
+#
+# @len: maximum progress value
+#
+# @offset: current progress value. On success this is equal to len.
+#          On failure this is less than len
+#
+# @speed: rate limit, bytes per second
+#
+# @error: #optional, error message. Only present on failure. This field
+#         contains a human-readable error message. There are no semantics
+#         other than that streaming has failed and clients should not try to
+#         interpret the error string
+#
+# Since: 1.1
+##
+{ 'event': 'BLOCK_JOB_COMPLETED',
+  'data': { 'type'  : 'BlockJobType',
+            'device': 'str',
+            'len'   : 'int',
+            'offset': 'int',
+            'speed' : 'int',
+            '*error': 'str' } }
+
+##
+# @BLOCK_JOB_CANCELLED
+#
+# Emitted when a block job has been cancelled
+#
+# @type: job type
+#
+# @device: device name
+#
+# @len: maximum progress value
+#
+# @offset: current progress value. On success this is equal to len.
+#          On failure this is less than len
+#
+# @speed: rate limit, bytes per second
+#
+# Since: 1.1
+##
+{ 'event': 'BLOCK_JOB_CANCELLED',
+  'data': { 'type'  : 'BlockJobType',
+            'device': 'str',
+            'len'   : 'int',
+            'offset': 'int',
+            'speed' : 'int' } }
+
+##
 # @BLOCK_JOB_ERROR
 #
 # Emitted when a block job encounters an error
@@ -1489,3 +1545,18 @@ 
   'data': { 'device'   : 'str',
             'operation': 'IoOperationType',
             'action'   : 'BlockdevOnError' } }
+
+##
+# @BLOCK_JOB_READY
+#
+# Emitted when a block job is ready to complete
+#
+# @device: device name
+#
+# Note: The "ready to complete" status is always reset by a @BLOCK_JOB_ERROR
+# event
+#
+# Since: 1.3
+##
+{ 'event': 'BLOCK_JOB_READY',
+  'data': { 'device': 'str' } }