From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42632) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WYG2E-0006Eo-OQ for qemu-devel@nongnu.org; Thu, 10 Apr 2014 10:33:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WYG28-0001XB-Jh for qemu-devel@nongnu.org; Thu, 10 Apr 2014 10:32:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:62529) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WYG28-0001X0-8K for qemu-devel@nongnu.org; Thu, 10 Apr 2014 10:32:52 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s3AEWp47026281 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Thu, 10 Apr 2014 10:32:51 -0400 Message-ID: <5346AB8F.7090606@redhat.com> Date: Thu, 10 Apr 2014 16:32:47 +0200 From: Max Reitz MIME-Version: 1.0 References: <1396961442-24046-1-git-send-email-mreitz@redhat.com> <1396961442-24046-4-git-send-email-mreitz@redhat.com> <20140408151458.GE6262@noname.str.redhat.com> In-Reply-To: <20140408151458.GE6262@noname.str.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 3/6] qemu-img: Implement commit like QMP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, Stefan Hajnoczi 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 requi= re >> 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 >> --- >> 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 +=3D snapshot.o qapi.o >> block-obj-$(CONFIG_WIN32) +=3D raw-win32.o win32-aio.o >> block-obj-$(CONFIG_POSIX) +=3D raw-posix.o >> block-obj-$(CONFIG_LINUX_AIO) +=3D linux-aio.o >> +block-obj-y +=3D mirror.o >> =20 >> ifeq ($(CONFIG_POSIX),y) >> block-obj-y +=3D nbd.o nbd-client.o sheepdog.o >> @@ -22,7 +23,6 @@ endif >> =20 >> common-obj-y +=3D stream.o >> common-obj-y +=3D commit.o >> -common-obj-y +=3D mirror.o >> common-obj-y +=3D backup.o >> =20 >> iscsi.o-cflags :=3D $(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 >> =20 >> @@ -682,12 +683,37 @@ fail: >> return ret; >> } >> =20 >> +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 =96 which apparently didn't=20 sound strange to me at all. I checked and the much more interesting fact=20 is that I assumed block_job_complete() would actually complete the block=20 job without any further need for aio_poll(); but it doesn't. I'll fix=20 both things (calling aio_poll() until the CB is called and checking the=20 return value). Max > Kevin