From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34220) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UQL9Q-0000Eu-CI for qemu-devel@nongnu.org; Thu, 11 Apr 2013 13:19:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UQL9L-0002ax-5z for qemu-devel@nongnu.org; Thu, 11 Apr 2013 13:19:08 -0400 Received: from mail-pa0-f50.google.com ([209.85.220.50]:47544) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UQL9K-0002YF-TT for qemu-devel@nongnu.org; Thu, 11 Apr 2013 13:19:03 -0400 Received: by mail-pa0-f50.google.com with SMTP id bg2so996501pad.23 for ; Thu, 11 Apr 2013 10:19:01 -0700 (PDT) Message-ID: <5166F0BB.9060007@inktank.com> Date: Thu, 11 Apr 2013 10:19:55 -0700 From: Josh Durgin MIME-Version: 1.0 References: <1364543983-8180-1-git-send-email-josh.durgin@inktank.com> <1364587403-30689-1-git-send-email-josh.durgin@inktank.com> <20130402141042.GL2341@dhcp-200-207.str.redhat.com> <5165713B.8070003@inktank.com> <20130411080232.GB5253@stefanha-thinkpad.redhat.com> <20130411084844.GD2449@dhcp-200-207.str.redhat.com> In-Reply-To: <20130411084844.GD2449@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] rbd: add an asynchronous flush List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, Stefan Hajnoczi On 04/11/2013 01:48 AM, Kevin Wolf wrote: > Am 11.04.2013 um 10:02 hat Stefan Hajnoczi geschrieben: >> On Wed, Apr 10, 2013 at 07:03:39AM -0700, Josh Durgin wrote: >>> On 04/02/2013 07:10 AM, Kevin Wolf wrote: >>>> Am 29.03.2013 um 21:03 hat Josh Durgin geschrieben: >>>>> The existing bdrv_co_flush_to_disk implementation uses rbd_flush(), >>>>> which is sychronous and causes the main qemu thread to block until it >>>>> is complete. This results in unresponsiveness and extra latency for >>>>> the guest. >>>>> >>>>> Fix this by using an asynchronous version of flush. This was added to >>>>> librbd with a special #define to indicate its presence, since it will >>>>> be backported to stable versions. Thus, there is no need to check the >>>>> version of librbd. >>>> >>>> librbd is linked dynamically and the version on the build host isn't >>>> necessarily the same as the version qemu is run with. So shouldn't this >>>> better be a runtime check? >>> >>> While we discuss runtime loading separately, would you mind taking this >>> patch as-is for now? >> >> Hi Josh, >> I'm happy with Patch v3 1/2. Does that work for you? > > Only patch 1/2 would add dead code as .bdrv_aio_flush would never be > called. I think we should rather take v1 of the series then, with the > #ifdefs at build time. Yes, that would be a problem with only v3 1/2. v2 of the original series is fine, but v1 was accidentally missing a hunk. To be clear, I think just http://patchwork.ozlabs.org/patch/232489/ would be good. Thanks, Josh