qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: Alberto Garcia <berto@igalia.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org,
	Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] block: disable I/O limits at the beginning of bdrv_close()
Date: Mon, 28 Sep 2015 08:18:33 +0800	[thread overview]
Message-ID: <20150928001833.GB10367@localhost.nay.redhat.com> (raw)
In-Reply-To: <w51wpvein98.fsf@maestria.local.igalia.com>

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

  reply	other threads:[~2015-09-28  0:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-25 13:41 [Qemu-devel] [PATCH] block: disable I/O limits at the beginning of bdrv_close() Alberto Garcia
2015-09-25 14:22 ` Eric Blake
2015-09-25 14:31   ` Alberto Garcia
2015-09-28  0:18     ` Fam Zheng [this message]
2015-09-28  9:04       ` Alberto Garcia
2015-09-29 19:51         ` Andrey Korolyov
2015-09-29 12:50 ` Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150928001833.GB10367@localhost.nay.redhat.com \
    --to=famz@redhat.com \
    --cc=berto@igalia.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).