[Qemu-devel] [PATCH v3 2/5] qemu-img: Implement commit like QMP

[Max Reitz]

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.

This new implementation does not empty the snapshot image, as opposed to
the old implementation using bdrv_commit(). However, as QMP's
block-commit apparently never did this and as qcow2 (which is probably
qemu's standard image format) does not even implement the required
function (bdrv_make_empty()), it does not seem necessary.

Signed-off-by: Max Reitz <address@hidden>
---
 block/Makefile.objs |  2 +-
 qemu-img.c          | 90 ++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 72 insertions(+), 20 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index fd88c03..2c37e80 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -9,6 +9,7 @@ block-obj-y += 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 += mirror.o
 
 ifeq ($(CONFIG_POSIX),y)
 block-obj-y += nbd.o nbd-client.o sheepdog.o
@@ -22,7 +23,6 @@ endif
 
 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 8455994..026742a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -30,6 +30,7 @@
 #include "qemu/osdep.h"
 #include "sysemu/sysemu.h"
 #include "block/block_int.h"
+#include "block/blockjob.h"
 #include "block/qapi.h"
 #include <getopt.h>
 
@@ -682,12 +683,53 @@ fail:
     return ret;
 }
 
+struct CommonBlockJobCBInfo {
+    Error **errp;
+    bool done;
+};
+
+static void common_block_job_cb(void *opaque, int ret)
+{
+    struct CommonBlockJobCBInfo *cbi = opaque;
+
+    if (ret < 0) {
+        error_setg_errno(cbi->errp, -ret, "Block job failed");
+    }
+
+    cbi->done = true;
+}
+
+static void run_block_job(BlockJob *job, struct CommonBlockJobCBInfo *cbi)
+{
+    BlockJobInfo *info;
+
+    do {
+        aio_poll(qemu_get_aio_context(), true);
+
+        info = block_job_query(job);
+
+        if (!info->busy && info->offset < info->len) {
+            block_job_resume(job);
+        }
+    } while (info->offset < info->len);
+
+    block_job_complete(job, cbi->errp);
+
+    while (!cbi->done) {
+        aio_poll(qemu_get_aio_context(), true);
+    }
+}
+
+/* Same as in block.c */
+#define COMMIT_BUF_SECTORS 2048
+
 static int img_commit(int argc, char **argv)
 {
     int c, ret, flags;
     const char *filename, *fmt, *cache;
-    BlockDriverState *bs;
+    BlockDriverState *bs, *base_bs;
     bool quiet = false;
+    Error *local_err = NULL;
 
     fmt = NULL;
     cache = BDRV_DEFAULT_CACHE;
@@ -728,29 +770,39 @@ static int img_commit(int argc, char **argv)
     if (!bs) {
         return 1;
     }
-    ret = bdrv_commit(bs);
-    switch(ret) {
-    case 0:
-        qprintf(quiet, "Image committed.\n");
-        break;
-    case -ENOENT:
-        error_report("No disk inserted");
-        break;
-    case -EACCES:
-        error_report("Image is read-only");
-        break;
-    case -ENOTSUP:
-        error_report("Image is already committed");
-        break;
-    default:
-        error_report("Error while committing image");
-        break;
+
+    /* This is different from QMP, which by default uses the deepest file in 
the
+     * backing chain (i.e., the very base); however, the traditional behavior 
of
+     * qemu-img commit is using the immediate backing file. */
+    base_bs = bs->backing_hd;
+    if (!base_bs) {
+        error_set(&local_err, QERR_BASE_NOT_FOUND, "NULL");
+        goto done;
     }
 
+    struct CommonBlockJobCBInfo cbi = {
+        .errp = &local_err,
+    };
+
+    commit_active_start(bs, base_bs, 0, COMMIT_BUF_SECTORS << BDRV_SECTOR_BITS,
+                        BLOCKDEV_ON_ERROR_REPORT, common_block_job_cb, &cbi,
+                        &local_err);
+    if (local_err) {
+        goto done;
+    }
+
+    run_block_job(bs->job, &cbi);
+
+done:
     bdrv_unref(bs);
-    if (ret) {
+
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return 1;
     }
+
+    qprintf(quiet, "Image committed.\n");
     return 0;
 }
 
-- 
1.9.1