Re: [Qemu-devel] [PATCH v13 08/14] qemu-img: Implement commit like QMP

[Kevin Wolf <kwolf@redhat.com>]

Am 23.10.2014 um 14:35 hat Max Reitz geschrieben:
> On 2014-10-23 at 13:59, Kevin Wolf wrote:
> >Am 22.10.2014 um 14:51 hat Max Reitz geschrieben:
> >>qemu-img should use QMP commands whenever possible in order to ensure
> >>feature completeness of both online and offline image operations. As
> >>qemu-img itself has no access to QMP (since this would basically require
> >>just everything being linked into qemu-img), imitate QMP's
> >>implementation of block-commit by using commit_active_start() and then
> >>waiting for the block job to finish.
> >>
> >>Signed-off-by: Max Reitz <mreitz@redhat.com>
> >>---
> >>  block/Makefile.objs |  3 +-
> >>  qemu-img.c          | 82 ++++++++++++++++++++++++++++++++++++++++-------------
> >>  2 files changed, 64 insertions(+), 21 deletions(-)
> >>
> >>diff --git a/block/Makefile.objs b/block/Makefile.objs
> >>index 27911b6..04b0e43 100644
> >>--- a/block/Makefile.objs
> >>+++ b/block/Makefile.objs
> >>@@ -9,7 +9,7 @@ block-obj-y += block-backend.o snapshot.o qapi.o
> >>  block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
> >>  block-obj-$(CONFIG_POSIX) += raw-posix.o
> >>  block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> >>-block-obj-y += null.o
> >>+block-obj-y += null.o mirror.o
> >>  block-obj-y += nbd.o nbd-client.o sheepdog.o
> >>  block-obj-$(CONFIG_LIBISCSI) += iscsi.o
> >>@@ -23,7 +23,6 @@ block-obj-y += accounting.o
> >>  common-obj-y += stream.o
> >>  common-obj-y += commit.o
> >>-common-obj-y += mirror.o
> >>  common-obj-y += backup.o
> >>  iscsi.o-cflags     := $(LIBISCSI_CFLAGS)
> >>diff --git a/qemu-img.c b/qemu-img.c
> >>index 09e7e72..f1f2857 100644
> >>--- a/qemu-img.c
> >>+++ b/qemu-img.c
> >>@@ -31,6 +31,7 @@
> >>  #include "sysemu/sysemu.h"
> >>  #include "sysemu/block-backend.h"
> >>  #include "block/block_int.h"
> >>+#include "block/blockjob.h"
> >>  #include "block/qapi.h"
> >>  #include <getopt.h>
> >>@@ -715,13 +716,47 @@ fail:
> >>      return ret;
> >>  }
> >>+typedef struct CommonBlockJobCBInfo {
> >>+    BlockDriverState *bs;
> >>+    Error **errp;
> >>+} CommonBlockJobCBInfo;
> >>+
> >>+static void common_block_job_cb(void *opaque, int ret)
> >>+{
> >>+    CommonBlockJobCBInfo *cbi = opaque;
> >>+
> >>+    if (ret < 0) {
> >>+        error_setg_errno(cbi->errp, -ret, "Block job failed");
> >>+    }
> >>+
> >>+    /* Drop this block job's reference */
> >>+    bdrv_unref(cbi->bs);
> >>+}
> >>+
> >>+static void run_block_job(BlockJob *job, Error **errp)
> >>+{
> >>+    AioContext *aio_context = bdrv_get_aio_context(job->bs);
> >>+
> >>+    do {
> >>+        aio_poll(aio_context, true);
> >>+
> >>+        if (!job->busy && !job->ready) {
> >>+            block_job_resume(job);
> >>+        }
> >I wasn't quite sure what this is for. With BLOCKDEV_ON_ERROR_REPORT, why
> >would the job ever be paused?
> 
> I remember you telling that I puzzled this code together until it
> worked. At one point in time, it didn't work without this. It works
> now. I'm going to trust you.

This was an honest question. It's probably risky to trust that if I
don't understand something, the opposite is true...

Kevin