qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1
@ 2012-09-28 15:22 Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 01/19] qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE Paolo Bonzini
                   ` (20 more replies)
  0 siblings, 21 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Kevin,

the following changes since commit ac05f3492421caeb05809ffa02c6198ede179e43:

  add a boot parameter to set reboot timeout (2012-09-25 20:05:04 -0500)

are available in the git repository at:

  git://github.com/bonzini/qemu.git blkmirror-job-1.3-part1

for you to fetch changes up to ed306a929f16fda8a85561430b1ac370bde65097:

  qemu-iotests: add tests for streaming error handling (2012-09-27 15:11:22 +0200)

This message has the diff from my post of Sep 25th.  You reviewed 18 patches,
these are 19 because of the new "qmp: add 'busy' member to BlockJobInfo" patch.

----------------------------------------------------------------
Jeff Cody (1):
      blockdev: rename block_stream_cb to a generic block_job_cb

Paolo Bonzini (18):
      qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE
      block: fix documentation of block_job_cancel_sync
      block: move job APIs to separate files
      block: add block_job_query
      qmp: add 'busy' member to BlockJobInfo
      block: add support for job pause/resume
      qmp: add block-job-pause and block-job-resume
      qemu-iotests: add test for pausing a streaming operation
      block: rename block_job_complete to block_job_completed
      iostatus: rename BlockErrorAction, BlockQMPEventAction
      iostatus: move BlockdevOnError declaration to QAPI
      iostatus: change is_read to a bool
      iostatus: reorganize io error code
      block: introduce block job error
      stream: add on-error argument
      blkdebug: process all set_state rules in the old state
      qemu-iotests: map underscore to dash in QMP argument names
      qemu-iotests: add tests for streaming error handling

 Makefile.objs                 |   5 +-
 QMP/qmp-events.txt            |  22 ++++
 block.c                       | 187 ++++++++----------------------
 block.h                       |  20 ++--
 block/Makefile.objs           |   3 +-
 block/blkdebug.c              |  12 +-
 block/stream.c                |  33 +++++-
 block_int.h                   | 162 ++------------------------
 blockdev.c                    |  86 +++++++++-----
 blockjob.c                    | 249 ++++++++++++++++++++++++++++++++++++++++
 blockjob.h                    | 243 +++++++++++++++++++++++++++++++++++++++
 hmp-commands.hx               |  35 +++++-
 hmp.c                         |  26 ++++-
 hmp.h                         |   2 +
 hw/fdc.c                      |   4 +-
 hw/ide/core.c                 |  22 +---
 hw/ide/pci.c                  |   4 +-
 hw/scsi-disk.c                |  25 ++--
 hw/scsi-generic.c             |   4 +-
 hw/virtio-blk.c               |  23 ++--
 monitor.c                     |   1 +
 monitor.h                     |   1 +
 qapi-schema.json              |  91 ++++++++++++++-
 qemu-tool.c                   |   6 +
 qerror.h                      |   6 +
 qmp-commands.hx               |  14 ++-
 tests/qemu-iotests/030        | 260 +++++++++++++++++++++++++++++++++++++++++-
 tests/qemu-iotests/030.out    |   4 +-
 tests/qemu-iotests/group      |   2 +-
 tests/qemu-iotests/iotests.py |  15 ++-
 trace-events                  |   4 +-
 31 file modificati, 1152 inserzioni(+), 419 rimozioni(-)
 create mode 100644 blockjob.c
 create mode 100644 blockjob.h

diff --git a/blockjob.c b/blockjob.c
index 5dd9c1e..b5c16f3 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -198,6 +198,7 @@ BlockJobInfo *block_job_query(BlockJob *job)
     info->type      = g_strdup(job->job_type->job_type);
     info->device    = g_strdup(bdrv_get_device_name(job->bs));
     info->len       = job->len;
+    info->busy      = job->busy;
     info->paused    = job->paused;
     info->offset    = job->offset;
     info->speed     = job->speed;
diff --git a/blockjob.h b/blockjob.h
index 45cc03d..c2261a9 100644
--- a/blockjob.h
+++ b/blockjob.h
@@ -199,7 +199,8 @@ void block_job_resume(BlockJob *job);
  * block_job_is_paused:
  * @job: The job being queried.
  *
- * Returns whether the job is currently paused.
+ * Returns whether the job is currently paused, or will pause
+ * as soon as it reaches a sleeping point.
  */
 bool block_job_is_paused(BlockJob *job);
 
@@ -207,12 +208,10 @@ bool block_job_is_paused(BlockJob *job);
  * block_job_cancel_sync:
  * @job: The job to be canceled.
  *
- * Synchronously cancel the job and wait for it to reach a quiescent
- * state.  Note that the completion callback will still be called
- * asynchronously, hence it is *not* valid to call #bdrv_delete
- * immediately after #block_job_cancel_sync.  Users of block jobs
- * will usually protect the BlockDriverState objects with a reference
- * count, should this be a concern.
+ * Synchronously cancel the job.  The completion callback is called
+ * before the function returns.  The job may actually complete
+ * instead of canceling itself; the circumstances under which this
+ * happens depend on the kind of job that is active.
  *
  * Returns the return value from the job if the job actually completed
  * during the call, or -ECANCELED if it was canceled.
diff --git a/qapi-schema.json b/qapi-schema.json
index 8719a9d..d31ff0e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1100,11 +1100,11 @@
 # @ignore: ignore the error, only report a QMP event (BLOCK_IO_ERROR
 #          or BLOCK_JOB_ERROR)
 #
+# @enospc: same as @stop on ENOSPC, same as @report otherwise.
+#
 # @stop: for guest operations, stop the virtual machine;
 #        for jobs, pause the job
 #
-# @enospc: same as @stop on ENOSPC, same as @report otherwise.
-#
 # Since: 1.3
 ##
 { 'enum': 'BlockdevOnError',
@@ -1121,19 +1121,23 @@
 #
 # @len: the maximum progress value
 #
-# @paused: whether the job is paused (since 1.2)
+# @busy: false if the job is known to be in a quiescent state, with
+#        no pending I/O.  Since 1.3.
+#
+# @paused: whether the job is paused or, if @busy is true, will
+#          pause itself as soon as possible.  Since 1.3.
 #
 # @offset: the current progress value
 #
 # @speed: the rate limit, bytes per second
 #
-# @io-status: the status of the job (since 1.2)
+# @io-status: the status of the job (since 1.3)
 #
 # Since: 1.1
 ##
 { 'type': 'BlockJobInfo',
   'data': {'type': 'str', 'device': 'str', 'len': 'int',
-           'offset': 'int', 'paused': 'bool', 'speed': 'int',
+           'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int',
            'io-status': 'BlockDeviceIoStatus'} }
 
 ##
@@ -1833,7 +1837,7 @@
 #
 # @on-error: #optional the action to take on an error (default report).
 #            'stop' and 'enospc' can only be used if the block device
-#            supports io-status (see BlockInfo).  Since 1.2.
+#            supports io-status (see BlockInfo).  Since 1.3.
 #
 # Returns: Nothing on success
 #          If @device does not exist, DeviceNotFound
@@ -1886,7 +1890,8 @@
 #
 # @device: the device name
 #
-# @force: #optional whether to allow cancellation of a paused job (default false)
+# @force: #optional whether to allow cancellation of a paused job (default
+#         false).  Since 1.3.
 #
 # Returns: Nothing on success
 #          If no background operation is active on this device, DeviceNotActive
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 01/19] qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 02/19] blockdev: rename block_stream_cb to a generic block_job_cb Paolo Bonzini
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

The DeviceNotActive text is not a particularly good match, add
a separate text while keeping the same class.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockdev.c | 4 ++--
 qerror.h   | 3 +++
 2 file modificati, 5 inserzioni(+), 2 rimozioni(-)

diff --git a/blockdev.c b/blockdev.c
index e5d450f..de5457d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1142,7 +1142,7 @@ void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp)
     BlockJob *job = find_block_job(device);
 
     if (!job) {
-        error_set(errp, QERR_DEVICE_NOT_ACTIVE, device);
+        error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
         return;
     }
 
@@ -1154,7 +1154,7 @@ void qmp_block_job_cancel(const char *device, Error **errp)
     BlockJob *job = find_block_job(device);
 
     if (!job) {
-        error_set(errp, QERR_DEVICE_NOT_ACTIVE, device);
+        error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
         return;
     }
 
diff --git a/qerror.h b/qerror.h
index d0a76a4..485c773 100644
--- a/qerror.h
+++ b/qerror.h
@@ -48,6 +48,9 @@ void assert_no_error(Error *err);
 #define QERR_BASE_NOT_FOUND \
     ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
 
+#define QERR_BLOCK_JOB_NOT_ACTIVE \
+    ERROR_CLASS_DEVICE_NOT_ACTIVE, "No active block job on device '%s'"
+
 #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
     ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
 
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 02/19] blockdev: rename block_stream_cb to a generic block_job_cb
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 01/19] qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 03/19] block: fix documentation of block_job_cancel_sync Paolo Bonzini
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, Jeff Cody

From: Jeff Cody <jcody@redhat.com>

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockdev.c   | 6 +++---
 trace-events | 2 +-
 2 file modificati, 4 inserzioni(+), 4 rimozioni(-)

diff --git a/blockdev.c b/blockdev.c
index de5457d..fa338fb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1065,12 +1065,12 @@ static QObject *qobject_from_block_job(BlockJob *job)
                               job->speed);
 }
 
-static void block_stream_cb(void *opaque, int ret)
+static void block_job_cb(void *opaque, int ret)
 {
     BlockDriverState *bs = opaque;
     QObject *obj;
 
-    trace_block_stream_cb(bs, bs->job, ret);
+    trace_block_job_cb(bs, bs->job, ret);
 
     assert(bs->job);
     obj = qobject_from_block_job(bs->job);
@@ -1112,7 +1112,7 @@ void qmp_block_stream(const char *device, bool has_base,
     }
 
     stream_start(bs, base_bs, base, has_speed ? speed : 0,
-                 block_stream_cb, bs, &local_err);
+                 block_job_cb, bs, &local_err);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
         return;
diff --git a/trace-events b/trace-events
index f5b5097..e90de71 100644
--- a/trace-events
+++ b/trace-events
@@ -77,7 +77,7 @@ stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs %p base
 
 # blockdev.c
 qmp_block_job_cancel(void *job) "job %p"
-block_stream_cb(void *bs, void *job, int ret) "bs %p job %p ret %d"
+block_job_cb(void *bs, void *job, int ret) "bs %p job %p ret %d"
 qmp_block_stream(void *bs, void *job) "bs %p job %p"
 
 # hw/virtio-blk.c
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 03/19] block: fix documentation of block_job_cancel_sync
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 01/19] qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 02/19] blockdev: rename block_stream_cb to a generic block_job_cb Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 04/19] block: move job APIs to separate files Paolo Bonzini
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Do this in a separate commit before we move the functions to
blockjob.h.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block_int.h | 12 +++++-------
 1 file modificato, 5 inserzioni(+), 7 rimozioni(-)

diff --git a/block_int.h b/block_int.h
index ac4245c..b025fa0 100644
--- a/block_int.h
+++ b/block_int.h
@@ -425,15 +425,13 @@ void block_job_cancel(BlockJob *job);
 bool block_job_is_cancelled(BlockJob *job);
 
 /**
- * block_job_cancel:
+ * block_job_cancel_sync:
  * @job: The job to be canceled.
  *
- * Asynchronously cancel the job and wait for it to reach a quiescent
- * state.  Note that the completion callback will still be called
- * asynchronously, hence it is *not* valid to call #bdrv_delete
- * immediately after #block_job_cancel_sync.  Users of block jobs
- * will usually protect the BlockDriverState objects with a reference
- * count, should this be a concern.
+ * Synchronously cancel the job.  The completion callback is called
+ * before the function returns.  The job may actually complete
+ * instead of canceling itself; the circumstances under which this
+ * happens depend on the kind of job that is active.
  *
  * Returns the return value from the job if the job actually completed
  * during the call, or -ECANCELED if it was canceled.
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 04/19] block: move job APIs to separate files
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 03/19] block: fix documentation of block_job_cancel_sync Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 05/19] block: add block_job_query Paolo Bonzini
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs       |   5 +-
 block.c             | 128 +------------------------------------
 block.h             |   2 +
 block/Makefile.objs |   3 +-
 block/stream.c      |   1 +
 block_int.h         | 151 --------------------------------------------
 blockdev.c          |   1 +
 blockjob.c          | 163 +++++++++++++++++++++++++++++++++++++++++++++++
 blockjob.h          | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 9 file modificati, 352 inserzioni(+), 281 rimozioni(-)
 create mode 100644 blockjob.c
 create mode 100644 blockjob.h

diff --git a/Makefile.objs b/Makefile.objs
index 4412757..9401516 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -42,7 +42,8 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
 # block-obj-y is code used by both qemu system emulation and qemu-img
 
 block-obj-y = cutils.o iov.o cache-utils.o qemu-option.o module.o async.o
-block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o
+block-obj-y += nbd.o block.o blockjob.o aio.o aes.o qemu-config.o
+block-obj-y += qemu-progress.o qemu-sockets.o
 block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y)
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
@@ -59,7 +60,7 @@ endif
 # suppress *all* target specific code in case of system emulation, i.e. a
 # single QEMU executable should support all CPUs and machines.
 
-common-obj-y = $(block-obj-y) blockdev.o
+common-obj-y = $(block-obj-y) blockdev.o block/
 common-obj-y += net.o net/
 common-obj-y += qom/
 common-obj-y += readline.o console.o cursor.o
diff --git a/block.c b/block.c
index 751ebdc..a4816ad 100644
--- a/block.c
+++ b/block.c
@@ -26,6 +26,7 @@
 #include "trace.h"
 #include "monitor.h"
 #include "block_int.h"
+#include "blockjob.h"
 #include "module.h"
 #include "qjson.h"
 #include "qemu-coroutine.h"
@@ -4247,130 +4248,3 @@ out:
 
     return ret;
 }
-
-void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
-                       int64_t speed, BlockDriverCompletionFunc *cb,
-                       void *opaque, Error **errp)
-{
-    BlockJob *job;
-
-    if (bs->job || bdrv_in_use(bs)) {
-        error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
-        return NULL;
-    }
-    bdrv_set_in_use(bs, 1);
-
-    job = g_malloc0(job_type->instance_size);
-    job->job_type      = job_type;
-    job->bs            = bs;
-    job->cb            = cb;
-    job->opaque        = opaque;
-    job->busy          = true;
-    bs->job = job;
-
-    /* Only set speed when necessary to avoid NotSupported error */
-    if (speed != 0) {
-        Error *local_err = NULL;
-
-        block_job_set_speed(job, speed, &local_err);
-        if (error_is_set(&local_err)) {
-            bs->job = NULL;
-            g_free(job);
-            bdrv_set_in_use(bs, 0);
-            error_propagate(errp, local_err);
-            return NULL;
-        }
-    }
-    return job;
-}
-
-void block_job_complete(BlockJob *job, int ret)
-{
-    BlockDriverState *bs = job->bs;
-
-    assert(bs->job == job);
-    job->cb(job->opaque, ret);
-    bs->job = NULL;
-    g_free(job);
-    bdrv_set_in_use(bs, 0);
-}
-
-void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
-{
-    Error *local_err = NULL;
-
-    if (!job->job_type->set_speed) {
-        error_set(errp, QERR_NOT_SUPPORTED);
-        return;
-    }
-    job->job_type->set_speed(job, speed, &local_err);
-    if (error_is_set(&local_err)) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
-    job->speed = speed;
-}
-
-void block_job_cancel(BlockJob *job)
-{
-    job->cancelled = true;
-    if (job->co && !job->busy) {
-        qemu_coroutine_enter(job->co, NULL);
-    }
-}
-
-bool block_job_is_cancelled(BlockJob *job)
-{
-    return job->cancelled;
-}
-
-struct BlockCancelData {
-    BlockJob *job;
-    BlockDriverCompletionFunc *cb;
-    void *opaque;
-    bool cancelled;
-    int ret;
-};
-
-static void block_job_cancel_cb(void *opaque, int ret)
-{
-    struct BlockCancelData *data = opaque;
-
-    data->cancelled = block_job_is_cancelled(data->job);
-    data->ret = ret;
-    data->cb(data->opaque, ret);
-}
-
-int block_job_cancel_sync(BlockJob *job)
-{
-    struct BlockCancelData data;
-    BlockDriverState *bs = job->bs;
-
-    assert(bs->job == job);
-
-    /* Set up our own callback to store the result and chain to
-     * the original callback.
-     */
-    data.job = job;
-    data.cb = job->cb;
-    data.opaque = job->opaque;
-    data.ret = -EINPROGRESS;
-    job->cb = block_job_cancel_cb;
-    job->opaque = &data;
-    block_job_cancel(job);
-    while (data.ret == -EINPROGRESS) {
-        qemu_aio_wait();
-    }
-    return (data.cancelled && data.ret == 0) ? -ECANCELED : data.ret;
-}
-
-void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns)
-{
-    /* Check cancellation *before* setting busy = false, too!  */
-    if (!block_job_is_cancelled(job)) {
-        job->busy = false;
-        co_sleep_ns(clock, ns);
-        job->busy = true;
-    }
-}
diff --git a/block.h b/block.h
index b1095d8..bd788e0 100644
--- a/block.h
+++ b/block.h
@@ -6,9 +6,11 @@
 #include "qemu-option.h"
 #include "qemu-coroutine.h"
 #include "qobject.h"
+#include "qapi-types.h"
 
 /* block.c */
 typedef struct BlockDriver BlockDriver;
+typedef struct BlockJob BlockJob;
 
 typedef struct BlockDriverInfo {
     /* in bytes, 0 if irrelevant */
diff --git a/block/Makefile.objs b/block/Makefile.objs
index b5754d3..c45affc 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -3,9 +3,10 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c
 block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-y += qed-check.o
 block-obj-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
-block-obj-y += stream.o
 block-obj-$(CONFIG_WIN32) += raw-win32.o
 block-obj-$(CONFIG_POSIX) += raw-posix.o
 block-obj-$(CONFIG_LIBISCSI) += iscsi.o
 block-obj-$(CONFIG_CURL) += curl.o
 block-obj-$(CONFIG_RBD) += rbd.o
+
+common-obj-y += stream.o
diff --git a/block/stream.c b/block/stream.c
index c4f87dd..57e4be7 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -13,6 +13,7 @@
 
 #include "trace.h"
 #include "block_int.h"
+#include "blockjob.h"
 #include "qemu/ratelimit.h"
 
 enum {
diff --git a/block_int.h b/block_int.h
index b025fa0..0da1067 100644
--- a/block_int.h
+++ b/block_int.h
@@ -67,73 +67,6 @@ typedef struct BlockIOBaseValue {
     uint64_t ios[2];
 } BlockIOBaseValue;
 
-typedef struct BlockJob BlockJob;
-
-/**
- * BlockJobType:
- *
- * A class type for block job objects.
- */
-typedef struct BlockJobType {
-    /** Derived BlockJob struct size */
-    size_t instance_size;
-
-    /** String describing the operation, part of query-block-jobs QMP API */
-    const char *job_type;
-
-    /** Optional callback for job types that support setting a speed limit */
-    void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);
-} BlockJobType;
-
-/**
- * BlockJob:
- *
- * Long-running operation on a BlockDriverState.
- */
-struct BlockJob {
-    /** The job type, including the job vtable.  */
-    const BlockJobType *job_type;
-
-    /** The block device on which the job is operating.  */
-    BlockDriverState *bs;
-
-    /**
-     * The coroutine that executes the job.  If not NULL, it is
-     * reentered when busy is false and the job is cancelled.
-     */
-    Coroutine *co;
-
-    /**
-     * Set to true if the job should cancel itself.  The flag must
-     * always be tested just before toggling the busy flag from false
-     * to true.  After a job has been cancelled, it should only yield
-     * if #qemu_aio_wait will ("sooner or later") reenter the coroutine.
-     */
-    bool cancelled;
-
-    /**
-     * Set to false by the job while it is in a quiescent state, where
-     * no I/O is pending and the job has yielded on any condition
-     * that is not detected by #qemu_aio_wait, such as a timer.
-     */
-    bool busy;
-
-    /** Offset that is published by the query-block-jobs QMP API */
-    int64_t offset;
-
-    /** Length that is published by the query-block-jobs QMP API */
-    int64_t len;
-
-    /** Speed that was set with @block_job_set_speed.  */
-    int64_t speed;
-
-    /** The completion function that will be called when the job completes.  */
-    BlockDriverCompletionFunc *cb;
-
-    /** The opaque value that is passed to the completion function.  */
-    void *opaque;
-};
-
 struct BlockDriver {
     const char *format_name;
     int instance_size;
@@ -355,90 +288,6 @@ int is_windows_drive(const char *filename);
 #endif
 
 /**
- * block_job_create:
- * @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.
- * @cb: Completion function for the job.
- * @opaque: Opaque pointer value passed to @cb.
- * @errp: Error object.
- *
- * Create a new long-running block device job and return it.  The job
- * will call @cb asynchronously when the job completes.  Note that
- * @bs may have been closed at the time the @cb it is called.  If
- * this is the case, the job may be reported as either cancelled or
- * completed.
- *
- * 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 BlockJobType *job_type, BlockDriverState *bs,
-                       int64_t speed, BlockDriverCompletionFunc *cb,
-                       void *opaque, Error **errp);
-
-/**
- * block_job_sleep_ns:
- * @job: The job that calls the function.
- * @clock: The clock to sleep on.
- * @ns: How many nanoseconds to stop for.
- *
- * Put the job to sleep (assuming that it wasn't canceled) for @ns
- * nanoseconds.  Canceling the job will interrupt the wait immediately.
- */
-void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns);
-
-/**
- * block_job_complete:
- * @job: The job being completed.
- * @ret: The status code.
- *
- * Call the completion function that was registered at creation time, and
- * free @job.
- */
-void block_job_complete(BlockJob *job, int ret);
-
-/**
- * block_job_set_speed:
- * @job: The job to set the speed for.
- * @speed: The new value
- * @errp: Error object.
- *
- * Set a rate-limiting parameter for the job; the actual meaning may
- * vary depending on the job type.
- */
-void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp);
-
-/**
- * block_job_cancel:
- * @job: The job to be canceled.
- *
- * Asynchronously cancel the specified job.
- */
-void block_job_cancel(BlockJob *job);
-
-/**
- * block_job_is_cancelled:
- * @job: The job being queried.
- *
- * Returns whether the job is scheduled for cancellation.
- */
-bool block_job_is_cancelled(BlockJob *job);
-
-/**
- * block_job_cancel_sync:
- * @job: The job to be canceled.
- *
- * Synchronously cancel the job.  The completion callback is called
- * before the function returns.  The job may actually complete
- * instead of canceling itself; the circumstances under which this
- * happens depend on the kind of job that is active.
- *
- * Returns the return value from the job if the job actually completed
- * during the call, or -ECANCELED if it was canceled.
- */
-int block_job_cancel_sync(BlockJob *job);
-
-/**
  * stream_start:
  * @bs: Block device to operate on.
  * @base: Block device that will become the new base, or %NULL to
diff --git a/blockdev.c b/blockdev.c
index fa338fb..7ab7d5e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -9,6 +9,7 @@
 
 #include "blockdev.h"
 #include "hw/block-common.h"
+#include "blockjob.h"
 #include "monitor.h"
 #include "qerror.h"
 #include "qemu-option.h"
diff --git a/blockjob.c b/blockjob.c
new file mode 100644
index 0000000..9737a43
--- /dev/null
+++ b/blockjob.c
@@ -0,0 +1,163 @@
+/*
+ * QEMU System Emulator block driver
+ *
+ * Copyright (c) 2011 IBM Corp.
+ * Copyright (c) 2012 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "config-host.h"
+#include "qemu-common.h"
+#include "trace.h"
+#include "monitor.h"
+#include "block.h"
+#include "blockjob.h"
+#include "block_int.h"
+#include "qjson.h"
+#include "qemu-coroutine.h"
+#include "qmp-commands.h"
+#include "qemu-timer.h"
+
+void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
+                       int64_t speed, BlockDriverCompletionFunc *cb,
+                       void *opaque, Error **errp)
+{
+    BlockJob *job;
+
+    if (bs->job || bdrv_in_use(bs)) {
+        error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
+        return NULL;
+    }
+    bdrv_set_in_use(bs, 1);
+
+    job = g_malloc0(job_type->instance_size);
+    job->job_type      = job_type;
+    job->bs            = bs;
+    job->cb            = cb;
+    job->opaque        = opaque;
+    job->busy          = true;
+    bs->job = job;
+
+    /* Only set speed when necessary to avoid NotSupported error */
+    if (speed != 0) {
+        Error *local_err = NULL;
+
+        block_job_set_speed(job, speed, &local_err);
+        if (error_is_set(&local_err)) {
+            bs->job = NULL;
+            g_free(job);
+            bdrv_set_in_use(bs, 0);
+            error_propagate(errp, local_err);
+            return NULL;
+        }
+    }
+    return job;
+}
+
+void block_job_complete(BlockJob *job, int ret)
+{
+    BlockDriverState *bs = job->bs;
+
+    assert(bs->job == job);
+    job->cb(job->opaque, ret);
+    bs->job = NULL;
+    g_free(job);
+    bdrv_set_in_use(bs, 0);
+}
+
+void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
+{
+    Error *local_err = NULL;
+
+    if (!job->job_type->set_speed) {
+        error_set(errp, QERR_NOT_SUPPORTED);
+        return;
+    }
+    job->job_type->set_speed(job, speed, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    job->speed = speed;
+}
+
+void block_job_cancel(BlockJob *job)
+{
+    job->cancelled = true;
+    if (job->co && !job->busy) {
+        qemu_coroutine_enter(job->co, NULL);
+    }
+}
+
+bool block_job_is_cancelled(BlockJob *job)
+{
+    return job->cancelled;
+}
+
+struct BlockCancelData {
+    BlockJob *job;
+    BlockDriverCompletionFunc *cb;
+    void *opaque;
+    bool cancelled;
+    int ret;
+};
+
+static void block_job_cancel_cb(void *opaque, int ret)
+{
+    struct BlockCancelData *data = opaque;
+
+    data->cancelled = block_job_is_cancelled(data->job);
+    data->ret = ret;
+    data->cb(data->opaque, ret);
+}
+
+int block_job_cancel_sync(BlockJob *job)
+{
+    struct BlockCancelData data;
+    BlockDriverState *bs = job->bs;
+
+    assert(bs->job == job);
+
+    /* Set up our own callback to store the result and chain to
+     * the original callback.
+     */
+    data.job = job;
+    data.cb = job->cb;
+    data.opaque = job->opaque;
+    data.ret = -EINPROGRESS;
+    job->cb = block_job_cancel_cb;
+    job->opaque = &data;
+    block_job_cancel(job);
+    while (data.ret == -EINPROGRESS) {
+        qemu_aio_wait();
+    }
+    return (data.cancelled && data.ret == 0) ? -ECANCELED : data.ret;
+}
+
+void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns)
+{
+    /* Check cancellation *before* setting busy = false, too!  */
+    if (!block_job_is_cancelled(job)) {
+        job->busy = false;
+        co_sleep_ns(clock, ns);
+        job->busy = true;
+    }
+}
diff --git a/blockjob.h b/blockjob.h
new file mode 100644
index 0000000..753f5bc
--- /dev/null
+++ b/blockjob.h
@@ -0,0 +1,179 @@
+/*
+ * Declarations for long-running block device operations
+ *
+ * Copyright (c) 2011 IBM Corp.
+ * Copyright (c) 2012 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef BLOCKJOB_H
+#define BLOCKJOB_H 1
+
+#include "block.h"
+
+/**
+ * BlockJobType:
+ *
+ * A class type for block job objects.
+ */
+typedef struct BlockJobType {
+    /** Derived BlockJob struct size */
+    size_t instance_size;
+
+    /** String describing the operation, part of query-block-jobs QMP API */
+    const char *job_type;
+
+    /** Optional callback for job types that support setting a speed limit */
+    void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);
+} BlockJobType;
+
+/**
+ * BlockJob:
+ *
+ * Long-running operation on a BlockDriverState.
+ */
+struct BlockJob {
+    /** The job type, including the job vtable.  */
+    const BlockJobType *job_type;
+
+    /** The block device on which the job is operating.  */
+    BlockDriverState *bs;
+
+    /**
+     * The coroutine that executes the job.  If not NULL, it is
+     * reentered when busy is false and the job is cancelled.
+     */
+    Coroutine *co;
+
+    /**
+     * Set to true if the job should cancel itself.  The flag must
+     * always be tested just before toggling the busy flag from false
+     * to true.  After a job has been cancelled, it should only yield
+     * if #qemu_aio_wait will ("sooner or later") reenter the coroutine.
+     */
+    bool cancelled;
+
+    /**
+     * Set to false by the job while it is in a quiescent state, where
+     * no I/O is pending and the job has yielded on any condition
+     * that is not detected by #qemu_aio_wait, such as a timer.
+     */
+    bool busy;
+
+    /** Offset that is published by the query-block-jobs QMP API */
+    int64_t offset;
+
+    /** Length that is published by the query-block-jobs QMP API */
+    int64_t len;
+
+    /** Speed that was set with @block_job_set_speed.  */
+    int64_t speed;
+
+    /** The completion function that will be called when the job completes.  */
+    BlockDriverCompletionFunc *cb;
+
+    /** The opaque value that is passed to the completion function.  */
+    void *opaque;
+};
+
+/**
+ * block_job_create:
+ * @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.
+ * @cb: Completion function for the job.
+ * @opaque: Opaque pointer value passed to @cb.
+ * @errp: Error object.
+ *
+ * Create a new long-running block device job and return it.  The job
+ * will call @cb asynchronously when the job completes.  Note that
+ * @bs may have been closed at the time the @cb it is called.  If
+ * this is the case, the job may be reported as either cancelled or
+ * completed.
+ *
+ * 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 BlockJobType *job_type, BlockDriverState *bs,
+                       int64_t speed, BlockDriverCompletionFunc *cb,
+                       void *opaque, Error **errp);
+
+/**
+ * block_job_sleep_ns:
+ * @job: The job that calls the function.
+ * @clock: The clock to sleep on.
+ * @ns: How many nanoseconds to stop for.
+ *
+ * Put the job to sleep (assuming that it wasn't canceled) for @ns
+ * nanoseconds.  Canceling the job will interrupt the wait immediately.
+ */
+void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns);
+
+/**
+ * block_job_complete:
+ * @job: The job being completed.
+ * @ret: The status code.
+ *
+ * Call the completion function that was registered at creation time, and
+ * free @job.
+ */
+void block_job_complete(BlockJob *job, int ret);
+
+/**
+ * block_job_set_speed:
+ * @job: The job to set the speed for.
+ * @speed: The new value
+ * @errp: Error object.
+ *
+ * Set a rate-limiting parameter for the job; the actual meaning may
+ * vary depending on the job type.
+ */
+void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp);
+
+/**
+ * block_job_cancel:
+ * @job: The job to be canceled.
+ *
+ * Asynchronously cancel the specified job.
+ */
+void block_job_cancel(BlockJob *job);
+
+/**
+ * block_job_is_cancelled:
+ * @job: The job being queried.
+ *
+ * Returns whether the job is scheduled for cancellation.
+ */
+bool block_job_is_cancelled(BlockJob *job);
+
+/**
+ * block_job_cancel_sync:
+ * @job: The job to be canceled.
+ *
+ * Synchronously cancel the job.  The completion callback is called
+ * before the function returns.  The job may actually complete
+ * instead of canceling itself; the circumstances under which this
+ * happens depend on the kind of job that is active.
+ *
+ * Returns the return value from the job if the job actually completed
+ * during the call, or -ECANCELED if it was canceled.
+ */
+int block_job_cancel_sync(BlockJob *job);
+
+#endif
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 05/19] block: add block_job_query
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 04/19] block: move job APIs to separate files Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 06/19] qmp: add 'busy' member to BlockJobInfo Paolo Bonzini
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Extract it out of the implementation of info block-jobs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockdev.c | 15 ++-------------
 blockjob.c | 11 +++++++++++
 blockjob.h |  8 ++++++++
 3 file modificati, 21 inserzioni(+), 13 rimozioni(-)

diff --git a/blockdev.c b/blockdev.c
index 7ab7d5e..5772c11 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1169,19 +1169,8 @@ static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs)
     BlockJob *job = bs->job;
 
     if (job) {
-        BlockJobInfoList *elem;
-        BlockJobInfo *info = g_new(BlockJobInfo, 1);
-        *info = (BlockJobInfo){
-            .type   = g_strdup(job->job_type->job_type),
-            .device = g_strdup(bdrv_get_device_name(bs)),
-            .len    = job->len,
-            .offset = job->offset,
-            .speed  = job->speed,
-        };
-
-        elem = g_new0(BlockJobInfoList, 1);
-        elem->value = info;
-
+        BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
+        elem->value = block_job_query(bs->job);
         (*prev)->next = elem;
         *prev = elem;
     }
diff --git a/blockjob.c b/blockjob.c
index 9737a43..dea63f8 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -161,3 +161,14 @@ void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns)
         job->busy = true;
     }
 }
+
+BlockJobInfo *block_job_query(BlockJob *job)
+{
+    BlockJobInfo *info = g_new0(BlockJobInfo, 1);
+    info->type   = g_strdup(job->job_type->job_type);
+    info->device = g_strdup(bdrv_get_device_name(job->bs));
+    info->len    = job->len;
+    info->offset = job->offset;
+    info->speed  = job->speed;
+    return info;
+}
diff --git a/blockjob.h b/blockjob.h
index 753f5bc..f3d8d58 100644
--- a/blockjob.h
+++ b/blockjob.h
@@ -163,6 +163,14 @@ void block_job_cancel(BlockJob *job);
 bool block_job_is_cancelled(BlockJob *job);
 
 /**
+ * block_job_query:
+ * @job: The job to get information about.
+ *
+ * Return information about a job.
+ */
+BlockJobInfo *block_job_query(BlockJob *job);
+
+/**
  * block_job_cancel_sync:
  * @job: The job to be canceled.
  *
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 06/19] qmp: add 'busy' member to BlockJobInfo
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 05/19] block: add block_job_query Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 07/19] block: add support for job pause/resume Paolo Bonzini
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Because pausing a job is asynchronous, we need to know whether it has
completed.  This is described by the "busy" field of BlockJob; copy it
to BlockJobInfo.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockjob.c       | 1 +
 qapi-schema.json | 5 ++++-
 2 file modificati, 5 inserzioni(+). 1 rimozione(-)

diff --git a/blockjob.c b/blockjob.c
index dea63f8..64c9d2d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -168,6 +168,7 @@ BlockJobInfo *block_job_query(BlockJob *job)
     info->type   = g_strdup(job->job_type->job_type);
     info->device = g_strdup(bdrv_get_device_name(job->bs));
     info->len    = job->len;
+    info->busy   = job->busy;
     info->offset = job->offset;
     info->speed  = job->speed;
     return info;
diff --git a/qapi-schema.json b/qapi-schema.json
index 14e4419..add93f9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1098,6 +1098,9 @@
 #
 # @len: the maximum progress value
 #
+# @busy: false if the job is known to be in a quiescent state, with
+#        no pending I/O.  Since 1.3.
+#
 # @offset: the current progress value
 #
 # @speed: the rate limit, bytes per second
@@ -1106,7 +1109,7 @@
 ##
 { 'type': 'BlockJobInfo',
   'data': {'type': 'str', 'device': 'str', 'len': 'int',
-           'offset': 'int', 'speed': 'int'} }
+           'offset': 'int', 'busy': 'bool', 'speed': 'int'} }
 
 ##
 # @query-block-jobs:
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 07/19] block: add support for job pause/resume
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 06/19] qmp: add 'busy' member to BlockJobInfo Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 08/19] qmp: add block-job-pause and block-job-resume Paolo Bonzini
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Job pausing reuses the existing support for cancellable sleeps.  A pause
happens at the next sleeping point and lasts until the coroutine is
re-entered explicitly.  Cancellation was already doing a forced resume,
so implement it explicitly in terms of resume.

Paused jobs cannot be canceled without first resuming them.  This ensures
that I/O errors are never missed by management.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockdev.c       |  4 ++++
 blockjob.c       | 35 ++++++++++++++++++++++++++++++-----
 blockjob.h       | 31 +++++++++++++++++++++++++++++++
 qapi-schema.json |  5 ++++-
 qerror.h         |  3 +++
 5 file modificati, 72 inserzioni(+), 6 rimozioni(-)

diff --git a/blockdev.c b/blockdev.c
index 5772c11..df0c449 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1158,6 +1158,10 @@ void qmp_block_job_cancel(const char *device, Error **errp)
         error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
         return;
     }
+    if (job->paused) {
+        error_set(errp, QERR_BLOCK_JOB_PAUSED, device);
+        return;
+    }
 
     trace_qmp_block_job_cancel(job);
     block_job_cancel(job);
diff --git a/blockjob.c b/blockjob.c
index 64c9d2d..8219f73 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -99,14 +99,30 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
     job->speed = speed;
 }
 
-void block_job_cancel(BlockJob *job)
+void block_job_pause(BlockJob *job)
 {
-    job->cancelled = true;
+    job->paused = true;
+}
+
+bool block_job_is_paused(BlockJob *job)
+{
+    return job->paused;
+}
+
+void block_job_resume(BlockJob *job)
+{
+    job->paused = false;
     if (job->co && !job->busy) {
         qemu_coroutine_enter(job->co, NULL);
     }
 }
 
+void block_job_cancel(BlockJob *job)
+{
+    job->cancelled = true;
+    block_job_resume(job);
+}
+
 bool block_job_is_cancelled(BlockJob *job)
 {
     return job->cancelled;
@@ -154,12 +170,20 @@ int block_job_cancel_sync(BlockJob *job)
 
 void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns)
 {
+    assert(job->busy);
+
     /* Check cancellation *before* setting busy = false, too!  */
-    if (!block_job_is_cancelled(job)) {
-        job->busy = false;
+    if (block_job_is_cancelled(job)) {
+        return;
+    }
+
+    job->busy = false;
+    if (block_job_is_paused(job)) {
+        qemu_coroutine_yield();
+    } else {
         co_sleep_ns(clock, ns);
-        job->busy = true;
     }
+    job->busy = true;
 }
 
 BlockJobInfo *block_job_query(BlockJob *job)
@@ -169,6 +193,7 @@ BlockJobInfo *block_job_query(BlockJob *job)
     info->device = g_strdup(bdrv_get_device_name(job->bs));
     info->len    = job->len;
     info->busy   = job->busy;
+    info->paused = job->paused;
     info->offset = job->offset;
     info->speed  = job->speed;
     return info;
diff --git a/blockjob.h b/blockjob.h
index f3d8d58..ece5afa 100644
--- a/blockjob.h
+++ b/blockjob.h
@@ -70,6 +70,12 @@ struct BlockJob {
     bool cancelled;
 
     /**
+     * Set to true if the job is either paused, or will pause itself
+     * as soon as possible (if busy == true).
+     */
+    bool paused;
+
+    /**
      * Set to false by the job while it is in a quiescent state, where
      * no I/O is pending and the job has yielded on any condition
      * that is not detected by #qemu_aio_wait, such as a timer.
@@ -171,6 +177,31 @@ bool block_job_is_cancelled(BlockJob *job);
 BlockJobInfo *block_job_query(BlockJob *job);
 
 /**
+ * block_job_pause:
+ * @job: The job to be paused.
+ *
+ * Asynchronously pause the specified job.
+ */
+void block_job_pause(BlockJob *job);
+
+/**
+ * block_job_resume:
+ * @job: The job to be resumed.
+ *
+ * Resume the specified job.
+ */
+void block_job_resume(BlockJob *job);
+
+/**
+ * block_job_is_paused:
+ * @job: The job being queried.
+ *
+ * Returns whether the job is currently paused, or will pause
+ * as soon as it reaches a sleeping point.
+ */
+bool block_job_is_paused(BlockJob *job);
+
+/**
  * block_job_cancel_sync:
  * @job: The job to be canceled.
  *
diff --git a/qapi-schema.json b/qapi-schema.json
index add93f9..6130687 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1101,6 +1101,9 @@
 # @busy: false if the job is known to be in a quiescent state, with
 #        no pending I/O.  Since 1.3.
 #
+# @paused: whether the job is paused or, if @busy is true, will
+#          pause itself as soon as possible.  Since 1.3.
+#
 # @offset: the current progress value
 #
 # @speed: the rate limit, bytes per second
@@ -1109,7 +1112,7 @@
 ##
 { 'type': 'BlockJobInfo',
   'data': {'type': 'str', 'device': 'str', 'len': 'int',
-           'offset': 'int', 'busy': 'bool', 'speed': 'int'} }
+           'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int'} }
 
 ##
 # @query-block-jobs:
diff --git a/qerror.h b/qerror.h
index 485c773..c91708c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -51,6 +51,9 @@ void assert_no_error(Error *err);
 #define QERR_BLOCK_JOB_NOT_ACTIVE \
     ERROR_CLASS_DEVICE_NOT_ACTIVE, "No active block job on device '%s'"
 
+#define QERR_BLOCK_JOB_PAUSED \
+    ERROR_CLASS_GENERIC_ERROR, "The block job for device '%s' is currently paused"
+
 #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
     ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
 
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 08/19] qmp: add block-job-pause and block-job-resume
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 07/19] block: add support for job pause/resume Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 09/19] qemu-iotests: add test for pausing a streaming operation Paolo Bonzini
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Add QMP commands matching the functionality.

Paused jobs cannot be canceled without first resuming them.  This
ensures that I/O errors are never missed by management.  However, an
optional force argument can be specified to allow that.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockdev.c       | 35 +++++++++++++++++++++++++++++++++--
 hmp-commands.hx  | 35 ++++++++++++++++++++++++++++++++---
 hmp.c            | 23 ++++++++++++++++++++++-
 hmp.h            |  2 ++
 qapi-schema.json | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 qmp-commands.hx  | 12 +++++++++++-
 trace-events     |  2 ++
 7 file modificati, 147 inserzioni(+), 8 rimozioni(-)

diff --git a/blockdev.c b/blockdev.c
index df0c449..a718380 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1150,15 +1150,20 @@ void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp)
     block_job_set_speed(job, speed, errp);
 }
 
-void qmp_block_job_cancel(const char *device, Error **errp)
+void qmp_block_job_cancel(const char *device,
+                          bool has_force, bool force, Error **errp)
 {
     BlockJob *job = find_block_job(device);
 
+    if (!has_force) {
+        force = false;
+    }
+
     if (!job) {
         error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
         return;
     }
-    if (job->paused) {
+    if (job->paused && !force) {
         error_set(errp, QERR_BLOCK_JOB_PAUSED, device);
         return;
     }
@@ -1167,6 +1172,32 @@ void qmp_block_job_cancel(const char *device, Error **errp)
     block_job_cancel(job);
 }
 
+void qmp_block_job_pause(const char *device, Error **errp)
+{
+    BlockJob *job = find_block_job(device);
+
+    if (!job) {
+        error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
+        return;
+    }
+
+    trace_qmp_block_job_pause(job);
+    block_job_pause(job);
+}
+
+void qmp_block_job_resume(const char *device, Error **errp)
+{
+    BlockJob *job = find_block_job(device);
+
+    if (!job) {
+        error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
+        return;
+    }
+
+    trace_qmp_block_job_resume(job);
+    block_job_resume(job);
+}
+
 static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs)
 {
     BlockJobInfoList **prev = opaque;
diff --git a/hmp-commands.hx b/hmp-commands.hx
index ed67e99..27d90a2 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -99,9 +99,10 @@ ETEXI
 
     {
         .name       = "block_job_cancel",
-        .args_type  = "device:B",
-        .params     = "device",
-        .help       = "stop an active background block operation",
+        .args_type  = "force:-f,device:B",
+        .params     = "[-f] device",
+        .help       = "stop an active background block operation (use -f"
+                      "\n\t\t\t if the operation is currently paused)",
         .mhandler.cmd = hmp_block_job_cancel,
     },
 
@@ -112,6 +113,34 @@ Stop an active block streaming operation.
 ETEXI
 
     {
+        .name       = "block_job_pause",
+        .args_type  = "device:B",
+        .params     = "device",
+        .help       = "pause an active background block operation",
+        .mhandler.cmd = hmp_block_job_pause,
+    },
+
+STEXI
+@item block_job_pause
+@findex block_job_pause
+Pause an active block streaming operation.
+ETEXI
+
+    {
+        .name       = "block_job_resume",
+        .args_type  = "device:B",
+        .params     = "device",
+        .help       = "resume a paused background block operation",
+        .mhandler.cmd = hmp_block_job_resume,
+    },
+
+STEXI
+@item block_job_resume
+@findex block_job_resume
+Resume a paused block streaming operation.
+ETEXI
+
+    {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
diff --git a/hmp.c b/hmp.c
index ba6fbd3..55601f7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -950,8 +950,29 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
 {
     Error *error = NULL;
     const char *device = qdict_get_str(qdict, "device");
+    bool force = qdict_get_try_bool(qdict, "force", 0);
 
-    qmp_block_job_cancel(device, &error);
+    qmp_block_job_cancel(device, true, force, &error);
+
+    hmp_handle_error(mon, &error);
+}
+
+void hmp_block_job_pause(Monitor *mon, const QDict *qdict)
+{
+    Error *error = NULL;
+    const char *device = qdict_get_str(qdict, "device");
+
+    qmp_block_job_pause(device, &error);
+
+    hmp_handle_error(mon, &error);
+}
+
+void hmp_block_job_resume(Monitor *mon, const QDict *qdict)
+{
+    Error *error = NULL;
+    const char *device = qdict_get_str(qdict, "device");
+
+    qmp_block_job_resume(device, &error);
 
     hmp_handle_error(mon, &error);
 }
diff --git a/hmp.h b/hmp.h
index 48b9c59..71ea384 100644
--- a/hmp.h
+++ b/hmp.h
@@ -64,6 +64,8 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
 void hmp_block_stream(Monitor *mon, const QDict *qdict);
 void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
 void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
+void hmp_block_job_pause(Monitor *mon, const QDict *qdict);
+void hmp_block_job_resume(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
diff --git a/qapi-schema.json b/qapi-schema.json
index 6130687..a37507a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1859,12 +1859,56 @@
 #
 # @device: the device name
 #
+# @force: #optional whether to allow cancellation of a paused job (default
+#         false).  Since 1.3.
+#
 # Returns: Nothing on success
 #          If no background operation is active on this device, DeviceNotActive
 #
 # Since: 1.1
 ##
-{ 'command': 'block-job-cancel', 'data': { 'device': 'str' } }
+{ 'command': 'block-job-cancel', 'data': { 'device': 'str', '*force': 'bool' } }
+
+##
+# @block-job-pause:
+#
+# Pause an active background block operation.
+#
+# This command returns immediately after marking the active background block
+# operation for pausing.  It is an error to call this command if no
+# operation is in progress.  Pausing an already paused job has no cumulative
+# effect; a single block-job-resume command will resume the job.
+#
+# The operation will pause as soon as possible.  No event is emitted when
+# the operation is actually paused.  Cancelling a paused job automatically
+# resumes it.
+#
+# @device: the device name
+#
+# Returns: Nothing on success
+#          If no background operation is active on this device, DeviceNotActive
+#
+# Since: 1.3
+##
+{ 'command': 'block-job-pause', 'data': { 'device': 'str' } }
+
+##
+# @block-job-resume:
+#
+# Resume an active background block operation.
+#
+# This command returns immediately after resuming a paused background block
+# operation.  It is an error to call this command if no operation is in
+# progress.  Resuming an already running job is not an error.
+#
+# @device: the device name
+#
+# Returns: Nothing on success
+#          If no background operation is active on this device, DeviceNotActive
+#
+# Since: 1.3
+##
+{ 'command': 'block-job-resume', 'data': { 'device': 'str' } }
 
 ##
 # @ObjectTypeInfo:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6e21ddb..85eacb5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -799,10 +799,20 @@ EQMP
 
     {
         .name       = "block-job-cancel",
-        .args_type  = "device:B",
+        .args_type  = "device:B,force:b?",
         .mhandler.cmd_new = qmp_marshal_input_block_job_cancel,
     },
     {
+        .name       = "block-job-pause",
+        .args_type  = "device:B",
+        .mhandler.cmd_new = qmp_marshal_input_block_job_pause,
+    },
+    {
+        .name       = "block-job-resume",
+        .args_type  = "device:B",
+        .mhandler.cmd_new = qmp_marshal_input_block_job_resume,
+    },
+    {
         .name       = "transaction",
         .args_type  = "actions:q",
         .mhandler.cmd_new = qmp_marshal_input_transaction,
diff --git a/trace-events b/trace-events
index e90de71..75f7357 100644
--- a/trace-events
+++ b/trace-events
@@ -77,6 +77,8 @@ stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs %p base
 
 # blockdev.c
 qmp_block_job_cancel(void *job) "job %p"
+qmp_block_job_pause(void *job) "job %p"
+qmp_block_job_resume(void *job) "job %p"
 block_job_cb(void *bs, void *job, int ret) "bs %p job %p ret %d"
 qmp_block_stream(void *bs, void *job) "bs %p job %p"
 
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 09/19] qemu-iotests: add test for pausing a streaming operation
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 08/19] qmp: add block-job-pause and block-job-resume Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 10/19] block: rename block_job_complete to block_job_completed Paolo Bonzini
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

These check that a paused streaming job does not advance its offset.

Sometimes the new test fails; the map is different between the source
and the destination of the streaming because qemu-io does not always
pack adjacent clusters that have the same allocated/unallocated state.
However, this also happens with the existing test_stream testcase, and
is better fixed in qemu-io.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qemu-iotests/030     | 40 ++++++++++++++++++++++++++++++++++++++--
 tests/qemu-iotests/030.out |  4 ++--
 tests/qemu-iotests/group   |  2 +-
 3 file modificati, 41 inserzioni(+), 5 rimozioni(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 55b16f8..dfacdf1 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -18,6 +18,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import time
 import os
 import iotests
 from iotests import qemu_img, qemu_io
@@ -98,6 +99,43 @@ class TestSingleDrive(ImageStreamingTestCase):
                          qemu_io('-c', 'map', test_img),
                          'image file map does not match backing file after streaming')
 
+    def test_stream_pause(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('block-job-pause', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        time.sleep(1)
+        result = self.vm.qmp('query-block-jobs')
+        offset = self.dictpath(result, 'return[0]/offset')
+
+        time.sleep(1)
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/offset', offset)
+
+        result = self.vm.qmp('block-job-resume', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        completed = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assert_qmp(event, 'data/type', 'stream')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/offset', self.image_len)
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    completed = True
+
+        self.assert_no_active_streams()
+        self.vm.shutdown()
+
+        self.assertEqual(qemu_io('-c', 'map', backing_img),
+                         qemu_io('-c', 'map', test_img),
+                         'image file map does not match backing file after streaming')
+
     def test_stream_partial(self):
         self.assert_no_active_streams()
 
@@ -173,8 +211,6 @@ class TestStreamStop(ImageStreamingTestCase):
         os.remove(backing_img)
 
     def test_stream_stop(self):
-        import time
-
         self.assert_no_active_streams()
 
         result = self.vm.qmp('block-stream', device='drive0')
diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
index 2f7d390..594c16f 100644
--- a/tests/qemu-iotests/030.out
+++ b/tests/qemu-iotests/030.out
@@ -1,5 +1,5 @@
-.......
+........
 ----------------------------------------------------------------------
-Ran 7 tests
+Ran 8 tests
 
 OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index ebb5ca4..fa4da7d 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -36,7 +36,7 @@
 027 rw auto quick
 028 rw backing auto
 029 rw auto quick
-030 rw auto
+030 rw auto backing
 031 rw auto quick
 032 rw auto
 033 rw auto
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 10/19] block: rename block_job_complete to block_job_completed
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (8 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 09/19] qemu-iotests: add test for pausing a streaming operation Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 11/19] iostatus: rename BlockErrorAction, BlockQMPEventAction Paolo Bonzini
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

The imperative will be used for the QMP command.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/stream.c | 4 ++--
 blockjob.c     | 2 +-
 blockjob.h     | 4 ++--
 3 file modificati, 5 inserzioni(+), 5 rimozioni(-)

diff --git a/block/stream.c b/block/stream.c
index 57e4be7..a8f585a 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -84,7 +84,7 @@ static void coroutine_fn stream_run(void *opaque)
 
     s->common.len = bdrv_getlength(bs);
     if (s->common.len < 0) {
-        block_job_complete(&s->common, s->common.len);
+        block_job_completed(&s->common, s->common.len);
         return;
     }
 
@@ -167,7 +167,7 @@ wait:
     }
 
     qemu_vfree(buf);
-    block_job_complete(&s->common, ret);
+    block_job_completed(&s->common, ret);
 }
 
 static void stream_set_speed(BlockJob *job, int64_t speed, Error **errp)
diff --git a/blockjob.c b/blockjob.c
index 8219f73..9577b84 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -71,7 +71,7 @@ void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
     return job;
 }
 
-void block_job_complete(BlockJob *job, int ret)
+void block_job_completed(BlockJob *job, int ret)
 {
     BlockDriverState *bs = job->bs;
 
diff --git a/blockjob.h b/blockjob.h
index ece5afa..b62ddb4 100644
--- a/blockjob.h
+++ b/blockjob.h
@@ -132,14 +132,14 @@ void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
 void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns);
 
 /**
- * block_job_complete:
+ * block_job_completed:
  * @job: The job being completed.
  * @ret: The status code.
  *
  * Call the completion function that was registered at creation time, and
  * free @job.
  */
-void block_job_complete(BlockJob *job, int ret);
+void block_job_completed(BlockJob *job, int ret);
 
 /**
  * block_job_set_speed:
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 11/19] iostatus: rename BlockErrorAction, BlockQMPEventAction
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (9 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 10/19] block: rename block_job_complete to block_job_completed Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 12/19] iostatus: move BlockdevOnError declaration to QAPI Paolo Bonzini
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

We want to remove knowledge of BLOCK_ERR_STOP_ENOSPC from drivers;
drivers should only be told whether to stop/report/ignore the error.
On the other hand, we want to keep using the nicer BlockErrorAction
name in the drivers.  So rename the enums, while leaving aside the
names of the enum values for now.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c         |  8 ++++----
 block.h         | 12 ++++++------
 block_int.h     |  2 +-
 hw/ide/core.c   |  2 +-
 hw/scsi-disk.c  |  2 +-
 hw/virtio-blk.c |  2 +-
 6 file modificati, 14 inserzioni(+), 14 rimozioni(-)

diff --git a/block.c b/block.c
index a4816ad..a4225e7 100644
--- a/block.c
+++ b/block.c
@@ -1387,7 +1387,7 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
 }
 
 void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               BlockQMPEventAction action, int is_read)
+                               BlockErrorAction action, int is_read)
 {
     QObject *data;
     const char *action_str;
@@ -2331,14 +2331,14 @@ void bdrv_set_io_limits(BlockDriverState *bs,
     bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
 }
 
-void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
-                       BlockErrorAction on_write_error)
+void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
+                       BlockdevOnError on_write_error)
 {
     bs->on_read_error = on_read_error;
     bs->on_write_error = on_write_error;
 }
 
-BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read)
+BlockdevOnError bdrv_get_on_error(BlockDriverState *bs, int is_read)
 {
     return is_read ? bs->on_read_error : bs->on_write_error;
 }
diff --git a/block.h b/block.h
index bd788e0..b4ef643 100644
--- a/block.h
+++ b/block.h
@@ -93,11 +93,11 @@ typedef struct BlockDevOps {
 typedef enum {
     BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC,
     BLOCK_ERR_STOP_ANY
-} BlockErrorAction;
+} BlockdevOnError;
 
 typedef enum {
     BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
-} BlockQMPEventAction;
+} BlockErrorAction;
 
 typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;
 
@@ -114,7 +114,7 @@ void bdrv_iostatus_disable(BlockDriverState *bs);
 bool bdrv_iostatus_is_enabled(const BlockDriverState *bs);
 void bdrv_iostatus_set_err(BlockDriverState *bs, int error);
 void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               BlockQMPEventAction action, int is_read);
+                               BlockErrorAction action, int is_read);
 void bdrv_info_print(Monitor *mon, const QObject *data);
 void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_stats_print(Monitor *mon, const QObject *data);
@@ -279,9 +279,9 @@ int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
                       int *pnum);
 
-void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
-                       BlockErrorAction on_write_error);
-BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
+void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
+                       BlockdevOnError on_write_error);
+BlockdevOnError bdrv_get_on_error(BlockDriverState *bs, int is_read);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index 0da1067..db487eb 100644
--- a/block_int.h
+++ b/block_int.h
@@ -262,7 +262,7 @@ struct BlockDriverState {
 
     /* NOTE: the following infos are only hints for real hardware
        drivers. They are not used by the block driver */
-    BlockErrorAction on_read_error, on_write_error;
+    BlockdevOnError on_read_error, on_write_error;
     bool iostatus_enabled;
     BlockDeviceIoStatus iostatus;
     char device_name[32];
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d6fb69c..57b9fa4 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -557,7 +557,7 @@ void ide_dma_error(IDEState *s)
 static int ide_handle_rw_error(IDEState *s, int error, int op)
 {
     int is_read = (op & BM_STATUS_RETRY_READ);
-    BlockErrorAction action = bdrv_get_on_error(s->bs, is_read);
+    BlockdevOnError action = bdrv_get_on_error(s->bs, is_read);
 
     if (action == BLOCK_ERR_IGNORE) {
         bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_IGNORE, is_read);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 95e9158..fef83a3 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -388,7 +388,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
 {
     int is_read = (r->req.cmd.xfer == SCSI_XFER_FROM_DEV);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-    BlockErrorAction action = bdrv_get_on_error(s->qdev.conf.bs, is_read);
+    BlockdevOnError action = bdrv_get_on_error(s->qdev.conf.bs, is_read);
 
     if (action == BLOCK_ERR_IGNORE) {
         bdrv_emit_qmp_error_event(s->qdev.conf.bs, BDRV_ACTION_IGNORE, is_read);
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 6f6d172..01e537d 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -66,7 +66,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
     int is_read)
 {
-    BlockErrorAction action = bdrv_get_on_error(req->dev->bs, is_read);
+    BlockdevOnError action = bdrv_get_on_error(req->dev->bs, is_read);
     VirtIOBlock *s = req->dev;
 
     if (action == BLOCK_ERR_IGNORE) {
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 12/19] iostatus: move BlockdevOnError declaration to QAPI
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (10 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 11/19] iostatus: rename BlockErrorAction, BlockQMPEventAction Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 13/19] iostatus: change is_read to a bool Paolo Bonzini
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

This will let block-stream reuse the enum.  Places that used the enums
are renamed accordingly.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c           |  6 +++---
 block.h           |  5 -----
 blockdev.c        | 12 ++++++------
 hw/fdc.c          |  4 ++--
 hw/ide/core.c     |  6 +++---
 hw/scsi-disk.c    |  6 +++---
 hw/scsi-generic.c |  4 ++--
 hw/virtio-blk.c   |  6 +++---
 qapi-schema.json  | 23 +++++++++++++++++++++++
 9 file modificati, 45 inserzioni(+), 27 rimozioni(-)

diff --git a/block.c b/block.c
index a4225e7..ab2f7e0 100644
--- a/block.c
+++ b/block.c
@@ -4050,9 +4050,9 @@ void bdrv_iostatus_enable(BlockDriverState *bs)
 bool bdrv_iostatus_is_enabled(const BlockDriverState *bs)
 {
     return (bs->iostatus_enabled &&
-           (bs->on_write_error == BLOCK_ERR_STOP_ENOSPC ||
-            bs->on_write_error == BLOCK_ERR_STOP_ANY    ||
-            bs->on_read_error == BLOCK_ERR_STOP_ANY));
+           (bs->on_write_error == BLOCKDEV_ON_ERROR_ENOSPC ||
+            bs->on_write_error == BLOCKDEV_ON_ERROR_STOP   ||
+            bs->on_read_error == BLOCKDEV_ON_ERROR_STOP));
 }
 
 void bdrv_iostatus_disable(BlockDriverState *bs)
diff --git a/block.h b/block.h
index b4ef643..d93cbc3 100644
--- a/block.h
+++ b/block.h
@@ -91,11 +91,6 @@ typedef struct BlockDevOps {
 #define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
 
 typedef enum {
-    BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC,
-    BLOCK_ERR_STOP_ANY
-} BlockdevOnError;
-
-typedef enum {
     BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
 } BlockErrorAction;
 
diff --git a/blockdev.c b/blockdev.c
index a718380..9fc794f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -241,13 +241,13 @@ static void drive_put_ref_bh_schedule(DriveInfo *dinfo)
 static int parse_block_error_action(const char *buf, int is_read)
 {
     if (!strcmp(buf, "ignore")) {
-        return BLOCK_ERR_IGNORE;
+        return BLOCKDEV_ON_ERROR_IGNORE;
     } else if (!is_read && !strcmp(buf, "enospc")) {
-        return BLOCK_ERR_STOP_ENOSPC;
+        return BLOCKDEV_ON_ERROR_ENOSPC;
     } else if (!strcmp(buf, "stop")) {
-        return BLOCK_ERR_STOP_ANY;
+        return BLOCKDEV_ON_ERROR_STOP;
     } else if (!strcmp(buf, "report")) {
-        return BLOCK_ERR_REPORT;
+        return BLOCKDEV_ON_ERROR_REPORT;
     } else {
         error_report("'%s' invalid %s error action",
                      buf, is_read ? "read" : "write");
@@ -433,7 +433,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
         return NULL;
     }
 
-    on_write_error = BLOCK_ERR_STOP_ENOSPC;
+    on_write_error = BLOCKDEV_ON_ERROR_ENOSPC;
     if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
         if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type != IF_NONE) {
             error_report("werror is not supported by this bus type");
@@ -446,7 +446,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
         }
     }
 
-    on_read_error = BLOCK_ERR_REPORT;
+    on_read_error = BLOCKDEV_ON_ERROR_REPORT;
     if ((buf = qemu_opt_get(opts, "rerror")) != NULL) {
         if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI && type != IF_NONE) {
             error_report("rerror is not supported by this bus type");
diff --git a/hw/fdc.c b/hw/fdc.c
index 08830c1..43b0f20 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1994,11 +1994,11 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
         drive->fdctrl = fdctrl;
 
         if (drive->bs) {
-            if (bdrv_get_on_error(drive->bs, 0) != BLOCK_ERR_STOP_ENOSPC) {
+            if (bdrv_get_on_error(drive->bs, 0) != BLOCKDEV_ON_ERROR_ENOSPC) {
                 error_report("fdc doesn't support drive option werror");
                 return -1;
             }
-            if (bdrv_get_on_error(drive->bs, 1) != BLOCK_ERR_REPORT) {
+            if (bdrv_get_on_error(drive->bs, 1) != BLOCKDEV_ON_ERROR_REPORT) {
                 error_report("fdc doesn't support drive option rerror");
                 return -1;
             }
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 57b9fa4..2620e87 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -559,13 +559,13 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
     int is_read = (op & BM_STATUS_RETRY_READ);
     BlockdevOnError action = bdrv_get_on_error(s->bs, is_read);
 
-    if (action == BLOCK_ERR_IGNORE) {
+    if (action == BLOCKDEV_ON_ERROR_IGNORE) {
         bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_IGNORE, is_read);
         return 0;
     }
 
-    if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC)
-            || action == BLOCK_ERR_STOP_ANY) {
+    if ((error == ENOSPC && action == BLOCKDEV_ON_ERROR_ENOSPC)
+            || action == BLOCKDEV_ON_ERROR_STOP) {
         s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
         s->bus->error_status = op;
         bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_STOP, is_read);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index fef83a3..c295326 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -390,13 +390,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     BlockdevOnError action = bdrv_get_on_error(s->qdev.conf.bs, is_read);
 
-    if (action == BLOCK_ERR_IGNORE) {
+    if (action == BLOCKDEV_ON_ERROR_IGNORE) {
         bdrv_emit_qmp_error_event(s->qdev.conf.bs, BDRV_ACTION_IGNORE, is_read);
         return 0;
     }
 
-    if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC)
-            || action == BLOCK_ERR_STOP_ANY) {
+    if ((error == ENOSPC && action == BLOCKDEV_ON_ERROR_ENOSPC)
+            || action == BLOCKDEV_ON_ERROR_STOP) {
 
         bdrv_emit_qmp_error_event(s->qdev.conf.bs, BDRV_ACTION_STOP, is_read);
         vm_stop(RUN_STATE_IO_ERROR);
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index a5eb663..d904534 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -400,11 +400,11 @@ static int scsi_generic_initfn(SCSIDevice *s)
         return -1;
     }
 
-    if (bdrv_get_on_error(s->conf.bs, 0) != BLOCK_ERR_STOP_ENOSPC) {
+    if (bdrv_get_on_error(s->conf.bs, 0) != BLOCKDEV_ON_ERROR_ENOSPC) {
         error_report("Device doesn't support drive option werror");
         return -1;
     }
-    if (bdrv_get_on_error(s->conf.bs, 1) != BLOCK_ERR_REPORT) {
+    if (bdrv_get_on_error(s->conf.bs, 1) != BLOCKDEV_ON_ERROR_REPORT) {
         error_report("Device doesn't support drive option rerror");
         return -1;
     }
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 01e537d..f178fa8 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -69,13 +69,13 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
     BlockdevOnError action = bdrv_get_on_error(req->dev->bs, is_read);
     VirtIOBlock *s = req->dev;
 
-    if (action == BLOCK_ERR_IGNORE) {
+    if (action == BLOCKDEV_ON_ERROR_IGNORE) {
         bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_IGNORE, is_read);
         return 0;
     }
 
-    if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC)
-            || action == BLOCK_ERR_STOP_ANY) {
+    if ((error == ENOSPC && action == BLOCKDEV_ON_ERROR_ENOSPC)
+            || action == BLOCKDEV_ON_ERROR_STOP) {
         req->next = s->rq;
         s->rq = req;
         bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_STOP, is_read);
diff --git a/qapi-schema.json b/qapi-schema.json
index a37507a..406c180 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1088,6 +1088,29 @@
 { 'command': 'query-pci', 'returns': ['PciInfo'] }
 
 ##
+# @BlockdevOnError:
+#
+# An enumeration of possible behaviors for errors on I/O operations.
+# The exact meaning depends on whether the I/O was initiated by a guest
+# or by a block job
+#
+# @report: for guest operations, report the error to the guest;
+#          for jobs, cancel the job
+#
+# @ignore: ignore the error, only report a QMP event (BLOCK_IO_ERROR
+#          or BLOCK_JOB_ERROR)
+#
+# @enospc: same as @stop on ENOSPC, same as @report otherwise.
+#
+# @stop: for guest operations, stop the virtual machine;
+#        for jobs, pause the job
+#
+# Since: 1.3
+##
+{ 'enum': 'BlockdevOnError',
+  'data': ['report', 'ignore', 'enospc', 'stop'] }
+
+##
 # @BlockJobInfo:
 #
 # Information about a long-running block device operation.
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 13/19] iostatus: change is_read to a bool
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (11 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 12/19] iostatus: move BlockdevOnError declaration to QAPI Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 14/19] iostatus: reorganize io error code Paolo Bonzini
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Do this while we are touching this part of the code, before introducing
more uses of "int is_read".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c         | 4 ++--
 block.h         | 4 ++--
 blockdev.c      | 2 +-
 hw/ide/core.c   | 2 +-
 hw/ide/pci.c    | 4 ++--
 hw/scsi-disk.c  | 2 +-
 hw/virtio-blk.c | 4 ++--
 7 file modificati, 11 inserzioni(+), 11 rimozioni(-)

diff --git a/block.c b/block.c
index ab2f7e0..85114c5 100644
--- a/block.c
+++ b/block.c
@@ -1387,7 +1387,7 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
 }
 
 void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               BlockErrorAction action, int is_read)
+                               BlockErrorAction action, bool is_read)
 {
     QObject *data;
     const char *action_str;
@@ -2338,7 +2338,7 @@ void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
     bs->on_write_error = on_write_error;
 }
 
-BlockdevOnError bdrv_get_on_error(BlockDriverState *bs, int is_read)
+BlockdevOnError bdrv_get_on_error(BlockDriverState *bs, bool is_read)
 {
     return is_read ? bs->on_read_error : bs->on_write_error;
 }
diff --git a/block.h b/block.h
index d93cbc3..433721d 100644
--- a/block.h
+++ b/block.h
@@ -109,7 +109,7 @@ void bdrv_iostatus_disable(BlockDriverState *bs);
 bool bdrv_iostatus_is_enabled(const BlockDriverState *bs);
 void bdrv_iostatus_set_err(BlockDriverState *bs, int error);
 void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               BlockErrorAction action, int is_read);
+                               BlockErrorAction action, bool is_read);
 void bdrv_info_print(Monitor *mon, const QObject *data);
 void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_stats_print(Monitor *mon, const QObject *data);
@@ -276,7 +276,7 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
 
 void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
                        BlockdevOnError on_write_error);
-BlockdevOnError bdrv_get_on_error(BlockDriverState *bs, int is_read);
+BlockdevOnError bdrv_get_on_error(BlockDriverState *bs, bool is_read);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
diff --git a/blockdev.c b/blockdev.c
index 9fc794f..4a7d2dd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -238,7 +238,7 @@ static void drive_put_ref_bh_schedule(DriveInfo *dinfo)
     qemu_bh_schedule(s->bh);
 }
 
-static int parse_block_error_action(const char *buf, int is_read)
+static int parse_block_error_action(const char *buf, bool is_read)
 {
     if (!strcmp(buf, "ignore")) {
         return BLOCKDEV_ON_ERROR_IGNORE;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 2620e87..c03db4a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -556,7 +556,7 @@ void ide_dma_error(IDEState *s)
 
 static int ide_handle_rw_error(IDEState *s, int error, int op)
 {
-    int is_read = (op & BM_STATUS_RETRY_READ);
+    bool is_read = (op & BM_STATUS_RETRY_READ) != 0;
     BlockdevOnError action = bdrv_get_on_error(s->bs, is_read);
 
     if (action == BLOCKDEV_ON_ERROR_IGNORE) {
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 88c0942..644533f 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -188,7 +188,7 @@ static void bmdma_restart_bh(void *opaque)
 {
     BMDMAState *bm = opaque;
     IDEBus *bus = bm->bus;
-    int is_read;
+    bool is_read;
     int error_status;
 
     qemu_bh_delete(bm->bh);
@@ -198,7 +198,7 @@ static void bmdma_restart_bh(void *opaque)
         return;
     }
 
-    is_read = !!(bus->error_status & BM_STATUS_RETRY_READ);
+    is_read = (bus->error_status & BM_STATUS_RETRY_READ) != 0;
 
     /* The error status must be cleared before resubmitting the request: The
      * request may fail again, and this case can only be distinguished if the
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c295326..2dd99a9 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -386,7 +386,7 @@ static void scsi_read_data(SCSIRequest *req)
  */
 static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
 {
-    int is_read = (r->req.cmd.xfer == SCSI_XFER_FROM_DEV);
+    bool is_read = (r->req.cmd.xfer == SCSI_XFER_FROM_DEV);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     BlockdevOnError action = bdrv_get_on_error(s->qdev.conf.bs, is_read);
 
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index f178fa8..1ac2483 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -64,7 +64,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 }
 
 static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
-    int is_read)
+    bool is_read)
 {
     BlockdevOnError action = bdrv_get_on_error(req->dev->bs, is_read);
     VirtIOBlock *s = req->dev;
@@ -98,7 +98,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
     trace_virtio_blk_rw_complete(req, ret);
 
     if (ret) {
-        int is_read = !(ldl_p(&req->out->type) & VIRTIO_BLK_T_OUT);
+        bool is_read = !(ldl_p(&req->out->type) & VIRTIO_BLK_T_OUT);
         if (virtio_blk_handle_rw_error(req, -ret, is_read))
             return;
     }
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 14/19] iostatus: reorganize io error code
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (12 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 13/19] iostatus: change is_read to a bool Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 15/19] block: introduce block job error Paolo Bonzini
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Move the common part of IDE/SCSI/virtio error handling to the block
layer.  The new function bdrv_error_action subsumes all three of
bdrv_emit_qmp_error_event, vm_stop, bdrv_iostatus_set_err.

The same scheme will be used for errors in block jobs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c         | 46 ++++++++++++++++++++++++++++++++++++++--------
 block.h         |  5 +++--
 hw/ide/core.c   | 20 +++++---------------
 hw/scsi-disk.c  | 23 +++++++----------------
 hw/virtio-blk.c | 19 +++++--------------
 qemu-tool.c     |  6 ++++++
 6 file modificati, 64 inserzioni(+), 55 rimozioni(-)

diff --git a/block.c b/block.c
index 85114c5..b7b04aa 100644
--- a/block.c
+++ b/block.c
@@ -29,6 +29,7 @@
 #include "blockjob.h"
 #include "module.h"
 #include "qjson.h"
+#include "sysemu.h"
 #include "qemu-coroutine.h"
 #include "qmp-commands.h"
 #include "qemu-timer.h"
@@ -1386,8 +1387,8 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
     }
 }
 
-void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               BlockErrorAction action, bool is_read)
+static void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
+                                      BlockErrorAction action, bool is_read)
 {
     QObject *data;
     const char *action_str;
@@ -2343,6 +2344,39 @@ BlockdevOnError bdrv_get_on_error(BlockDriverState *bs, bool is_read)
     return is_read ? bs->on_read_error : bs->on_write_error;
 }
 
+BlockErrorAction bdrv_get_error_action(BlockDriverState *bs, bool is_read, int error)
+{
+    BlockdevOnError on_err = is_read ? bs->on_read_error : bs->on_write_error;
+
+    switch (on_err) {
+    case BLOCKDEV_ON_ERROR_ENOSPC:
+        return (error == ENOSPC) ? BDRV_ACTION_STOP : BDRV_ACTION_REPORT;
+    case BLOCKDEV_ON_ERROR_STOP:
+        return BDRV_ACTION_STOP;
+    case BLOCKDEV_ON_ERROR_REPORT:
+        return BDRV_ACTION_REPORT;
+    case BLOCKDEV_ON_ERROR_IGNORE:
+        return BDRV_ACTION_IGNORE;
+    default:
+        abort();
+    }
+}
+
+/* This is done by device models because, while the block layer knows
+ * about the error, it does not know whether an operation comes from
+ * the device or the block layer (from a job, for example).
+ */
+void bdrv_error_action(BlockDriverState *bs, BlockErrorAction action,
+                       bool is_read, int error)
+{
+    assert(error >= 0);
+    bdrv_emit_qmp_error_event(bs, action, is_read);
+    if (action == BDRV_ACTION_STOP) {
+        vm_stop(RUN_STATE_IO_ERROR);
+        bdrv_iostatus_set_err(bs, error);
+    }
+}
+
 int bdrv_is_read_only(BlockDriverState *bs)
 {
     return bs->read_only;
@@ -4067,14 +4101,10 @@ void bdrv_iostatus_reset(BlockDriverState *bs)
     }
 }
 
-/* XXX: Today this is set by device models because it makes the implementation
-   quite simple. However, the block layer knows about the error, so it's
-   possible to implement this without device models being involved */
 void bdrv_iostatus_set_err(BlockDriverState *bs, int error)
 {
-    if (bdrv_iostatus_is_enabled(bs) &&
-        bs->iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
-        assert(error >= 0);
+    assert(bdrv_iostatus_is_enabled(bs));
+    if (bs->iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
         bs->iostatus = error == ENOSPC ? BLOCK_DEVICE_IO_STATUS_NOSPACE :
                                          BLOCK_DEVICE_IO_STATUS_FAILED;
     }
diff --git a/block.h b/block.h
index 433721d..7db23b8 100644
--- a/block.h
+++ b/block.h
@@ -108,8 +108,6 @@ void bdrv_iostatus_reset(BlockDriverState *bs);
 void bdrv_iostatus_disable(BlockDriverState *bs);
 bool bdrv_iostatus_is_enabled(const BlockDriverState *bs);
 void bdrv_iostatus_set_err(BlockDriverState *bs, int error);
-void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               BlockErrorAction action, bool is_read);
 void bdrv_info_print(Monitor *mon, const QObject *data);
 void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_stats_print(Monitor *mon, const QObject *data);
@@ -277,6 +275,9 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
 void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
                        BlockdevOnError on_write_error);
 BlockdevOnError bdrv_get_on_error(BlockDriverState *bs, bool is_read);
+BlockErrorAction bdrv_get_error_action(BlockDriverState *bs, bool is_read, int error);
+void bdrv_error_action(BlockDriverState *bs, BlockErrorAction action,
+                       bool is_read, int error);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index c03db4a..d683a8c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -557,31 +557,21 @@ void ide_dma_error(IDEState *s)
 static int ide_handle_rw_error(IDEState *s, int error, int op)
 {
     bool is_read = (op & BM_STATUS_RETRY_READ) != 0;
-    BlockdevOnError action = bdrv_get_on_error(s->bs, is_read);
+    BlockErrorAction action = bdrv_get_error_action(s->bs, is_read, error);
 
-    if (action == BLOCKDEV_ON_ERROR_IGNORE) {
-        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_IGNORE, is_read);
-        return 0;
-    }
-
-    if ((error == ENOSPC && action == BLOCKDEV_ON_ERROR_ENOSPC)
-            || action == BLOCKDEV_ON_ERROR_STOP) {
+    if (action == BDRV_ACTION_STOP) {
         s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
         s->bus->error_status = op;
-        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_STOP, is_read);
-        vm_stop(RUN_STATE_IO_ERROR);
-        bdrv_iostatus_set_err(s->bs, error);
-    } else {
+    } else if (action == BDRV_ACTION_REPORT) {
         if (op & BM_STATUS_DMA_RETRY) {
             dma_buf_commit(s);
             ide_dma_error(s);
         } else {
             ide_rw_error(s);
         }
-        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_REPORT, is_read);
     }
-
-    return 1;
+    bdrv_error_action(s->bs, action, is_read, error);
+    return action != BDRV_ACTION_IGNORE;
 }
 
 void ide_dma_cb(void *opaque, int ret)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2dd99a9..99bb02e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -388,21 +388,9 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
 {
     bool is_read = (r->req.cmd.xfer == SCSI_XFER_FROM_DEV);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-    BlockdevOnError action = bdrv_get_on_error(s->qdev.conf.bs, is_read);
+    BlockErrorAction action = bdrv_get_error_action(s->qdev.conf.bs, is_read, error);
 
-    if (action == BLOCKDEV_ON_ERROR_IGNORE) {
-        bdrv_emit_qmp_error_event(s->qdev.conf.bs, BDRV_ACTION_IGNORE, is_read);
-        return 0;
-    }
-
-    if ((error == ENOSPC && action == BLOCKDEV_ON_ERROR_ENOSPC)
-            || action == BLOCKDEV_ON_ERROR_STOP) {
-
-        bdrv_emit_qmp_error_event(s->qdev.conf.bs, BDRV_ACTION_STOP, is_read);
-        vm_stop(RUN_STATE_IO_ERROR);
-        bdrv_iostatus_set_err(s->qdev.conf.bs, error);
-        scsi_req_retry(&r->req);
-    } else {
+    if (action == BDRV_ACTION_REPORT) {
         switch (error) {
         case ENOMEDIUM:
             scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
@@ -417,9 +405,12 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
             scsi_check_condition(r, SENSE_CODE(IO_ERROR));
             break;
         }
-        bdrv_emit_qmp_error_event(s->qdev.conf.bs, BDRV_ACTION_REPORT, is_read);
     }
-    return 1;
+    bdrv_error_action(s->qdev.conf.bs, action, is_read, error);
+    if (action == BDRV_ACTION_STOP) {
+        scsi_req_retry(&r->req);
+    }
+    return action != BDRV_ACTION_IGNORE;
 }
 
 static void scsi_write_complete(void * opaque, int ret)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 1ac2483..e25cc96 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -66,29 +66,20 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
     bool is_read)
 {
-    BlockdevOnError action = bdrv_get_on_error(req->dev->bs, is_read);
+    BlockErrorAction action = bdrv_get_error_action(req->dev->bs, is_read, error);
     VirtIOBlock *s = req->dev;
 
-    if (action == BLOCKDEV_ON_ERROR_IGNORE) {
-        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_IGNORE, is_read);
-        return 0;
-    }
-
-    if ((error == ENOSPC && action == BLOCKDEV_ON_ERROR_ENOSPC)
-            || action == BLOCKDEV_ON_ERROR_STOP) {
+    if (action == BDRV_ACTION_STOP) {
         req->next = s->rq;
         s->rq = req;
-        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_STOP, is_read);
-        vm_stop(RUN_STATE_IO_ERROR);
-        bdrv_iostatus_set_err(s->bs, error);
-    } else {
+    } else if (action == BDRV_ACTION_REPORT) {
         virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
         bdrv_acct_done(s->bs, &req->acct);
         g_free(req);
-        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_REPORT, is_read);
     }
 
-    return 1;
+    bdrv_error_action(s->bs, action, is_read, error);
+    return action != BDRV_ACTION_IGNORE;
 }
 
 static void virtio_blk_rw_complete(void *opaque, int ret)
diff --git a/qemu-tool.c b/qemu-tool.c
index 18205ba..f2f9813 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -19,6 +19,7 @@
 #include "qemu-log.h"
 #include "migration.h"
 #include "main-loop.h"
+#include "sysemu.h"
 #include "qemu_socket.h"
 #include "slirp/libslirp.h"
 
@@ -37,6 +38,11 @@ const char *qemu_get_vm_name(void)
 
 Monitor *cur_mon;
 
+void vm_stop(RunState state)
+{
+    abort();
+}
+
 int monitor_cur_is_qmp(void)
 {
     return 0;
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 15/19] block: introduce block job error
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (13 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 14/19] iostatus: reorganize io error code Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 16/19] stream: add on-error argument Paolo Bonzini
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

The following behaviors are possible:

'report': The behavior is the same as in 1.1.  An I/O error,
respectively during a read or a write, will complete the job immediately
with an error code.

'ignore': An I/O error, respectively during a read or a write, will be
ignored.  For streaming, the job will complete with an error and the
backing file will be left in place.  For mirroring, the sector will be
marked again as dirty and re-examined later.

'stop': The job will be paused and the job iostatus will be set to
failed or nospace, while the VM will keep running.  This can only be
specified if the block device has rerror=stop and werror=stop or enospc.

'enospc': Behaves as 'stop' for ENOSPC errors, 'report' for others.

In all cases, even for 'report', the I/O error is reported as a QMP
event BLOCK_JOB_ERROR, with the same arguments as BLOCK_IO_ERROR.

It is possible that while stopping the VM a BLOCK_IO_ERROR event will be
reported and will clobber the event from BLOCK_JOB_ERROR, or vice versa.
This is not really avoidable since stopping the VM completes all pending
I/O requests.  In fact, it is already possible now that a series of
BLOCK_IO_ERROR events are reported with rerror=stop, because vm_stop
calls bdrv_drain_all and this can generate further errors.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 QMP/qmp-events.txt | 22 +++++++++++++++++++
 block.c            |  9 ++++----
 block_int.h        |  4 ++++
 blockjob.c         | 63 ++++++++++++++++++++++++++++++++++++++++++++++++------
 blockjob.h         | 25 ++++++++++++++++++++++
 monitor.c          |  1 +
 monitor.h          |  1 +
 qapi-schema.json   |  7 +++++-
 8 file modificati, 120 inserzioni(+), 12 rimozioni(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 2878058..ae19db2 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -94,6 +94,28 @@ 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 } }
+
 DEVICE_TRAY_MOVED
 -----------------
 
diff --git a/block.c b/block.c
index b7b04aa..83a695b 100644
--- a/block.c
+++ b/block.c
@@ -1387,8 +1387,9 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
     }
 }
 
-static void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                                      BlockErrorAction action, bool is_read)
+void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
+                               enum MonitorEvent ev,
+                               BlockErrorAction action, bool is_read)
 {
     QObject *data;
     const char *action_str;
@@ -1411,7 +1412,7 @@ static void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
                               bdrv->device_name,
                               action_str,
                               is_read ? "read" : "write");
-    monitor_protocol_event(QEVENT_BLOCK_IO_ERROR, data);
+    monitor_protocol_event(ev, data);
 
     qobject_decref(data);
 }
@@ -2370,7 +2371,7 @@ void bdrv_error_action(BlockDriverState *bs, BlockErrorAction action,
                        bool is_read, int error)
 {
     assert(error >= 0);
-    bdrv_emit_qmp_error_event(bs, action, is_read);
+    bdrv_emit_qmp_error_event(bs, QEVENT_BLOCK_IO_ERROR, action, is_read);
     if (action == BDRV_ACTION_STOP) {
         vm_stop(RUN_STATE_IO_ERROR);
         bdrv_iostatus_set_err(bs, error);
diff --git a/block_int.h b/block_int.h
index db487eb..5257b10 100644
--- a/block_int.h
+++ b/block_int.h
@@ -31,6 +31,7 @@
 #include "qemu-timer.h"
 #include "qapi-types.h"
 #include "qerror.h"
+#include "monitor.h"
 
 #define BLOCK_FLAG_ENCRYPT          1
 #define BLOCK_FLAG_COMPAT6          4
@@ -286,6 +287,9 @@ void bdrv_set_io_limits(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/blockjob.c b/blockjob.c
index 9577b84..b5c16f3 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -112,6 +112,7 @@ bool block_job_is_paused(BlockJob *job)
 void block_job_resume(BlockJob *job)
 {
     job->paused = false;
+    block_job_iostatus_reset(job);
     if (job->co && !job->busy) {
         qemu_coroutine_enter(job->co, NULL);
     }
@@ -128,6 +129,11 @@ bool block_job_is_cancelled(BlockJob *job)
     return job->cancelled;
 }
 
+void block_job_iostatus_reset(BlockJob *job)
+{
+    job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
+}
+
 struct BlockCancelData {
     BlockJob *job;
     BlockDriverCompletionFunc *cb;
@@ -189,12 +195,55 @@ void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns)
 BlockJobInfo *block_job_query(BlockJob *job)
 {
     BlockJobInfo *info = g_new0(BlockJobInfo, 1);
-    info->type   = g_strdup(job->job_type->job_type);
-    info->device = g_strdup(bdrv_get_device_name(job->bs));
-    info->len    = job->len;
-    info->busy   = job->busy;
-    info->paused = job->paused;
-    info->offset = job->offset;
-    info->speed  = job->speed;
+    info->type      = g_strdup(job->job_type->job_type);
+    info->device    = g_strdup(bdrv_get_device_name(job->bs));
+    info->len       = job->len;
+    info->busy      = job->busy;
+    info->paused    = job->paused;
+    info->offset    = job->offset;
+    info->speed     = job->speed;
+    info->io_status = job->iostatus;
     return info;
 }
+
+static void block_job_iostatus_set_err(BlockJob *job, int error)
+{
+    if (job->iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
+        job->iostatus = error == ENOSPC ? BLOCK_DEVICE_IO_STATUS_NOSPACE :
+                                          BLOCK_DEVICE_IO_STATUS_FAILED;
+    }
+}
+
+
+BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
+                                        BlockdevOnError on_err,
+                                        int is_read, int error)
+{
+    BlockErrorAction action;
+
+    switch (on_err) {
+    case BLOCKDEV_ON_ERROR_ENOSPC:
+        action = (error == ENOSPC) ? BDRV_ACTION_STOP : BDRV_ACTION_REPORT;
+        break;
+    case BLOCKDEV_ON_ERROR_STOP:
+        action = BDRV_ACTION_STOP;
+        break;
+    case BLOCKDEV_ON_ERROR_REPORT:
+        action = BDRV_ACTION_REPORT;
+        break;
+    case BLOCKDEV_ON_ERROR_IGNORE:
+        action = BDRV_ACTION_IGNORE;
+        break;
+    default:
+        abort();
+    }
+    bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action, is_read);
+    if (action == BDRV_ACTION_STOP) {
+        block_job_pause(job);
+        block_job_iostatus_set_err(job, error);
+        if (bs != job->bs) {
+            bdrv_iostatus_set_err(bs, error);
+        }
+    }
+    return action;
+}
diff --git a/blockjob.h b/blockjob.h
index b62ddb4..c2261a9 100644
--- a/blockjob.h
+++ b/blockjob.h
@@ -82,6 +82,9 @@ struct BlockJob {
      */
     bool busy;
 
+    /** Status that is published by the query-block-jobs QMP API */
+    BlockDeviceIoStatus iostatus;
+
     /** Offset that is published by the query-block-jobs QMP API */
     int64_t offset;
 
@@ -215,4 +218,26 @@ bool block_job_is_paused(BlockJob *job);
  */
 int block_job_cancel_sync(BlockJob *job);
 
+/**
+ * block_job_iostatus_reset:
+ * @job: The job whose I/O status should be reset.
+ *
+ * Reset I/O status on @job.
+ */
+void block_job_iostatus_reset(BlockJob *job);
+
+/**
+ * block_job_error_action:
+ * @job: The job to signal an error for.
+ * @bs: The block device on which to set an I/O error.
+ * @on_err: The error action setting.
+ * @is_read: Whether the operation was a read.
+ * @error: The error that was reported.
+ *
+ * Report an I/O error for a block job and possibly stop the VM.  Return the
+ * action that was selected based on @on_err and @error.
+ */
+BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
+                                        BlockdevOnError on_err,
+                                        int is_read, int error);
 #endif
diff --git a/monitor.c b/monitor.c
index 67064e2..d4bd5fe 100644
--- a/monitor.c
+++ b/monitor.c
@@ -450,6 +450,7 @@ static const char *monitor_event_names[] = {
     [QEVENT_SPICE_DISCONNECTED] = "SPICE_DISCONNECTED",
     [QEVENT_BLOCK_JOB_COMPLETED] = "BLOCK_JOB_COMPLETED",
     [QEVENT_BLOCK_JOB_CANCELLED] = "BLOCK_JOB_CANCELLED",
+    [QEVENT_BLOCK_JOB_ERROR] = "BLOCK_JOB_ERROR",
     [QEVENT_DEVICE_TRAY_MOVED] = "DEVICE_TRAY_MOVED",
     [QEVENT_SUSPEND] = "SUSPEND",
     [QEVENT_SUSPEND_DISK] = "SUSPEND_DISK",
diff --git a/monitor.h b/monitor.h
index 64c1561..43040af 100644
--- a/monitor.h
+++ b/monitor.h
@@ -38,6 +38,7 @@ typedef enum MonitorEvent {
     QEVENT_SPICE_DISCONNECTED,
     QEVENT_BLOCK_JOB_COMPLETED,
     QEVENT_BLOCK_JOB_CANCELLED,
+    QEVENT_BLOCK_JOB_ERROR,
     QEVENT_DEVICE_TRAY_MOVED,
     QEVENT_SUSPEND,
     QEVENT_SUSPEND_DISK,
diff --git a/qapi-schema.json b/qapi-schema.json
index 406c180..d5502fe 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1131,11 +1131,14 @@
 #
 # @speed: the rate limit, bytes per second
 #
+# @io-status: the status of the job (since 1.3)
+#
 # Since: 1.1
 ##
 { 'type': 'BlockJobInfo',
   'data': {'type': 'str', 'device': 'str', 'len': 'int',
-           'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int'} }
+           'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int',
+           'io-status': 'BlockDeviceIoStatus'} }
 
 ##
 # @query-block-jobs:
@@ -1924,6 +1927,8 @@
 # operation.  It is an error to call this command if no operation is in
 # progress.  Resuming an already running job is not an error.
 #
+# This command also clears the error status of the job.
+#
 # @device: the device name
 #
 # Returns: Nothing on success
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 16/19] stream: add on-error argument
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (14 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 15/19] block: introduce block job error Paolo Bonzini
@ 2012-09-28 15:22 ` Paolo Bonzini
  2012-09-28 15:23 ` [Qemu-devel] [PATCH 17/19] blkdebug: process all set_state rules in the old state Paolo Bonzini
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

This patch adds support for error management to streaming.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/stream.c   | 28 +++++++++++++++++++++++++++-
 block_int.h      |  3 ++-
 blockdev.c       | 11 ++++++++---
 hmp.c            |  3 ++-
 qapi-schema.json |  9 +++++++--
 qmp-commands.hx  |  2 +-
 6 file modificati, 47 inserzioni(+), 9 rimozioni(-)

diff --git a/block/stream.c b/block/stream.c
index a8f585a..0c0fc7a 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -31,6 +31,7 @@ typedef struct StreamBlockJob {
     BlockJob common;
     RateLimit limit;
     BlockDriverState *base;
+    BlockdevOnError on_error;
     char backing_file_id[1024];
 } StreamBlockJob;
 
@@ -78,6 +79,7 @@ static void coroutine_fn stream_run(void *opaque)
     BlockDriverState *bs = s->common.bs;
     BlockDriverState *base = s->base;
     int64_t sector_num, end;
+    int error = 0;
     int ret = 0;
     int n = 0;
     void *buf;
@@ -142,7 +144,19 @@ wait:
             ret = stream_populate(bs, sector_num, n, buf);
         }
         if (ret < 0) {
-            break;
+            BlockErrorAction action =
+                block_job_error_action(&s->common, s->common.bs, s->on_error,
+                                       true, -ret);
+            if (action == BDRV_ACTION_STOP) {
+                n = 0;
+                continue;
+            }
+            if (error == 0) {
+                error = ret;
+            }
+            if (action == BDRV_ACTION_REPORT) {
+                break;
+            }
         }
         ret = 0;
 
@@ -154,6 +168,9 @@ wait:
         bdrv_disable_copy_on_read(bs);
     }
 
+    /* Do not remove the backing file if an error was there but ignored.  */
+    ret = error;
+
     if (!block_job_is_cancelled(&s->common) && sector_num == end && ret == 0) {
         const char *base_id = NULL, *base_fmt = NULL;
         if (base) {
@@ -189,11 +206,19 @@ static BlockJobType stream_job_type = {
 
 void stream_start(BlockDriverState *bs, BlockDriverState *base,
                   const char *base_id, int64_t speed,
+                  BlockdevOnError on_error,
                   BlockDriverCompletionFunc *cb,
                   void *opaque, Error **errp)
 {
     StreamBlockJob *s;
 
+    if ((on_error == BLOCKDEV_ON_ERROR_STOP ||
+         on_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
+        !bdrv_iostatus_is_enabled(bs)) {
+        error_set(errp, QERR_INVALID_PARAMETER, "on-error");
+        return;
+    }
+
     s = block_job_create(&stream_job_type, bs, speed, cb, opaque, errp);
     if (!s) {
         return;
@@ -204,6 +229,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base,
         pstrcpy(s->backing_file_id, sizeof(s->backing_file_id), base_id);
     }
 
+    s->on_error = on_error;
     s->common.co = qemu_coroutine_create(stream_run);
     trace_stream_start(bs, base, s, s->common.co, opaque);
     qemu_coroutine_enter(s->common.co, s);
diff --git a/block_int.h b/block_int.h
index 5257b10..057baae 100644
--- a/block_int.h
+++ b/block_int.h
@@ -299,6 +299,7 @@ void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
  * @base_id: The file name that will be written to @bs as the new
  * backing file if the job completes.  Ignored if @base is %NULL.
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
+ * @on_error: The action to take upon error.
  * @cb: Completion function for the job.
  * @opaque: Opaque pointer value passed to @cb.
  * @errp: Error object.
@@ -310,7 +311,7 @@ void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
  * @base_id in the written image and to @base in the live BlockDriverState.
  */
 void stream_start(BlockDriverState *bs, BlockDriverState *base,
-                  const char *base_id, int64_t speed,
+                  const char *base_id, int64_t speed, BlockdevOnError on_error,
                   BlockDriverCompletionFunc *cb,
                   void *opaque, Error **errp);
 
diff --git a/blockdev.c b/blockdev.c
index 4a7d2dd..425759e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1091,13 +1091,18 @@ static void block_job_cb(void *opaque, int ret)
 }
 
 void qmp_block_stream(const char *device, bool has_base,
-                      const char *base, bool has_speed,
-                      int64_t speed, Error **errp)
+                      const char *base, bool has_speed, int64_t speed,
+                      bool has_on_error, BlockdevOnError on_error,
+                      Error **errp)
 {
     BlockDriverState *bs;
     BlockDriverState *base_bs = NULL;
     Error *local_err = NULL;
 
+    if (!has_on_error) {
+        on_error = BLOCKDEV_ON_ERROR_REPORT;
+    }
+
     bs = bdrv_find(device);
     if (!bs) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
@@ -1113,7 +1118,7 @@ void qmp_block_stream(const char *device, bool has_base,
     }
 
     stream_start(bs, base_bs, base, has_speed ? speed : 0,
-                 block_job_cb, bs, &local_err);
+                 on_error, block_job_cb, bs, &local_err);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
         return;
diff --git a/hmp.c b/hmp.c
index 55601f7..df789b2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -930,7 +930,8 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
     int64_t speed = qdict_get_try_int(qdict, "speed", 0);
 
     qmp_block_stream(device, base != NULL, base,
-                     qdict_haskey(qdict, "speed"), speed, &error);
+                     qdict_haskey(qdict, "speed"), speed,
+                     BLOCKDEV_ON_ERROR_REPORT, true, &error);
 
     hmp_handle_error(mon, &error);
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index d5502fe..d31ff0e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1835,13 +1835,18 @@
 #
 # @speed:  #optional the maximum speed, in bytes per second
 #
+# @on-error: #optional the action to take on an error (default report).
+#            'stop' and 'enospc' can only be used if the block device
+#            supports io-status (see BlockInfo).  Since 1.3.
+#
 # Returns: Nothing on success
 #          If @device does not exist, DeviceNotFound
 #
 # Since: 1.1
 ##
-{ 'command': 'block-stream', 'data': { 'device': 'str', '*base': 'str',
-                                       '*speed': 'int' } }
+{ 'command': 'block-stream',
+  'data': { 'device': 'str', '*base': 'str', '*speed': 'int',
+            '*on-error': 'BlockdevOnError' } }
 
 ##
 # @block-job-set-speed:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 85eacb5..07fa8fe 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -787,7 +787,7 @@ EQMP
 
     {
         .name       = "block-stream",
-        .args_type  = "device:B,base:s?,speed:o?",
+        .args_type  = "device:B,base:s?,speed:o?,on-error:s?",
         .mhandler.cmd_new = qmp_marshal_input_block_stream,
     },
 
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 17/19] blkdebug: process all set_state rules in the old state
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (15 preceding siblings ...)
  2012-09-28 15:22 ` [Qemu-devel] [PATCH 16/19] stream: add on-error argument Paolo Bonzini
