From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52809) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S2kxW-0003XZ-Ke for qemu-devel@nongnu.org; Wed, 29 Feb 2012 09:56:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S2kx3-0007Yz-Mi for qemu-devel@nongnu.org; Wed, 29 Feb 2012 09:56:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:15354) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S2kx3-0007YZ-EP for qemu-devel@nongnu.org; Wed, 29 Feb 2012 09:56:21 -0500 Message-ID: <4F4E3C8D.2090105@redhat.com> Date: Wed, 29 Feb 2012 15:56:13 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1330522650-15357-1-git-send-email-pbonzini@redhat.com> <1330522650-15357-6-git-send-email-pbonzini@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/6] Add blkmirror block driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, stefanha@linux.vnet.ibm.com, jcody@redhat.com, Marcelo Tosatti , qemu-devel@nongnu.org, fsimonce@redhat.com, eblake@redhat.com Il 29/02/2012 15:37, Stefan Hajnoczi ha scritto: >> Makefile.objs | 2 +- >> block/blkmirror.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> docs/blkmirror.txt | 16 ++++++ >> 3 files changed, 170 insertions(+), 1 deletions(-) >> create mode 100644 block/blkmirror.c >> create mode 100644 docs/blkmirror.txt > > Mostly happy here, I just recommend tweaking the name of this block > driver and documenting clearly that this is not a general-purpose > mirroring driver, given that it points image B's backing file at image > A's backing file. I see this driver as internal functionality and > it's fairly easy for users to misuse it and be surprised by the > results. I was thinking of adding something like no_user later. And of course docs/blkmirror.txt is totally obsolete, I'll merge it into block/blkmirror.c. >> +static int blkmirror_co_writev(BlockDriverState *bs, >> + int64_t sector_num, int nb_sectors, >> + QEMUIOVector *qiov) >> +{ >> + int ret; >> + >> + /* bs->backing_hd is set after initialization. */ >> + bs->file->backing_hd = bs->backing_hd; >> + >> + ret = bdrv_co_writev(bs->backing_hd, sector_num, nb_sectors, qiov); >> + if (ret >= 0) { >> + ret = bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov); >> + } >> + >> + return ret; >> +} > > Have you done performance tests? It seems suboptimal to use > .bdrv_co_writev() and perform writes sequentially, even with > cache=unsafe. No, not yet. I can add back AIO here. Paolo