diff mbox

[2.1,21/36] qapi event: convert BLOCK_IO_ERROR and BLOCK_JOB_ERROR

Message ID 1403073840-32603-22-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>

Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c                   | 36 ++++--------------------------------
 blockjob.c                |  6 +++++-
 docs/qmp/qmp-events.txt   | 47 -----------------------------------------------
 include/block/block_int.h |  3 ---
 qapi-schema.json          | 14 ++++++++++++++
 qapi/block-core.json      | 39 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 62 insertions(+), 83 deletions(-)

Comments

Markus Armbruster June 26, 2014, 3:37 p.m. UTC | #1 
Paolo Bonzini <pbonzini@redhat.com> writes:

> From: Wenchao Xia <wenchaoqemu@gmail.com>
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

This broke tests/qemu-iotests/041.  Apparently, the BLOCK_JOB_ERROR
event changed from

    {'timestamp': {'seconds': 1403796871, 'microseconds': 446502},
     'data': {'device': 'drive0',
              'action': 'ignore',
              'operation': 'write'},
     'event': 'BLOCK_JOB_ERROR'}

to

    {'timestamp': {'seconds': 1403796674, 'microseconds': 63271},
     'data': {'device': '',
              'action': 'report',
              'operation': 'write'},
     'event': 'BLOCK_JOB_ERROR'}

losing the value of device, which breaks qemu-iotests/041's tests of
data/device.

