From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35395) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj1dC-0005Wx-Ia for qemu-devel@nongnu.org; Thu, 24 Mar 2016 05:32:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aj1dB-0008DN-Or for qemu-devel@nongnu.org; Thu, 24 Mar 2016 05:32:42 -0400 Sender: Paolo Bonzini References: <1458660792-3035-1-git-send-email-kwolf@redhat.com> <1458660792-3035-11-git-send-email-kwolf@redhat.com> <56F30A9F.6040300@redhat.com> <20160324082510.GB4310@noname.redhat.com> From: Paolo Bonzini Message-ID: <56F3B431.2080503@redhat.com> Date: Thu, 24 Mar 2016 10:32:33 +0100 MIME-Version: 1.0 In-Reply-To: <20160324082510.GB4310@noname.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 10/12] block: Drain throttling queue with BdrvChild callback List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org On 24/03/2016 09:25, Kevin Wolf wrote: > I think your cancellation series (allows to) gets rid of most if not all > blk_drain() callers in the device emulation, so it becomes harder for > guests to trigger one. Ideally only the monitor should allow triggering > a drain. More precesely you still need to call drain, but indeed they won't be able to game throttling. > On the other hand, your other series introduces bdrv_drain() calls where > we have open-coded nested event loops waiting for a single request > today. I'm pretty sure that these can be triggered by the guest and that > throttling the drain would be desirable therefore. The open-coded nested event loops are typically triggered only from qemu-io, bdrv_create, etc. They shouldn't be worse than the "disable throttling for sync I/O" that we used to have. > Okay. Actually, such a pair of callbacks - not only into the > BlockBackend, but from there into the guest device - was a thought > already when we introduced aio_disable_external(). Do you think it would > make sense to change things in the mid term so that the users of a > BlockBackend just get drain_begin/end callbacks? Yes, aio_disable_external and aio_disable_internal can be moved to the BdrvChildRole callbacks too. Paolo