@ 2012-09-28 15:23 ` Paolo Bonzini
  2012-09-28 15:23 ` [Qemu-devel] [PATCH 18/19] qemu-iotests: map underscore to dash in QMP argument names Paolo Bonzini
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Currently it is impossible to write a blkdebug script that ping-pongs
between two states, because the second set-state rule will use the
state that is set in the first.  If you have

    [set-state]
    event = "..."
    state = "1"
    new_state = "2"

    [set-state]
    event = "..."
    state = "2"
    new_state = "1"

for example the state will remain locked at 1.  This can be fixed
by first processing all rules, and then setting the state.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/blkdebug.c | 12 +++++++-----
 1 file modificato, 7 inserzioni(+), 5 rimozioni(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 59dcea0..1206d52 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -28,6 +28,7 @@
 
 typedef struct BDRVBlkdebugState {
     int state;
+    int new_state;
     QLIST_HEAD(, BlkdebugRule) rules[BLKDBG_EVENT_MAX];
     QSIMPLEQ_HEAD(, BlkdebugRule) active_rules;
 } BDRVBlkdebugState;
@@ -403,12 +404,12 @@ static void blkdebug_close(BlockDriverState *bs)
 }
 
 static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
-    int old_state, bool injected)
+    bool injected)
 {
     BDRVBlkdebugState *s = bs->opaque;
 
     /* Only process rules for the current state */
-    if (rule->state && rule->state != old_state) {
+    if (rule->state && rule->state != s->state) {
         return injected;
     }
 
@@ -423,7 +424,7 @@ static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
         break;
 
     case ACTION_SET_STATE:
-        s->state = rule->options.set_state.new_state;
+        s->new_state = rule->options.set_state.new_state;
         break;
     }
     return injected;
@@ -433,15 +434,16 @@ static void blkdebug_debug_event(BlockDriverState *bs, BlkDebugEvent event)
 {
     BDRVBlkdebugState *s = bs->opaque;
     struct BlkdebugRule *rule;
-    int old_state = s->state;
     bool injected;
 
     assert((int)event >= 0 && event < BLKDBG_EVENT_MAX);
 
     injected = false;
+    s->new_state = s->state;
     QLIST_FOREACH(rule, &s->rules[event], next) {
-        injected = process_rule(bs, rule, old_state, injected);
+        injected = process_rule(bs, rule, injected);
     }
+    s->state = s->new_state;
 }
 
 static int64_t blkdebug_getlength(BlockDriverState *bs)
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 18/19] qemu-iotests: map underscore to dash in QMP argument names
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (16 preceding siblings ...)
  2012-09-28 15:23 ` [Qemu-devel] [PATCH 17/19] blkdebug: process all set_state rules in the old state Paolo Bonzini
@ 2012-09-28 15:23 ` Paolo Bonzini
  2012-09-28 15:23 ` [Qemu-devel] [PATCH 19/19] qemu-iotests: add tests for streaming error handling Paolo Bonzini
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

iotests.py provides a convenience function that uses Python keyword
arguments to represent QMP command arguments.  However, almost all
QMP commands use dashes for argument names (the sole exception is
block_set_io_throttle), and dashes are not allowed in a keyword
argument name.  Hence provide automatic conversion of underscores
to dashes.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qemu-iotests/iotests.py | 8 +++++++-
 1 file modificato, 7 inserzioni(+). 1 rimozione(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e05b1d6..a94ea75 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -19,6 +19,7 @@
 import os
 import re
 import subprocess
+import string
 import unittest
 import sys; sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'QMP'))
 import qmp
@@ -96,9 +97,14 @@ class VM(object):
             os.remove(self._qemu_log_path)
             self._popen = None
 
+    underscore_to_dash = string.maketrans('_', '-')
     def qmp(self, cmd, **args):
         '''Invoke a QMP command and return the result dict'''
-        return self._qmp.cmd(cmd, args=args)
+        qmp_args = dict()
+        for k in args.keys():
+            qmp_args[k.translate(self.underscore_to_dash)] = args[k]
+
+        return self._qmp.cmd(cmd, args=qmp_args)
 
     def get_qmp_events(self, wait=False):
         '''Poll for queued QMP events and return a list of dicts'''
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 19/19] qemu-iotests: add tests for streaming error handling
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (17 preceding siblings ...)
  2012-09-28 15:23 ` [Qemu-devel] [PATCH 18/19] qemu-iotests: map underscore to dash in QMP argument names Paolo Bonzini