Please fix.
Paolo Bonzini June 26, 2014, 3:40 p.m. UTC | #2 
Il 26/06/2014 17:37, Markus Armbruster ha scritto:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> From: Wenchao Xia <wenchaoqemu@gmail.com>
>>
>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> This broke tests/qemu-iotests/041.  Apparently, the BLOCK_JOB_ERROR
> event changed from
> 
>     {'timestamp': {'seconds': 1403796871, 'microseconds': 446502},
>      'data': {'device': 'drive0',

-    bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action, is_read);
+    qapi_event_send_block_job_error(bdrv_get_device_name(bs),

Should have been job->bs.

>               'action': 'ignore',
>               'operation': 'write'},
>      'event': 'BLOCK_JOB_ERROR'}
> 
> to
> 
>     {'timestamp': {'seconds': 1403796674, 'microseconds': 63271},
>      'data': {'device': '',
>               'action': 'report',

I suppose ignore vs. report is just from pasting two different sections?

Paolo

>               'operation': 'write'},
>      'event': 'BLOCK_JOB_ERROR'}
> 
> losing the value of device, which breaks qemu-iotests/041's tests of
> data/device.
> 
> Please fix.
> 
>
Luiz Capitulino June 26, 2014, 3:44 p.m. UTC | #3 
On Thu, 26 Jun 2014 17:40:57 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 26/06/2014 17:37, Markus Armbruster ha scritto:
> > Paolo Bonzini <pbonzini@redhat.com> writes:
> > 
> >> From: Wenchao Xia <wenchaoqemu@gmail.com>
> >>
> >> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > 
> > This broke tests/qemu-iotests/041.  Apparently, the BLOCK_JOB_ERROR
> > event changed from
> > 
> >     {'timestamp': {'seconds': 1403796871, 'microseconds': 446502},
> >      'data': {'device': 'drive0',
> 
> -    bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action, is_read);
> +    qapi_event_send_block_job_error(bdrv_get_device_name(bs),
> 
> Should have been job->bs.

Yeah, patch is underway.

> 
> >               'action': 'ignore',
> >               'operation': 'write'},
> >      'event': 'BLOCK_JOB_ERROR'}
> > 
> > to
> > 
> >     {'timestamp': {'seconds': 1403796674, 'microseconds': 63271},
> >      'data': {'device': '',
> >               'action': 'report',
> 
> I suppose ignore vs. report is just from pasting two different sections?
> 
> Paolo
> 
> >               'operation': 'write'},
> >      'event': 'BLOCK_JOB_ERROR'}
> > 
> > losing the value of device, which breaks qemu-iotests/041's tests of
> > data/device.
> > 
> > Please fix.
> > 
> > 
>
Markus Armbruster June 26, 2014, 3:58 p.m. UTC | #4 
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 26/06/2014 17:37, Markus Armbruster ha scritto:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> From: Wenchao Xia <wenchaoqemu@gmail.com>
>>>
>>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> 
>> This broke tests/qemu-iotests/041.  Apparently, the BLOCK_JOB_ERROR
>> event changed from
>> 
>>     {'timestamp': {'seconds': 1403796871, 'microseconds': 446502},
>>      'data': {'device': 'drive0',
>
> - bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action,
> is_read);
> +    qapi_event_send_block_job_error(bdrv_get_device_name(bs),
>
> Should have been job->bs.

Will you post a patch, or would you like me to do that?

>>               'action': 'ignore',
>>               'operation': 'write'},
>>      'event': 'BLOCK_JOB_ERROR'}
>> 
>> to
>> 
>>     {'timestamp': {'seconds': 1403796674, 'microseconds': 63271},
>>      'data': {'device': '',
>>               'action': 'report',
>
> I suppose ignore vs. report is just from pasting two different sections?

Could well be; it's almost time for Fußball & Bier ;)
Luiz Capitulino June 26, 2014, 4 p.m. UTC | #5 
On Thu, 26 Jun 2014 17:58:21 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > Il 26/06/2014 17:37, Markus Armbruster ha scritto:
> >> Paolo Bonzini <pbonzini@redhat.com> writes:
> >> 
> >>> From: Wenchao Xia <wenchaoqemu@gmail.com>
> >>>
> >>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> 
> >> This broke tests/qemu-iotests/041.  Apparently, the BLOCK_JOB_ERROR
> >> event changed from
> >> 
> >>     {'timestamp': {'seconds': 1403796871, 'microseconds': 446502},
> >>      'data': {'device': 'drive0',
> >
> > - bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action,
> > is_read);
> > +    qapi_event_send_block_job_error(bdrv_get_device_name(bs),
> >
> > Should have been job->bs.
> 
> Will you post a patch, or would you like me to do that?

I was going to do it myself, but it would be great if you could do that.

> 
> >>               'action': 'ignore',
> >>               'operation': 'write'},
> >>      'event': 'BLOCK_JOB_ERROR'}
> >> 
> >> to
> >> 
> >>     {'timestamp': {'seconds': 1403796674, 'microseconds': 63271},
> >>      'data': {'device': '',
> >>               'action': 'report',
> >
> > I suppose ignore vs. report is just from pasting two different sections?
> 
> Could well be; it's almost time for Fußball & Bier ;)
>
Markus Armbruster June 27, 2014, 12:21 p.m. UTC | #6 
Markus Armbruster <armbru@redhat.com> writes:

> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> Il 26/06/2014 17:37, Markus Armbruster ha scritto:
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>> 
>>>> From: Wenchao Xia <wenchaoqemu@gmail.com>
>>>>
>>>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> 
>>> This broke tests/qemu-iotests/041.  Apparently, the BLOCK_JOB_ERROR
>>> event changed from
>>> 
>>>     {'timestamp': {'seconds': 1403796871, 'microseconds': 446502},
>>>      'data': {'device': 'drive0',
>>
>> - bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action,
>> is_read);
>> +    qapi_event_send_block_job_error(bdrv_get_device_name(bs),
>>
>> Should have been job->bs.
>
> Will you post a patch, or would you like me to do that?
>
>>>               'action': 'ignore',
>>>               'operation': 'write'},
>>>      'event': 'BLOCK_JOB_ERROR'}
>>> 
>>> to
>>> 
>>>     {'timestamp': {'seconds': 1403796674, 'microseconds': 63271},
>>>      'data': {'device': '',
>>>               'action': 'report',
>>
>> I suppose ignore vs. report is just from pasting two different sections?
>
> Could well be; it's almost time for Fußball & Bier ;)

Nope, it's a genuine error in the patch.  Working on a fix.
diff mbox

Patch

diff --git a/block.c b/block.c
index 0d0e5cd..041f17a 100644
--- a/block.c
+++ b/block.c
@@ -24,7 +24,6 @@ 
 #include "config-host.h"
 #include "qemu-common.h"
 #include "trace.h"
-#include "monitor/monitor.h"
 #include "block/block_int.h"
 #include "block/blockjob.h"
 #include "qemu/module.h"
@@ -2133,36 +2132,6 @@  void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
     bs->dev_opaque = opaque;
 }
 
-void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               enum MonitorEvent ev,
-                               BlockErrorAction action, bool is_read)
-{
-    QObject *data;
-    const char *action_str;
-
-    switch (action) {
-    case BLOCK_ERROR_ACTION_REPORT:
-        action_str = "report";
-        break;
-    case BLOCK_ERROR_ACTION_IGNORE:
-        action_str = "ignore";
-        break;
-    case BLOCK_ERROR_ACTION_STOP:
-        action_str = "stop";
-        break;
-    default:
-        abort();
-    }
-
-    data = qobject_from_jsonf("{ 'device': %s, 'action': %s, 'operation': %s }",
-                              bdrv->device_name,
-                              action_str,
-                              is_read ? "read" : "write");
-    monitor_protocol_event(ev, data);
-
-    qobject_decref(data);
-}
-
 static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load)
 {
     if (bs->dev_ops && bs->dev_ops->change_media_cb) {
@@ -3619,7 +3588,10 @@  void bdrv_error_action(BlockDriverState *bs, BlockErrorAction action,
                        bool is_read, int error)
 {
     assert(error >= 0);
-    bdrv_emit_qmp_error_event(bs, QEVENT_BLOCK_IO_ERROR, action, is_read);
+    qapi_event_send_block_io_error(bdrv_get_device_name(bs),
+                                   is_read ? IO_OPERATION_TYPE_READ :
+                                   IO_OPERATION_TYPE_WRITE,
+                                   action, &error_abort);
     if (action == BLOCK_ERROR_ACTION_STOP) {
         vm_stop(RUN_STATE_IO_ERROR);
         bdrv_iostatus_set_err(bs, error);
diff --git a/blockjob.c b/blockjob.c
index bc63d42..ee2a6fb 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -34,6 +34,7 @@ 
 #include "block/coroutine.h"
 #include "qmp-commands.h"
 #include "qemu/timer.h"
+#include "qapi-event.h"
 
 void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
                        int64_t speed, BlockDriverCompletionFunc *cb,
@@ -277,7 +278,10 @@  BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
     default:
         abort();
     }
-    bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action, is_read);
+    qapi_event_send_block_job_error(bdrv_get_device_name(bs),
+                                    is_read ? IO_OPERATION_TYPE_READ :
+                                    IO_OPERATION_TYPE_WRITE,
+                                    action, &error_abort);
     if (action == BLOCK_ERROR_ACTION_STOP) {
         block_job_pause(job);
         block_job_iostatus_set_err(job, error);
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index 1ee6f53..f67a9ec 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -40,31 +40,6 @@  Example:
         "size": 65536 },
     "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
 
-BLOCK_IO_ERROR
---------------
-
-Emitted when a disk I/O error occurs.
-
-Data:
-
-- "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):
-    "ignore": error has been ignored
-    "report": error has been reported to the device
-    "stop": error caused VM to be stopped
-
-Example:
-
-{ "event": "BLOCK_IO_ERROR",
-    "data": { "device": "ide0-hd1",
-              "operation": "write",
-              "action": "stop" },
-    "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
-
-Note: If action is "stop", a STOP event will eventually follow the
-BLOCK_IO_ERROR event.
-
 BLOCK_JOB_CANCELLED
 -------------------
 
@@ -118,28 +93,6 @@  Example:
                "speed": 0 },
      "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
 
-BLOCK_JOB_ERROR
----------------
-
-Emitted when a block job encounters an error.
-
-Data:
-
-- "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):
-    "ignore": error has been ignored, the job may fail later
-    "report": error will be reported and the job canceled
-    "stop": error caused job to be paused
-
-Example:
-
-{ "event": "BLOCK_JOB_ERROR",
-    "data": { "device": "ide0-hd1",
-              "operation": "write",
-              "action": "stop" },
-    "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
-
 BLOCK_JOB_READY
 ---------------
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7aa2213..715c761 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -425,9 +425,6 @@  void bdrv_attach_aio_context(BlockDriverState *bs,
 #ifdef _WIN32
 int is_windows_drive(const char *filename);
 #endif
-void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               enum MonitorEvent ev,
-                               BlockErrorAction action, bool is_read);
 
 /**
  * stream_start:
diff --git a/qapi-schema.json b/qapi-schema.json
index 33d76bc..0a8df60 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3165,4 +3165,18 @@ 
 { 'enum': 'WatchdogExpirationAction',
   'data': [ 'reset', 'shutdown', 'poweroff', 'pause', 'debug', 'none' ] }
 
+##
+# @IoOperationType
+#
+# An enumeration of the I/O operation types
+#
+# @read: read operation
+#
+# @write: write operation
+#
+# Since: 2.1
+##
+{ 'enum': 'IoOperationType',
+  'data': [ 'read', 'write' ] }
+
 { 'include': 'qapi-event.json' }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b7f36c6..682864d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1426,3 +1426,42 @@ 
 ##
 { 'enum': 'BlockErrorAction',
   'data': [ 'ignore', 'report', 'stop' ] }
+
+
+##
+# @BLOCK_IO_ERROR
+#
+# Emitted when a disk I/O error occurs
+#
+# @device: device name
+#
+# @operation: I/O operation
+#
+# @action: action that has been taken
+#
+# Note: If action is "stop", a STOP event will eventually follow the
+# BLOCK_IO_ERROR event
+#
+# Since: 0.13.0
+##
+{ 'event': 'BLOCK_IO_ERROR',
+  'data': { 'device': 'str', 'operation': 'IoOperationType',
+            'action': 'BlockErrorAction' } }
+
+##
+# @BLOCK_JOB_ERROR
+#
+# Emitted when a block job encounters an error
+#
+# @device: device name
+#
+# @operation: I/O operation
+#
+# @action: action that has been taken
+#
+# Since: 1.3
+##
+{ 'event': 'BLOCK_JOB_ERROR',
+  'data': { 'device'   : 'str',
+            'operation': 'IoOperationType',
+            'action'   : 'BlockdevOnError' } }