From: Stefan Hajnoczi <stefanha@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
Alberto Garcia <berto@igalia.com>,
Stefan Hajnoczi <stefanha@redhat.com>
Subject: [Qemu-devel] [PULL for-2.10 07/15] throttle: Make burst_length 64bit and add range checks
Date: Thu, 31 Aug 2017 09:22:02 +0100 [thread overview]
Message-ID: <20170831082210.8362-8-stefanha@redhat.com> (raw)
In-Reply-To: <20170831082210.8362-1-stefanha@redhat.com>
From: Alberto Garcia <berto@igalia.com>
LeakyBucket.burst_length is defined as an unsigned integer but the
code never checks for overflows and it only makes sure that the value
is not 0.
In practice this means that the user can set something like
throttling.iops-total-max-length=4294967300 despite being larger than
UINT_MAX and the final value after casting to unsigned int will be 4.
This patch changes the data type to uint64_t. This does not increase
the storage size of LeakyBucket, and allows us to assign the value
directly from qemu_opt_get_number() or BlockIOThrottle and then do the
checks directly in throttle_is_valid().
The value of burst_length does not have a specific upper limit,
but since the bucket size is defined by max * burst_length we have
to prevent overflows. Instead of going for UINT64_MAX or something
similar this patch reuses THROTTLE_VALUE_MAX, which allows I/O bursts
of 1 GiB/s for 10 days in a row.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 1b2e3049803f71cafb2e1fa1be4fb47147a0d398.1503580370.git.berto@igalia.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/qemu/throttle.h | 2 +-
util/throttle.c | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 6e31155fd4..8e01885d29 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -81,7 +81,7 @@ typedef struct LeakyBucket {
uint64_t max; /* leaky bucket max burst in units */
double level; /* bucket level in units */
double burst_level; /* bucket level in units (for computing bursts) */
- unsigned burst_length; /* max length of the burst period, in seconds */
+ uint64_t burst_length; /* max length of the burst period, in seconds */
} LeakyBucket;
/* The following structure is used to configure a ThrottleState
diff --git a/util/throttle.c b/util/throttle.c
index 80660ffd2c..b8c524336c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -354,6 +354,11 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
return false;
}
+ if (bkt->max && bkt->burst_length > THROTTLE_VALUE_MAX / bkt->max) {
+ error_setg(errp, "burst length too high for this burst rate");
+ return false;
+ }
+
if (bkt->max && !bkt->avg) {
error_setg(errp, "bps_max/iops_max require corresponding"
" bps/iops values");
--
2.13.5
next prev parent reply other threads:[~2017-08-31 8:22 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-31 8:21 [Qemu-devel] [PULL for-2.10 00/15] Block patches Stefan Hajnoczi
2017-08-31 8:21 ` [Qemu-devel] [PULL for-2.10 01/15] nvme: Fix get/set number of queues feature, again Stefan Hajnoczi
2017-08-31 8:21 ` [Qemu-devel] [PULL for-2.10 02/15] throttle: Fix wrong variable name in the header documentation Stefan Hajnoczi
2017-08-31 8:21 ` [Qemu-devel] [PULL for-2.10 03/15] throttle: Update the throttle_fix_bucket() documentation Stefan Hajnoczi
2017-08-31 8:21 ` [Qemu-devel] [PULL for-2.10 04/15] throttle: Make throttle_is_valid() a bit less verbose Stefan Hajnoczi
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 05/15] throttle: Remove throttle_fix_bucket() / throttle_unfix_bucket() Stefan Hajnoczi
2017-09-12 17:37 ` Peter Maydell
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 06/15] throttle: Make LeakyBucket.avg and LeakyBucket.max integer types Stefan Hajnoczi
2017-08-31 8:22 ` Stefan Hajnoczi [this message]
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 08/15] throttle: Test the valid range of config values Stefan Hajnoczi
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 09/15] oslib-posix: Print errors before aborting on qemu_alloc_stack() Stefan Hajnoczi
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 10/15] misc: Remove unused Error variables Stefan Hajnoczi
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 11/15] scripts: add argparse module for Python 2.6 compatibility Stefan Hajnoczi
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 12/15] docker.py: Python 2.6 argparse compatibility Stefan Hajnoczi
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 13/15] tests: migration/guestperf " Stefan Hajnoczi
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 14/15] qemu-doc: Add UUID support in initiator name Stefan Hajnoczi
2017-08-31 8:22 ` [Qemu-devel] [PULL for-2.10 15/15] qcow2: allocate cluster_cache/cluster_data on demand Stefan Hajnoczi
2017-08-31 8:37 ` [Qemu-devel] [PULL for-2.10 00/15] Block patches no-reply
2017-08-31 13:47 ` Eric Blake
2017-08-31 14:51 ` Peter Maydell
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=20170831082210.8362-8-stefanha@redhat.com \
--to=stefanha@redhat.com \
--cc=berto@igalia.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/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).