From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40850) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPvbv-0006bD-Q1 for qemu-devel@nongnu.org; Wed, 10 Apr 2013 10:02:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UPvbs-00026e-Tc for qemu-devel@nongnu.org; Wed, 10 Apr 2013 10:02:51 -0400 Received: from mail-pb0-f54.google.com ([209.85.160.54]:53455) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPvbs-00026Q-FB for qemu-devel@nongnu.org; Wed, 10 Apr 2013 10:02:48 -0400 Received: by mail-pb0-f54.google.com with SMTP id xa7so284096pbc.27 for ; Wed, 10 Apr 2013 07:02:47 -0700 (PDT) Message-ID: <5165713B.8070003@inktank.com> Date: Wed, 10 Apr 2013 07:03:39 -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> In-Reply-To: <20130402141042.GL2341@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/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? Josh >> Implement this as bdrv_aio_flush, since it matches other aio functions >> in the rbd block driver, and leave out bdrv_co_flush_to_disk when the >> asynchronous version is available. >> >> Reported-by: Oliver Francke >> Signed-off-by: Josh Durgin > > Looks good otherwise. > > Kevin >