From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52509) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R1yVG-0002xP-Jt for qemu-devel@nongnu.org; Fri, 09 Sep 2011 06:40:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R1yVF-0003Lz-HR for qemu-devel@nongnu.org; Fri, 09 Sep 2011 06:40:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18222) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R1yVF-0003Lf-74 for qemu-devel@nongnu.org; Fri, 09 Sep 2011 06:40:09 -0400 Message-ID: <4E69EDB1.8020000@redhat.com> Date: Fri, 09 Sep 2011 12:42:57 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1315495505-28906-1-git-send-email-pbonzini@redhat.com> <4E69D595.8090000@redhat.com> <4E69EA89.6090706@redhat.com> In-Reply-To: <4E69EA89.6090706@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 00/12] nbd improvements List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, Nicholas Thomas Am 09.09.2011 12:29, schrieb Paolo Bonzini: > On 09/09/2011 11:00 AM, Kevin Wolf wrote: >> There is anonther patch enabling AIO for NBD on the list [1], by >> Nicholas Thomas (CCed), that lacked review so far. Can you guys please >> review each others approach and then converge on a solution? I guess >> Paolo's patches 1-7 can be applied in any case, probably causing minor >> conflicts, but for the rest we need to decide which one to pick. > > Stefan also pointed me to Nicholas's patches yesterday. I would go with > mine, if only because his work predates coroutines (at least the older > versions) and are much more complex. > > On the other hand, Nicholas's work includes timeout and reconnect. I'm > not sure how complicated it is to include it in my series, but probably > not much. With coroutines, preserving the list of outstanding I/O > requests is done implicitly by the CoMutex, so you basically have to > check errno for ECONNRESET and similar errors, reconnect, and retry > issuing the current request only. > > Timeout can be done with a QEMUTimer that shuts down the socket > (shutdown(2) I mean). This triggers an EPIPE when writing, or a > zero-sized read when reading. The timeout can be set every time the > coroutine is (re)entered, and reset before exiting nbd_co_readv/writev. > > What do you think? I haven't really had a look at your patches yet (I hope to do so later today), but from the patch descriptions I would think that indeed your patches could be the better base for a converged series. What I would like you and Nick to do is to see what is missing from your series compared to his one (you describe a couple of things, but let's see if Nick knows anything else) and to agree on the next steps. I think that a possible way is to merge your series and do the other improvements on top, but as I said I haven't really looked into the details yet. Kevin