[Qemu-devel] [PATCH v4 00/15] block: generic image streaming

[Qemu-devel] [PATCH v4 00/15] block: generic image streaming

[Stefan Hajnoczi]

This series adds the 'block_stream' command which copies the contents of a
backing file into the image file while the VM is running.  The series builds on
the zero detection features which I sent out before Christmas. I suggest
grabbing my git tree to try it out without merging this dependency:

https://github.com/stefanha/qemu/tree/image-streaming-api

The image streaming HMP/QMP commands are documented in the patch and also
described here:

http://wiki.qemu.org/Features/LiveBlockMigration/ImageStreamingAPI

The basic idea is to execute 'block_stream virtio0' while the guest is running.
Progress can be monitored using 'info block-jobs'.  When the streaming
operation completes it raises a QMP event.

Note: The last patch includes includes a Python test script called
test-stream.py, I do not propose to merge it.  When run in a QEMU source tree
it performs basic image streaming QMP tests.

v4:
 * Drop SQMP/EQMP docs from qmp-commands.hx [Luiz]
 * Follow QAPI doc conventions [Luiz]
 * Document QMP events in QMP/qmp-events.txt [Luiz]
 * Protect against hotplug, resize, eject, etc [Kevin]
 * Move block job functions from header to block.c [Kevin]
 * Return error from bdrg_change_backing_file() [Kevin]
 * Merge Marcelo's block_stream base partial streaming series [Marcelo]

Marcelo Tosatti (4):
  block: add bdrv_find_backing_image
  add QERR_BASE_ID_NOT_FOUND
  block stream: add support for partial streaming
  add doc to describe live block operations

Stefan Hajnoczi (11):
  coroutine: add co_sleep_ns() coroutine sleep function
  block: check bdrv_in_use() before blockdev operations
  block: add BlockJob interface for long-running operations
  block: add image streaming block job
  block: rate-limit streaming operations
  qmp: add block_stream command
  qmp: add block_job_set_speed command
  qmp: add block_job_cancel command
  qmp: add query-block-jobs
  blockdev: make image streaming safe across hotplug
  test: add image streaming test cases

 Makefile.objs           |    2 +
 QMP/qmp-events.txt      |   53 ++++++++++++
 block.c                 |  133 ++++++++++++++++++++++++++++++
 block.h                 |    5 +
 block/stream.c          |  192 +++++++++++++++++++++++++++++++++++++++++++
 block_int.h             |   44 ++++++++++
 blockdev.c              |  199 ++++++++++++++++++++++++++++++++++++++++++++-
 docs/live-block-ops.txt |   58 +++++++++++++
 hmp-commands.hx         |   41 +++++++++
 hmp.c                   |   68 +++++++++++++++
 hmp.h                   |    4 +
 monitor.c               |   13 +++
 monitor.h               |    2 +
 qapi-schema.json        |  115 ++++++++++++++++++++++++++
 qemu-coroutine-sleep.c  |   38 +++++++++
 qemu-coroutine.h        |    6 ++
 qerror.c                |    8 ++
 qerror.h                |    6 ++
 qmp-commands.hx         |   24 ++++++
 test-stream.py          |  208 +++++++++++++++++++++++++++++++++++++++++++++++
 trace-events            |    9 ++
 21 files changed, 1227 insertions(+), 1 deletions(-)
 create mode 100644 block/stream.c
 create mode 100644 docs/live-block-ops.txt
 create mode 100644 qemu-coroutine-sleep.c
 create mode 100644 test-stream.py

-- 
1.7.7.3

[Qemu-devel] [PATCH v4 01/15] coroutine: add co_sleep_ns() coroutine sleep function

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 Makefile.objs          |    1 +
 qemu-coroutine-sleep.c |   38 ++++++++++++++++++++++++++++++++++++++
 qemu-coroutine.h       |    6 ++++++
 3 files changed, 45 insertions(+), 0 deletions(-)
 create mode 100644 qemu-coroutine-sleep.c

diff --git a/Makefile.objs b/Makefile.objs
index 47fdc82..64d84de 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -13,6 +13,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
 #######################################################################
 # coroutines
 coroutine-obj-y = qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
+coroutine-obj-y += qemu-coroutine-sleep.o
 ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
 coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
 else
diff --git a/qemu-coroutine-sleep.c b/qemu-coroutine-sleep.c
new file mode 100644
index 0000000..fd65274
--- /dev/null
+++ b/qemu-coroutine-sleep.c
@@ -0,0 +1,38 @@
+/*
+ * QEMU coroutine sleep
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Stefan Hajnoczi    <stefanha@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-coroutine.h"
+#include "qemu-timer.h"
+
+typedef struct CoSleepCB {
+    QEMUTimer *ts;
+    Coroutine *co;
+} CoSleepCB;
+
+static void co_sleep_cb(void *opaque)
+{
+    CoSleepCB *sleep_cb = opaque;
+
+    qemu_free_timer(sleep_cb->ts);
+    qemu_coroutine_enter(sleep_cb->co, NULL);
+}
+
+void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns)
+{
+    CoSleepCB sleep_cb = {
+        .co = qemu_coroutine_self(),
+    };
+    sleep_cb.ts = qemu_new_timer(clock, SCALE_NS, co_sleep_cb, &sleep_cb);
+    qemu_mod_timer(sleep_cb.ts, qemu_get_clock_ns(clock) + ns);
+    qemu_coroutine_yield();
+}
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index 8a55fe1..bae1ffe 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -17,6 +17,7 @@
 
 #include <stdbool.h>
 #include "qemu-queue.h"
+#include "qemu-timer.h"
 
 /**
  * Coroutines are a mechanism for stack switching and can be used for
@@ -199,4 +200,9 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
  */
 void qemu_co_rwlock_unlock(CoRwlock *lock);
 
+/**
+ * Yield the coroutine for a given duration
+ */
+void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns);
+
 #endif /* QEMU_COROUTINE_H */
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH v4 01/15] coroutine: add co_sleep_ns() coroutine sleep function

[Kevin Wolf]

Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  Makefile.objs          |    1 +
>  qemu-coroutine-sleep.c |   38 ++++++++++++++++++++++++++++++++++++++
>  qemu-coroutine.h       |    6 ++++++
>  3 files changed, 45 insertions(+), 0 deletions(-)
>  create mode 100644 qemu-coroutine-sleep.c

> diff --git a/qemu-coroutine.h b/qemu-coroutine.h
> index 8a55fe1..bae1ffe 100644
> --- a/qemu-coroutine.h
> +++ b/qemu-coroutine.h
> @@ -17,6 +17,7 @@
>  
>  #include <stdbool.h>
>  #include "qemu-queue.h"
> +#include "qemu-timer.h"
>  
>  /**
>   * Coroutines are a mechanism for stack switching and can be used for
> @@ -199,4 +200,9 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
>   */
>  void qemu_co_rwlock_unlock(CoRwlock *lock);
>  
> +/**
> + * Yield the coroutine for a given duration
> + */
> +void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns);
> +
>  #endif /* QEMU_COROUTINE_H */

As you mentioned on IRC yesterday, timers don't work in the tools. There
should probably be a warning in the comment (or a fix before this is merged)

Kevin

Re: [Qemu-devel] [PATCH v4 01/15] coroutine: add co_sleep_ns() coroutine sleep function

[Stefan Hajnoczi]

On Thu, Jan 12, 2012 at 10:13 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>> ---
>>  Makefile.objs          |    1 +
>>  qemu-coroutine-sleep.c |   38 ++++++++++++++++++++++++++++++++++++++
>>  qemu-coroutine.h       |    6 ++++++
>>  3 files changed, 45 insertions(+), 0 deletions(-)
>>  create mode 100644 qemu-coroutine-sleep.c
>
>> diff --git a/qemu-coroutine.h b/qemu-coroutine.h
>> index 8a55fe1..bae1ffe 100644
>> --- a/qemu-coroutine.h
>> +++ b/qemu-coroutine.h
>> @@ -17,6 +17,7 @@
>>
>>  #include <stdbool.h>
>>  #include "qemu-queue.h"
>> +#include "qemu-timer.h"
>>
>>  /**
>>   * Coroutines are a mechanism for stack switching and can be used for
>> @@ -199,4 +200,9 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
>>   */
>>  void qemu_co_rwlock_unlock(CoRwlock *lock);
>>
>> +/**
>> + * Yield the coroutine for a given duration
>> + */
>> +void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns);
>> +
>>  #endif /* QEMU_COROUTINE_H */
>
> As you mentioned on IRC yesterday, timers don't work in the tools. There
> should probably be a warning in the comment (or a fix before this is merged)

This is something I'm looking at right now and will probably want to
discuss with Paolo.

In a coroutine we're probably using a main loop and timers should be
available there.  In general, the problem we're starting to see is
that some block layer features are using timers (I/O throttling, QED
deferred dirty bit clearing, image streaming).  The question is do we
isolate this functionality so it is unavailable from a qemu-tool world
when there's no main loop, or do we move everything into a main loop?

Stefan

Re: [Qemu-devel] [PATCH v4 01/15] coroutine: add co_sleep_ns() coroutine sleep function

[Paolo Bonzini]

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

On 01/12/2012 11:58 AM, Stefan Hajnoczi wrote:
> This is something I'm looking at right now and will probably want to
> discuss with Paolo.
>
> In a coroutine we're probably using a main loop and timers should be
> available there.  In general, the problem we're starting to see is
> that some block layer features are using timers (I/O throttling, QED
> deferred dirty bit clearing, image streaming).  The question is do we
> isolate this functionality so it is unavailable from a qemu-tool world
> when there's no main loop, or do we move everything into a main loop?

Yes, I would move things into a main loop just like qemu-nbd does.  I 
even had some prototype patches for it.  The qemu-img part is broken 
because it uses callbacks that haven't been made asynchronous yet; the 
one for qemu-io should be okay modulo some more testing.

Paolo

[-- Attachment #2: tools-co.patch --]
[-- Type: text/x-patch, Size: 8122 bytes --]

>From 9b67defc5eacb9254a8140a4daa52949f0fe60e2 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Mon, 26 Sep 2011 10:59:46 +0200
Subject: [PATCH 1/2] qemu-io: use main_loop_wait

This will let timers run during aio_read and aio_write commands,
though not during synchronous commands.

Not-tested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cmd.c     |   10 +++++-----
 qemu-io.c |    8 +++++---
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/cmd.c b/cmd.c
index 0806e18..7ffbb71 100644
--- a/cmd.c
+++ b/cmd.c
@@ -25,6 +25,7 @@
 
 #include "cmd.h"
 #include "qemu-aio.h"
+#include "main-loop.h"
 
 #define _(x)	x	/* not gettext support yet */
 
@@ -146,7 +147,7 @@ static void prep_fetchline(void *opaque)
 {
     int *fetchable = opaque;
 
-    qemu_aio_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL);
     *fetchable= 1;
 }
 
@@ -193,12 +194,11 @@ void command_loop(void)
         if (!prompted) {
             printf("%s", get_prompt());
             fflush(stdout);
-            qemu_aio_set_fd_handler(STDIN_FILENO, prep_fetchline, NULL, NULL,
-                                    NULL, &fetchable);
+            qemu_set_fd_handler(STDIN_FILENO, prep_fetchline, NULL, &fetchable);
             prompted = 1;
         }
 
-        qemu_aio_wait();
+        main_loop_wait(false);
 
         if (!fetchable) {
             continue;
@@ -221,7 +221,7 @@ void command_loop(void)
         prompted = 0;
         fetchable = 0;
     }
-    qemu_aio_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL);
 }
 
 /* from libxcmd/input.c */
diff --git a/qemu-io.c b/qemu-io.c
index ffa62fb..6e1bee3 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -15,6 +15,7 @@
 #include <libgen.h>
 
 #include "qemu-common.h"
+#include "main-loop.h"
 #include "block_int.h"
 #include "cmd.h"
 
@@ -249,7 +250,7 @@ static int do_aio_readv(QEMUIOVector *qiov, int64_t offset, int *total)
     bdrv_aio_readv(bs, offset >> 9, qiov, qiov->size >> 9,
                    aio_rw_done, &async_ret);
     while (async_ret == NOT_DONE) {
-        qemu_aio_wait();
+        main_loop_wait(false);
     }
 
     *total = qiov->size;
@@ -263,7 +264,7 @@ static int do_aio_writev(QEMUIOVector *qiov, int64_t offset, int *total)
     bdrv_aio_writev(bs, offset >> 9, qiov, qiov->size >> 9,
                     aio_rw_done, &async_ret);
     while (async_ret == NOT_DONE) {
-        qemu_aio_wait();
+        main_loop_wait(false);
     }
 
     *total = qiov->size;
@@ -306,7 +307,7 @@ static int do_aio_multiwrite(BlockRequest* reqs, int num_reqs, int *total)
     }
 
     while (async_ret.num_done < num_reqs) {
-        qemu_aio_wait();
+        main_loop_wait(false);
     }
 
     return async_ret.error < 0 ? async_ret.error : 1;
@@ -1793,6 +1794,7 @@ int main(int argc, char **argv)
         exit(1);
     }
 
+    qemu_init_main_loop();
     bdrv_init();
 
     /* initialize commands */
-- 
1.7.7.1


>From ac2958b0992dcd155ab76885cac044a9fec5cc06 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 18 Oct 2011 12:20:44 +0200
Subject: [PATCH 2/2] qemu-img: run commands in coroutine context

The new function qemu_coroutine_schedule is in qemu-coroutine-lock.c
since it's where the other dependencies on the main loop reside.

Not-tested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 main-loop.h           |   20 ++++++--------------
 qemu-coroutine-lock.c |   24 ++++++++++++++++++++++++
 qemu-coroutine.h      |    8 ++++++++
 qemu-img.c            |   40 ++++++++++++++++++++++++++++++++--------
 4 files changed, 70 insertions(+), 22 deletions(-)

diff --git a/main-loop.h b/main-loop.h
index f971013..4e99485 100644
--- a/main-loop.h
+++ b/main-loop.h
@@ -59,24 +59,16 @@ int qemu_init_main_loop(void);
  * It is sometimes useful to put a whole program in a coroutine.  In this
  * case, the coroutine actually should be started from within the main loop,
  * so that the main loop can run whenever the coroutine yields.  To do this,
- * you can use a bottom half to enter the coroutine as soon as the main loop
- * starts:
+ * you can use qemu_coroutine_schedule.
  *
- *     void enter_co_bh(void *opaque) {
- *         QEMUCoroutine *co = opaque;
- *         qemu_coroutine_enter(co, NULL);
- *     }
- *
- *     ...
- *     QEMUCoroutine *co = qemu_coroutine_create(coroutine_entry);
- *     QEMUBH *start_bh = qemu_bh_new(enter_co_bh, co);
- *     qemu_bh_schedule(start_bh);
- *     while (...) {
+ *     int ret = -1;
+ *     Coroutine *co = qemu_coroutine_create(coroutine_entry);
+ *     qemu_init_main_loop();
+ *     qemu_coroutine_schedule(co, &ret);
+ *     while (ret == -1) {
  *         main_loop_wait(false);
  *     }
  *
- * (In the future we may provide a wrapper for this).
- *
  * @nonblocking: Whether the caller should block until an event occurs.
  */
 int main_loop_wait(int nonblocking);
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 26ad76b..45a2fc2 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -169,3 +169,27 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock)
     }
     lock->writer = true;
 }
