From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MmB1w-0002hE-Dp
	for qemu-devel@nongnu.org; Fri, 11 Sep 2009 14:39:32 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MmB1r-0002eT-Hj
	for qemu-devel@nongnu.org; Fri, 11 Sep 2009 14:39:31 -0400
Received: from [199.232.76.173] (port=42599 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MmB1r-0002eO-4j
	for qemu-devel@nongnu.org; Fri, 11 Sep 2009 14:39:27 -0400
Received: from verein.lst.de ([213.95.11.210]:41205)
	by monty-python.gnu.org with esmtps
	(TLS-1.0:DHE_RSA_3DES_EDE_CBC_SHA1:24) (Exim 4.60)
	(envelope-from <hch@lst.de>) id 1MmB1q-0008VN-Jc
	for qemu-devel@nongnu.org; Fri, 11 Sep 2009 14:39:26 -0400
Date: Fri, 11 Sep 2009 20:39:25 +0200
From: Christoph Hellwig <hch@lst.de>
Subject: Re: [Qemu-devel] [PATCH v3 2/2] virtio-blk: Use bdrv_aio_multiwrite
Message-ID: <20090911183925.GC13621@lst.de>
References: <1252511618-19497-1-git-send-email-kwolf@redhat.com>
	<1252511618-19497-3-git-send-email-kwolf@redhat.com>
	<20090910224141.GA25700@lst.de> <4AA9F7DC.1050608@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4AA9F7DC.1050608@redhat.com>
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>, qemu-devel@nongnu.org

On Fri, Sep 11, 2009 at 09:10:20AM +0200, Kevin Wolf wrote:
> >> +    blkreq[*num_writes].sector = req->out->sector;
> >> +    blkreq[*num_writes].nb_sectors = req->qiov.size / 512;
> >> +    blkreq[*num_writes].qiov = &req->qiov;
> >> +    blkreq[*num_writes].cb = virtio_blk_rw_complete;
> >> +    blkreq[*num_writes].opaque = req;
> >> +    blkreq[*num_writes].error = 0;
> >> +
> >> +    (*num_writes)++;
> > 
> > If you pass the completion routine to the function and map the error case
> > to calling completion routine (which is the usual way to handle errors
> > anyway) this function could become copletely generic.
> 
> Except that VirtIOBlockReq doesn't seem to be a type commonly used in
> generic code.

Yeah, we'd need to pass it only as opaque cookie and the qiov/setor
separately, making the whole thing look more similar to how the block
API works elsewhere.

> > Any chance to just use this batches subsmission unconditionally and
> > also for reads?  I'd hate to grow even more confusing I/O methods
> > in the block later.
> 
> If we want to completely obsolete bdrv_aio_readv/writev by batch
> submission functions (not only in block.c but also in each block
> driver), we certainly can do that. I think this would make a lot of
> sense, but it's quite some work and definitely out of scope for this
> patch which is basically meant to be a qcow2 performance fix.

I'm generally not a big fan of incomplete transitions, history tells
they will remaing incomplete for a long time or even forever and grow
more and more of the old calls.  The persistant existance of the non-AIO
block APIs in qemu is one of those cases..