From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42085) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhHkT-0000EN-11 for qemu-devel@nongnu.org; Thu, 23 Oct 2014 08:44:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XhHkM-0005Ix-Rx for qemu-devel@nongnu.org; Thu, 23 Oct 2014 08:44:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:3913) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhHkM-0005It-Ku for qemu-devel@nongnu.org; Thu, 23 Oct 2014 08:44:06 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9NCi5hh020011 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 23 Oct 2014 08:44:06 -0400 Message-ID: <5448F813.5000501@redhat.com> Date: Thu, 23 Oct 2014 14:44:03 +0200 From: Max Reitz MIME-Version: 1.0 References: <1413982283-10186-1-git-send-email-mreitz@redhat.com> <1413982283-10186-9-git-send-email-mreitz@redhat.com> <20141023115916.GK3522@noname.redhat.com> <5448F627.2080409@redhat.com> <20141023124000.GM3522@noname.redhat.com> In-Reply-To: <20141023124000.GM3522@noname.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v13 08/14] 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 2014-10-23 at 14:40, Kevin Wolf wrote: > 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 >>>> --- >>>> 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 >>>> @@ -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... I remember having had trouble with the block job just pausing and then qemu-img would sleep indefinitely. Maybe I'm mistaken. Maybe it actually was necessary back in April. Right now I don't see a way how the block job should be paused aside from errors and manual QMP intervention. Max