+
+typedef struct {
+    QEMUBH *bh;
+    Coroutine *co;
+    void *opaque;
+} CoroutineBHArgs;
+
+static void qemu_coroutine_schedule_bh(void *opaque)
+{
+    CoroutineBHArgs args = *(CoroutineBHArgs *)opaque;
+    g_free(opaque);
+    qemu_bh_delete(args.bh);
+    qemu_coroutine_enter(args.co, args.opaque);
+}
+
+void qemu_coroutine_schedule(Coroutine *co, void *opaque)
+{
+    CoroutineBHArgs *args = g_malloc(sizeof(CoroutineBHArgs));
+    args->bh = qemu_bh_new(qemu_coroutine_schedule_bh, args);
+    args->co = co;
+    args->opaque = opaque;
+    qemu_bh_schedule(args->bh);
+}
+
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index 8a55fe1..8cdef58 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -199,4 +199,12 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
  */
 void qemu_co_rwlock_unlock(CoRwlock *lock);
 
+/**
+ * Transfer control to a coroutine from a bottom half
+ *
+ * The opaque argument is passed as the argument to the entry point when
+ * entering the coroutine for the first time.  It is subsequently ignored.
+ */
+void qemu_coroutine_schedule(Coroutine *coroutine, void *opaque);
+
 #endif /* QEMU_COROUTINE_H */
diff --git a/qemu-img.c b/qemu-img.c
index 01cc0d3..2123476 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -24,6 +24,8 @@
 #include "qemu-common.h"
 #include "qemu-option.h"
 #include "qemu-error.h"
+#include "qemu-coroutine.h"
+#include "main-loop.h"
 #include "osdep.h"
 #include "sysemu.h"
 #include "block_int.h"
@@ -1674,27 +1676,49 @@ static const img_cmd_t img_cmds[] = {
     { NULL, NULL, },
 };
 
-int main(int argc, char **argv)
+typedef struct {
+    int argc;
+    char **argv;
+    int ret;
+} MainArgs;
+
+static void main_co_entry(void *opaque)
 {
+    MainArgs *args = opaque;
     const img_cmd_t *cmd;
     const char *cmdname;
 
-    error_set_progname(argv[0]);
-
-    bdrv_init();
-    if (argc < 2)
+    if (args->argc < 2)
         help();
-    cmdname = argv[1];
-    argc--; argv++;
+    cmdname = args->argv[1];
+    args->argc--; args->argv++;
 
     /* find the command */
     for(cmd = img_cmds; cmd->name != NULL; cmd++) {
         if (!strcmp(cmdname, cmd->name)) {
-            return cmd->handler(argc, argv);
+            args->ret = cmd->handler(args->argc, args->argv);
+            return;
         }
     }
 
     /* not found */
     help();
+    args->ret = 0;
+}
+
+#define NOT_DONE 0x7fffffff
+
+int main(int argc, char **argv)
+{
+    error_set_progname(argv[0]);
+    MainArgs args = { .argc = argc, .argv = argv, .ret = NOT_DONE };
+    Coroutine *co = qemu_coroutine_create(main_co_entry);
+
+    qemu_init_main_loop();
+    bdrv_init();
+    qemu_coroutine_schedule(co, &args);
+    while (args.ret == NOT_DONE) {
+        main_loop_wait(false);
+    }
     return 0;
 }
-- 
1.7.7.1

Re: [Qemu-devel] [PATCH v4 01/15] coroutine: add co_sleep_ns() coroutine sleep function

[Stefan Hajnoczi]

On Thu, Jan 12, 2012 at 10:13 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>> ---
>>  Makefile.objs          |    1 +
>>  qemu-coroutine-sleep.c |   38 ++++++++++++++++++++++++++++++++++++++
>>  qemu-coroutine.h       |    6 ++++++
>>  3 files changed, 45 insertions(+), 0 deletions(-)
>>  create mode 100644 qemu-coroutine-sleep.c
>
>> diff --git a/qemu-coroutine.h b/qemu-coroutine.h
>> index 8a55fe1..bae1ffe 100644
>> --- a/qemu-coroutine.h
>> +++ b/qemu-coroutine.h
>> @@ -17,6 +17,7 @@
>>
>>  #include <stdbool.h>
>>  #include "qemu-queue.h"
>> +#include "qemu-timer.h"
>>
>>  /**
>>   * Coroutines are a mechanism for stack switching and can be used for
>> @@ -199,4 +200,9 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
>>   */
>>  void qemu_co_rwlock_unlock(CoRwlock *lock);
>>
>> +/**
>> + * Yield the coroutine for a given duration
>> + */
>> +void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns);
>> +
>>  #endif /* QEMU_COROUTINE_H */
>
> As you mentioned on IRC yesterday, timers don't work in the tools. There
> should probably be a warning in the comment (or a fix before this is merged)

I will add a comment to prevent new callers using this
inappropriately.  qemu-img/qemu-io do not use image streaming so we
never hit an abort(3).

But I still have an interest in solving the larger problem because QED
broken in qemu-tools!  So I'll look into using the main loop in
qemu-io/qemu-img based on Paolo's patches independently of this image
streaming series.

Stefan

[Qemu-devel] [PATCH v4 02/15] block: check bdrv_in_use() before blockdev operations

[Stefan Hajnoczi]

Long-running block operations like block migration and image streaming
must have continual access to their block device.  It is not safe to
perform operations like hotplug, eject, change, resize, commit, or
external snapshot while a long-running operation is in progress.

This patch adds the missing bdrv_in_use() checks so that block migration
and image streaming never have the rug pulled out from underneath them.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c    |    4 ++++
 blockdev.c |   16 +++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index 967a583..daf92c2 100644
--- a/block.c
+++ b/block.c
@@ -1020,6 +1020,10 @@ int bdrv_commit(BlockDriverState *bs)
         return -EACCES;
     }
 
+    if (bdrv_in_use(bs) || bdrv_in_use(bs->backing_hd)) {
+        return -EBUSY;
+    }
+
     backing_drv = bs->backing_hd->drv;
     ro = bs->backing_hd->read_only;
     strncpy(filename, bs->backing_hd->filename, sizeof(filename));
diff --git a/blockdev.c b/blockdev.c
index c832782..6d78b36 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -592,12 +592,18 @@ void do_commit(Monitor *mon, const QDict *qdict)
     if (!strcmp(device, "all")) {
         bdrv_commit_all();
     } else {
+        int ret;
+
         bs = bdrv_find(device);
         if (!bs) {
             qerror_report(QERR_DEVICE_NOT_FOUND, device);
             return;
         }
-        bdrv_commit(bs);
+        ret = bdrv_commit(bs);
+        if (ret == -EBUSY) {
+            qerror_report(QERR_DEVICE_IN_USE, device);
+            return;
+        }
     }
 }
 
@@ -616,6 +622,10 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return;
     }
+    if (bdrv_in_use(bs)) {
+        error_set(errp, QERR_DEVICE_IN_USE, device);
+        return;
+    }
 
     pstrcpy(old_filename, sizeof(old_filename), bs->filename);
 
