From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43751) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1REhgL-0000ej-Ug for qemu-devel@nongnu.org; Fri, 14 Oct 2011 09:20:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1REhgE-0003yZ-7a for qemu-devel@nongnu.org; Fri, 14 Oct 2011 09:20:13 -0400 Received: from mtagate7.uk.ibm.com ([194.196.100.167]:52851) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1REhgD-0003wa-Up for qemu-devel@nongnu.org; Fri, 14 Oct 2011 09:20:06 -0400 Received: from d06nrmr1507.portsmouth.uk.ibm.com (d06nrmr1507.portsmouth.uk.ibm.com [9.149.38.233]) by mtagate7.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p9EDK3un024914 for ; Fri, 14 Oct 2011 13:20:03 GMT Received: from d06av12.portsmouth.uk.ibm.com (d06av12.portsmouth.uk.ibm.com [9.149.37.247]) by d06nrmr1507.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p9EDK3Gf2547874 for ; Fri, 14 Oct 2011 14:20:03 +0100 Received: from d06av12.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av12.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p9EDK3dv016683 for ; Fri, 14 Oct 2011 07:20:03 -0600 Date: Fri, 14 Oct 2011 14:20:02 +0100 From: Stefan Hajnoczi Message-ID: <20111014132002.GA1486@stefanha-thinkpad.localdomain> References: <1318581692-18338-1-git-send-email-pbonzini@redhat.com> <1318581692-18338-3-git-send-email-pbonzini@redhat.com> <4E98183E.5040303@redhat.com> <4E981D6D.1070407@redhat.com> <4E982302.1030100@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4E982302.1030100@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/4] block: unify flush implementations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Paolo Bonzini , qemu-devel@nongnu.org On Fri, Oct 14, 2011 at 01:54:42PM +0200, Kevin Wolf wrote: > Am 14.10.2011 13:30, schrieb Paolo Bonzini: > > On 10/14/2011 01:08 PM, Kevin Wolf wrote: > >> Am 14.10.2011 10:41, schrieb Paolo Bonzini: > > Let me show how this might go. Right now you have > > > > bdrv_read/write > > -> bdrv_rw_co > > -> create qiov > > -> create coroutine or just fast-path to bdrv_rw_co_entry > > -> bdrv_rw_co_entry > > -> bdrv_co_do_readv/writev > > -> driver > > > > bdrv_co_readv/writev > > -> bdrv_co_do_readv/writev > > -> driver > > > > But starting from here, you might just as well reorganize it like this: > > > > bdrv_read/writev > > -> bdrv_rw_co > > -> create qiov > > -> bdrv_readv/writev > > > > bdrv_readv/writev > > -> create coroutine or just fast-path to bdrv_rw_co_entry > > -> bdrv_rw_co_entry > > -> bdrv_co_do_readv/writev > > -> driver > > > > and just drop bdrv_co_readv, since it would just be hard-coding the > > fast-path of bdrv_readv. > > I guess it's all a matter of taste. Stefan, what do you think? > > > Since some amount of synchronous I/O would likely always be there, for > > example in qemu-img, I think this unification would make more sense than > > providing two separate entrypoints for bdrv_co_flush and bdrv_flush. > > Actually, I'm not so sure about qemu-img. I think we have thought of > scenarios where converting it to a coroutine based version with a main > loop would be helpful (can't remember the details, though). I'd like to completely remove synchronous bdrv_*(), including for qemu-img. It's just too tempting to call these functions in contexts where it is not okay to do so. The bdrv_co_*() functions are all tagged as coroutine_fn and make it clear that they can yield. We already have an event loop in qemu-img except it's the nested event loop in synchronous bdrv_*() emulation functions. The nested event loop is a mini event loop and can't really do things like timers. It would be nicer to remove it in favor of a single top-level event loop with the qemu-img code running in a coroutine. So I'm in favor of keeping bdrv_co_*() explicit for now and refactoring both hw/ and qemu-tool users of synchronous functions. Stefan