From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38886) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agMFh-0008Ff-9q for qemu-devel@nongnu.org; Wed, 16 Mar 2016 20:57:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agMFf-0001hI-VK for qemu-devel@nongnu.org; Wed, 16 Mar 2016 20:57:25 -0400 Date: Thu, 17 Mar 2016 08:57:14 +0800 From: Fam Zheng Message-ID: <20160317005714.GB23821@ad.usersys.redhat.com> References: <1455645388-32401-1-git-send-email-pbonzini@redhat.com> <1455645388-32401-8-git-send-email-pbonzini@redhat.com> <20160316163908.GA2012@stefanha-x1.localdomain> <56E99ACC.3050904@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56E99ACC.3050904@redhat.com> Subject: Re: [Qemu-devel] [PATCH 07/16] block: change drain to look only at one child at a time List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , qemu-block@nongnu.org, qemu-devel@nongnu.org, Stefan Hajnoczi On Wed, 03/16 18:41, Paolo Bonzini wrote: > > > On 16/03/2016 17:39, Stefan Hajnoczi wrote: > > The tree looks like this: > > > > [NBD export] > > / > > v > > [guest] temporary qcow2 > > \ / > > v v > > disk > > > > Block backend access is in square brackets. Nodes without square > > brackets are BDS nodes. > > > > If the guest wants to drain the disk, it's possible for new I/O requests > > to enter the disk BDS while we're recursing to disk's children because > > the NBD export socket fd is in the same AIOContext. The socket fd is > > therefore handled during aio_poll() calls. > > > > I'm not 100% sure that this is a problem, but I wonder if you've thought > > about this? > > I hadn't, but I think this is handled by using > bdrv_drained_begin/bdrv_drained_end instead of bdrv_drain. The NBD > export registers its callback as "external", and it is thus disabled > between bdrv_drained_begin and bdrv_drained_end. > > It will indeed become more complex when BDSes won't have anymore a "home It probably means BBs won't have a "home AioContext" too, in that case. > AioContext" due to multiqueue. I suspect that we should rethink the > strategy for enabling and disabling external callbacks. For example we > could add callbacks to each BlockBackend that enable/disable external > callbacks, and when bdrv_drained_begin is called on a BDS, we call the > callbacks for all BlockBackends that are included in this BDS. I'm not > sure if there's a way to go from a BDS to all the BBs above it. If none of the bdrv_drained_begin callers is in hot path, I think we can simply call disable on each aio context that can send request to BB/BDS. Fam