@@ -667,6 +677,10 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
 
 static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
 {
+    if (bdrv_in_use(bs)) {
+        qerror_report(QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
+        return -1;
+    }
     if (!bdrv_dev_has_removable_media(bs)) {
         qerror_report(QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
         return -1;
-- 
1.7.7.3

[Qemu-devel] [PATCH v4 03/15] block: add BlockJob interface for long-running operations

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c     |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 block_int.h |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index daf92c2..5bfaa3a 100644
--- a/block.c
+++ b/block.c
@@ -3877,3 +3877,51 @@ out:
 
     return ret;
 }
+
+void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
+                       BlockDriverCompletionFunc *cb, void *opaque)
+{
+    BlockJob *job;
+
+    if (bdrv_in_use(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;
+    bs->job = job;
+    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);
+}
+
+int block_job_set_speed(BlockJob *job, int64_t value)
+{
+    if (!job->job_type->set_speed) {
+        return -ENOTSUP;
+    }
+    return job->job_type->set_speed(job, value);
+}
+
+void block_job_cancel(BlockJob *job)
+{
+    job->cancelled = true;
+}
+
+bool block_job_is_cancelled(BlockJob *job)
+{
+    return job->cancelled;
+}
diff --git a/block_int.h b/block_int.h
index 5362180..316443e 100644
--- a/block_int.h
+++ b/block_int.h
@@ -69,6 +69,36 @@ typedef struct BlockIOBaseValue {
     uint64_t ios[2];
 } BlockIOBaseValue;
 
+typedef void BlockJobCancelFunc(void *opaque);
+typedef struct BlockJob BlockJob;
+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 */
+    int (*set_speed)(BlockJob *job, int64_t value);
+} BlockJobType;
+
+/**
+ * Long-running operation on a BlockDriverState
+ */
+struct BlockJob {
+    const BlockJobType *job_type;
+    BlockDriverState *bs;
+    bool cancelled;
+
+    /* These fields are published by the query-block-jobs QMP API */
+    int64_t offset;
+    int64_t len;
+    int64_t speed;
+
+    BlockDriverCompletionFunc *cb;
+    void *opaque;
+};
+
 struct BlockDriver {
     const char *format_name;
     int instance_size;
@@ -269,6 +299,9 @@ struct BlockDriverState {
     void *private;
 
     QLIST_HEAD(, BdrvTrackedRequest) tracked_requests;
+
+    /* long-running background operation */
+    BlockJob *job;
 };
 
 struct BlockDriverAIOCB {
@@ -292,4 +325,11 @@ void bdrv_set_io_limits(BlockDriverState *bs,
 int is_windows_drive(const char *filename);
 #endif
 
+void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
+                       BlockDriverCompletionFunc *cb, void *opaque);
+void block_job_complete(BlockJob *job, int ret);
+int block_job_set_speed(BlockJob *job, int64_t value);
+void block_job_cancel(BlockJob *job);
+bool block_job_is_cancelled(BlockJob *job);
+
 #endif /* BLOCK_INT_H */
-- 
1.7.7.3

[Qemu-devel] [PATCH v4 04/15] block: add image streaming block job

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 Makefile.objs  |    1 +
 block/stream.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block_int.h    |    3 +
 trace-events   |    4 ++
 4 files changed, 127 insertions(+), 0 deletions(-)
 create mode 100644 block/stream.c

diff --git a/Makefile.objs b/Makefile.objs
index 64d84de..fde3769 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -35,6 +35,7 @@ block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow
 block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-nested-y += qed-check.o
 block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
+block-nested-y += stream.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
 block-nested-$(CONFIG_LIBISCSI) += iscsi.o
diff --git a/block/stream.c b/block/stream.c
new file mode 100644
index 0000000..8ff98cf
--- /dev/null
+++ b/block/stream.c
@@ -0,0 +1,119 @@
+/*
+ * Image streaming
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "trace.h"
+#include "block_int.h"
+
+enum {
+    /*
+     * Size of data buffer for populating the image file.  This should be large
+     * enough to process multiple clusters in a single call, so that populating
+     * contiguous regions of the image is efficient.
+     */
+    STREAM_BUFFER_SIZE = 512 * 1024, /* in bytes */
+};
+
+typedef struct StreamBlockJob {
+    BlockJob common;
+    BlockDriverState *base;
+} StreamBlockJob;
+
+static int coroutine_fn stream_populate(BlockDriverState *bs,
+                                        int64_t sector_num, int nb_sectors,
+                                        void *buf)
+{
+    struct iovec iov = {
+        .iov_base = buf,
+        .iov_len  = nb_sectors * BDRV_SECTOR_SIZE,
+    };
+    QEMUIOVector qiov;
+
+    qemu_iovec_init_external(&qiov, &iov, 1);
+
+    /* Copy-on-read the unallocated clusters */
+    return bdrv_co_readv(bs, sector_num, nb_sectors, &qiov);
+}
+
+static void coroutine_fn stream_run(void *opaque)
+{
+    StreamBlockJob *s = opaque;
+    BlockDriverState *bs = s->common.bs;
+    int64_t sector_num, end;
+    int ret = 0;
+    int n;
+    void *buf;
+
+    buf = qemu_blockalign(bs, STREAM_BUFFER_SIZE);
+    s->common.len = bdrv_getlength(bs);
+    bdrv_get_geometry(bs, (uint64_t *)&end);
+
+    bdrv_enable_copy_on_read(bs);
+
+    for (sector_num = 0; sector_num < end; sector_num += n) {
+        if (block_job_is_cancelled(&s->common)) {
+            break;
+        }
+
+        /* TODO rate-limit */
+        /* Note that even when no rate limit is applied we need to yield with
+         * no pending I/O here so that qemu_aio_flush() is able to return.
+         */
+        co_sleep_ns(rt_clock, 0);
+
+        ret = bdrv_co_is_allocated(bs, sector_num,
+                                   STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
+        trace_stream_one_iteration(s, sector_num, n, ret);
+        if (ret == 0) {
+            ret = stream_populate(bs, sector_num, n, buf);
+        }
+        if (ret < 0) {
+            break;
+        }
+
+        /* Publish progress */
+        s->common.offset += n * BDRV_SECTOR_SIZE;
+    }
+
+    bdrv_disable_copy_on_read(bs);
+
+    if (sector_num == end && ret == 0) {
+        ret = bdrv_change_backing_file(bs, NULL, NULL);
+    }
+
+    qemu_vfree(buf);
+    block_job_complete(&s->common, ret);
+}
+
+static BlockJobType stream_job_type = {
+    .instance_size = sizeof(StreamBlockJob),
+    .job_type      = "stream",
+};
+
+int stream_start(BlockDriverState *bs, BlockDriverState *base,
+                 BlockDriverCompletionFunc *cb, void *opaque)
+{
+    StreamBlockJob *s;
+    Coroutine *co;
+
+    if (bs->job) {
+        return -EBUSY;
+    }
+
+    s = block_job_create(&stream_job_type, bs, cb, opaque);
+    s->base = base;
+
+    co = qemu_coroutine_create(stream_run);
+    trace_stream_start(bs, base, s, co, opaque);
+    qemu_coroutine_enter(co, s);
+    return 0;
+}
diff --git a/block_int.h b/block_int.h
index 316443e..c7c9178 100644
--- a/block_int.h
+++ b/block_int.h
@@ -332,4 +332,7 @@ int block_job_set_speed(BlockJob *job, int64_t value);
 void block_job_cancel(BlockJob *job);
 bool block_job_is_cancelled(BlockJob *job);
 
+int stream_start(BlockDriverState *bs, BlockDriverState *base,
+                 BlockDriverCompletionFunc *cb, void *opaque);
+
 #endif /* BLOCK_INT_H */
diff --git a/trace-events b/trace-events
index 360f039..c5368fa 100644
--- a/trace-events
+++ b/trace-events
@@ -70,6 +70,10 @@ bdrv_co_write_zeroes(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_
 bdrv_co_io_em(void *bs, int64_t sector_num, int nb_sectors, int is_write, void *acb) "bs %p sector_num %"PRId64" nb_sectors %d is_write %d acb %p"
 bdrv_co_copy_on_readv(void *bs, int64_t sector_num, int nb_sectors, int64_t cluster_sector_num, int cluster_nb_sectors) "bs %p sector_num %"PRId64" nb_sectors %d cluster_sector_num %"PRId64" cluster_nb_sectors %d"
 
+# block/stream.c
+stream_one_iteration(void *s, int64_t sector_num, int nb_sectors, int is_allocated) "s %p sector_num %"PRId64" nb_sectors %d is_allocated %d"
+stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs %p base %p s %p co %p opaque %p"
+
 # hw/virtio-blk.c
 virtio_blk_req_complete(void *req, int status) "req %p status %d"
 virtio_blk_rw_complete(void *req, int ret) "req %p ret %d"
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH v4 04/15] block: add image streaming block job

[Luiz Capitulino]

On Fri,  6 Jan 2012 14:01:30 +0000
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote:

> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  Makefile.objs  |    1 +
>  block/stream.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block_int.h    |    3 +
>  trace-events   |    4 ++
>  4 files changed, 127 insertions(+), 0 deletions(-)
>  create mode 100644 block/stream.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 64d84de..fde3769 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -35,6 +35,7 @@ block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow
>  block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>  block-nested-y += qed-check.o
>  block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
> +block-nested-y += stream.o
>  block-nested-$(CONFIG_WIN32) += raw-win32.o
>  block-nested-$(CONFIG_POSIX) += raw-posix.o
>  block-nested-$(CONFIG_LIBISCSI) += iscsi.o
> diff --git a/block/stream.c b/block/stream.c
> new file mode 100644
> index 0000000..8ff98cf
> --- /dev/null
> +++ b/block/stream.c
> @@ -0,0 +1,119 @@
> +/*
> + * Image streaming
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "trace.h"
> +#include "block_int.h"
> +
> +enum {
> +    /*
> +     * Size of data buffer for populating the image file.  This should be large
> +     * enough to process multiple clusters in a single call, so that populating
> +     * contiguous regions of the image is efficient.
> +     */
> +    STREAM_BUFFER_SIZE = 512 * 1024, /* in bytes */
> +};
> +
> +typedef struct StreamBlockJob {
> +    BlockJob common;
> +    BlockDriverState *base;
> +} StreamBlockJob;
> +
> +static int coroutine_fn stream_populate(BlockDriverState *bs,
> +                                        int64_t sector_num, int nb_sectors,
> +                                        void *buf)
> +{
> +    struct iovec iov = {
> +        .iov_base = buf,
> +        .iov_len  = nb_sectors * BDRV_SECTOR_SIZE,
> +    };
> +    QEMUIOVector qiov;
> +
> +    qemu_iovec_init_external(&qiov, &iov, 1);
> +
> +    /* Copy-on-read the unallocated clusters */
> +    return bdrv_co_readv(bs, sector_num, nb_sectors, &qiov);
> +}
> +
> +static void coroutine_fn stream_run(void *opaque)
> +{
> +    StreamBlockJob *s = opaque;
> +    BlockDriverState *bs = s->common.bs;
> +    int64_t sector_num, end;
> +    int ret = 0;
> +    int n;
> +    void *buf;
> +
> +    buf = qemu_blockalign(bs, STREAM_BUFFER_SIZE);
> +    s->common.len = bdrv_getlength(bs);
> +    bdrv_get_geometry(bs, (uint64_t *)&end);
> +
> +    bdrv_enable_copy_on_read(bs);
> +
> +    for (sector_num = 0; sector_num < end; sector_num += n) {
> +        if (block_job_is_cancelled(&s->common)) {
> +            break;
> +        }
> +
> +        /* TODO rate-limit */
> +        /* Note that even when no rate limit is applied we need to yield with
> +         * no pending I/O here so that qemu_aio_flush() is able to return.
> +         */
> +        co_sleep_ns(rt_clock, 0);
> +
> +        ret = bdrv_co_is_allocated(bs, sector_num,
> +                                   STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
> +        trace_stream_one_iteration(s, sector_num, n, ret);
> +        if (ret == 0) {
> +            ret = stream_populate(bs, sector_num, n, buf);
> +        }
> +        if (ret < 0) {
> +            break;
> +        }
> +
> +        /* Publish progress */
> +        s->common.offset += n * BDRV_SECTOR_SIZE;
> +    }
> +
> +    bdrv_disable_copy_on_read(bs);
> +
> +    if (sector_num == end && ret == 0) {
> +        ret = bdrv_change_backing_file(bs, NULL, NULL);
> +    }
> +
> +    qemu_vfree(buf);
> +    block_job_complete(&s->common, ret);
> +}
> +
> +static BlockJobType stream_job_type = {
> +    .instance_size = sizeof(StreamBlockJob),
> +    .job_type      = "stream",
> +};
> +
> +int stream_start(BlockDriverState *bs, BlockDriverState *base,
> +                 BlockDriverCompletionFunc *cb, void *opaque)
> +{
> +    StreamBlockJob *s;
> +    Coroutine *co;
> +
> +    if (bs->job) {
> +        return -EBUSY;
> +    }
> +
> +    s = block_job_create(&stream_job_type, bs, cb, opaque);
> +    s->base = base;

This is missing a check against NULL.

> +
> +    co = qemu_coroutine_create(stream_run);
> +    trace_stream_start(bs, base, s, co, opaque);
> +    qemu_coroutine_enter(co, s);
> +    return 0;
> +}
> diff --git a/block_int.h b/block_int.h
> index 316443e..c7c9178 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -332,4 +332,7 @@ int block_job_set_speed(BlockJob *job, int64_t value);
>  void block_job_cancel(BlockJob *job);
>  bool block_job_is_cancelled(BlockJob *job);
>  
> +int stream_start(BlockDriverState *bs, BlockDriverState *base,
> +                 BlockDriverCompletionFunc *cb, void *opaque);
> +
>  #endif /* BLOCK_INT_H */
> diff --git a/trace-events b/trace-events
> index 360f039..c5368fa 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -70,6 +70,10 @@ bdrv_co_write_zeroes(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_
>  bdrv_co_io_em(void *bs, int64_t sector_num, int nb_sectors, int is_write, void *acb) "bs %p sector_num %"PRId64" nb_sectors %d is_write %d acb %p"
>  bdrv_co_copy_on_readv(void *bs, int64_t sector_num, int nb_sectors, int64_t cluster_sector_num, int cluster_nb_sectors) "bs %p sector_num %"PRId64" nb_sectors %d cluster_sector_num %"PRId64" cluster_nb_sectors %d"
>  
> +# block/stream.c
> +stream_one_iteration(void *s, int64_t sector_num, int nb_sectors, int is_allocated) "s %p sector_num %"PRId64" nb_sectors %d is_allocated %d"
> +stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs %p base %p s %p co %p opaque %p"
> +
>  # hw/virtio-blk.c
>  virtio_blk_req_complete(void *req, int status) "req %p status %d"
>  virtio_blk_rw_complete(void *req, int ret) "req %p ret %d"

Re: [Qemu-devel] [PATCH v4 04/15] block: add image streaming block job

[Stefan Hajnoczi]

On Wed, Jan 11, 2012 at 03:18:28PM -0200, Luiz Capitulino wrote:
> On Fri,  6 Jan 2012 14:01:30 +0000
> Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote:
> > +int stream_start(BlockDriverState *bs, BlockDriverState *base,
> > +                 BlockDriverCompletionFunc *cb, void *opaque)
> > +{
> > +    StreamBlockJob *s;
> > +    Coroutine *co;
> > +
> > +    if (bs->job) {
> > +        return -EBUSY;
> > +    }
> > +
> > +    s = block_job_create(&stream_job_type, bs, cb, opaque);
> > +    s->base = base;
> 
> This is missing a check against NULL.

Good catch.

Re: [Qemu-devel] [PATCH v4 04/15] block: add image streaming block job

[Kevin Wolf]

Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  Makefile.objs  |    1 +
>  block/stream.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block_int.h    |    3 +
>  trace-events   |    4 ++
>  4 files changed, 127 insertions(+), 0 deletions(-)
>  create mode 100644 block/stream.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 64d84de..fde3769 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -35,6 +35,7 @@ block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow
>  block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>  block-nested-y += qed-check.o
>  block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
> +block-nested-y += stream.o
>  block-nested-$(CONFIG_WIN32) += raw-win32.o
>  block-nested-$(CONFIG_POSIX) += raw-posix.o
>  block-nested-$(CONFIG_LIBISCSI) += iscsi.o
> diff --git a/block/stream.c b/block/stream.c
> new file mode 100644
> index 0000000..8ff98cf
> --- /dev/null
> +++ b/block/stream.c
> @@ -0,0 +1,119 @@
> +/*
> + * Image streaming
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "trace.h"
> +#include "block_int.h"
> +
> +enum {
> +    /*
> +     * Size of data buffer for populating the image file.  This should be large
> +     * enough to process multiple clusters in a single call, so that populating
> +     * contiguous regions of the image is efficient.
> +     */
> +    STREAM_BUFFER_SIZE = 512 * 1024, /* in bytes */
> +};
> +
> +typedef struct StreamBlockJob {
> +    BlockJob common;
> +    BlockDriverState *base;
> +} StreamBlockJob;
> +
> +static int coroutine_fn stream_populate(BlockDriverState *bs,
> +                                        int64_t sector_num, int nb_sectors,
> +                                        void *buf)
> +{
> +    struct iovec iov = {
> +        .iov_base = buf,
> +        .iov_len  = nb_sectors * BDRV_SECTOR_SIZE,
> +    };
> +    QEMUIOVector qiov;
> +
> +    qemu_iovec_init_external(&qiov, &iov, 1);
> +
> +    /* Copy-on-read the unallocated clusters */
> +    return bdrv_co_readv(bs, sector_num, nb_sectors, &qiov);
> +}
> +
> +static void coroutine_fn stream_run(void *opaque)
> +{
> +    StreamBlockJob *s = opaque;
> +    BlockDriverState *bs = s->common.bs;
> +    int64_t sector_num, end;
> +    int ret = 0;
> +    int n;
> +    void *buf;
> +
> +    buf = qemu_blockalign(bs, STREAM_BUFFER_SIZE);
> +    s->common.len = bdrv_getlength(bs);

No error check?

> +    bdrv_get_geometry(bs, (uint64_t *)&end);

Why call bdrv_getlength() twice? end = s->common.len >> BDRV_SECTOR_BITS
should be the same.

Kevin

Re: [Qemu-devel] [PATCH v4 04/15] block: add image streaming block job

[Stefan Hajnoczi]

On Thu, Jan 12, 2012 at 10:59 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
>> +    buf = qemu_blockalign(bs, STREAM_BUFFER_SIZE);
>> +    s->common.len = bdrv_getlength(bs);
>
> No error check?

Will fix.

>> +    bdrv_get_geometry(bs, (uint64_t *)&end);
>
> Why call bdrv_getlength() twice? end = s->common.len >> BDRV_SECTOR_BITS
> should be the same.

Okay, I'll change it.  I got sick of BDRV_SECTOR_* and called twice instead.

Re: [Qemu-devel] [PATCH v4 04/15] block: add image streaming block job

[Kevin Wolf]

Am 12.01.2012 12:39, schrieb Stefan Hajnoczi:
> On Thu, Jan 12, 2012 at 10:59 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
>>> +    buf = qemu_blockalign(bs, STREAM_BUFFER_SIZE);
>>> +    s->common.len = bdrv_getlength(bs);
>>
>> No error check?
> 
> Will fix.
> 
>>> +    bdrv_get_geometry(bs, (uint64_t *)&end);
>>
>> Why call bdrv_getlength() twice? end = s->common.len >> BDRV_SECTOR_BITS
>> should be the same.
> 
> Okay, I'll change it.  I got sick of BDRV_SECTOR_* and called twice instead.

Well, you can try and change everything in the streaming code to bytes
instead of sectors. We should probably do this sooner or later anyway.
Sectors of 512 bytes are a completely arbitrary unit that doesn't make
much sense generally.

Kevin

Re: [Qemu-devel] [PATCH v4 04/15] block: add image streaming block job

[Stefan Hajnoczi]

On Thu, Jan 12, 2012 at 12:53 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 12.01.2012 12:39, schrieb Stefan Hajnoczi:
>> On Thu, Jan 12, 2012 at 10:59 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
>>>> +    buf = qemu_blockalign(bs, STREAM_BUFFER_SIZE);
>>>> +    s->common.len = bdrv_getlength(bs);
>>>
>>> No error check?
>>
>> Will fix.
>>
>>>> +    bdrv_get_geometry(bs, (uint64_t *)&end);
>>>
>>> Why call bdrv_getlength() twice? end = s->common.len >> BDRV_SECTOR_BITS
>>> should be the same.
>>
>> Okay, I'll change it.  I got sick of BDRV_SECTOR_* and called twice instead.
>
> Well, you can try and change everything in the streaming code to bytes
> instead of sectors. We should probably do this sooner or later anyway.
> Sectors of 512 bytes are a completely arbitrary unit that doesn't make
> much sense generally.

That doesn't work because block layer interfaces use nb_sectors.  We
still need to convert.

Stefan

Re: [Qemu-devel] [PATCH v4 04/15] block: add image streaming block job

[Kevin Wolf]

Am 12.01.2012 14:05, schrieb Stefan Hajnoczi:
> On Thu, Jan 12, 2012 at 12:53 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 12.01.2012 12:39, schrieb Stefan Hajnoczi:
>>> On Thu, Jan 12, 2012 at 10:59 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>>>> Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
>>>>> +    buf = qemu_blockalign(bs, STREAM_BUFFER_SIZE);
>>>>> +    s->common.len = bdrv_getlength(bs);
>>>>
>>>> No error check?
>>>
>>> Will fix.
>>>
>>>>> +    bdrv_get_geometry(bs, (uint64_t *)&end);
>>>>
>>>> Why call bdrv_getlength() twice? end = s->common.len >> BDRV_SECTOR_BITS
>>>> should be the same.
>>>
>>> Okay, I'll change it.  I got sick of BDRV_SECTOR_* and called twice instead.
>>
>> Well, you can try and change everything in the streaming code to bytes
>> instead of sectors. We should probably do this sooner or later anyway.
>> Sectors of 512 bytes are a completely arbitrary unit that doesn't make
>> much sense generally.
> 
> That doesn't work because block layer interfaces use nb_sectors.  We
> still need to convert.

Sure, somewhere you'll have the conversion. You can only push it a bit
closer to the invocation of the block drivers if you like. Everything
else would be a major refactoring (but eventually I think we'll do it).

Kevin

[Qemu-devel] [PATCH v4 05/15] block: rate-limit streaming operations

[Stefan Hajnoczi]

This patch implements rate-limiting for image streaming.  If we've
exceeded the bandwidth quota for a 100 ms time slice we sleep the
coroutine until the next slice begins.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/stream.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/block/stream.c b/block/stream.c
index 8ff98cf..5d5d672 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -23,8 +23,39 @@ enum {
     STREAM_BUFFER_SIZE = 512 * 1024, /* in bytes */
 };
 
+#define SLICE_TIME 100000000ULL /* ns */
+
+typedef struct {
+    int64_t next_slice_time;
+    uint64_t slice_quota;
+    uint64_t dispatched;
+} RateLimit;
+
+static int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
+{
+    int64_t delay_ns = 0;
+    int64_t now = qemu_get_clock_ns(rt_clock);
+
+    if (limit->next_slice_time < now) {
+        limit->next_slice_time = now + SLICE_TIME;
+        limit->dispatched = 0;
+    }
+    if (limit->dispatched + n > limit->slice_quota) {
+        delay_ns = limit->next_slice_time - now;
+    } else {
+        limit->dispatched += n;
+    }
+    return delay_ns;
+}
+
+static void ratelimit_set_speed(RateLimit *limit, uint64_t speed)
+{
+    limit->slice_quota = speed / (1000000000ULL / SLICE_TIME);
+}
+
 typedef struct StreamBlockJob {
     BlockJob common;
+    RateLimit limit;
     BlockDriverState *base;
 } StreamBlockJob;
 
@@ -60,20 +91,24 @@ static void coroutine_fn stream_run(void *opaque)
     bdrv_enable_copy_on_read(bs);
 
     for (sector_num = 0; sector_num < end; sector_num += n) {
+retry:
         if (block_job_is_cancelled(&s->common)) {
             break;
         }
 
-        /* TODO rate-limit */
-        /* Note that even when no rate limit is applied we need to yield with
-         * no pending I/O here so that qemu_aio_flush() is able to return.
-         */
-        co_sleep_ns(rt_clock, 0);
-
         ret = bdrv_co_is_allocated(bs, sector_num,
                                    STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
         trace_stream_one_iteration(s, sector_num, n, ret);
         if (ret == 0) {
+            if (s->common.speed) {
+                uint64_t delay_ns = ratelimit_calculate_delay(&s->limit, n);
+                if (delay_ns > 0) {
+                    co_sleep_ns(rt_clock, delay_ns);
+
+                    /* Recheck cancellation and that sectors are unallocated */
+                    goto retry;
+                }
+            }
             ret = stream_populate(bs, sector_num, n, buf);
         }
         if (ret < 0) {
@@ -82,6 +117,11 @@ static void coroutine_fn stream_run(void *opaque)
 
         /* Publish progress */
         s->common.offset += n * BDRV_SECTOR_SIZE;
+
+        /* Note that even when no rate limit is applied we need to yield
+         * with no pending I/O here so that qemu_aio_flush() returns.
+         */
+        co_sleep_ns(rt_clock, 0);
     }
 
     bdrv_disable_copy_on_read(bs);
@@ -94,9 +134,22 @@ static void coroutine_fn stream_run(void *opaque)
     block_job_complete(&s->common, ret);
 }
 
+static int stream_set_speed(BlockJob *job, int64_t value)
+{
+    StreamBlockJob *s = container_of(job, StreamBlockJob, common);
+
+    if (value < 0) {
+        return -EINVAL;
+    }
+    job->speed = value;
+    ratelimit_set_speed(&s->limit, value / BDRV_SECTOR_SIZE);
+    return 0;
+}
+
 static BlockJobType stream_job_type = {
     .instance_size = sizeof(StreamBlockJob),
     .job_type      = "stream",
+    .set_speed     = stream_set_speed,
 };
 
 int stream_start(BlockDriverState *bs, BlockDriverState *base,
-- 
1.7.7.3

[Qemu-devel] [PATCH v4 06/15] qmp: add block_stream command

[Stefan Hajnoczi]

Add the block_stream command, which starts copy backing file contents
into the image file.  Also add the BLOCK_JOB_COMPLETED QMP event which
is emitted when image streaming completes.  Later patches add control
over the background copy speed, cancelation, and querying running
streaming operations.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 QMP/qmp-events.txt |   29 ++++++++++++++++++++++
 blockdev.c         |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 hmp-commands.hx    |   13 ++++++++++
 hmp.c              |   11 ++++++++
 hmp.h              |    1 +
 monitor.c          |    3 ++
 monitor.h          |    1 +
 qapi-schema.json   |   32 ++++++++++++++++++++++++
 qerror.c           |    4 +++
 qerror.h           |    3 ++
 qmp-commands.hx    |    6 ++++
 trace-events       |    4 +++
 12 files changed, 174 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index af586ec..a80e604 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -264,3 +264,32 @@ Example:
 
 Note: If action is "reset", "shutdown", or "pause" the WATCHDOG event is
 followed respectively by the RESET, SHUTDOWN, or STOP events.
+
+
+BLOCK_JOB_COMPLETED
+-------------------
+
+Emitted when a block job has completed.
+
+Data:
+
+- "type":     Job type ("stream" for image streaming, json-string)
+- "device":   Device name (json-string)
+- "len":      Maximum progress value (json-int)
+- "offset":   Current progress value (json-int)
+              On success this is equal to len.
+              On failure this is less than len.
+- "speed":    Rate limit, bytes per second (json-int)
+- "error":    Error message (json-string)
+              Only present on failure.  This field contains a human-readable
+              error message.  There are no semantics other than that streaming
+              has failed and clients should not try to interpret the error
+              string.
+
+Example:
+
+{ "event": "BLOCK_JOB_COMPLETED",
+     "data": { "type": "stream", "device": "virtio-disk0",
+               "len": 10737418240, "offset": 10737418240,
+               "speed": 0 },
+     "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
diff --git a/blockdev.c b/blockdev.c
index 6d78b36..ba973b0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -13,9 +13,11 @@
 #include "qerror.h"
 #include "qemu-option.h"
 #include "qemu-config.h"
+#include "qemu-objects.h"
 #include "sysemu.h"
 #include "block_int.h"
 #include "qmp-commands.h"
+#include "trace.h"
 
 static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
 
@@ -880,3 +882,68 @@ void qmp_block_resize(const char *device, int64_t size, Error **errp)
         return;
     }
 }
+
+static QObject *qobject_from_block_job(BlockJob *job)
+{
+    return qobject_from_jsonf("{ 'type': %s,"
+                              "'device': %s,"
+                              "'len': %" PRId64 ","
+                              "'offset': %" PRId64 ","
+                              "'speed': %" PRId64 " }",
+                              job->job_type->job_type,
+                              bdrv_get_device_name(job->bs),
+                              job->len,
+                              job->offset,
+                              job->speed);
+}
+
+static void block_stream_cb(void *opaque, int ret)
+{
+    BlockDriverState *bs = opaque;
+    QObject *obj;
+
+    trace_block_stream_cb(bs, bs->job, ret);
+
+    assert(bs->job);
+    obj = qobject_from_block_job(bs->job);
+    if (ret < 0) {
+        QDict *dict = qobject_to_qdict(obj);
+        qdict_put(dict, "error", qstring_from_str(strerror(-ret)));
+    }
+
+    monitor_protocol_event(QEVENT_BLOCK_JOB_COMPLETED, obj);
+    qobject_decref(obj);
+}
+
+void qmp_block_stream(const char *device, bool has_base,
+                      const char *base, Error **errp)
+{
+    BlockDriverState *bs;
+    int ret;
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    /* Base device not supported */
+    if (base) {
+        error_set(errp, QERR_NOT_SUPPORTED);
+        return;
+    }
+
+    ret = stream_start(bs, NULL, block_stream_cb, bs);
+    if (ret < 0) {
+        switch (ret) {
+        case -EBUSY:
+            error_set(errp, QERR_DEVICE_IN_USE, device);
+            return;
+        default:
+            error_set(errp, QERR_NOT_SUPPORTED);
+            return;
+        }
+    }
+
+    trace_qmp_block_stream(bs, bs->job);
+}
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 14838b7..8d9dbd6 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -69,6 +69,19 @@ but should be used with extreme caution.  Note that this command only
 resizes image files, it can not resize block devices like LVM volumes.
 ETEXI
 
+    {
+        .name       = "block_stream",
+        .args_type  = "device:B,base:s?",
+        .params     = "device [base]",
+        .help       = "copy data from a backing file into a block device",
+        .mhandler.cmd = hmp_block_stream,
+    },
+
+STEXI
+@item block_stream
+@findex block_stream
+Copy data from a backing file into a block device.
+ETEXI
 
     {
         .name       = "eject",
diff --git a/hmp.c b/hmp.c
index e7659d5..b6e5913 100644
--- a/hmp.c
+++ b/hmp.c
@@ -679,3 +679,14 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
     int64_t value = qdict_get_int(qdict, "value");
     qmp_migrate_set_speed(value, NULL);
 }
+
+void hmp_block_stream(Monitor *mon, const QDict *qdict)
+{
+    Error *error = NULL;
+    const char *device = qdict_get_str(qdict, "device");
+    const char *base = qdict_get_try_str(qdict, "base");
+
+    qmp_block_stream(device, base != NULL, base, &error);
+
+    hmp_handle_error(mon, &error);
+}
diff --git a/hmp.h b/hmp.h
index 093242d..b55c295 100644
--- a/hmp.h
+++ b/hmp.h
@@ -49,5 +49,6 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
+void hmp_block_stream(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/monitor.c b/monitor.c
index 7334401..bb42580 100644
--- a/monitor.c
+++ b/monitor.c
@@ -479,6 +479,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_SPICE_DISCONNECTED:
             event_name = "SPICE_DISCONNECTED";
             break;
+        case QEVENT_BLOCK_JOB_COMPLETED:
+            event_name = "BLOCK_JOB_COMPLETED";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index cfa2f67..7324236 100644
--- a/monitor.h
+++ b/monitor.h
@@ -35,6 +35,7 @@ typedef enum MonitorEvent {
     QEVENT_SPICE_CONNECTED,
     QEVENT_SPICE_INITIALIZED,
     QEVENT_SPICE_DISCONNECTED,
+    QEVENT_BLOCK_JOB_COMPLETED,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 44cf764..2b1cc8c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1275,3 +1275,35 @@
 { 'command': 'qom-set',
   'data': { 'path': 'str', 'property': 'str', 'value': 'visitor' },
   'gen': 'no' }
+
+##
+# @block_stream:
+#
+# Copy data from a backing file into a block device.
+#
+# The block streaming operation is performed in the background until the entire
+# backing file has been copied.  This command returns immediately once streaming
+# has started.  The status of ongoing block streaming operations can be checked
+# with query-block-jobs.  The operation can be stopped before it has completed
+# using the block_job_cancel command.
+#
+# If a base file is specified then sectors are not copied from that base file and
+# its backing chain.  When streaming completes the image file will have the base
+# file as its backing file.  This can be used to stream a subset of the backing
+# file chain instead of flattening the entire image.
+#
+# On successful completion the image file is updated to drop the backing file
+# and the BLOCK_JOB_COMPLETED event is emitted.
+#
+# @device: the device name
+#
+# @base:   the common backing file name
+#
+# Returns: Nothing on success
+#          If streaming is already active on this device, DeviceInUse
+#          If @device is does not exist, DeviceNotFound
+#          If image streaming is not supported by this device, NotSupported
+#
+# Since: 1.1
+##
+{ 'command': 'block_stream', 'data': { 'device': 'str', '*base': 'str' } }
diff --git a/qerror.c b/qerror.c
index 9a75d06..feb3d35 100644
--- a/qerror.c
+++ b/qerror.c
@@ -182,6 +182,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "No '%(bus)' bus found for device '%(device)'",
     },
     {
+        .error_fmt = QERR_NOT_SUPPORTED,
+        .desc      = "Not supported",
+    },
+    {
         .error_fmt = QERR_OPEN_FILE_FAILED,
         .desc      = "Could not open '%(filename)'",
     },
diff --git a/qerror.h b/qerror.h
index efda232..095ba9d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -153,6 +153,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_NO_BUS_FOR_DEVICE \
     "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
 
+#define QERR_NOT_SUPPORTED \
+    "{ 'class': 'NotSupported', 'data': {} }"
+
 #define QERR_OPEN_FILE_FAILED \
     "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
 
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7e3f4b9..b9ebb76 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -655,6 +655,12 @@ Example:
 EQMP
 
     {
+        .name       = "block_stream",
+        .args_type  = "device:B,base:s?",
+        .mhandler.cmd_new = qmp_marshal_input_block_stream,
+    },
+
+    {
         .name       = "blockdev-snapshot-sync",
         .args_type  = "device:B,snapshot-file:s,format:s?",
         .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
diff --git a/trace-events b/trace-events
index c5368fa..6ff0d43 100644
--- a/trace-events
+++ b/trace-events
@@ -74,6 +74,10 @@ bdrv_co_copy_on_readv(void *bs, int64_t sector_num, int nb_sectors, int64_t clus
 stream_one_iteration(void *s, int64_t sector_num, int nb_sectors, int is_allocated) "s %p sector_num %"PRId64" nb_sectors %d is_allocated %d"
 stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs %p base %p s %p co %p opaque %p"
 
+# blockdev.c
+block_stream_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
 virtio_blk_req_complete(void *req, int status) "req %p status %d"
 virtio_blk_rw_complete(void *req, int ret) "req %p ret %d"
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH v4 06/15] qmp: add block_stream command

[Luiz Capitulino]

On Fri,  6 Jan 2012 14:01:32 +0000
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote:

> Add the block_stream command, which starts copy backing file contents
> into the image file.  Also add the BLOCK_JOB_COMPLETED QMP event which
> is emitted when image streaming completes.  Later patches add control
> over the background copy speed, cancelation, and querying running
> streaming operations.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  QMP/qmp-events.txt |   29 ++++++++++++++++++++++
>  blockdev.c         |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hmp-commands.hx    |   13 ++++++++++
>  hmp.c              |   11 ++++++++
>  hmp.h              |    1 +
>  monitor.c          |    3 ++
>  monitor.h          |    1 +
>  qapi-schema.json   |   32 ++++++++++++++++++++++++
>  qerror.c           |    4 +++
>  qerror.h           |    3 ++
>  qmp-commands.hx    |    6 ++++
>  trace-events       |    4 +++
>  12 files changed, 174 insertions(+), 0 deletions(-)
> 
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index af586ec..a80e604 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -264,3 +264,32 @@ Example:
>  
>  Note: If action is "reset", "shutdown", or "pause" the WATCHDOG event is
>  followed respectively by the RESET, SHUTDOWN, or STOP events.
> +
> +
> +BLOCK_JOB_COMPLETED
> +-------------------
> +
> +Emitted when a block job has completed.
> +
> +Data:
> +
> +- "type":     Job type ("stream" for image streaming, json-string)
> +- "device":   Device name (json-string)
> +- "len":      Maximum progress value (json-int)
> +- "offset":   Current progress value (json-int)
> +              On success this is equal to len.
> +              On failure this is less than len.
> +- "speed":    Rate limit, bytes per second (json-int)
> +- "error":    Error message (json-string)

"error" is optional, so it should be "(json-string, optional)"

> +              Only present on failure.  This field contains a human-readable
> +              error message.  There are no semantics other than that streaming
> +              has failed and clients should not try to interpret the error
> +              string.
> +
> +Example:
> +
> +{ "event": "BLOCK_JOB_COMPLETED",
> +     "data": { "type": "stream", "device": "virtio-disk0",
> +               "len": 10737418240, "offset": 10737418240,
> +               "speed": 0 },
> +     "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
> diff --git a/blockdev.c b/blockdev.c
> index 6d78b36..ba973b0 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -13,9 +13,11 @@
>  #include "qerror.h"
>  #include "qemu-option.h"
>  #include "qemu-config.h"
> +#include "qemu-objects.h"
>  #include "sysemu.h"
>  #include "block_int.h"
>  #include "qmp-commands.h"
> +#include "trace.h"
>  
>  static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
>  
> @@ -880,3 +882,68 @@ void qmp_block_resize(const char *device, int64_t size, Error **errp)
>          return;
>      }
>  }
> +
> +static QObject *qobject_from_block_job(BlockJob *job)
> +{
> +    return qobject_from_jsonf("{ 'type': %s,"
> +                              "'device': %s,"
> +                              "'len': %" PRId64 ","
> +                              "'offset': %" PRId64 ","
> +                              "'speed': %" PRId64 " }",
> +                              job->job_type->job_type,
> +                              bdrv_get_device_name(job->bs),
> +                              job->len,
> +                              job->offset,
> +                              job->speed);
> +}
> +
> +static void block_stream_cb(void *opaque, int ret)
> +{
> +    BlockDriverState *bs = opaque;
> +    QObject *obj;
> +
> +    trace_block_stream_cb(bs, bs->job, ret);
> +
> +    assert(bs->job);
> +    obj = qobject_from_block_job(bs->job);
> +    if (ret < 0) {
> +        QDict *dict = qobject_to_qdict(obj);
> +        qdict_put(dict, "error", qstring_from_str(strerror(-ret)));
> +    }
> +
> +    monitor_protocol_event(QEVENT_BLOCK_JOB_COMPLETED, obj);
> +    qobject_decref(obj);
> +}
> +
> +void qmp_block_stream(const char *device, bool has_base,
> +                      const char *base, Error **errp)
> +{
> +    BlockDriverState *bs;
> +    int ret;
> +
> +    bs = bdrv_find(device);
> +    if (!bs) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +        return;
> +    }
> +
> +    /* Base device not supported */
> +    if (base) {
> +        error_set(errp, QERR_NOT_SUPPORTED);
> +        return;
> +    }
> +
> +    ret = stream_start(bs, NULL, block_stream_cb, bs);
> +    if (ret < 0) {
> +        switch (ret) {
> +        case -EBUSY:
> +            error_set(errp, QERR_DEVICE_IN_USE, device);
> +            return;
> +        default:
> +            error_set(errp, QERR_NOT_SUPPORTED);
> +            return;
> +        }
> +    }
> +
> +    trace_qmp_block_stream(bs, bs->job);
> +}
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 14838b7..8d9dbd6 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -69,6 +69,19 @@ but should be used with extreme caution.  Note that this command only
>  resizes image files, it can not resize block devices like LVM volumes.
>  ETEXI
>  
> +    {
> +        .name       = "block_stream",
> +        .args_type  = "device:B,base:s?",
> +        .params     = "device [base]",
> +        .help       = "copy data from a backing file into a block device",
> +        .mhandler.cmd = hmp_block_stream,
> +    },
> +
> +STEXI
> +@item block_stream
> +@findex block_stream
> +Copy data from a backing file into a block device.
> +ETEXI
>  
>      {
>          .name       = "eject",
> diff --git a/hmp.c b/hmp.c
> index e7659d5..b6e5913 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -679,3 +679,14 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
>      int64_t value = qdict_get_int(qdict, "value");
>      qmp_migrate_set_speed(value, NULL);
>  }
> +
> +void hmp_block_stream(Monitor *mon, const QDict *qdict)
> +{
> +    Error *error = NULL;
> +    const char *device = qdict_get_str(qdict, "device");
> +    const char *base = qdict_get_try_str(qdict, "base");
> +
> +    qmp_block_stream(device, base != NULL, base, &error);
> +
> +    hmp_handle_error(mon, &error);
> +}
> diff --git a/hmp.h b/hmp.h
> index 093242d..b55c295 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -49,5 +49,6 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
>  void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
>  void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
>  void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
> +void hmp_block_stream(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/monitor.c b/monitor.c
> index 7334401..bb42580 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -479,6 +479,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>          case QEVENT_SPICE_DISCONNECTED:
>              event_name = "SPICE_DISCONNECTED";
>              break;
> +        case QEVENT_BLOCK_JOB_COMPLETED:
> +            event_name = "BLOCK_JOB_COMPLETED";
> +            break;
>          default:
>              abort();
>              break;
> diff --git a/monitor.h b/monitor.h
> index cfa2f67..7324236 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -35,6 +35,7 @@ typedef enum MonitorEvent {
>      QEVENT_SPICE_CONNECTED,
>      QEVENT_SPICE_INITIALIZED,
>      QEVENT_SPICE_DISCONNECTED,
> +    QEVENT_BLOCK_JOB_COMPLETED,
>      QEVENT_MAX,
>  } MonitorEvent;
>  
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 44cf764..2b1cc8c 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1275,3 +1275,35 @@
>  { 'command': 'qom-set',
>    'data': { 'path': 'str', 'property': 'str', 'value': 'visitor' },
>    'gen': 'no' }
> +
> +##
> +# @block_stream:
> +#
> +# Copy data from a backing file into a block device.
> +#
> +# The block streaming operation is performed in the background until the entire
> +# backing file has been copied.  This command returns immediately once streaming
> +# has started.  The status of ongoing block streaming operations can be checked
> +# with query-block-jobs.  The operation can be stopped before it has completed
> +# using the block_job_cancel command.
> +#
> +# If a base file is specified then sectors are not copied from that base file and
> +# its backing chain.  When streaming completes the image file will have the base
> +# file as its backing file.  This can be used to stream a subset of the backing
> +# file chain instead of flattening the entire image.
> +#
> +# On successful completion the image file is updated to drop the backing file
> +# and the BLOCK_JOB_COMPLETED event is emitted.
> +#
> +# @device: the device name
> +#
> +# @base:   the common backing file name

@base is optional, so it should be documented like this:

 @base:   #optional the common backing file name

> +#
> +# Returns: Nothing on success
> +#          If streaming is already active on this device, DeviceInUse
> +#          If @device is does not exist, DeviceNotFound
> +#          If image streaming is not supported by this device, NotSupported
> +#
> +# Since: 1.1
> +##
> +{ 'command': 'block_stream', 'data': { 'device': 'str', '*base': 'str' } }
> diff --git a/qerror.c b/qerror.c
> index 9a75d06..feb3d35 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -182,6 +182,10 @@ static const QErrorStringTable qerror_table[] = {
>          .desc      = "No '%(bus)' bus found for device '%(device)'",
>      },
>      {
> +        .error_fmt = QERR_NOT_SUPPORTED,
> +        .desc      = "Not supported",
> +    },
> +    {
>          .error_fmt = QERR_OPEN_FILE_FAILED,
>          .desc      = "Could not open '%(filename)'",
>      },
> diff --git a/qerror.h b/qerror.h
> index efda232..095ba9d 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -153,6 +153,9 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_NO_BUS_FOR_DEVICE \
>      "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
>  
> +#define QERR_NOT_SUPPORTED \
> +    "{ 'class': 'NotSupported', 'data': {} }"
> +
>  #define QERR_OPEN_FILE_FAILED \
>      "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
>  
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 7e3f4b9..b9ebb76 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -655,6 +655,12 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "block_stream",
> +        .args_type  = "device:B,base:s?",
> +        .mhandler.cmd_new = qmp_marshal_input_block_stream,
> +    },
> +
> +    {
>          .name       = "blockdev-snapshot-sync",
>          .args_type  = "device:B,snapshot-file:s,format:s?",
>          .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
> diff --git a/trace-events b/trace-events
> index c5368fa..6ff0d43 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -74,6 +74,10 @@ bdrv_co_copy_on_readv(void *bs, int64_t sector_num, int nb_sectors, int64_t clus
>  stream_one_iteration(void *s, int64_t sector_num, int nb_sectors, int is_allocated) "s %p sector_num %"PRId64" nb_sectors %d is_allocated %d"
>  stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs %p base %p s %p co %p opaque %p"
>  
> +# blockdev.c
> +block_stream_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
>  virtio_blk_req_complete(void *req, int status) "req %p status %d"
>  virtio_blk_rw_complete(void *req, int ret) "req %p ret %d"

Re: [Qemu-devel] [PATCH v4 06/15] qmp: add block_stream command

[Stefan Hajnoczi]

On Wed, Jan 11, 2012 at 5:23 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Fri,  6 Jan 2012 14:01:32 +0000
> Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote:
>> +- "error":    Error message (json-string)
>
> "error" is optional, so it should be "(json-string, optional)"
>

Ok.

>> +# @base:   the common backing file name
>
> @base is optional, so it should be documented like this:
>
>  @base:   #optional the common backing file name

Ok.

[Qemu-devel] [PATCH v4 07/15] qmp: add block_job_set_speed command

[Stefan Hajnoczi]

Add block_job_set_speed, which sets the maximum speed for a background
block operation.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 blockdev.c       |   25 +++++++++++++++++++++++++
 hmp-commands.hx  |   14 ++++++++++++++
 hmp.c            |   11 +++++++++++
 hmp.h            |    1 +
 qapi-schema.json |   22 ++++++++++++++++++++++
 qmp-commands.hx  |    6 ++++++
 6 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index ba973b0..2dfca40 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -947,3 +947,28 @@ void qmp_block_stream(const char *device, bool has_base,
 
     trace_qmp_block_stream(bs, bs->job);
 }
+
+static BlockJob *find_block_job(const char *device)
+{
+    BlockDriverState *bs;
+
+    bs = bdrv_find(device);
+    if (!bs || !bs->job) {
+        return NULL;
+    }
+    return bs->job;
+}
+
+void qmp_block_job_set_speed(const char *device, int64_t value, Error **errp)
+{
+    BlockJob *job = find_block_job(device);
+
+    if (!job) {
+        error_set(errp, QERR_DEVICE_NOT_ACTIVE, device);
+        return;
+    }
+
+    if (block_job_set_speed(job, value) < 0) {
+        error_set(errp, QERR_NOT_SUPPORTED);
+    }
+}
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8d9dbd6..5dffcb2 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -84,6 +84,20 @@ Copy data from a backing file into a block device.
 ETEXI
 
     {
+        .name       = "block_job_set_speed",
+        .args_type  = "device:B,value:o",
+        .params     = "device value",
+        .help       = "set maximum speed for a background block operation",
+        .mhandler.cmd = hmp_block_job_set_speed,
+    },
+
+STEXI
+@item block_job_set_stream
+@findex block_job_set_stream
+Set maximum speed for a background block operation.
+ETEXI
+
+    {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
diff --git a/hmp.c b/hmp.c
index b6e5913..1144d53 100644
--- a/hmp.c
+++ b/hmp.c
@@ -690,3 +690,14 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
 
     hmp_handle_error(mon, &error);
 }
+
+void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict)
+{
+    Error *error = NULL;
+    const char *device = qdict_get_str(qdict, "device");
+    int64_t value = qdict_get_int(qdict, "value");
+
+    qmp_block_job_set_speed(device, value, &error);
+
+    hmp_handle_error(mon, &error);
+}
diff --git a/hmp.h b/hmp.h
index b55c295..2c871ea 100644
--- a/hmp.h
+++ b/hmp.h
@@ -50,5 +50,6 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_speed(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);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 2b1cc8c..7b39371 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1307,3 +1307,25 @@
 # Since: 1.1
 ##
 { 'command': 'block_stream', 'data': { 'device': 'str', '*base': 'str' } }
+
+##
+# @block_job_set_speed:
+#
+# Set maximum speed for a background block operation.
+#
+# This command can only be issued when there is an active block job.
+#
+# Throttling can be disabled by setting the speed to 0.
+#
+# @device: the device name
+#
+# @value:  the maximum speed, in bytes per second
+#
+# Returns: Nothing on success
+#          If the job type does not support throttling, NotSupported
+#          If streaming is not active on this device, DeviceNotActive
+#
+# Since: 1.1
+##
+{ 'command': 'block_job_set_speed',
+  'data': { 'device': 'str', 'value': 'int' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b9ebb76..dc6bc2e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -661,6 +661,12 @@ EQMP
     },
 
     {
+        .name       = "block_job_set_speed",
+        .args_type  = "device:B,value:o",
+        .mhandler.cmd_new = qmp_marshal_input_block_job_set_speed,
+    },
+
+    {
         .name       = "blockdev-snapshot-sync",
         .args_type  = "device:B,snapshot-file:s,format:s?",
         .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
-- 
1.7.7.3

[Qemu-devel] [PATCH v4 08/15] qmp: add block_job_cancel command

[Stefan Hajnoczi]

Add block_job_cancel, which stops an active block streaming operation.
When the operation has been cancelled the new BLOCK_JOB_CANCELLED event
is emitted.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 QMP/qmp-events.txt |   24 ++++++++++++++++++++++++
 blockdev.c         |   19 ++++++++++++++++++-
 hmp-commands.hx    |   14 ++++++++++++++
 hmp.c              |   10 ++++++++++
 hmp.h              |    1 +
 monitor.c          |    3 +++
 monitor.h          |    1 +
 qapi-schema.json   |   29 +++++++++++++++++++++++++++++
 qmp-commands.hx    |    6 ++++++
 trace-events       |    1 +
 10 files changed, 107 insertions(+), 1 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index a80e604..310c4c6 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -293,3 +293,27 @@ Example:
                "len": 10737418240, "offset": 10737418240,
                "speed": 0 },
      "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
+
+
+BLOCK_JOB_CANCELLED
+-------------------
+
+Emitted when a block job has been cancelled.
+
+Data:
+
+- "type":     Job type ("stream" for image streaming, json-string)
+- "device":   Device name (json-string)
+- "len":      Maximum progress value (json-int)
+- "offset":   Current progress value (json-int)
+              On success this is equal to len.
+              On failure this is less than len.
+- "speed":    Rate limit, bytes per second (json-int)
+
+Example:
+
+{ "event": "BLOCK_JOB_CANCELLED",
+     "data": { "type": "stream", "device": "virtio-disk0",
+               "len": 10737418240, "offset": 134217728,
+               "speed": 0 },
+     "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
diff --git a/blockdev.c b/blockdev.c
index 2dfca40..35de3bc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -911,7 +911,11 @@ static void block_stream_cb(void *opaque, int ret)
         qdict_put(dict, "error", qstring_from_str(strerror(-ret)));
     }
 
-    monitor_protocol_event(QEVENT_BLOCK_JOB_COMPLETED, obj);
+    if (block_job_is_cancelled(bs->job)) {
+        monitor_protocol_event(QEVENT_BLOCK_JOB_CANCELLED, obj);
+    } else {
+        monitor_protocol_event(QEVENT_BLOCK_JOB_COMPLETED, obj);
+    }
     qobject_decref(obj);
 }
 
@@ -972,3 +976,16 @@ void qmp_block_job_set_speed(const char *device, int64_t value, Error **errp)
         error_set(errp, QERR_NOT_SUPPORTED);
     }
 }
+
+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);
+        return;
+    }
+
+    trace_qmp_block_job_cancel(job);
+    block_job_cancel(job);
+}
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 5dffcb2..14d6122 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -98,6 +98,20 @@ Set maximum speed for a background block operation.
 ETEXI
 
     {
+        .name       = "block_job_cancel",
+        .args_type  = "device:B",
+        .params     = "device",
+        .help       = "stop an active block streaming operation",
+        .mhandler.cmd = hmp_block_job_cancel,
+    },
+
+STEXI
+@item block_job_cancel
+@findex block_job_cancel
+Stop an active block streaming operation.
+ETEXI
+
+    {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
diff --git a/hmp.c b/hmp.c
index 1144d53..851885b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -701,3 +701,13 @@ void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict)
 
     hmp_handle_error(mon, &error);
 }
+
+void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
+{
+    Error *error = NULL;
+    const char *device = qdict_get_str(qdict, "device");
+
+    qmp_block_job_cancel(device, &error);
+
+    hmp_handle_error(mon, &error);
+}
diff --git a/hmp.h b/hmp.h
index 2c871ea..0ad2004 100644
--- a/hmp.h
+++ b/hmp.h
@@ -51,5 +51,6 @@ void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_speed(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);
 
 #endif
diff --git a/monitor.c b/monitor.c
index bb42580..01850ca 100644
--- a/monitor.c
+++ b/monitor.c
@@ -482,6 +482,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_BLOCK_JOB_COMPLETED:
             event_name = "BLOCK_JOB_COMPLETED";
             break;
+        case QEVENT_BLOCK_JOB_CANCELLED:
+            event_name = "BLOCK_JOB_CANCELLED";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index 7324236..86c997d 100644
--- a/monitor.h
+++ b/monitor.h
@@ -36,6 +36,7 @@ typedef enum MonitorEvent {
     QEVENT_SPICE_INITIALIZED,
     QEVENT_SPICE_DISCONNECTED,
     QEVENT_BLOCK_JOB_COMPLETED,
+    QEVENT_BLOCK_JOB_CANCELLED,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 7b39371..d3fd36e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1329,3 +1329,32 @@
 ##
 { 'command': 'block_job_set_speed',
   'data': { 'device': 'str', 'value': 'int' } }
+
+##
+# @block_job_cancel:
+#
+# Stop an active block streaming operation.
+#
+# This command returns immediately after marking the active block streaming
+# operation for cancellation.  It is an error to call this command if no
+# operation is in progress.
+#
+# The operation will cancel as soon as possible and then emit the
+# BLOCK_JOB_CANCELLED event.  Before that happens the job is still visible when
+# enumerated using query-block-jobs.
+#
+# The image file retains its backing file unless the streaming operation happens
+# to complete just as it is being cancelled.
+#
+# A new block streaming operation can be started at a later time to finish
+# copying all data from the backing file.
+#
+# @device: the device name
+#
+# Returns: Nothing on success
+#          If streaming is not active on this device, DeviceNotActive
+#          If cancellation already in progress, DeviceInUse
+#
+# Since: 1.1
+##
+{ 'command': 'block_job_cancel', 'data': { 'device': 'str' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index dc6bc2e..0a0335f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -667,6 +667,12 @@ EQMP
     },
 
     {
+        .name       = "block_job_cancel",
+        .args_type  = "device:B",
+        .mhandler.cmd_new = qmp_marshal_input_block_job_cancel,
+    },
+
+    {
         .name       = "blockdev-snapshot-sync",
         .args_type  = "device:B,snapshot-file:s,format:s?",
         .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
diff --git a/trace-events b/trace-events
index 6ff0d43..196a872 100644
--- a/trace-events
+++ b/trace-events
@@ -75,6 +75,7 @@ stream_one_iteration(void *s, int64_t sector_num, int nb_sectors, int is_allocat
 stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs %p base %p s %p co %p opaque %p"
 
 # 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"
 qmp_block_stream(void *bs, void *job) "bs %p job %p"
 
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH v4 08/15] qmp: add block_job_cancel command

[Eric Blake]

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

On 01/06/2012 07:01 AM, Stefan Hajnoczi wrote:
> Add block_job_cancel, which stops an active block streaming operation.
> When the operation has been cancelled the new BLOCK_JOB_CANCELLED event
> is emitted.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

> +++ b/hmp-commands.hx
> @@ -98,6 +98,20 @@ Set maximum speed for a background block operation.
>  ETEXI
>  
>      {
> +        .name       = "block_job_cancel",
> +        .args_type  = "device:B",
> +        .params     = "device",
> +        .help       = "stop an active block streaming operation",
> +        .mhandler.cmd = hmp_block_job_cancel,
> +    },
> +

Looking at this from libvirt's perspective, would it be possible to give
this a different name?  Then libvirt would know that if
block_job_cancel_async exists, we have the official semantics; and if it
doesn't exist, then we can try block_job_cancel as a fallback to see if
we have the old blocking semantics.

But by using the same name as the old unofficial blocking command, it's
difficult to tell if we should expect an event, or whether completion of
the command means completion of the cancel.

On the other hand, I guess we can rely on completion of the command,
followed by reading block job status to see if the job is still in
flight, will tell us whether we need to worry about waiting for an event
- if the job is complete (whether or not this command was the blocking
variant), we are done; if the job is ongoing, we have the new semantics
and can expect an event; and that only leaves the race of calling the
command, then the job completes, then we query and see it done, then the
event comes, where we just have to be ready to ignore an unexpected event.

> +##
> +# @block_job_cancel:
> +#
> +# Stop an active block streaming operation.
> +#
> +# This command returns immediately after marking the active block streaming
> +# operation for cancellation.  It is an error to call this command if no
> +# operation is in progress.
> +#
> +# The operation will cancel as soon as possible and then emit the
> +# BLOCK_JOB_CANCELLED event.  Before that happens the job is still visible when
> +# enumerated using query-block-jobs.

Is there any policy on _ vs - in command names?  It seems awkward to
have block_job_cancel but query-block-jobs.

-- 
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: 620 bytes --]

Re: [Qemu-devel] [PATCH v4 08/15] qmp: add block_job_cancel command

[Kevin Wolf]

Am 20.01.2012 01:02, schrieb Eric Blake:
> On 01/06/2012 07:01 AM, Stefan Hajnoczi wrote:
>> Add block_job_cancel, which stops an active block streaming operation.
>> When the operation has been cancelled the new BLOCK_JOB_CANCELLED event
>> is emitted.
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> 
>> +++ b/hmp-commands.hx
>> @@ -98,6 +98,20 @@ Set maximum speed for a background block operation.
>>  ETEXI
>>  
>>      {
>> +        .name       = "block_job_cancel",
>> +        .args_type  = "device:B",
>> +        .params     = "device",
>> +        .help       = "stop an active block streaming operation",
>> +        .mhandler.cmd = hmp_block_job_cancel,
>> +    },
>> +
> 
> Looking at this from libvirt's perspective, would it be possible to give
> this a different name?  Then libvirt would know that if
> block_job_cancel_async exists, we have the official semantics; and if it
> doesn't exist, then we can try block_job_cancel as a fallback to see if
> we have the old blocking semantics.
> 
> But by using the same name as the old unofficial blocking command, it's
> difficult to tell if we should expect an event, or whether completion of
> the command means completion of the cancel.
> 
> On the other hand, I guess we can rely on completion of the command,
> followed by reading block job status to see if the job is still in
> flight, will tell us whether we need to worry about waiting for an event
> - if the job is complete (whether or not this command was the blocking
> variant), we are done; if the job is ongoing, we have the new semantics
> and can expect an event; and that only leaves the race of calling the
> command, then the job completes, then we query and see it done, then the
> event comes, where we just have to be ready to ignore an unexpected event.

You're quoting the HMP part, is that intentional? You shouldn't be using
this at all.

Anyway, are there even any qemu versions out there that implement an
older interface?

>> +##
>> +# @block_job_cancel:
>> +#
>> +# Stop an active block streaming operation.
>> +#
>> +# This command returns immediately after marking the active block streaming
>> +# operation for cancellation.  It is an error to call this command if no
>> +# operation is in progress.
>> +#
>> +# The operation will cancel as soon as possible and then emit the
>> +# BLOCK_JOB_CANCELLED event.  Before that happens the job is still visible when
>> +# enumerated using query-block-jobs.
> 
> Is there any policy on _ vs - in command names?  It seems awkward to
> have block_job_cancel but query-block-jobs.

block_job_cancel is HMP, whereas query-block-jobs is a QMP command. QMP
uses - consistently. Not sure if HMP is consistent, but it tends to use _.

Kevin

Re: [Qemu-devel] [PATCH v4 08/15] qmp: add block_job_cancel command

[Luiz Capitulino]

On Fri, 20 Jan 2012 09:30:45 +0100
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 20.01.2012 01:02, schrieb Eric Blake:
> > On 01/06/2012 07:01 AM, Stefan Hajnoczi wrote:
> >> Add block_job_cancel, which stops an active block streaming operation.
> >> When the operation has been cancelled the new BLOCK_JOB_CANCELLED event
> >> is emitted.
> >>
> >> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > 
> >> +++ b/hmp-commands.hx
> >> @@ -98,6 +98,20 @@ Set maximum speed for a background block operation.
> >>  ETEXI
> >>  
> >>      {
> >> +        .name       = "block_job_cancel",
> >> +        .args_type  = "device:B",
> >> +        .params     = "device",
> >> +        .help       = "stop an active block streaming operation",
> >> +        .mhandler.cmd = hmp_block_job_cancel,
> >> +    },
> >> +
> > 
> > Looking at this from libvirt's perspective, would it be possible to give
> > this a different name?  Then libvirt would know that if
> > block_job_cancel_async exists, we have the official semantics; and if it
> > doesn't exist, then we can try block_job_cancel as a fallback to see if
> > we have the old blocking semantics.
> > 
> > But by using the same name as the old unofficial blocking command, it's
> > difficult to tell if we should expect an event, or whether completion of
> > the command means completion of the cancel.
> > 
> > On the other hand, I guess we can rely on completion of the command,
> > followed by reading block job status to see if the job is still in
> > flight, will tell us whether we need to worry about waiting for an event
> > - if the job is complete (whether or not this command was the blocking
> > variant), we are done; if the job is ongoing, we have the new semantics
> > and can expect an event; and that only leaves the race of calling the
> > command, then the job completes, then we query and see it done, then the
> > event comes, where we just have to be ready to ignore an unexpected event.
> 
> You're quoting the HMP part, is that intentional? You shouldn't be using
> this at all.
> 
> Anyway, are there even any qemu versions out there that implement an
> older interface?
> 
> >> +##
> >> +# @block_job_cancel:
> >> +#
> >> +# Stop an active block streaming operation.
> >> +#
> >> +# This command returns immediately after marking the active block streaming
> >> +# operation for cancellation.  It is an error to call this command if no
> >> +# operation is in progress.
> >> +#
> >> +# The operation will cancel as soon as possible and then emit the
> >> +# BLOCK_JOB_CANCELLED event.  Before that happens the job is still visible when
> >> +# enumerated using query-block-jobs.
> > 
> > Is there any policy on _ vs - in command names?  It seems awkward to
> > have block_job_cancel but query-block-jobs.
> 
> block_job_cancel is HMP, whereas query-block-jobs is a QMP command. QMP
> uses - consistently. Not sure if HMP is consistent, but it tends to use _.

This very series broke QMP's consistency because it was designed when we
were following HMP's inconsistencies...

Re: [Qemu-devel] [PATCH v4 08/15] qmp: add block_job_cancel command

[Eric Blake]

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

On 01/20/2012 01:30 AM, Kevin Wolf wrote:

>>> +++ b/hmp-commands.hx

>> Looking at this from libvirt's perspective, would it be possible to give
>> this a different name?  Then libvirt would know that if
>> block_job_cancel_async exists, we have the official semantics; and if it
>> doesn't exist, then we can try block_job_cancel as a fallback to see if
>> we have the old blocking semantics.
>>
>> But by using the same name as the old unofficial blocking command, it's
>> difficult to tell if we should expect an event, or whether completion of
>> the command means completion of the cancel.
>>

> 
> You're quoting the HMP part, is that intentional? You shouldn't be using
> this at all.

Oh - I missed the .hx file name when trimming my reply.  I agree that
libvirt will be using the QMP, not HMP command.

> 
> Anyway, are there even any qemu versions out there that implement an
> older interface?

The older interface was backported into RHEL 6.2 qemu, so libvirt has to
maintain the distinction (although it can equally be argued that the
distinction only exists in the RHEL build, and therefore, only the RHEL
build of libvirt has to deal with the issues of an early backport of a
qemu feature).

>> Is there any policy on _ vs - in command names?  It seems awkward to
>> have block_job_cancel but query-block-jobs.
> 
> block_job_cancel is HMP, whereas query-block-jobs is a QMP command. QMP
> uses - consistently. Not sure if HMP is consistent, but it tends to use _.

OK, on RHEL 6.2, 'help' on HMP includes help for block_job_cancel, and
'{"execute":"query-commands"}' on QMP includes
{"name":"block_job_cancel"}, with underscores in both locations.  So if
your new QMP command is block-job-cancel, you've met the goal of having
a different spelling from the RHEL backport of the earlier semantics.
And if you stuck with underscores, then it's RHEL's problem to solve :)

-- 
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: 620 bytes --]

Re: [Qemu-devel] [PATCH v4 08/15] qmp: add block_job_cancel command

[Stefan Hajnoczi]

On Fri, Jan 20, 2012 at 7:55 PM, Eric Blake <eblake@redhat.com> wrote:
> On 01/20/2012 01:30 AM, Kevin Wolf wrote:
>>> Is there any policy on _ vs - in command names?  It seems awkward to
>>> have block_job_cancel but query-block-jobs.
>>
>> block_job_cancel is HMP, whereas query-block-jobs is a QMP command. QMP
>> uses - consistently. Not sure if HMP is consistent, but it tends to use _.
>
> OK, on RHEL 6.2, 'help' on HMP includes help for block_job_cancel, and
> '{"execute":"query-commands"}' on QMP includes
> {"name":"block_job_cancel"}, with underscores in both locations.  So if
> your new QMP command is block-job-cancel, you've met the goal of having
> a different spelling from the RHEL backport of the earlier semantics.
> And if you stuck with underscores, then it's RHEL's problem to solve :)

Hi Eric,
qemu.git/master has "query-block-jobs" with all other
streaming/blockjob commands using underscores.  I suspect this is
identical to RHEL 6.2.

Stefan

[Qemu-devel] [PATCH v4 09/15] qmp: add query-block-jobs

[Stefan Hajnoczi]

Add query-block-jobs, which shows the progress of ongoing block device
operations.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 blockdev.c       |   33 +++++++++++++++++++++++++++++++++
 hmp.c            |   36 ++++++++++++++++++++++++++++++++++++
 hmp.h            |    1 +
 monitor.c        |    7 +++++++
 qapi-schema.json |   32 ++++++++++++++++++++++++++++++++
 qmp-commands.hx  |    6 ++++++
 6 files changed, 115 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 35de3bc..4549c9e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -989,3 +989,36 @@ void qmp_block_job_cancel(const char *device, Error **errp)
     trace_qmp_block_job_cancel(job);
     block_job_cancel(job);
 }
+
+static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs)
+{
+    BlockJobInfoList **prev = opaque;
+    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;
+
+        (*prev)->next = elem;
+        *prev = elem;
+    }
+}
+
+BlockJobInfoList *qmp_query_block_jobs(Error **errp)
+{
+    /* Dummy is a fake list element for holding the head pointer */
+    BlockJobInfoList dummy = {};
+    BlockJobInfoList *prev = &dummy;
+    bdrv_iterate(do_qmp_query_block_jobs_one, &prev);
+    return dummy.next;
+}
diff --git a/hmp.c b/hmp.c
index 851885b..76e89f8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -507,6 +507,42 @@ void hmp_info_pci(Monitor *mon)
     qapi_free_PciInfoList(info);
 }
 
