From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:49491) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QQbd9-0004S5-Of for qemu-devel@nongnu.org; Sun, 29 May 2011 04:45:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QQbd8-0006ls-PJ for qemu-devel@nongnu.org; Sun, 29 May 2011 04:45:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:23313) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QQbd8-0006lo-IM for qemu-devel@nongnu.org; Sun, 29 May 2011 04:45:50 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p4T8jmBG032270 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 29 May 2011 04:45:48 -0400 Message-ID: <4DE207B8.1070902@redhat.com> Date: Sun, 29 May 2011 11:45:44 +0300 From: Avi Kivity MIME-Version: 1.0 References: <20110523213115.164535428@amt.cnet> <20110523213410.726580546@amt.cnet> In-Reply-To: <20110523213410.726580546@amt.cnet> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [patch 2/7] Add blkmirror block driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcelo Tosatti Cc: kwolf@redhat.com, Jes.Sorensen@redhat.com, dlaor@redhat.com, qemu-devel@nongnu.org On 05/24/2011 12:31 AM, Marcelo Tosatti wrote: > Mirrored writes are used by live block copy. > > > + > +typedef struct { > + BlockDriverState *bs[2]; > +} BdrvMirrorState; Can be generalized one day to N. > + > +/* Valid blkmirror filenames look like blkmirror:path/to/image1:path/to/image2 */ > +static int blkmirror_open(BlockDriverState *bs, const char *filename, int flags) > +{ > + BdrvMirrorState *m = bs->opaque; > + int ret; > + char *raw, *c; > + > + /* Parse the blkmirror: prefix */ > + if (strncmp(filename, "blkmirror:", strlen("blkmirror:"))) { Actual format should be documented somewhere. > + return -EINVAL; > + } > + filename += strlen("blkmirror:"); > + > + /* Parse the raw image filename */ > + c = strchr(filename, ':'); > + if (c == NULL) { > + return -EINVAL; > + } > + > + raw = strdup(filename); > + raw[c - filename] = '\0'; > + ret = bdrv_file_open(&m->bs[0], raw, flags); > + free(raw); > + if (ret< 0) { > + return ret; > + } > + filename = c + 1; > + > + ret = bdrv_file_open(&m->bs[1], filename, flags); > + if (ret< 0) { > + bdrv_delete(m->bs[0]); > + return ret; > + } > + > + return 0; > +} > + > > + > +static BlockDriverAIOCB *blkmirror_aio_readv(BlockDriverState *bs, > + int64_t sector_num, > + QEMUIOVector *qiov, > + int nb_sectors, > + BlockDriverCompletionFunc *cb, > + void *opaque) > +{ > + BdrvMirrorState *m = bs->opaque; > + return bdrv_aio_readv(m->bs[0], sector_num, qiov, nb_sectors, cb, opaque); We could one day be clever and round-robin once we're synced. > +} > + > +typedef struct DupAIOCB { > + BlockDriverAIOCB common; > + int count; > + > + BlockDriverCompletionFunc *cb; > + void *opaque; > + > + BlockDriverAIOCB *src_aiocb; > + BlockDriverAIOCB *dest_aiocb; > + > + int ret; > +} DupAIOCB; > + > +static void dup_aio_cancel(BlockDriverAIOCB *blockacb) > +{ > + DupAIOCB *acb = container_of(blockacb, DupAIOCB, common); > + > + bdrv_aio_cancel(acb->src_aiocb); > + bdrv_aio_cancel(acb->dest_aiocb); Shouldn't we cancel just the ones that haven't completed yet? > + qemu_aio_release(acb); > +} > + > +static AIOPool dup_aio_pool = { > + .aiocb_size = sizeof(DupAIOCB), > + .cancel = dup_aio_cancel, > +}; > + > +static void blkmirror_aio_cb(void *opaque, int ret) > +{ > + DupAIOCB *dcb = opaque; > + > + dcb->count--; > + assert(dcb->count>= 0); > + if (dcb->count == 0) { > + if (dcb->ret< 0) { > + ret = dcb->ret; > + } > + dcb->common.cb(dcb->opaque, ret); > + qemu_aio_release(dcb); > + } > + dcb->ret = ret; > +} It would be nicer to do if (ret < 0) { dcb->ret = ret; } unconditionally. The current code works, but only for N=2, and is a little obfuscated. I see you aren't keeping sync/unsync state here. Will read on. -- error compiling committee.c: too many arguments to function