@ 2012-09-28 15:23 ` Paolo Bonzini
  2012-09-28 15:57 ` [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Eric Blake
  2012-09-28 18:23 ` Kevin Wolf
  20 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-09-28 15:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Add a test for each of report/ignore/stop.  The tests use blkdebug
to generate an error in the middle of a script.  The error is
recoverable (once = "on") so that we can test resuming a job after
stopping for an error.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qemu-iotests/030        | 220 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/030.out    |   4 +-
 tests/qemu-iotests/iotests.py |   7 ++
 3 file modificati, 229 inserzioni(+), 2 rimozioni(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index dfacdf1..dd4ef11 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -195,6 +195,226 @@ class TestSmallerBackingFile(ImageStreamingTestCase):
         self.assert_no_active_streams()
         self.vm.shutdown()
 
+class TestErrors(ImageStreamingTestCase):
+    image_len = 2 * 1024 * 1024 # MB
+
+    # this should match STREAM_BUFFER_SIZE/512 in block/stream.c
+    STREAM_BUFFER_SIZE = 512 * 1024
+
+    def create_blkdebug_file(self, name, event, errno):
+        file = open(name, 'w')
+        file.write('''
+[inject-error]
+state = "1"
+event = "%s"
+errno = "%d"
+immediately = "off"
+once = "on"
+sector = "%d"
+
+[set-state]
+state = "1"
+event = "%s"
+new_state = "2"
+
+[set-state]
+state = "2"
+event = "%s"
+new_state = "1"
+''' % (event, errno, self.STREAM_BUFFER_SIZE / 512, event, event))
+        file.close()
+
+class TestEIO(TestErrors):
+    def setUp(self):
+        self.blkdebug_file = backing_img + ".blkdebug"
+        self.create_image(backing_img, TestErrors.image_len)
+        self.create_blkdebug_file(self.blkdebug_file, "read_aio", 5)
+        qemu_img('create', '-f', iotests.imgfmt,
+                 '-o', 'backing_file=blkdebug:%s:%s,backing_fmt=raw'
+                       % (self.blkdebug_file, backing_img),
+                 test_img)
+        self.vm = iotests.VM().add_drive(test_img)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+        os.remove(backing_img)
+        os.remove(self.blkdebug_file)
+
+    def test_report(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        completed = False
+        error = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_ERROR':
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/operation', 'read')
+                    error = True
+                elif event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assertTrue(error, 'job completed unexpectedly')
+                    self.assert_qmp(event, 'data/type', 'stream')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/error', 'Input/output error')
+                    self.assert_qmp(event, 'data/offset', self.STREAM_BUFFER_SIZE)
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    completed = True
+
+        self.assert_no_active_streams()
+        self.vm.shutdown()
+
+    def test_ignore(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0', on_error='ignore')
+        self.assert_qmp(result, 'return', {})
+
+        error = False
+        completed = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_ERROR':
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/operation', 'read')
+                    result = self.vm.qmp('query-block-jobs')
+                    self.assert_qmp(result, 'return[0]/paused', False)
+                    error = True
+                elif event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assertTrue(error, 'job completed unexpectedly')
+                    self.assert_qmp(event, 'data/type', 'stream')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/error', 'Input/output error')
+                    self.assert_qmp(event, 'data/offset', self.image_len)
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    completed = True
+
+        self.assert_no_active_streams()
+        self.vm.shutdown()
+
+    def test_stop(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0', on_error='stop')
+        self.assert_qmp(result, 'return', {})
+
+        error = False
+        completed = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_ERROR':
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/operation', 'read')
+
+                    result = self.vm.qmp('query-block-jobs')
+                    self.assert_qmp(result, 'return[0]/paused', True)
+                    self.assert_qmp(result, 'return[0]/offset', self.STREAM_BUFFER_SIZE)
+                    self.assert_qmp(result, 'return[0]/io-status', 'failed')
+
+                    result = self.vm.qmp('block-job-resume', device='drive0')
+                    self.assert_qmp(result, 'return', {})
+
+                    result = self.vm.qmp('query-block-jobs')
+                    self.assert_qmp(result, 'return[0]/paused', False)
+                    self.assert_qmp(result, 'return[0]/io-status', 'ok')
+                    error = True
+                elif event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assertTrue(error, 'job completed unexpectedly')
+                    self.assert_qmp(event, 'data/type', 'stream')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp_absent(event, 'data/error')
+                    self.assert_qmp(event, 'data/offset', self.image_len)
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    completed = True
+
+        self.assert_no_active_streams()
+        self.vm.shutdown()
+
+    def test_enospc(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0', on_error='enospc')
+        self.assert_qmp(result, 'return', {})
+
+        completed = False
+        error = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_ERROR':
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/operation', 'read')
+                    error = True
+                elif event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assertTrue(error, 'job completed unexpectedly')
+                    self.assert_qmp(event, 'data/type', 'stream')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/error', 'Input/output error')
+                    self.assert_qmp(event, 'data/offset', self.STREAM_BUFFER_SIZE)
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    completed = True
+
+        self.assert_no_active_streams()
+        self.vm.shutdown()
+
+class TestENOSPC(TestErrors):
+    def setUp(self):
+        self.blkdebug_file = backing_img + ".blkdebug"
+        self.create_image(backing_img, TestErrors.image_len)
+        self.create_blkdebug_file(self.blkdebug_file, "read_aio", 28)
+        qemu_img('create', '-f', iotests.imgfmt,
+                 '-o', 'backing_file=blkdebug:%s:%s,backing_fmt=raw'
+                       % (self.blkdebug_file, backing_img),
+                 test_img)
+        self.vm = iotests.VM().add_drive(test_img)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+        os.remove(backing_img)
+        os.remove(self.blkdebug_file)
+
+    def test_enospc(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0', on_error='enospc')
+        self.assert_qmp(result, 'return', {})
+
+        error = False
+        completed = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_ERROR':
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/operation', 'read')
+
+                    result = self.vm.qmp('query-block-jobs')
+                    self.assert_qmp(result, 'return[0]/paused', True)
+                    self.assert_qmp(result, 'return[0]/offset', self.STREAM_BUFFER_SIZE)
+                    self.assert_qmp(result, 'return[0]/io-status', 'nospace')
+
+                    result = self.vm.qmp('block-job-resume', device='drive0')
+                    self.assert_qmp(result, 'return', {})
+
+                    result = self.vm.qmp('query-block-jobs')
+                    self.assert_qmp(result, 'return[0]/paused', False)
+                    self.assert_qmp(result, 'return[0]/io-status', 'ok')
+                    error = True
+                elif event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assertTrue(error, 'job completed unexpectedly')
+                    self.assert_qmp(event, 'data/type', 'stream')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp_absent(event, 'data/error')
+                    self.assert_qmp(event, 'data/offset', self.image_len)
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    completed = True
+
+        self.assert_no_active_streams()
+        self.vm.shutdown()
 
 class TestStreamStop(ImageStreamingTestCase):
     image_len = 8 * 1024 * 1024 * 1024 # GB
diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
index 594c16f..fa16b5c 100644
--- a/tests/qemu-iotests/030.out
+++ b/tests/qemu-iotests/030.out
@@ -1,5 +1,5 @@
-........
+.............
 ----------------------------------------------------------------------
-Ran 8 tests
+Ran 13 tests
 
 OK
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index a94ea75..3c60b2d 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -138,6 +138,13 @@ class QMPTestCase(unittest.TestCase):
                     self.fail('invalid index "%s" in path "%s" in "%s"' % (idx, path, str(d)))
         return d
 
+    def assert_qmp_absent(self, d, path):
+        try:
+            result = self.dictpath(d, path)
+        except AssertionError:
+            return
+        self.fail('path "%s" has value "%s"' % (path, str(result)))
+
     def assert_qmp(self, d, path, value):
         '''Assert that the value for a specific path in a QMP dict matches'''
         result = self.dictpath(d, path)
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (18 preceding siblings ...)
  2012-09-28 15:23 ` [Qemu-devel] [PATCH 19/19] qemu-iotests: add tests for streaming error handling Paolo Bonzini
@ 2012-09-28 15:57 ` Eric Blake
  2012-09-28 18:23 ` Kevin Wolf
  20 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2012-09-28 15:57 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kwolf, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

On 09/28/2012 09:22 AM, Paolo Bonzini wrote:
> Kevin,
> 
> the following changes since commit ac05f3492421caeb05809ffa02c6198ede179e43:
> 
>   add a boot parameter to set reboot timeout (2012-09-25 20:05:04 -0500)
> 
> are available in the git repository at:
> 
>   git://github.com/bonzini/qemu.git blkmirror-job-1.3-part1
> 
> for you to fetch changes up to ed306a929f16fda8a85561430b1ac370bde65097:
> 
>   qemu-iotests: add tests for streaming error handling (2012-09-27 15:11:22 +0200)
> 
> This message has the diff from my post of Sep 25th.  You reviewed 18 patches,
> these are 19 because of the new "qmp: add 'busy' member to BlockJobInfo" patch.

I reviewed the interdiff, and it appears to address all the concerns
from v3 for these patches, other than my request to include 'type' as
part of the new event, possibly as an enum rather than freeform string.
 But since we agreed to defer that to a followup patch, it does not hold
up this series.  I also did one last sanity read through the patches.

Series:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1
  2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
                   ` (19 preceding siblings ...)
  2012-09-28 15:57 ` [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Eric Blake
@ 2012-09-28 18:23 ` Kevin Wolf
  2012-10-01 13:37   ` Paolo Bonzini
  20 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2012-09-28 18:23 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Am 28.09.2012 17:22, schrieb Paolo Bonzini:
> Kevin,
> 
> the following changes since commit ac05f3492421caeb05809ffa02c6198ede179e43:
> 
>   add a boot parameter to set reboot timeout (2012-09-25 20:05:04 -0500)
> 
> are available in the git repository at:
> 
>   git://github.com/bonzini/qemu.git blkmirror-job-1.3-part1
> 
> for you to fetch changes up to ed306a929f16fda8a85561430b1ac370bde65097:
> 
>   qemu-iotests: add tests for streaming error handling (2012-09-27 15:11:22 +0200)
> 
> This message has the diff from my post of Sep 25th.  You reviewed 18 patches,
> these are 19 because of the new "qmp: add 'busy' member to BlockJobInfo" patch.
> 
> ----------------------------------------------------------------
> Jeff Cody (1):
>       blockdev: rename block_stream_cb to a generic block_job_cb
> 
> Paolo Bonzini (18):
>       qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE
>       block: fix documentation of block_job_cancel_sync
>       block: move job APIs to separate files
>       block: add block_job_query
>       qmp: add 'busy' member to BlockJobInfo
>       block: add support for job pause/resume
>       qmp: add block-job-pause and block-job-resume
>       qemu-iotests: add test for pausing a streaming operation
>       block: rename block_job_complete to block_job_completed
>       iostatus: rename BlockErrorAction, BlockQMPEventAction
>       iostatus: move BlockdevOnError declaration to QAPI
>       iostatus: change is_read to a bool
>       iostatus: reorganize io error code
>       block: introduce block job error
>       stream: add on-error argument
>       blkdebug: process all set_state rules in the old state
>       qemu-iotests: map underscore to dash in QMP argument names
>       qemu-iotests: add tests for streaming error handling
> 
>  Makefile.objs                 |   5 +-
>  QMP/qmp-events.txt            |  22 ++++
>  block.c                       | 187 ++++++++----------------------
>  block.h                       |  20 ++--
>  block/Makefile.objs           |   3 +-
>  block/blkdebug.c              |  12 +-
>  block/stream.c                |  33 +++++-
>  block_int.h                   | 162 ++------------------------
>  blockdev.c                    |  86 +++++++++-----
>  blockjob.c                    | 249 ++++++++++++++++++++++++++++++++++++++++
>  blockjob.h                    | 243 +++++++++++++++++++++++++++++++++++++++
>  hmp-commands.hx               |  35 +++++-
>  hmp.c                         |  26 ++++-
>  hmp.h                         |   2 +
>  hw/fdc.c                      |   4 +-
>  hw/ide/core.c                 |  22 +---
>  hw/ide/pci.c                  |   4 +-
>  hw/scsi-disk.c                |  25 ++--
>  hw/scsi-generic.c             |   4 +-
>  hw/virtio-blk.c               |  23 ++--
>  monitor.c                     |   1 +
>  monitor.h                     |   1 +
>  qapi-schema.json              |  91 ++++++++++++++-
>  qemu-tool.c                   |   6 +
>  qerror.h                      |   6 +
>  qmp-commands.hx               |  14 ++-
>  tests/qemu-iotests/030        | 260 +++++++++++++++++++++++++++++++++++++++++-
>  tests/qemu-iotests/030.out    |   4 +-
>  tests/qemu-iotests/group      |   2 +-
>  tests/qemu-iotests/iotests.py |  15 ++-
>  trace-events                  |   4 +-
>  31 file modificati, 1152 inserzioni(+), 419 rimozioni(-)
>  create mode 100644 blockjob.c
>  create mode 100644 blockjob.h

Thanks, applied all to the block branch.

Kevin

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1
  2012-09-28 18:23 ` Kevin Wolf
@ 2012-10-01 13:37   ` Paolo Bonzini
  0 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2012-10-01 13:37 UTC (permalink / raw)
  To: qemu-devel, Kevin Wolf

Il 28/09/2012 20:23, Kevin Wolf ha scritto:
> Am 28.09.2012 17:22, schrieb Paolo Bonzini:
>> Kevin,
>>
>> the following changes since commit ac05f3492421caeb05809ffa02c6198ede179e43:
>>
>>   add a boot parameter to set reboot timeout (2012-09-25 20:05:04 -0500)
>>
>> are available in the git repository at:
>>
>>   git://github.com/bonzini/qemu.git blkmirror-job-1.3-part1
>>
>> for you to fetch changes up to ed306a929f16fda8a85561430b1ac370bde65097:
>>
>>   qemu-iotests: add tests for streaming error handling (2012-09-27 15:11:22 +0200)
>>
>> This message has the diff from my post of Sep 25th.  You reviewed 18 patches,
>> these are 19 because of the new "qmp: add 'busy' member to BlockJobInfo" patch.
>>
>> ----------------------------------------------------------------
>> Jeff Cody (1):
>>       blockdev: rename block_stream_cb to a generic block_job_cb
>>
>> Paolo Bonzini (18):
>>       qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE
>>       block: fix documentation of block_job_cancel_sync
>>       block: move job APIs to separate files
>>       block: add block_job_query
>>       qmp: add 'busy' member to BlockJobInfo
>>       block: add support for job pause/resume
>>       qmp: add block-job-pause and block-job-resume
>>       qemu-iotests: add test for pausing a streaming operation
>>       block: rename block_job_complete to block_job_completed
>>       iostatus: rename BlockErrorAction, BlockQMPEventAction
>>       iostatus: move BlockdevOnError declaration to QAPI
>>       iostatus: change is_read to a bool
>>       iostatus: reorganize io error code
>>       block: introduce block job error
>>       stream: add on-error argument
>>       blkdebug: process all set_state rules in the old state
>>       qemu-iotests: map underscore to dash in QMP argument names
>>       qemu-iotests: add tests for streaming error handling
>>
>>  Makefile.objs                 |   5 +-
>>  QMP/qmp-events.txt            |  22 ++++
>>  block.c                       | 187 ++++++++----------------------
>>  block.h                       |  20 ++--
>>  block/Makefile.objs           |   3 +-
>>  block/blkdebug.c              |  12 +-
>>  block/stream.c                |  33 +++++-
>>  block_int.h                   | 162 ++------------------------
>>  blockdev.c                    |  86 +++++++++-----
>>  blockjob.c                    | 249 ++++++++++++++++++++++++++++++++++++++++
>>  blockjob.h                    | 243 +++++++++++++++++++++++++++++++++++++++
>>  hmp-commands.hx               |  35 +++++-
>>  hmp.c                         |  26 ++++-
>>  hmp.h                         |   2 +
>>  hw/fdc.c                      |   4 +-
>>  hw/ide/core.c                 |  22 +---
>>  hw/ide/pci.c                  |   4 +-
>>  hw/scsi-disk.c                |  25 ++--
>>  hw/scsi-generic.c             |   4 +-
>>  hw/virtio-blk.c               |  23 ++--
>>  monitor.c                     |   1 +
>>  monitor.h                     |   1 +
>>  qapi-schema.json              |  91 ++++++++++++++-
>>  qemu-tool.c                   |   6 +
>>  qerror.h                      |   6 +
>>  qmp-commands.hx               |  14 ++-
>>  tests/qemu-iotests/030        | 260 +++++++++++++++++++++++++++++++++++++++++-
>>  tests/qemu-iotests/030.out    |   4 +-
>>  tests/qemu-iotests/group      |   2 +-
>>  tests/qemu-iotests/iotests.py |  15 ++-
>>  trace-events                  |   4 +-
>>  31 file modificati, 1152 inserzioni(+), 419 rimozioni(-)
>>  create mode 100644 blockjob.c
>>  create mode 100644 blockjob.h
> 
> Thanks, applied all to the block branch.

Great, I rebased and only got mostly trivial conflicts.  I pushed the
outcome to the branch blkmirror-job-20120925-rebase of my github
repository.  Do I need to repost, or are you going to continue your
review on the previous patchset?  The next obvious points to step are:

- part 2: "qemu-iotests: add mirroring test case" or "qemu-iotests: add
testcases for mirroring on-source-error/on-target-error"

- part 3: "mirror: perform COW if the cluster size is bigger than the
granularity" or "mirror: allow customizing the granularity"

- part 4: "mirror: support arbitrarily-sized iterations"

Paolo

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2012-10-01 13:37 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-28 15:22 [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 01/19] qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 02/19] blockdev: rename block_stream_cb to a generic block_job_cb Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 03/19] block: fix documentation of block_job_cancel_sync Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 04/19] block: move job APIs to separate files Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 05/19] block: add block_job_query Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 06/19] qmp: add 'busy' member to BlockJobInfo Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 07/19] block: add support for job pause/resume Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 08/19] qmp: add block-job-pause and block-job-resume Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 09/19] qemu-iotests: add test for pausing a streaming operation Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 10/19] block: rename block_job_complete to block_job_completed Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 11/19] iostatus: rename BlockErrorAction, BlockQMPEventAction Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 12/19] iostatus: move BlockdevOnError declaration to QAPI Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 13/19] iostatus: change is_read to a bool Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 14/19] iostatus: reorganize io error code Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 15/19] block: introduce block job error Paolo Bonzini
2012-09-28 15:22 ` [Qemu-devel] [PATCH 16/19] stream: add on-error argument Paolo Bonzini
2012-09-28 15:23 ` [Qemu-devel] [PATCH 17/19] blkdebug: process all set_state rules in the old state Paolo Bonzini
2012-09-28 15:23 ` [Qemu-devel] [PATCH 18/19] qemu-iotests: map underscore to dash in QMP argument names Paolo Bonzini
2012-09-28 15:23 ` [Qemu-devel] [PATCH 19/19] qemu-iotests: add tests for streaming error handling Paolo Bonzini
2012-09-28 15:57 ` [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1 Eric Blake
2012-09-28 18:23 ` Kevin Wolf
2012-10-01 13:37   ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).