+void hmp_info_block_jobs(Monitor *mon)
+{
+    BlockJobInfoList *list;
+    Error *err = NULL;
+
+    list = qmp_query_block_jobs(&err);
+    assert(!err);
+
+    if (!list) {
+        monitor_printf(mon, "No active jobs\n");
+        return;
+    }
+
+    while (list) {
+        if (strcmp(list->value->type, "stream") == 0) {
+            monitor_printf(mon, "Streaming device %s: Completed %" PRId64
+                           " of %" PRId64 " bytes, speed limit %" PRId64
+                           " bytes/s\n",
+                           list->value->device,
+                           list->value->offset,
+                           list->value->len,
+                           list->value->speed);
+        } else {
+            monitor_printf(mon, "Type %s, device %s: Completed %" PRId64
+                           " of %" PRId64 " bytes, speed limit %" PRId64
+                           " bytes/s\n",
+                           list->value->type,
+                           list->value->device,
+                           list->value->offset,
+                           list->value->len,
+                           list->value->speed);
+        }
+        list = list->next;
+    }
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
     monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 0ad2004..23bfca2 100644
--- a/hmp.h
+++ b/hmp.h
@@ -32,6 +32,7 @@ void hmp_info_vnc(Monitor *mon);
 void hmp_info_spice(Monitor *mon);
 void hmp_info_balloon(Monitor *mon);
 void hmp_info_pci(Monitor *mon);
+void hmp_info_block_jobs(Monitor *mon);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/monitor.c b/monitor.c
index 01850ca..f96a296 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2483,6 +2483,13 @@ static mon_cmd_t info_cmds[] = {
         .mhandler.info = hmp_info_blockstats,
     },
     {
+        .name       = "block-jobs",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show progress of ongoing block device operations",
+        .mhandler.info = hmp_info_block_jobs,
+    },
+    {
         .name       = "registers",
         .args_type  = "",
         .params     = "",
diff --git a/qapi-schema.json b/qapi-schema.json
index d3fd36e..a27861a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -845,6 +845,38 @@
 { 'command': 'query-pci', 'returns': ['PciInfo'] }
 
 ##
+# @BlockJobInfo:
+#
+# Information about a long-running block device operation.
+#
+# @type: the job type ('stream' for image streaming)
+#
+# @device: the block device name
+#
+# @len: the maximum progress value
+#
+# @offset: the current progress value
+#
+# @speed: the rate limit, bytes per second
+#
+# Since: 1.1
+##
+{ 'type': 'BlockJobInfo',
+  'data': {'type': 'str', 'device': 'str', 'len': 'int',
+           'offset': 'int', 'speed': 'int'} }
+
+##
+# @query-block-jobs:
+#
+# Return information about long-running block device operations.
+#
+# Returns: a list of @BlockJobInfo for each active block job
+#
+# Since: 1.1
+##
+{ 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
+
+##
 # @quit:
 #
 # This command will cause the QEMU process to exit gracefully.  While every
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 0a0335f..4be6632 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2029,6 +2029,12 @@ EQMP
     },
 
     {
+        .name       = "query-block-jobs",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_block_jobs,
+    },
+
+    {
         .name       = "qom-list",
         .args_type  = "path:s",
         .mhandler.cmd_new = qmp_marshal_input_qom_list,
-- 
1.7.7.3

[Qemu-devel] [PATCH v4 10/15] blockdev: make image streaming safe across hotplug

[Stefan Hajnoczi]

Unplugging a storage interface like virtio-blk causes the host block
device to be deleted too.  Long-running operations like block migration
must take a DriveInfo reference to prevent the BlockDriverState from
being freed.  For image streaming we can do the same thing.

Note that it is not possible to acquire/release the drive reference in
block.c where the block job functions live because
drive_get_ref()/drive_put_ref() are blockdev.c functions.  Calling them
from block.c would be a layering violation - tools like qemu-img don't
even link against blockdev.c.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 blockdev.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 4549c9e..45a6ba6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -202,6 +202,37 @@ void drive_get_ref(DriveInfo *dinfo)
     dinfo->refcount++;
 }
 
+typedef struct {
+    QEMUBH *bh;
+    DriveInfo *dinfo;
+} DrivePutRefBH;
+
+static void drive_put_ref_bh(void *opaque)
+{
+    DrivePutRefBH *s = opaque;
+
+    drive_put_ref(s->dinfo);
+    qemu_bh_delete(s->bh);
+    g_free(s);
+}
+
+/*
+ * Release a drive reference in a BH
+ *
+ * It is not possible to use drive_put_ref() from a callback function when the
+ * callers still need the drive.  In such cases we schedule a BH to release the
+ * reference.
+ */
+static void drive_put_ref_bh_schedule(DriveInfo *dinfo)
+{
+    DrivePutRefBH *s;
+
+    s = g_new(DrivePutRefBH, 1);
+    s->bh = qemu_bh_new(drive_put_ref_bh, s);
+    s->dinfo = dinfo;
+    qemu_bh_schedule(s->bh);
+}
+
 static int parse_block_error_action(const char *buf, int is_read)
 {
     if (!strcmp(buf, "ignore")) {
@@ -917,6 +948,8 @@ static void block_stream_cb(void *opaque, int ret)
         monitor_protocol_event(QEVENT_BLOCK_JOB_COMPLETED, obj);
     }
     qobject_decref(obj);
+
+    drive_put_ref_bh_schedule(drive_get_by_blockdev(bs));
 }
 
 void qmp_block_stream(const char *device, bool has_base,
@@ -949,6 +982,11 @@ void qmp_block_stream(const char *device, bool has_base,
         }
     }
 
+    /* Grab a reference so hotplug does not delete the BlockDriverState from
+     * underneath us.
+     */
+    drive_get_ref(drive_get_by_blockdev(bs));
+
     trace_qmp_block_stream(bs, bs->job);
 }
 
-- 
1.7.7.3

[Qemu-devel] [PATCH v4 11/15] block: add bdrv_find_backing_image

[Stefan Hajnoczi]

From: Marcelo Tosatti <mtosatti@redhat.com>

Add bdrv_find_backing_image: given a BlockDriverState pointer, and an id,
traverse the backing image chain to locate the id.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c |   17 +++++++++++++++++
 block.h |    1 +
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 5bfaa3a..9b688a0 100644
--- a/block.c
+++ b/block.c
@@ -2614,6 +2614,23 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
     return -ENOTSUP;
 }
 
+BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *id)
+{
+    if (!bs->drv) {
+        return NULL;
+    }
+
+    if (bs->backing_hd) {
+        if (strcmp(bs->backing_file, id) == 0) {
+            return bs->backing_hd;
+        } else {
+            return bdrv_find_backing_image(bs->backing_hd, id);
+        }
+    }
+
+    return NULL;
+}
+
 #define NB_SUFFIXES 4
 
 char *get_human_readable_size(char *buf, int buf_size, int64_t size)
diff --git a/block.h b/block.h
index 51b90c7..a1d9b56 100644
--- a/block.h
+++ b/block.h
@@ -153,6 +153,7 @@ int coroutine_fn bdrv_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
     int nb_sectors);
 int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs, int64_t sector_num,
     int nb_sectors, int *pnum);
+BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *id);
 int bdrv_truncate(BlockDriverState *bs, int64_t offset);
 int64_t bdrv_getlength(BlockDriverState *bs);
 int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH v4 11/15] block: add bdrv_find_backing_image

[Kevin Wolf]

Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
> From: Marcelo Tosatti <mtosatti@redhat.com>
> 
> Add bdrv_find_backing_image: given a BlockDriverState pointer, and an id,
> traverse the backing image chain to locate the id.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  block.c |   17 +++++++++++++++++
>  block.h |    1 +
>  2 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 5bfaa3a..9b688a0 100644
> --- a/block.c
> +++ b/block.c
> @@ -2614,6 +2614,23 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
>      return -ENOTSUP;
>  }
>  
> +BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *id)
> +{
> +    if (!bs->drv) {
> +        return NULL;
> +    }
> +
> +    if (bs->backing_hd) {
> +        if (strcmp(bs->backing_file, id) == 0) {
> +            return bs->backing_hd;

So it's not really just some id, but the backing file name? I would find
it clearer to reflect that in the parameter name and the QMP error in
the next patch.

Kevin

Re: [Qemu-devel] [PATCH v4 11/15] block: add bdrv_find_backing_image

[Stefan Hajnoczi]

On Thu, Jan 12, 2012 at 12:17 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
>> +BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *id)
>> +{
>> +    if (!bs->drv) {
>> +        return NULL;
>> +    }
>> +
>> +    if (bs->backing_hd) {
>> +        if (strcmp(bs->backing_file, id) == 0) {
>> +            return bs->backing_hd;
>
> So it's not really just some id, but the backing file name? I would find
> it clearer to reflect that in the parameter name and the QMP error in
> the next patch.

Okay, thanks.

[Qemu-devel] [PATCH v4 12/15] add QERR_BASE_ID_NOT_FOUND

[Stefan Hajnoczi]

From: Marcelo Tosatti <mtosatti@redhat.com>

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 qerror.c |    4 ++++
 qerror.h |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index feb3d35..42f5fc2 100644
--- a/qerror.c
+++ b/qerror.c
@@ -276,6 +276,10 @@ static const QErrorStringTable qerror_table[] = {
         .error_fmt = QERR_INVALID_PARAMETER_COMBINATION,
         .desc      = "Invalid parameter combination",
     },
+    {
+        .error_fmt = QERR_BASE_ID_NOT_FOUND,
+        .desc      = "The base id %(base_id) has not been found",
+    },
     {}
 };
 
diff --git a/qerror.h b/qerror.h
index 095ba9d..50304c7 100644
--- a/qerror.h
+++ b/qerror.h
@@ -225,4 +225,7 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_INVALID_PARAMETER_COMBINATION \
     "{ 'class': 'InvalidParameterCombination', 'data': {} }"
 
+#define QERR_BASE_ID_NOT_FOUND \
+    "{ 'class': 'BaseIdNotFound', 'data': { 'base_id': %s } }"
+
 #endif /* QERROR_H */
-- 
1.7.7.3

[Qemu-devel] [PATCH v4 13/15] block stream: add support for partial streaming

[Stefan Hajnoczi]

From: Marcelo Tosatti <mtosatti@redhat.com>

Add support for streaming data from an intermediate section of the
image chain (see patch and documentation for details).

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c        |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block.h        |    4 +++
 block/stream.c |   28 +++++++++++++++++++++---
 block_int.h    |    3 +-
 blockdev.c     |   11 ++++++---
 5 files changed, 101 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 9b688a0..d2143b1 100644
--- a/block.c
+++ b/block.c
@@ -2263,6 +2263,70 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     return data.ret;
 }
 
+/*
+ * Given an image chain: [BASE] -> [INTER1] -> [INTER2] -> [TOP]
+ *
+ * Return true if the given sector is allocated in top or base.
+ * Return false if the given sector is allocated in intermediate images.
+ *
+ * 'pnum' is set to the number of sectors (including and immediately following
+ *  the specified sector) that are known to be in the same
+ *  allocated/unallocated state.
+ *
+ */
+int coroutine_fn bdrv_co_is_allocated_base(BlockDriverState *top,
+                                           BlockDriverState *base,
+                                           int64_t sector_num,
+                                           int nb_sectors, int *pnum)
+{
+    BlockDriverState *intermediate;
+    int ret, n;
+
+    ret = bdrv_co_is_allocated(top, sector_num, nb_sectors, &n);
+    if (ret) {
+        *pnum = n;
+        return ret;
+    }
+
+    /*
+     * Is the unallocated chunk [sector_num, n] also
+     * unallocated between base and top?
+     */
+    intermediate = top->backing_hd;
+
+    while (intermediate) {
+        int pnum_inter;
+
+        /* reached base */
+        if (intermediate == base) {
+            *pnum = n;
+            return 1;
+        }
+        ret = bdrv_co_is_allocated(intermediate, sector_num, nb_sectors,
+                                   &pnum_inter);
+        if (ret < 0) {
+            return ret;
+        } else if (ret) {
+            *pnum = pnum_inter;
+            return 0;
+        }
+
+        /*
+         * [sector_num, nb_sectors] is unallocated on top but intermediate
+         * might have
+         *
+         * [sector_num+x, nr_sectors] allocated.
+         */
+        if (n > pnum_inter) {
+            n = pnum_inter;
+        }
+
+        intermediate = intermediate->backing_hd;
+    }
+
+    return 1;
+}
+
 void bdrv_mon_event(const BlockDriverState *bdrv,
                     BlockMonEventAction action, int is_read)
 {
diff --git a/block.h b/block.h
index a1d9b56..0e786a9 100644
--- a/block.h
+++ b/block.h
@@ -229,6 +229,10 @@ int bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
 int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
                       int *pnum);
+int coroutine_fn bdrv_co_is_allocated_base(BlockDriverState *top,
+                                           BlockDriverState *base,
+                                           int64_t sector_num, int nb_sectors,
+                                           int *pnum);
 
 #define BIOS_ATA_TRANSLATION_AUTO   0
 #define BIOS_ATA_TRANSLATION_NONE   1
diff --git a/block/stream.c b/block/stream.c
index 5d5d672..c6f548d 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -57,6 +57,7 @@ typedef struct StreamBlockJob {
     BlockJob common;
     RateLimit limit;
     BlockDriverState *base;
+    char backing_file_id[1024];
 } StreamBlockJob;
 
 static int coroutine_fn stream_populate(BlockDriverState *bs,
@@ -79,6 +80,7 @@ static void coroutine_fn stream_run(void *opaque)
 {
     StreamBlockJob *s = opaque;
     BlockDriverState *bs = s->common.bs;
+    BlockDriverState *base = s->base;
     int64_t sector_num, end;
     int ret = 0;
     int n;
@@ -96,8 +98,17 @@ retry:
             break;
         }
 
-        ret = bdrv_co_is_allocated(bs, sector_num,
-                                   STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
+
+        if (base) {
+            ret = bdrv_co_is_allocated_base(bs, base, sector_num,
+                                            STREAM_BUFFER_SIZE /
+                                            BDRV_SECTOR_SIZE,
+                                            &n);
+        } else {
+            ret = bdrv_co_is_allocated(bs, sector_num,
+                                       STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE,
+                                       &n);
+        }
         trace_stream_one_iteration(s, sector_num, n, ret);
         if (ret == 0) {
             if (s->common.speed) {
@@ -114,6 +125,7 @@ retry:
         if (ret < 0) {
             break;
         }
+        ret = 0;
 
         /* Publish progress */
         s->common.offset += n * BDRV_SECTOR_SIZE;
@@ -127,7 +139,11 @@ retry:
     bdrv_disable_copy_on_read(bs);
 
     if (sector_num == end && ret == 0) {
-        ret = bdrv_change_backing_file(bs, NULL, NULL);
+        const char *base_id = NULL;
+        if (base) {
+            base_id = s->backing_file_id;
+        }
+        ret = bdrv_change_backing_file(bs, base_id, NULL);
     }
 
     qemu_vfree(buf);
@@ -153,7 +169,8 @@ static BlockJobType stream_job_type = {
 };
 
 int stream_start(BlockDriverState *bs, BlockDriverState *base,
-                 BlockDriverCompletionFunc *cb, void *opaque)
+                 const char *base_id, BlockDriverCompletionFunc *cb,
+                 void *opaque)
 {
     StreamBlockJob *s;
     Coroutine *co;
@@ -164,6 +181,9 @@ int stream_start(BlockDriverState *bs, BlockDriverState *base,
 
     s = block_job_create(&stream_job_type, bs, cb, opaque);
     s->base = base;
+    if (base_id) {
+        pstrcpy(s->backing_file_id, sizeof(s->backing_file_id), base_id);
+    }
 
     co = qemu_coroutine_create(stream_run);
     trace_stream_start(bs, base, s, co, opaque);
diff --git a/block_int.h b/block_int.h
index c7c9178..ed92884 100644
--- a/block_int.h
+++ b/block_int.h
@@ -333,6 +333,7 @@ void block_job_cancel(BlockJob *job);
 bool block_job_is_cancelled(BlockJob *job);
 
 int stream_start(BlockDriverState *bs, BlockDriverState *base,
-                 BlockDriverCompletionFunc *cb, void *opaque);
+                 const char *base_id, BlockDriverCompletionFunc *cb,
+                 void *opaque);
 
 #endif /* BLOCK_INT_H */
diff --git a/blockdev.c b/blockdev.c
index 45a6ba6..5da1097 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -956,6 +956,7 @@ void qmp_block_stream(const char *device, bool has_base,
                       const char *base, Error **errp)
 {
     BlockDriverState *bs;
+    BlockDriverState *base_bs = NULL;
     int ret;
 
     bs = bdrv_find(device);
@@ -964,13 +965,15 @@ void qmp_block_stream(const char *device, bool has_base,
         return;
     }
 
-    /* Base device not supported */
     if (base) {
-        error_set(errp, QERR_NOT_SUPPORTED);
-        return;
+        base_bs = bdrv_find_backing_image(bs, base);
+        if (base_bs == NULL) {
+            error_set(errp, QERR_BASE_ID_NOT_FOUND, base);
+            return;
+        }
     }
 
-    ret = stream_start(bs, NULL, block_stream_cb, bs);
+    ret = stream_start(bs, base_bs, base, block_stream_cb, bs);
     if (ret < 0) {
         switch (ret) {
         case -EBUSY:
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH v4 13/15] block stream: add support for partial streaming

[Kevin Wolf]

Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
> From: Marcelo Tosatti <mtosatti@redhat.com>
> 
> Add support for streaming data from an intermediate section of the
> image chain (see patch and documentation for details).
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  block.c        |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block.h        |    4 +++
>  block/stream.c |   28 +++++++++++++++++++++---
>  block_int.h    |    3 +-
>  blockdev.c     |   11 ++++++---
>  5 files changed, 101 insertions(+), 9 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 9b688a0..d2143b1 100644
> --- a/block.c
> +++ b/block.c
> @@ -2263,6 +2263,70 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
>      return data.ret;
>  }
>  
> +/*
> + * Given an image chain: [BASE] -> [INTER1] -> [INTER2] -> [TOP]
> + *
> + * Return true if the given sector is allocated in top or base.
> + * Return false if the given sector is allocated in intermediate images.

This description is inexact. A sector could be allocated both in base in
an intermediate image.

Also initially I thought that we not only need to check whether the
sector is allocated in BASE, but also in any parents of BASE. You don't
do this: Clusters that are completely unallocated all through the chain
are reported as allocated.

After reading all of the patch, I believe this provides the right
semantics: "Normal" image streaming would copy them into the topmost
file, but if you keep a backing file, you want to copy as little as
possible and keep using the backing file whenever possible.

So I suggest to fix the description rather than the implementation.

Maybe we should also rename the function. With this semantics it's not a
generic is_allocated function any more, but something quite specific to
streaming with a base file.

Kevin

Re: [Qemu-devel] [PATCH v4 13/15] block stream: add support for partial streaming

[Stefan Hajnoczi]

On Thu, Jan 12, 2012 at 12:42 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 06.01.2012 15:01, schrieb Stefan Hajnoczi:
>> From: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> Add support for streaming data from an intermediate section of the
>> image chain (see patch and documentation for details).
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>> ---
>>  block.c        |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  block.h        |    4 +++
>>  block/stream.c |   28 +++++++++++++++++++++---
>>  block_int.h    |    3 +-
>>  blockdev.c     |   11 ++++++---
>>  5 files changed, 101 insertions(+), 9 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 9b688a0..d2143b1 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2263,6 +2263,70 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
>>      return data.ret;
>>  }
>>
>> +/*
>> + * Given an image chain: [BASE] -> [INTER1] -> [INTER2] -> [TOP]
>> + *
>> + * Return true if the given sector is allocated in top or base.
>> + * Return false if the given sector is allocated in intermediate images.
>
> This description is inexact. A sector could be allocated both in base in
> an intermediate image.
>
> Also initially I thought that we not only need to check whether the
> sector is allocated in BASE, but also in any parents of BASE. You don't
> do this: Clusters that are completely unallocated all through the chain
> are reported as allocated.
>
> After reading all of the patch, I believe this provides the right
> semantics: "Normal" image streaming would copy them into the topmost
> file, but if you keep a backing file, you want to copy as little as
> possible and keep using the backing file whenever possible.
>
> So I suggest to fix the description rather than the implementation.
>
> Maybe we should also rename the function. With this semantics it's not a
> generic is_allocated function any more, but something quite specific to
> streaming with a base file.

I have moved the function into block/stream.c and renamed it to just
is_allocated_base().  The description is updated.

This makes it clearer that it's a special-case is_allocated-like function.

Stefan

[Qemu-devel] [PATCH v4 14/15] add doc to describe live block operations

[Stefan Hajnoczi]

From: Marcelo Tosatti <mtosatti@redhat.com>

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 docs/live-block-ops.txt |   58 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 58 insertions(+), 0 deletions(-)
 create mode 100644 docs/live-block-ops.txt

diff --git a/docs/live-block-ops.txt b/docs/live-block-ops.txt
new file mode 100644
index 0000000..a257087
--- /dev/null
+++ b/docs/live-block-ops.txt
@@ -0,0 +1,58 @@
+LIVE BLOCK OPERATIONS
+=====================
+
+High level description of live block operations. Note these are not
+supported for use with the raw format at the moment.
+
+Snapshot live merge
+===================
+
+Given a snapshot chain, described in this document in the following
+format:
+
+[A] -> [B] -> [C] -> [D]
+
+Where the rightmost object ([D] in the example) described is the current
+image which the guest OS has write access to. To the left of it is its base
+image, and so on accordingly until the leftmost image, which has no
+base.
+
+The snapshot live merge operation transforms such a chain into a
+smaller one with fewer elements, such as this transformation relative
+to the first example:
+
+[A] -> [D]
+
+Currently only forward merge with target being the active image is
+supported, that is, data copy is performed in the right direction with
+destination being the rightmost image.
+
+The operation is implemented in QEMU through image streaming facilities.
+
+The basic idea is to execute 'block_stream virtio0' while the guest is
+running. Progress can be monitored using 'info block-jobs'. When the
+streaming operation completes it raises a QMP event. 'block_stream'
+copies data from the backing file(s) into the active image. When finished,
+it adjusts the backing file pointer.
+
+The 'base' parameter specifies an image which data need not be streamed from.
+This image will be used as the backing file for the active image when the
+operation is finished.
+
+In the example above, the command would be:
+
+(qemu) block_stream virtio0 A
+
+
+Live block copy
+===============
+
+To copy an in use image to another destination in the filesystem, one
+should create a live snapshot in the desired destination, then stream
+into that image. Example:
+
+(qemu) snapshot_blkdev ide0-hd0 /new-path/disk.img qcow2
+
+(qemu) block_stream ide0-hd0
+
+
-- 
1.7.7.3

[Qemu-devel] [PATCH v4 15/15] test: add image streaming test cases

[Stefan Hajnoczi]

python test-stream.py

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 test-stream.py |  208 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 208 insertions(+), 0 deletions(-)
 create mode 100644 test-stream.py

diff --git a/test-stream.py b/test-stream.py
new file mode 100644
index 0000000..16cbe5d
--- /dev/null
+++ b/test-stream.py
@@ -0,0 +1,208 @@
+#!/usr/bin/env python
+import unittest
+import subprocess
+import re
+import os
+import sys; sys.path.append('QMP/')
+import qmp
+
+def qemu_img(*args):
+    devnull = open('/dev/null', 'r+')
+    return subprocess.call(['./qemu-img'] + list(args), stdin=devnull, stdout=devnull)
+
+def qemu_io(*args):
+    args = ['./qemu-io'] + list(args)
+    return subprocess.Popen(args, stdout=subprocess.PIPE).communicate()[0]
+
+class VM(object):
+    def __init__(self):
+        self._monitor_path = '/tmp/qemu-mon.%d' % os.getpid()
+        self._qemu_log_path = '/tmp/qemu-log.%d' % os.getpid()
+        self._args = ['x86_64-softmmu/qemu-system-x86_64',
+                      '-chardev', 'socket,id=mon,path=' + self._monitor_path,
+                      '-mon', 'chardev=mon,mode=control', '-nographic']
+        self._num_drives = 0
+
+    def add_drive(self, path, opts=''):
+        options = ['if=virtio',
+                   'cache=none',
+                   'file=%s' % path,
+                   'id=drive%d' % self._num_drives]
+        if opts:
+            options.append(opts)
+
+        self._args.append('-drive')
+        self._args.append(','.join(options))
+        self._num_drives += 1
+        return self
+
+    def launch(self):
+        devnull = open('/dev/null', 'rb')
+        qemulog = open(self._qemu_log_path, 'wb')
+        self._qmp = qmp.QEMUMonitorProtocol(self._monitor_path, server=True)
+        self._popen = subprocess.Popen(self._args, stdin=devnull, stdout=qemulog,
+                                       stderr=subprocess.STDOUT)
+        self._qmp.accept()
+
+    def shutdown(self):
+        self._qmp.cmd('quit')
+        self._popen.wait()
+        os.remove(self._monitor_path)
+        os.remove(self._qemu_log_path)
+
+    def qmp(self, cmd, **args):
+        return self._qmp.cmd(cmd, args=args)
+
+    def get_qmp_events(self, wait=False):
+        events = self._qmp.get_events(wait=wait)
+        self._qmp.clear_events()
+        return events
+
+index_re = re.compile(r'([^\[]+)\[([^\]]+)\]')
+
+class QMPTestCase(unittest.TestCase):
+    def dictpath(self, d, path):
+        """Traverse a path in a nested dict"""
+        for component in path.split('/'):
+            m = index_re.match(component)
+            if m:
+                component, idx = m.groups()
+                idx = int(idx)
+
+            if not isinstance(d, dict) or component not in d:
+                self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
+            d = d[component]
+
+            if m:
+                if not isinstance(d, list):
+                    self.fail('path component "%s" in "%s" is not a list in "%s"' % (component, path, str(d)))
+                try:
+                    d = d[idx]
+                except IndexError:
+                    self.fail('invalid index "%s" in path "%s" in "%s"' % (idx, path, str(d)))
+        return d
+
+    def assert_qmp(self, d, path, value):
+        result = self.dictpath(d, path)
+        self.assertEqual(result, value, 'values not equal "%s" and "%s"' % (str(result), str(value)))
+
+    def assert_no_active_streams(self):
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return', [])
+
+class TestSingleDrive(QMPTestCase):
+    image_len = 1 * 1024 * 1024 # MB
+
+    def setUp(self):
+        qemu_img('create', 'backing.img', str(TestSingleDrive.image_len))
+        qemu_img('create', '-f', 'qed', '-o', 'backing_file=backing.img', 'test.qed')
+        self.vm = VM().add_drive('test.qed')
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove('test.qed')
+        os.remove('backing.img')
+
+    def test_stream(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block_stream', 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.assertFalse('sectors not allocated' in qemu_io('-c', 'map', 'test.qed'),
+                         'image file not fully populated after streaming')
+
+    def test_device_not_found(self):
+        result = self.vm.qmp('block_stream', device='nonexistent')
+        self.assert_qmp(result, 'error/class', 'DeviceNotFound')
+
+class TestStreamStop(QMPTestCase):
+    image_len = 8 * 1024 * 1024 * 1024 # GB
+
+    def setUp(self):
+        qemu_img('create', 'backing.img', str(TestStreamStop.image_len))
+        qemu_img('create', '-f', 'qed', '-o', 'backing_file=backing.img', 'test.qed')
+        self.vm = VM().add_drive('test.qed')
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove('test.qed')
+        os.remove('backing.img')
+
+    def test_stream_stop(self):
+        import time
+
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block_stream', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        time.sleep(1)
+        events = self.vm.get_qmp_events(wait=False)
+        self.assertEqual(events, [], 'unexpected QMP event: %s' % events)
+
+        self.vm.qmp('block_job_cancel', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        cancelled = False
+        while not cancelled:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_CANCELLED':
+                    self.assert_qmp(event, 'data/type', 'stream')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    cancelled = True
+
+        self.assert_no_active_streams()
+
+class TestSetSpeed(QMPTestCase):
+    image_len = 80 * 1024 * 1024 # MB
+
+    def setUp(self):
+        qemu_img('create', 'backing.img', str(TestSetSpeed.image_len))
+        qemu_img('create', '-f', 'qed', '-o', 'backing_file=backing.img', 'test.qed')
+        self.vm = VM().add_drive('test.qed')
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove('test.qed')
+        os.remove('backing.img')
+
+    # This doesn't print or verify anything, only use it via "test-stream.py TestSetSpeed"
+    def test_stream_set_speed(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_set_speed', device='drive0', value=8 * 1024 * 1024)
+        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()
+
+if __name__ == '__main__':
+    unittest.main()
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH v4 00/15] block: generic image streaming

[Luiz Capitulino]

On Fri,  6 Jan 2012 14:01:26 +0000
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote:

> This series adds the 'block_stream' command which copies the contents of a
> backing file into the image file while the VM is running.  The series builds on
> the zero detection features which I sent out before Christmas. I suggest
> grabbing my git tree to try it out without merging this dependency:
> 
> https://github.com/stefanha/qemu/tree/image-streaming-api
> 
> The image streaming HMP/QMP commands are documented in the patch and also
> described here:
> 
> http://wiki.qemu.org/Features/LiveBlockMigration/ImageStreamingAPI

From a QMP perspective (and given that we'll make this obsolete when
we have a proper async API):

 Acked-by: Luiz Capitulino <lcapitulino@redhat.com>

> 
> The basic idea is to execute 'block_stream virtio0' while the guest is running.
> Progress can be monitored using 'info block-jobs'.  When the streaming
> operation completes it raises a QMP event.
> 
> Note: The last patch includes includes a Python test script called
> test-stream.py, I do not propose to merge it.  When run in a QEMU source tree
> it performs basic image streaming QMP tests.
> 
> v4:
>  * Drop SQMP/EQMP docs from qmp-commands.hx [Luiz]
>  * Follow QAPI doc conventions [Luiz]
>  * Document QMP events in QMP/qmp-events.txt [Luiz]
>  * Protect against hotplug, resize, eject, etc [Kevin]
>  * Move block job functions from header to block.c [Kevin]
>  * Return error from bdrg_change_backing_file() [Kevin]
>  * Merge Marcelo's block_stream base partial streaming series [Marcelo]
> 
> Marcelo Tosatti (4):
>   block: add bdrv_find_backing_image
>   add QERR_BASE_ID_NOT_FOUND
>   block stream: add support for partial streaming
>   add doc to describe live block operations
> 
> Stefan Hajnoczi (11):
>   coroutine: add co_sleep_ns() coroutine sleep function
>   block: check bdrv_in_use() before blockdev operations
>   block: add BlockJob interface for long-running operations
>   block: add image streaming block job
>   block: rate-limit streaming operations
>   qmp: add block_stream command
>   qmp: add block_job_set_speed command
>   qmp: add block_job_cancel command
>   qmp: add query-block-jobs
>   blockdev: make image streaming safe across hotplug
>   test: add image streaming test cases
> 
>  Makefile.objs           |    2 +
>  QMP/qmp-events.txt      |   53 ++++++++++++
>  block.c                 |  133 ++++++++++++++++++++++++++++++
>  block.h                 |    5 +
>  block/stream.c          |  192 +++++++++++++++++++++++++++++++++++++++++++
>  block_int.h             |   44 ++++++++++
>  blockdev.c              |  199 ++++++++++++++++++++++++++++++++++++++++++++-
>  docs/live-block-ops.txt |   58 +++++++++++++
>  hmp-commands.hx         |   41 +++++++++
>  hmp.c                   |   68 +++++++++++++++
>  hmp.h                   |    4 +
>  monitor.c               |   13 +++
>  monitor.h               |    2 +
>  qapi-schema.json        |  115 ++++++++++++++++++++++++++
>  qemu-coroutine-sleep.c  |   38 +++++++++
>  qemu-coroutine.h        |    6 ++
>  qerror.c                |    8 ++
>  qerror.h                |    6 ++
>  qmp-commands.hx         |   24 ++++++
>  test-stream.py          |  208 +++++++++++++++++++++++++++++++++++++++++++++++
>  trace-events            |    9 ++
>  21 files changed, 1227 insertions(+), 1 deletions(-)
>  create mode 100644 block/stream.c
>  create mode 100644 docs/live-block-ops.txt
>  create mode 100644 qemu-coroutine-sleep.c
>  create mode 100644 test-stream.py
>