From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44761) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZgM9U-0006CM-GA for qemu-devel@nongnu.org; Sun, 27 Sep 2015 20:18:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZgM9T-0004cm-NE for qemu-devel@nongnu.org; Sun, 27 Sep 2015 20:18:44 -0400 Date: Mon, 28 Sep 2015 08:18:33 +0800 From: Fam Zheng Message-ID: <20150928001833.GB10367@localhost.nay.redhat.com> References: <1443188504-20296-1-git-send-email-berto@igalia.com> <560558A2.3020804@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] block: disable I/O limits at the beginning of bdrv_close() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia Cc: Kevin Wolf , qemu-block@nongnu.org, qemu-devel@nongnu.org, Max Reitz , Stefan Hajnoczi On Fri, 09/25 16:31, Alberto Garcia wrote: > On Fri 25 Sep 2015 04:22:26 PM CEST, Eric Blake wrote: > > >> Disabling I/O limits from a BDS also drains all pending throttled > >> requests, so it should be done at the beginning of bdrv_close() with > >> the rest of the bdrv_drain() calls before the BlockDriver is closed. > > > > Can this be abused? If I have a guest running in a cloud where the > > cloud provider has put severe throttling limits on me, but lets me > > hotplug to my heart's content, couldn't I just repeatedly plug/unplug > > the disk to get around the throttling (every time I unplug, all writes > > flush at full speed, then I immediately replug to start batching up a > > new set of writes). In other words, shouldn't the draining still be > > throttled, to prevent my abuse? > > I didn't think about this case, and I don't know how practical this is, > but note that bdrv_drain() (which is already at the beginning of > bdrv_close()) flushes the I/O queue explicitly bypassing the limits, so > other cases where a user can trigger a bdrv_drain() would also be > vulnerable to this. > Yes, the issue is pre-existing. This patch only reordered things inside bdrv_close() so it's no worse. But indeed there is this vulnerability, maybe we should throttle the queue in all cases? Fam