On 08.04.2014 17:14, Kevin Wolf wrote:
Am 08.04.2014 um 14:50 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.
Leaves us with the HMP commit command that uses the old bdrv_commit()
function. I wonder if we can get rid of it by letting the HMP command
stop the VM, do a live commit, and then restart the VM.
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.
In fact, I think since qcow2 has discard support it would actually be
possible to write a sensible implementation of bdrv_make_empty(). That's
a separate feature, though, and can go in a different patch series.
Signed-off-by: Max Reitz <mre...@redhat.com>
---
block/Makefile.objs | 2 +-
qemu-img.c | 70 ++++++++++++++++++++++++++++++++++++++---------------
2 files changed, 52 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..e86911f 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,37 @@ fail:
return ret;
}
+static void dummy_block_job_cb(void *opaque, int ret)
+{
+}
Why don't we need to check the return value?
I didn't check it, because it was not called – which apparently didn't
sound strange to me at all. I checked and the much more interesting fact
is that I assumed block_job_complete() would actually complete the block
job without any further need for aio_poll(); but it doesn't. I'll fix
both things (calling aio_poll() until the CB is called and checking the
return value).
Max
Kevin