[Qemu-devel] [PATCH 00/13] throttle: Allow I/O bursts for a user-defined period of time

[Qemu-devel] [PATCH 00/13] throttle: Allow I/O bursts for a user-defined period of time

[Alberto Garcia]

Hello everyone,

the current throttling code in QEMU allows limiting the I/O rate on
block devices. Limits can be set in operations per second (IOPS) or
bytes per second, allowing separate limits for read and write
operations on both cases.

In its basic usage the user can set a limit of, say, 100 IOPS on a
block device passing this option to -drive:

  throttling.iops-total=100

In addition to that, QEMU can also allow the user to do I/O bursts
exceeding that limit up to a configurable rate:

  throttling.iops-total=100,throttling.iops-total-max=2000

With this, the user can do a burst of 2000 IOPS before they're
throttled down to 100 IOPS. Then, after a sufficiently long period of
unused I/O they will be able to do a burst again.

This patch series introduces the possibility to do bursts for longer
period of times. A new setting called throttling.iops-total-max-length
is used to define for how long bursts can be sustained.

So adding throttling.iops-total-max-length=60 to the previous
configuration allows the user to do I/O at a rate of 2000 IOPS for 1
minute before going down to the base rate of 100 IOPS.

This is essentially the same as described in this AWS blog post:

   https://aws.amazon.com/blogs/aws/new-ssd-backed-elastic-block-storage/

As described in the article, a use case for this feature is to allow
better performance when booting the OS or restarting a service while
keeping the average I/O limits lower the rest of the time.

Comments:

 - There are 6 different settings for setting I/O limits: iops-total,
   iops-read, iops-write, bps-total, bps-read, bps-write. This series
   adds one new setting to set the length for each one of those.

   I don't know if there's a good use case that requires such
   fine-grained control. It's of course also possible to make it
   simpler and add just one 'burst-length' setting that would apply
   for all cases, but the current solution is IMHO simple enough and
   consistent with the current API, and if we need to extend it later
   the result is probably going to be ugly.

 - With this series we set "a maximum of X operations/second for a
   period of T seconds". If would also be possible to make it "a
   maximum of X operations/second up to a total of Y operations". It
   would be equivalent (Y = X * T) but I thought the current proposal
   makes a more clear API.

And I think that's all.

Thanks!

Berto

Alberto Garcia (13):
  throttle: Make throttle_compute_timer() static
  throttle: Make throttle_conflicting() set errp
  throttle: Make throttle_max_is_missing_limit() set errp
  throttle: Make throttle_is_valid() set errp
  throttle: Set always an average value when setting a maximum value
  throttle: Merge all functions that check the configuration into one
  throttle: Use throttle_config_init() to initialize ThrottleConfig
  throttle: Add support for burst periods
  throttle: Add command-line settings to define the burst periods
  qapi: Add burst length parameters to block_set_io_throttle
  qapi: Add burst length fields to BlockDeviceInfo
  throttle: Check that burst_level leaks correctly
  throttle: Test throttle_compute_wait() during bursts

 block/qapi.c            |  20 ++++++++
 blockdev.c              |  99 ++++++++++++++++++++++++++----------
 hmp.c                   |  12 +++++
 include/qemu/throttle.h |  55 ++++++++++++++------
 qapi/block-core.json    |  90 ++++++++++++++++++++++++++++-----
 tests/test-throttle.c   |  88 ++++++++++++++++++++++++--------
 util/throttle.c         | 132 ++++++++++++++++++++++++++++++++----------------
 7 files changed, 377 insertions(+), 119 deletions(-)

-- 
2.7.0

[Qemu-devel] [PATCH 01/13] throttle: Make throttle_compute_timer() static

[Alberto Garcia]

This function is only used internally in throttle.c

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 include/qemu/throttle.h | 6 ------
 util/throttle.c         | 8 ++++----
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index d0c98ed..52c98d9 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -84,12 +84,6 @@ void throttle_leak_bucket(LeakyBucket *bkt, int64_t delta);
 
 int64_t throttle_compute_wait(LeakyBucket *bkt);
 
-/* expose timer computation function for unit tests */
-bool throttle_compute_timer(ThrottleState *ts,
-                            bool is_write,
-                            int64_t now,
-                            int64_t *next_timestamp);
-
 /* init/destroy cycle */
 void throttle_init(ThrottleState *ts);
 
diff --git a/util/throttle.c b/util/throttle.c
index 2f9b23d..c21043a 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -137,10 +137,10 @@ static int64_t throttle_compute_wait_for(ThrottleState *ts,
  * @next_timestamp: the resulting timer
  * @ret:        true if a timer must be set
  */
-bool throttle_compute_timer(ThrottleState *ts,
-                            bool is_write,
-                            int64_t now,
-                            int64_t *next_timestamp)
+static bool throttle_compute_timer(ThrottleState *ts,
+                                   bool is_write,
+                                   int64_t now,
+                                   int64_t *next_timestamp)
 {
     int64_t wait;
 
-- 
2.7.0

[Qemu-devel] [PATCH 02/13] throttle: Make throttle_conflicting() set errp

[Alberto Garcia]

The caller does not need to set it, and this will allow us to refactor
this function later.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 blockdev.c              |  4 +---
 include/qemu/throttle.h |  2 +-
 tests/test-throttle.c   | 12 ++++++------
 util/throttle.c         | 11 +++++++++--
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e1b6b0f..d8e5b57 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -345,9 +345,7 @@ static bool parse_stats_intervals(BlockAcctStats *stats, QList *intervals,
 
 static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
 {
-    if (throttle_conflicting(cfg)) {
-        error_setg(errp, "bps/iops/max total values and read/write values"
-                         " cannot be used at the same time");
+    if (throttle_conflicting(cfg, errp)) {
         return false;
     }
 
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 52c98d9..69cf171 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -106,7 +106,7 @@ bool throttle_timers_are_initialized(ThrottleTimers *tt);
 /* configuration */
 bool throttle_enabled(ThrottleConfig *cfg);
 
-bool throttle_conflicting(ThrottleConfig *cfg);
+bool throttle_conflicting(ThrottleConfig *cfg, Error **errp);
 
 bool throttle_is_valid(ThrottleConfig *cfg);
 
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index a95039f..769865d 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -254,31 +254,31 @@ static void test_conflicts_for_one_set(bool is_max,
                                        int write)
 {
     memset(&cfg, 0, sizeof(cfg));
-    g_assert(!throttle_conflicting(&cfg));
+    g_assert(!throttle_conflicting(&cfg, NULL));
 
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, read,  1);
-    g_assert(throttle_conflicting(&cfg));
+    g_assert(throttle_conflicting(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, write, 1);
-    g_assert(throttle_conflicting(&cfg));
+    g_assert(throttle_conflicting(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, read,  1);
     set_cfg_value(is_max, write, 1);
-    g_assert(throttle_conflicting(&cfg));
+    g_assert(throttle_conflicting(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
-    g_assert(!throttle_conflicting(&cfg));
+    g_assert(!throttle_conflicting(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, read,  1);
     set_cfg_value(is_max, write, 1);
-    g_assert(!throttle_conflicting(&cfg));
+    g_assert(!throttle_conflicting(&cfg, NULL));
 }
 
 static void test_conflicting_config(void)
diff --git a/util/throttle.c b/util/throttle.c
index c21043a..564e132 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -252,8 +252,9 @@ bool throttle_enabled(ThrottleConfig *cfg)
  *
  * @cfg: the throttling configuration to inspect
  * @ret: true if any conflict detected else false
+ * @errp: error object
  */
-bool throttle_conflicting(ThrottleConfig *cfg)
+bool throttle_conflicting(ThrottleConfig *cfg, Error **errp)
 {
     bool bps_flag, ops_flag;
     bool bps_max_flag, ops_max_flag;
@@ -274,7 +275,13 @@ bool throttle_conflicting(ThrottleConfig *cfg)
                    (cfg->buckets[THROTTLE_OPS_READ].max ||
                    cfg->buckets[THROTTLE_OPS_WRITE].max);
 
-    return bps_flag || ops_flag || bps_max_flag || ops_max_flag;
+    if (bps_flag || ops_flag || bps_max_flag || ops_max_flag) {
+        error_setg(errp, "bps/iops/max total values and read/write values"
+                   " cannot be used at the same time");
+        return true;
+    }
+
+    return false;
 }
 
 /* check if a throttling configuration is valid
-- 
2.7.0

[Qemu-devel] [PATCH 03/13] throttle: Make throttle_max_is_missing_limit() set errp

[Alberto Garcia]

The caller does not need to set it, and this will allow us to refactor
this function later.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 blockdev.c              | 4 +---
 include/qemu/throttle.h | 2 +-
 tests/test-throttle.c   | 6 +++---
 util/throttle.c         | 5 ++++-
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index d8e5b57..5472147 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -355,9 +355,7 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
         return false;
     }
 
-    if (throttle_max_is_missing_limit(cfg)) {
-        error_setg(errp, "bps_max/iops_max require corresponding"
-                         " bps/iops values");
+    if (throttle_max_is_missing_limit(cfg, errp)) {
         return false;
     }
 
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 69cf171..03bdec0 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -110,7 +110,7 @@ bool throttle_conflicting(ThrottleConfig *cfg, Error **errp);
 
 bool throttle_is_valid(ThrottleConfig *cfg);
 
-bool throttle_max_is_missing_limit(ThrottleConfig *cfg);
+bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp);
 
 void throttle_config(ThrottleState *ts,
                      ThrottleTimers *tt,
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 769865d..9cbca4b 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -337,15 +337,15 @@ static void test_max_is_missing_limit(void)
         memset(&cfg, 0, sizeof(cfg));
         cfg.buckets[i].max = 100;
         cfg.buckets[i].avg = 0;
-        g_assert(throttle_max_is_missing_limit(&cfg));
+        g_assert(throttle_max_is_missing_limit(&cfg, NULL));
 
         cfg.buckets[i].max = 0;
         cfg.buckets[i].avg = 0;
-        g_assert(!throttle_max_is_missing_limit(&cfg));
+        g_assert(!throttle_max_is_missing_limit(&cfg, NULL));
 
         cfg.buckets[i].max = 0;
         cfg.buckets[i].avg = 100;
-        g_assert(!throttle_max_is_missing_limit(&cfg));
+        g_assert(!throttle_max_is_missing_limit(&cfg, NULL));
     }
 }
 
diff --git a/util/throttle.c b/util/throttle.c
index 564e132..77010b4 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -306,13 +306,16 @@ bool throttle_is_valid(ThrottleConfig *cfg)
 
 /* check if bps_max/iops_max is used without bps/iops
  * @cfg: the throttling configuration to inspect
+ * @errp: error object
  */
-bool throttle_max_is_missing_limit(ThrottleConfig *cfg)
+bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp)
 {
     int i;
 
     for (i = 0; i < BUCKETS_COUNT; i++) {
         if (cfg->buckets[i].max && !cfg->buckets[i].avg) {
+            error_setg(errp, "bps_max/iops_max require corresponding"
+                       " bps/iops values");
             return true;
         }
     }
-- 
2.7.0

[Qemu-devel] [PATCH 04/13] throttle: Make throttle_is_valid() set errp

[Alberto Garcia]

The caller does not need to set it, and this will allow us to refactor
this function later.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 blockdev.c              | 4 +---
 include/qemu/throttle.h | 2 +-
 tests/test-throttle.c   | 2 +-
 util/throttle.c         | 5 ++++-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 5472147..9cc4200 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -349,9 +349,7 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
         return false;
     }
 
-    if (!throttle_is_valid(cfg)) {
-        error_setg(errp, "bps/iops/max values must be within [0, %lld]",
-                   THROTTLE_VALUE_MAX);
+    if (!throttle_is_valid(cfg, errp)) {
         return false;
     }
 
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 03bdec0..ecae621 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -108,7 +108,7 @@ bool throttle_enabled(ThrottleConfig *cfg);
 
 bool throttle_conflicting(ThrottleConfig *cfg, Error **errp);
 
-bool throttle_is_valid(ThrottleConfig *cfg);
+bool throttle_is_valid(ThrottleConfig *cfg, Error **errp);
 
 bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp);
 
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 9cbca4b..5f0b7d4 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -314,7 +314,7 @@ static void test_is_valid_for_value(int value, bool should_be_valid)
         for (index = 0; index < BUCKETS_COUNT; index++) {
             memset(&cfg, 0, sizeof(cfg));
             set_cfg_value(is_max, index, value);
-            g_assert(throttle_is_valid(&cfg) == should_be_valid);
+            g_assert(throttle_is_valid(&cfg, NULL) == should_be_valid);
         }
     }
 }
diff --git a/util/throttle.c b/util/throttle.c
index 77010b4..9046dd8 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -287,8 +287,9 @@ bool throttle_conflicting(ThrottleConfig *cfg, Error **errp)
 /* check if a throttling configuration is valid
  * @cfg: the throttling configuration to inspect
  * @ret: true if valid else false
+ * @errp: error object
  */
-bool throttle_is_valid(ThrottleConfig *cfg)
+bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
 {
     int i;
 
@@ -297,6 +298,8 @@ bool throttle_is_valid(ThrottleConfig *cfg)
             cfg->buckets[i].max < 0 ||
             cfg->buckets[i].avg > THROTTLE_VALUE_MAX ||
             cfg->buckets[i].max > THROTTLE_VALUE_MAX) {
+            error_setg(errp, "bps/iops/max values must be within [0, %lld]",
+                       THROTTLE_VALUE_MAX);
             return false;
         }
     }
-- 
2.7.0

[Qemu-devel] [PATCH 05/13] throttle: Set always an average value when setting a maximum value

[Alberto Garcia]

When testing the ranges of valid values, set_cfg_value() creates
sometimes invalid throttling configurations by setting bucket.max
while leaving bucket.avg uninitialized.

While this doesn't break the current tests, it will as soon as
we unify all functions that check the validity of the throttling
configuration.

This patch ensures that the value of bucket.avg is valid when setting
bucket.max.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 tests/test-throttle.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 5f0b7d4..5031eb7 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -221,6 +221,8 @@ static void set_cfg_value(bool is_max, int index, int value)
 {
     if (is_max) {
         cfg.buckets[index].max = value;
+        /* If max is set, avg should never be 0 */
+        cfg.buckets[index].avg = MAX(cfg.buckets[index].avg, 1);
     } else {
         cfg.buckets[index].avg = value;
     }
-- 
2.7.0

[Qemu-devel] [PATCH 06/13] throttle: Merge all functions that check the configuration into one

[Alberto Garcia]

There's no need to keep throttle_conflicting(), throttle_is_valid()
and throttle_max_is_missing_limit() as separate functions, so this
patch merges all three into one.

As a consequence, check_throttle_config() becomes redundant and can be
replaced with throttle_is_valid().

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 blockdev.c              | 21 ++-------------------
 include/qemu/throttle.h |  4 ----
 tests/test-throttle.c   | 18 +++++++++---------
 util/throttle.c         | 40 ++++++++--------------------------------
 4 files changed, 19 insertions(+), 64 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 9cc4200..8824c87 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -343,23 +343,6 @@ static bool parse_stats_intervals(BlockAcctStats *stats, QList *intervals,
     return true;
 }
 
-static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
-{
-    if (throttle_conflicting(cfg, errp)) {
-        return false;
-    }
-
-    if (!throttle_is_valid(cfg, errp)) {
-        return false;
-    }
-
-    if (throttle_max_is_missing_limit(cfg, errp)) {
-        return false;
-    }
-
-    return true;
-}
-
 typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType;
 
 /* All parameters but @opts are optional and may be set to NULL. */
@@ -434,7 +417,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
         throttle_cfg->op_size =
             qemu_opt_get_number(opts, "throttling.iops-size", 0);
 
-        if (!check_throttle_config(throttle_cfg, errp)) {
+        if (!throttle_is_valid(throttle_cfg, errp)) {
             return;
         }
     }
@@ -2652,7 +2635,7 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
         cfg.op_size = iops_size;
     }
 
-    if (!check_throttle_config(&cfg, errp)) {
+    if (!throttle_is_valid(&cfg, errp)) {
         goto out;
     }
 
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index ecae621..aec0785 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -106,12 +106,8 @@ bool throttle_timers_are_initialized(ThrottleTimers *tt);
 /* configuration */
 bool throttle_enabled(ThrottleConfig *cfg);
 
-bool throttle_conflicting(ThrottleConfig *cfg, Error **errp);
-
 bool throttle_is_valid(ThrottleConfig *cfg, Error **errp);
 
-bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp);
-
 void throttle_config(ThrottleState *ts,
                      ThrottleTimers *tt,
                      ThrottleConfig *cfg);
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 5031eb7..caf93ef 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -256,31 +256,31 @@ static void test_conflicts_for_one_set(bool is_max,
                                        int write)
 {
     memset(&cfg, 0, sizeof(cfg));
-    g_assert(!throttle_conflicting(&cfg, NULL));
+    g_assert(throttle_is_valid(&cfg, NULL));
 
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, read,  1);
-    g_assert(throttle_conflicting(&cfg, NULL));
+    g_assert(!throttle_is_valid(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, write, 1);
-    g_assert(throttle_conflicting(&cfg, NULL));
+    g_assert(!throttle_is_valid(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, read,  1);
     set_cfg_value(is_max, write, 1);
-    g_assert(throttle_conflicting(&cfg, NULL));
+    g_assert(!throttle_is_valid(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
-    g_assert(!throttle_conflicting(&cfg, NULL));
+    g_assert(throttle_is_valid(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, read,  1);
     set_cfg_value(is_max, write, 1);
-    g_assert(!throttle_conflicting(&cfg, NULL));
+    g_assert(throttle_is_valid(&cfg, NULL));
 }
 
 static void test_conflicting_config(void)
@@ -339,15 +339,15 @@ static void test_max_is_missing_limit(void)
         memset(&cfg, 0, sizeof(cfg));
         cfg.buckets[i].max = 100;
         cfg.buckets[i].avg = 0;
-        g_assert(throttle_max_is_missing_limit(&cfg, NULL));
+        g_assert(!throttle_is_valid(&cfg, NULL));
 
         cfg.buckets[i].max = 0;
         cfg.buckets[i].avg = 0;
-        g_assert(!throttle_max_is_missing_limit(&cfg, NULL));
+        g_assert(throttle_is_valid(&cfg, NULL));
 
         cfg.buckets[i].max = 0;
         cfg.buckets[i].avg = 100;
-        g_assert(!throttle_max_is_missing_limit(&cfg, NULL));
+        g_assert(throttle_is_valid(&cfg, NULL));
     }
 }
 
diff --git a/util/throttle.c b/util/throttle.c
index 9046dd8..f8bf03c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -248,14 +248,14 @@ bool throttle_enabled(ThrottleConfig *cfg)
     return false;
 }
 
-/* return true if any two throttling parameters conflicts
- *
+/* check if a throttling configuration is valid
  * @cfg: the throttling configuration to inspect
- * @ret: true if any conflict detected else false
+ * @ret: true if valid else false
  * @errp: error object
  */
-bool throttle_conflicting(ThrottleConfig *cfg, Error **errp)
+bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
 {
+    int i;
     bool bps_flag, ops_flag;
     bool bps_max_flag, ops_max_flag;
 
@@ -278,21 +278,9 @@ bool throttle_conflicting(ThrottleConfig *cfg, Error **errp)
     if (bps_flag || ops_flag || bps_max_flag || ops_max_flag) {
         error_setg(errp, "bps/iops/max total values and read/write values"
                    " cannot be used at the same time");
-        return true;
+        return false;
     }
 
-    return false;
-}
-
-/* check if a throttling configuration is valid
- * @cfg: the throttling configuration to inspect
- * @ret: true if valid else false
- * @errp: error object
- */
-bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
-{
-    int i;
-
     for (i = 0; i < BUCKETS_COUNT; i++) {
         if (cfg->buckets[i].avg < 0 ||
             cfg->buckets[i].max < 0 ||
@@ -302,27 +290,15 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
                        THROTTLE_VALUE_MAX);
             return false;
         }
-    }
 
-    return true;
-}
-
-/* check if bps_max/iops_max is used without bps/iops
- * @cfg: the throttling configuration to inspect
- * @errp: error object
- */
-bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp)
-{
-    int i;
-
-    for (i = 0; i < BUCKETS_COUNT; i++) {
         if (cfg->buckets[i].max && !cfg->buckets[i].avg) {
             error_setg(errp, "bps_max/iops_max require corresponding"
                        " bps/iops values");
-            return true;
+            return false;
         }
     }
-    return false;
+
+    return true;
 }
 
 /* fix bucket parameters */
-- 
2.7.0

[Qemu-devel] [PATCH 07/13] throttle: Use throttle_config_init() to initialize ThrottleConfig

[Alberto Garcia]

We can currently initialize ThrottleConfig by zeroing all its fields,
but this will change with the new fields to define the length of the
burst periods.

This patch introduces a new throttle_config_init() function and uses it
to replace all memset() calls that initialize ThrottleConfig directly.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 blockdev.c              |  4 ++--
 include/qemu/throttle.h |  2 ++
 tests/test-throttle.c   | 28 +++++++++++++++++-----------
 util/throttle.c         | 10 ++++++++++
 4 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 8824c87..c9e91ab 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -387,7 +387,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
     }
 
     if (throttle_cfg) {
-        memset(throttle_cfg, 0, sizeof(*throttle_cfg));
+        throttle_config_init(throttle_cfg);
         throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
             qemu_opt_get_number(opts, "throttling.bps-total", 0);
         throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
@@ -2603,7 +2603,7 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
         goto out;
     }
 
-    memset(&cfg, 0, sizeof(cfg));
+    throttle_config_init(&cfg);
     cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps;
     cfg.buckets[THROTTLE_BPS_READ].avg  = bps_rd;
     cfg.buckets[THROTTLE_BPS_WRITE].avg = bps_wr;
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index aec0785..8ec8951 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -114,6 +114,8 @@ void throttle_config(ThrottleState *ts,
 
 void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg);
 
+void throttle_config_init(ThrottleConfig *cfg);
+
 /* usage */
 bool throttle_schedule_timer(ThrottleState *ts,
                              ThrottleTimers *tt,
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index caf93ef..cf81033 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -34,6 +34,9 @@ static bool double_cmp(double x, double y)
 /* tests for single bucket operations */
 static void test_leak_bucket(void)
 {
+    throttle_config_init(&cfg);
+    bkt = cfg.buckets[THROTTLE_BPS_TOTAL];
+
     /* set initial value */
     bkt.avg = 150;
     bkt.max = 15;
@@ -63,6 +66,9 @@ static void test_compute_wait(void)
     int64_t wait;
     int64_t result;
 
+    throttle_config_init(&cfg);
+    bkt = cfg.buckets[THROTTLE_BPS_TOTAL];
+
     /* no operation limit set */
     bkt.avg = 0;
     bkt.max = 15;
@@ -232,17 +238,17 @@ static void test_enabled(void)
 {
     int i;
 
-    memset(&cfg, 0, sizeof(cfg));
+    throttle_config_init(&cfg);
     g_assert(!throttle_enabled(&cfg));
 
     for (i = 0; i < BUCKETS_COUNT; i++) {
-        memset(&cfg, 0, sizeof(cfg));
+        throttle_config_init(&cfg);
         set_cfg_value(false, i, 150);
         g_assert(throttle_enabled(&cfg));
     }
 
     for (i = 0; i < BUCKETS_COUNT; i++) {
-        memset(&cfg, 0, sizeof(cfg));
+        throttle_config_init(&cfg);
         set_cfg_value(false, i, -150);
         g_assert(!throttle_enabled(&cfg));
     }
@@ -255,29 +261,29 @@ static void test_conflicts_for_one_set(bool is_max,
                                        int read,
                                        int write)
 {
-    memset(&cfg, 0, sizeof(cfg));
+    throttle_config_init(&cfg);
     g_assert(throttle_is_valid(&cfg, NULL));
 
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, read,  1);
     g_assert(!throttle_is_valid(&cfg, NULL));
 
-    memset(&cfg, 0, sizeof(cfg));
+    throttle_config_init(&cfg);
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, write, 1);
     g_assert(!throttle_is_valid(&cfg, NULL));
 
-    memset(&cfg, 0, sizeof(cfg));
+    throttle_config_init(&cfg);
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, read,  1);
     set_cfg_value(is_max, write, 1);
     g_assert(!throttle_is_valid(&cfg, NULL));
 
-    memset(&cfg, 0, sizeof(cfg));
+    throttle_config_init(&cfg);
     set_cfg_value(is_max, total, 1);
     g_assert(throttle_is_valid(&cfg, NULL));
 
-    memset(&cfg, 0, sizeof(cfg));
+    throttle_config_init(&cfg);
     set_cfg_value(is_max, read,  1);
     set_cfg_value(is_max, write, 1);
     g_assert(throttle_is_valid(&cfg, NULL));
@@ -314,7 +320,7 @@ static void test_is_valid_for_value(int value, bool should_be_valid)
     int is_max, index;
     for (is_max = 0; is_max < 2; is_max++) {
         for (index = 0; index < BUCKETS_COUNT; index++) {
-            memset(&cfg, 0, sizeof(cfg));
+            throttle_config_init(&cfg);
             set_cfg_value(is_max, index, value);
             g_assert(throttle_is_valid(&cfg, NULL) == should_be_valid);
         }
@@ -336,7 +342,7 @@ static void test_max_is_missing_limit(void)
     int i;
 
     for (i = 0; i < BUCKETS_COUNT; i++) {
-        memset(&cfg, 0, sizeof(cfg));
+        throttle_config_init(&cfg);
         cfg.buckets[i].max = 100;
         cfg.buckets[i].avg = 0;
         g_assert(!throttle_is_valid(&cfg, NULL));
@@ -551,7 +557,7 @@ static void test_groups(void)
     g_assert(bdrv1->throttle_state == bdrv3->throttle_state);
 
     /* Setting the config of a group member affects the whole group */
-    memset(&cfg1, 0, sizeof(cfg1));
+    throttle_config_init(&cfg1);
     cfg1.buckets[THROTTLE_BPS_READ].avg  = 500000;
     cfg1.buckets[THROTTLE_BPS_WRITE].avg = 285000;
     cfg1.buckets[THROTTLE_OPS_READ].avg  = 20000;
diff --git a/util/throttle.c b/util/throttle.c
index f8bf03c..6a01cee 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -171,10 +171,20 @@ void throttle_timers_attach_aio_context(ThrottleTimers *tt,
                                   tt->write_timer_cb, tt->timer_opaque);
 }
 
+/*
+ * Initialize the ThrottleConfig structure to a valid state
+ * @cfg: the config to initialize
+ */
+void throttle_config_init(ThrottleConfig *cfg)
+{
+    memset(cfg, 0, sizeof(*cfg));
+}
+
 /* To be called first on the ThrottleState */
 void throttle_init(ThrottleState *ts)
 {
     memset(ts, 0, sizeof(ThrottleState));
+    throttle_config_init(&ts->cfg);
 }
 
 /* To be called first on the ThrottleTimers */
-- 
2.7.0

[Qemu-devel] [PATCH 08/13] throttle: Add support for burst periods

[Alberto Garcia]

This patch adds support for burst periods to the throttling code.
With this feature the user can keep performing bursts as defined by
the LeakyBucket.max rate for a configurable period of time.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 include/qemu/throttle.h | 41 +++++++++++++++++++++++----
 util/throttle.c         | 73 ++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 96 insertions(+), 18 deletions(-)

diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 8ec8951..63df690 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -2,7 +2,7 @@
  * QEMU throttling infrastructure
  *
  * Copyright (C) Nodalink, EURL. 2013-2014
- * Copyright (C) Igalia, S.L. 2015
+ * Copyright (C) Igalia, S.L. 2015-2016
  *
  * Authors:
  *   Benoît Canet <benoit.canet@nodalink.com>
@@ -42,16 +42,47 @@ typedef enum {
 } BucketType;
 
 /*
- * The max parameter of the leaky bucket throttling algorithm can be used to
- * allow the guest to do bursts.
- * The max value is a pool of I/O that the guest can use without being throttled
- * at all. Throttling is triggered once this pool is empty.
+ * This module implements I/O limits using the leaky bucket
+ * algorithm. The code is independent of the I/O units, but it is
+ * currently used for bytes per second and operations per second.
+ *
+ * Three parameters can be set by the user:
+ *
+ * - avg: the desired I/O limits in units per second.
+ * - max: the limit during bursts, also in units per second.
+ * - burst_length: the maximum length of the burst period, in seconds.
+ *
+ * Here's how it works:
+ *
+ * - The bucket level (number of performed I/O units) is kept in
+ *   bkt.level and leaks at a rate of bkt.avg units per second.
+ *
+ * - The size of the bucket is bkt.max * bkt.burst_length. Once the
+ *   bucket is full no more I/O is performed until the bucket leaks
+ *   again. This is what makes the I/O rate bkt.avg.
+ *
+ * - The bkt.avg rate does not apply until the bucket is full,
+ *   allowing the user to do bursts until then. The I/O limit during
+ *   bursts is bkt.max. To enforce this limit we keep an additional
+ *   bucket in bkt.burst_length that leaks at a rate of bkt.max units
+ *   per second.
+ *
+ * - Because of all of the above, the user can perform I/O at a
+ *   maximum of bkt.max units per second for at most bkt.burst_length
+ *   seconds in a row. After that the bucket will be full and the I/O
+ *   rate will go down to bkt.avg.
+ *
+ * - Since the bucket always leaks at a rate of bkt.avg, this also
+ *   determines how much the user needs to wait before being able to
+ *   do bursts again.
  */
 
 typedef struct LeakyBucket {
     double  avg;              /* average goal in units per second */
     double  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 */
 } LeakyBucket;
 
 /* The following structure is used to configure a ThrottleState
diff --git a/util/throttle.c b/util/throttle.c
index 6a01cee..371c769 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -41,6 +41,14 @@ void throttle_leak_bucket(LeakyBucket *bkt, int64_t delta_ns)
 
     /* make the bucket leak */
     bkt->level = MAX(bkt->level - leak, 0);
+
+    /* if we allow bursts for more than one second we also need to
+     * keep track of bkt->burst_level so the bkt->max goal per second
+     * is attained */
+    if (bkt->burst_length > 1) {
+        leak = (bkt->max * (double) delta_ns) / NANOSECONDS_PER_SECOND;
+        bkt->burst_level = MAX(bkt->burst_level - leak, 0);
+    }
 }
 
 /* Calculate the time delta since last leak and make proportionals leaks
@@ -91,13 +99,24 @@ int64_t throttle_compute_wait(LeakyBucket *bkt)
         return 0;
     }
 
-    extra = bkt->level - bkt->max;
+    /* If the bucket is full then we have to wait */
+    extra = bkt->level - bkt->max * bkt->burst_length;
+    if (extra > 0) {
+        return throttle_do_compute_wait(bkt->avg, extra);
+    }
 
-    if (extra <= 0) {
-        return 0;
+    /* If the bucket is not full yet we have to make sure that we
+     * fulfill the goal of bkt->max units per second. */
+    if (bkt->burst_length > 1) {
+        /* We use 1/10 of the max value to smooth the throttling.
+         * See throttle_fix_bucket() for more details. */
+        extra = bkt->burst_level - bkt->max / 10;
+        if (extra > 0) {
+            return throttle_do_compute_wait(bkt->max, extra);
+        }
     }
 
-    return throttle_do_compute_wait(bkt->avg, extra);
+    return 0;
 }
 
 /* This function compute the time that must be waited while this IO
@@ -177,7 +196,11 @@ void throttle_timers_attach_aio_context(ThrottleTimers *tt,
  */
 void throttle_config_init(ThrottleConfig *cfg)
 {
+    unsigned i;
     memset(cfg, 0, sizeof(*cfg));
+    for (i = 0; i < BUCKETS_COUNT; i++) {
+        cfg->buckets[i].burst_length = 1;
+    }
 }
 
 /* To be called first on the ThrottleState */
@@ -301,6 +324,16 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
             return false;
         }
 
+        if (!cfg->buckets[i].burst_length) {
+            error_setg(errp, "the burst length cannot be 0");
+            return false;
+        }
+
+        if (cfg->buckets[i].burst_length > 1 && !cfg->buckets[i].max) {
+            error_setg(errp, "burst length set without burst rate");
+            return false;
+        }
+
         if (cfg->buckets[i].max && !cfg->buckets[i].avg) {
             error_setg(errp, "bps_max/iops_max require corresponding"
                        " bps/iops values");
@@ -317,7 +350,7 @@ static void throttle_fix_bucket(LeakyBucket *bkt)
     double min;
 
     /* zero bucket level */
-    bkt->level = 0;
+    bkt->level = bkt->burst_level = 0;
 
     /* The following is done to cope with the Linux CFQ block scheduler
      * which regroup reads and writes by block of 100ms in the guest.
@@ -420,22 +453,36 @@ bool throttle_schedule_timer(ThrottleState *ts,
  */
 void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
 {
+    const BucketType bucket_types_size[2][2] = {
+        { THROTTLE_BPS_TOTAL, THROTTLE_BPS_READ },
+        { THROTTLE_BPS_TOTAL, THROTTLE_BPS_WRITE }
+    };
+    const BucketType bucket_types_units[2][2] = {
+        { THROTTLE_OPS_TOTAL, THROTTLE_OPS_READ },
+        { THROTTLE_OPS_TOTAL, THROTTLE_OPS_WRITE }
+    };
     double units = 1.0;
+    unsigned i;
 
     /* if cfg.op_size is defined and smaller than size we compute unit count */
     if (ts->cfg.op_size && size > ts->cfg.op_size) {
         units = (double) size / ts->cfg.op_size;
     }
 
-    ts->cfg.buckets[THROTTLE_BPS_TOTAL].level += size;
-    ts->cfg.buckets[THROTTLE_OPS_TOTAL].level += units;
+    for (i = 0; i < 2; i++) {
+        LeakyBucket *bkt;
 
-    if (is_write) {
-        ts->cfg.buckets[THROTTLE_BPS_WRITE].level += size;
-        ts->cfg.buckets[THROTTLE_OPS_WRITE].level += units;
-    } else {
-        ts->cfg.buckets[THROTTLE_BPS_READ].level += size;
-        ts->cfg.buckets[THROTTLE_OPS_READ].level += units;
+        bkt = &ts->cfg.buckets[bucket_types_size[is_write][i]];
+        bkt->level += size;
+        if (bkt->burst_length > 1) {
+            bkt->burst_level += size;
+        }
+
+        bkt = &ts->cfg.buckets[bucket_types_units[is_write][i]];
+        bkt->level += units;
+        if (bkt->burst_length > 1) {
+            bkt->burst_level += units;
+        }
     }
 }
 
-- 
2.7.0

[Qemu-devel] [PATCH 09/13] throttle: Add command-line settings to define the burst periods

[Alberto Garcia]

This patch adds all the throttling.*-max-length command-line
parameters to define the length of the burst periods.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 blockdev.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index c9e91ab..5a2a9b4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -414,6 +414,19 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
         throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
             qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
 
+        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
+            qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
+        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
+            qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
+        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
+            qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
+        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
+            qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
+        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
+            qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
+        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
+            qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
+
         throttle_cfg->op_size =
             qemu_opt_get_number(opts, "throttling.iops-size", 0);
 
@@ -4064,6 +4077,30 @@ QemuOptsList qemu_common_drive_opts = {
             .type = QEMU_OPT_NUMBER,
             .help = "total bytes write burst",
         },{
+            .name = "throttling.iops-total-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the iops-total-max burst period, in seconds",
+        },{
+            .name = "throttling.iops-read-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the iops-read-max burst period, in seconds",
+        },{
+            .name = "throttling.iops-write-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the iops-write-max burst period, in seconds",
+        },{
+            .name = "throttling.bps-total-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the bps-total-max burst period, in seconds",
+        },{
+            .name = "throttling.bps-read-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the bps-read-max burst period, in seconds",
+        },{
+            .name = "throttling.bps-write-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the bps-write-max burst period, in seconds",
+        },{
             .name = "throttling.iops-size",
             .type = QEMU_OPT_NUMBER,
             .help = "when limiting by iops max size of an I/O in bytes",
-- 
2.7.0

[Qemu-devel] [PATCH 10/13] qapi: Add burst length parameters to block_set_io_throttle

[Alberto Garcia]

This patch adds the new bps_*_max_length and iops_*_max_length
parameters to the block_set_io_throttle command.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 blockdev.c           | 31 +++++++++++++++++++++++++++++++
 hmp.c                | 12 ++++++++++++
 qapi/block-core.json | 51 +++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 88 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 5a2a9b4..4576109 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2590,6 +2590,18 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
                                int64_t iops_rd_max,
                                bool has_iops_wr_max,
                                int64_t iops_wr_max,
+                               bool has_bps_max_length,
+                               int64_t bps_max_length,
+                               bool has_bps_rd_max_length,
+                               int64_t bps_rd_max_length,
+                               bool has_bps_wr_max_length,
+                               int64_t bps_wr_max_length,
+                               bool has_iops_max_length,
+                               int64_t iops_max_length,
+                               bool has_iops_rd_max_length,
+                               int64_t iops_rd_max_length,
+                               bool has_iops_wr_max_length,
+                               int64_t iops_wr_max_length,
                                bool has_iops_size,
                                int64_t iops_size,
                                bool has_group,
@@ -2644,6 +2656,25 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
         cfg.buckets[THROTTLE_OPS_WRITE].max = iops_wr_max;
     }
 
+    if (has_bps_max_length) {
+        cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = bps_max_length;
+    }
+    if (has_bps_rd_max_length) {
+        cfg.buckets[THROTTLE_BPS_READ].burst_length = bps_rd_max_length;
+    }
+    if (has_bps_wr_max_length) {
+        cfg.buckets[THROTTLE_BPS_WRITE].burst_length = bps_wr_max_length;
+    }
+    if (has_iops_max_length) {
+        cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = iops_max_length;
+    }
+    if (has_iops_rd_max_length) {
+        cfg.buckets[THROTTLE_OPS_READ].burst_length = iops_rd_max_length;
+    }
+    if (has_iops_wr_max_length) {
+        cfg.buckets[THROTTLE_OPS_WRITE].burst_length = iops_wr_max_length;
+    }
+
     if (has_iops_size) {
         cfg.op_size = iops_size;
     }
diff --git a/hmp.c b/hmp.c
index cb03a15..39f2227 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1411,6 +1411,18 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
                               0,
                               false,
                               0,
+                              false, /* no burst length via HMP */
+                              0,
+                              false,
+                              0,
+                              false,
+                              0,
+                              false,
+                              0,
+                              false,
+                              0,
+                              false,
+                              0,
                               false, /* No default I/O size */
                               0,
                               false,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 33012b8..126d834 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1298,17 +1298,53 @@
 #
 # @iops_wr: write I/O operations per second
 #
-# @bps_max: #optional total max in bytes (Since 1.7)
+# @bps_max: #optional total throughput limit during bursts,
+#                     in bytes (Since 1.7)
 #
-# @bps_rd_max: #optional read max in bytes (Since 1.7)
+# @bps_rd_max: #optional read throughput limit during bursts,
+#                        in bytes (Since 1.7)
 #
-# @bps_wr_max: #optional write max in bytes (Since 1.7)
+# @bps_wr_max: #optional write throughput limit during bursts,
+#                        in bytes (Since 1.7)
 #
-# @iops_max: #optional total I/O operations max (Since 1.7)
+# @iops_max: #optional total I/O operations per second during bursts,
+#                      in bytes (Since 1.7)
 #
-# @iops_rd_max: #optional read I/O operations max (Since 1.7)
+# @iops_rd_max: #optional read I/O operations per second during bursts,
+#                         in bytes (Since 1.7)
 #
-# @iops_wr_max: #optional write I/O operations max (Since 1.7)
+# @iops_wr_max: #optional write I/O operations per second during bursts,
+#                         in bytes (Since 1.7)
+#
+# @bps_max_length: #optional maximum length of the @bps_max burst
+#                            period, in seconds. It must only
+#                            be set if @bps_max is set as well.
+#                            Defaults to 1. (Since 2.6)
+#
+# @bps_rd_max_length: #optional maximum length of the @bps_rd_max
+#                               burst period, in seconds. It must only
+#                               be set if @bps_rd_max is set as well.
+#                               Defaults to 1. (Since 2.6)
+#
+# @bps_wr_max_length: #optional maximum length of the @bps_wr_max
+#                               burst period, in seconds. It must only
+#                               be set if @bps_wr_max is set as well.
+#                               Defaults to 1. (Since 2.6)
+#
+# @iops_max_length: #optional maximum length of the @iops burst
+#                             period, in seconds. It must only
+#                             be set if @iops_max is set as well.
+#                             Defaults to 1. (Since 2.6)
+#
+# @iops_rd_max_length: #optional maximum length of the @iops_rd_max
+#                                burst period, in seconds. It must only
+#                                be set if @iops_rd_max is set as well.
+#                                Defaults to 1. (Since 2.6)
+#
+# @iops_wr_max_length: #optional maximum length of the @iops_wr_max
+#                                burst period, in seconds. It must only
+#                                be set if @iops_wr_max is set as well.
+#                                Defaults to 1. (Since 2.6)
 #
 # @iops_size: #optional an I/O size in bytes (Since 1.7)
 #
@@ -1325,6 +1361,9 @@
             '*bps_max': 'int', '*bps_rd_max': 'int',
             '*bps_wr_max': 'int', '*iops_max': 'int',
             '*iops_rd_max': 'int', '*iops_wr_max': 'int',
+            '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
+            '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
+            '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
             '*iops_size': 'int', '*group': 'str' } }
 
 ##
-- 
2.7.0

[Qemu-devel] [PATCH 11/13] qapi: Add burst length fields to BlockDeviceInfo

[Alberto Garcia]

This patch adds the new bps_*_max_length and iops_*_max_length
parameters to the BlockDeviceInfo struct.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qapi.c         | 20 ++++++++++++++++++++
 qapi/block-core.json | 39 +++++++++++++++++++++++++++++++++------
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 2e83105..3b99622 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -92,6 +92,26 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, Error **errp)
         info->has_iops_wr_max = cfg.buckets[THROTTLE_OPS_WRITE].max;
         info->iops_wr_max     = cfg.buckets[THROTTLE_OPS_WRITE].max;
 
+        info->has_bps_max_length     = info->has_bps_max;
+        info->bps_max_length         =
+            cfg.buckets[THROTTLE_BPS_TOTAL].burst_length;
+        info->has_bps_rd_max_length  = info->has_bps_rd_max;
+        info->bps_rd_max_length      =
+            cfg.buckets[THROTTLE_BPS_READ].burst_length;
+        info->has_bps_wr_max_length  = info->has_bps_wr_max;
+        info->bps_wr_max_length      =
+            cfg.buckets[THROTTLE_BPS_WRITE].burst_length;
+
+        info->has_iops_max_length    = info->has_iops_max;
+        info->iops_max_length        =
+            cfg.buckets[THROTTLE_OPS_TOTAL].burst_length;
+        info->has_iops_rd_max_length = info->has_iops_rd_max;
+        info->iops_rd_max_length     =
+            cfg.buckets[THROTTLE_OPS_READ].burst_length;
+        info->has_iops_wr_max_length = info->has_iops_wr_max;
+        info->iops_wr_max_length     =
+            cfg.buckets[THROTTLE_OPS_WRITE].burst_length;
+
         info->has_iops_size = cfg.op_size;
         info->iops_size = cfg.op_size;
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 126d834..fbbc709 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -273,17 +273,41 @@
 #
 # @image: the info of image used (since: 1.6)
 #
-# @bps_max: #optional total max in bytes (Since 1.7)
+# @bps_max: #optional total throughput limit during bursts,
+#                     in bytes (Since 1.7)
 #
-# @bps_rd_max: #optional read max in bytes (Since 1.7)
+# @bps_rd_max: #optional read throughput limit during bursts,
+#                        in bytes (Since 1.7)
 #
-# @bps_wr_max: #optional write max in bytes (Since 1.7)
+# @bps_wr_max: #optional write throughput limit during bursts,
+#                        in bytes (Since 1.7)
 #
-# @iops_max: #optional total I/O operations max (Since 1.7)
+# @iops_max: #optional total I/O operations per second during bursts,
+#                      in bytes (Since 1.7)
 #
-# @iops_rd_max: #optional read I/O operations max (Since 1.7)
+# @iops_rd_max: #optional read I/O operations per second during bursts,
+#                         in bytes (Since 1.7)
 #
-# @iops_wr_max: #optional write I/O operations max (Since 1.7)
+# @iops_wr_max: #optional write I/O operations per second during bursts,
+#                         in bytes (Since 1.7)
+#
+# @bps_max_length: #optional maximum length of the @bps_max burst
+#                            period, in seconds. (Since 2.6)
+#
+# @bps_rd_max_length: #optional maximum length of the @bps_rd_max
+#                               burst period, in seconds. (Since 2.6)
+#
+# @bps_wr_max_length: #optional maximum length of the @bps_wr_max
+#                               burst period, in seconds. (Since 2.6)
+#
+# @iops_max_length: #optional maximum length of the @iops burst
+#                             period, in seconds. (Since 2.6)
+#
+# @iops_rd_max_length: #optional maximum length of the @iops_rd_max
+#                                burst period, in seconds. (Since 2.6)
+#
+# @iops_wr_max_length: #optional maximum length of the @iops_wr_max
+#                                burst period, in seconds. (Since 2.6)
 #
 # @iops_size: #optional an I/O size in bytes (Since 1.7)
 #
@@ -308,6 +332,9 @@
             '*bps_max': 'int', '*bps_rd_max': 'int',
             '*bps_wr_max': 'int', '*iops_max': 'int',
             '*iops_rd_max': 'int', '*iops_wr_max': 'int',
+            '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
+            '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
+            '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
             '*iops_size': 'int', '*group': 'str', 'cache': 'BlockdevCacheInfo',
             'write_threshold': 'int' } }
 
-- 
2.7.0

[Qemu-devel] [PATCH 12/13] throttle: Check that burst_level leaks correctly

[Alberto Garcia]

This patch expands test_leak_bucket() to check that burst_level leaks
correctly.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 tests/test-throttle.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index cf81033..11ede23 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -59,6 +59,22 @@ static void test_leak_bucket(void)
     g_assert(bkt.avg == 150);
     g_assert(bkt.max == 15);
     g_assert(double_cmp(bkt.level, 0));
+
+    /* check that burst_level leaks correctly */
+    bkt.burst_level = 6;
+    bkt.max = 250;
+    bkt.burst_length = 2; /* otherwise burst_level will not leak */
+    throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 100);
+    g_assert(double_cmp(bkt.burst_level, 3.5));
+
+    throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 100);
+    g_assert(double_cmp(bkt.burst_level, 1));
+
+    throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 100);
+    g_assert(double_cmp(bkt.burst_level, 0));
+
+    throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 100);
+    g_assert(double_cmp(bkt.burst_level, 0));
 }
 
 static void test_compute_wait(void)
-- 
2.7.0

[Qemu-devel] [PATCH 13/13] throttle: Test throttle_compute_wait() during bursts

[Alberto Garcia]

This test simulates an I/O burst for more than two seconds and checks
that it works as expected.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 tests/test-throttle.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 11ede23..2501d6f 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -79,6 +79,7 @@ static void test_leak_bucket(void)
 
 static void test_compute_wait(void)
 {
+    unsigned i;
     int64_t wait;
     int64_t result;
 
@@ -114,6 +115,27 @@ static void test_compute_wait(void)
     /* time required to do half an operation */
     result = (int64_t)  NANOSECONDS_PER_SECOND / 150 / 2;
     g_assert(wait == result);
+
+    /* Perform I/O for 2.2 seconds at a rate of bkt.max */
+    bkt.burst_length = 2;
+    bkt.level = 0;
+    bkt.avg = 10;
+    bkt.max = 200;
+    for (i = 0; i < 22; i++) {
+        double units = bkt.max / 10;
+        bkt.level += units;
+        bkt.burst_level += units;
+        throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 10);
+        wait = throttle_compute_wait(&bkt);
+        g_assert(double_cmp(bkt.burst_level, 0));
+        g_assert(double_cmp(bkt.level, (i + 1) * (bkt.max - bkt.avg) / 10));
+        /* We can do bursts for the 2 seconds we have configured in
+         * burst_length. We have 100 extra miliseconds of burst
+         * because bkt.level has been leaking during this time.
+         * After that, we have to wait. */
+        result = i < 21 ? 0 : 1.8 * NANOSECONDS_PER_SECOND;
+        g_assert(wait == result);
+    }
 }
 
 /* functions to test ThrottleState initialization/destroy methods */
-- 
2.7.0

Re: [Qemu-devel] [PATCH 00/13] throttle: Allow I/O bursts for a user-defined period of time

[Kevin Wolf]

Am 05.02.2016 um 11:59 hat Alberto Garcia geschrieben:
> Hello everyone,
> 
> the current throttling code in QEMU allows limiting the I/O rate on
> block devices. Limits can be set in operations per second (IOPS) or
> bytes per second, allowing separate limits for read and write
> operations on both cases.
> 
> In its basic usage the user can set a limit of, say, 100 IOPS on a
> block device passing this option to -drive:
> 
>   throttling.iops-total=100
> 
> In addition to that, QEMU can also allow the user to do I/O bursts
> exceeding that limit up to a configurable rate:
> 
>   throttling.iops-total=100,throttling.iops-total-max=2000
> 
> With this, the user can do a burst of 2000 IOPS before they're
> throttled down to 100 IOPS. Then, after a sufficiently long period of
> unused I/O they will be able to do a burst again.
> 
> This patch series introduces the possibility to do bursts for longer
> period of times. A new setting called throttling.iops-total-max-length
> is used to define for how long bursts can be sustained.
> 
> So adding throttling.iops-total-max-length=60 to the previous
> configuration allows the user to do I/O at a rate of 2000 IOPS for 1
> minute before going down to the base rate of 100 IOPS.
> 
> This is essentially the same as described in this AWS blog post:
> 
>    https://aws.amazon.com/blogs/aws/new-ssd-backed-elastic-block-storage/
> 
> As described in the article, a use case for this feature is to allow
> better performance when booting the OS or restarting a service while
> keeping the average I/O limits lower the rest of the time.
> 
> Comments:
> 
>  - There are 6 different settings for setting I/O limits: iops-total,
>    iops-read, iops-write, bps-total, bps-read, bps-write. This series
>    adds one new setting to set the length for each one of those.
> 
>    I don't know if there's a good use case that requires such
>    fine-grained control. It's of course also possible to make it
>    simpler and add just one 'burst-length' setting that would apply
>    for all cases, but the current solution is IMHO simple enough and
>    consistent with the current API, and if we need to extend it later
>    the result is probably going to be ugly.
> 
>  - With this series we set "a maximum of X operations/second for a
>    period of T seconds". If would also be possible to make it "a
>    maximum of X operations/second up to a total of Y operations". It
>    would be equivalent (Y = X * T) but I thought the current proposal
>    makes a more clear API.
> 
> And I think that's all.

Congratulations, you found the unmaintained spot in the block layer! :-)

Anyway, the cover letter makes sense to me. And thanks for this great
writeup! Can we turn it into documentation? Throttling seems to be quite
underdocumented, and your explanation above let me understand for the
first time what these *-max options actually were.

I'll have to see whether I can review the meat of this series next week,
but for now:

Patches 1-7: Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH 00/13] throttle: Allow I/O bursts for a user-defined period of time

[Alberto Garcia]

On Fri 12 Feb 2016 06:19:06 PM CET, Kevin Wolf <kwolf@redhat.com> wrote:
> Anyway, the cover letter makes sense to me. And thanks for this great
> writeup! Can we turn it into documentation? Throttling seems to be quite
> underdocumented, and your explanation above let me understand for the
> first time what these *-max options actually were.

Yeah, I have it half written, it will go in the next version of the
series.

> Patches 1-7: Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Thanks!

Berto

Re: [Qemu-devel] [Qemu-block] [PATCH 00/13] throttle: Allow I/O bursts for a user-defined period of time

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 982 bytes --]

On Fri, Feb 05, 2016 at 12:59:10PM +0200, Alberto Garcia wrote:
>  - With this series we set "a maximum of X operations/second for a
>    period of T seconds". If would also be possible to make it "a
>    maximum of X operations/second up to a total of Y operations". It
>    would be equivalent (Y = X * T) but I thought the current proposal
>    makes a more clear API.

I find the diagram in the blog post clear.  The QEMU code is a little
harder to understand, it seems like there are too many variables and
special cases.  There are 4 core variables:

1. Refill rate (aka avg), e.g. 30 IOPS
2. Max bucket level (aka max * burst_length), e.g. 5.4 million IOPS
3. Burst rate (aka max), e.g. 3000 IOPS
4. Current bucket level

However, your patch to the core algorithm is small and makes sense to
me.  Maybe I just don't appreciate some of the subtleties that require
the existing code to use more variables.

Looks good to me.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 08/13] throttle: Add support for burst periods

[Kevin Wolf]

Am 05.02.2016 um 11:59 hat Alberto Garcia geschrieben:
> This patch adds support for burst periods to the throttling code.
> With this feature the user can keep performing bursts as defined by
> the LeakyBucket.max rate for a configurable period of time.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  include/qemu/throttle.h | 41 +++++++++++++++++++++++----
>  util/throttle.c         | 73 ++++++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 96 insertions(+), 18 deletions(-)
> 
> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
> index 8ec8951..63df690 100644
> --- a/include/qemu/throttle.h
> +++ b/include/qemu/throttle.h
> @@ -2,7 +2,7 @@
>   * QEMU throttling infrastructure
>   *
>   * Copyright (C) Nodalink, EURL. 2013-2014
> - * Copyright (C) Igalia, S.L. 2015
> + * Copyright (C) Igalia, S.L. 2015-2016
>   *
>   * Authors:
>   *   Benoît Canet <benoit.canet@nodalink.com>
> @@ -42,16 +42,47 @@ typedef enum {
>  } BucketType;
>  
>  /*
> - * The max parameter of the leaky bucket throttling algorithm can be used to
> - * allow the guest to do bursts.
> - * The max value is a pool of I/O that the guest can use without being throttled
> - * at all. Throttling is triggered once this pool is empty.
> + * This module implements I/O limits using the leaky bucket
> + * algorithm. The code is independent of the I/O units, but it is
> + * currently used for bytes per second and operations per second.
> + *
> + * Three parameters can be set by the user:
> + *
> + * - avg: the desired I/O limits in units per second.
> + * - max: the limit during bursts, also in units per second.
> + * - burst_length: the maximum length of the burst period, in seconds.
> + *
> + * Here's how it works:
> + *
> + * - The bucket level (number of performed I/O units) is kept in
> + *   bkt.level and leaks at a rate of bkt.avg units per second.
> + *
> + * - The size of the bucket is bkt.max * bkt.burst_length. Once the
> + *   bucket is full no more I/O is performed until the bucket leaks
> + *   again. This is what makes the I/O rate bkt.avg.
> + *
> + * - The bkt.avg rate does not apply until the bucket is full,
> + *   allowing the user to do bursts until then. The I/O limit during
> + *   bursts is bkt.max. To enforce this limit we keep an additional
> + *   bucket in bkt.burst_length that leaks at a rate of bkt.max units
> + *   per second.
> + *
> + * - Because of all of the above, the user can perform I/O at a
> + *   maximum of bkt.max units per second for at most bkt.burst_length
> + *   seconds in a row. After that the bucket will be full and the I/O
> + *   rate will go down to bkt.avg.
> + *
> + * - Since the bucket always leaks at a rate of bkt.avg, this also
> + *   determines how much the user needs to wait before being able to
> + *   do bursts again.
>   */

Good summary, thanks!

>  typedef struct LeakyBucket {
>      double  avg;              /* average goal in units per second */
>      double  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 */
>  } LeakyBucket;
>  
>  /* The following structure is used to configure a ThrottleState
> diff --git a/util/throttle.c b/util/throttle.c
> index 6a01cee..371c769 100644
> --- a/util/throttle.c
> +++ b/util/throttle.c
> @@ -41,6 +41,14 @@ void throttle_leak_bucket(LeakyBucket *bkt, int64_t delta_ns)
>  
>      /* make the bucket leak */
>      bkt->level = MAX(bkt->level - leak, 0);
> +
> +    /* if we allow bursts for more than one second we also need to
> +     * keep track of bkt->burst_level so the bkt->max goal per second
> +     * is attained */
> +    if (bkt->burst_length > 1) {
> +        leak = (bkt->max * (double) delta_ns) / NANOSECONDS_PER_SECOND;
> +        bkt->burst_level = MAX(bkt->burst_level - leak, 0);
> +    }
>  }
>  
>  /* Calculate the time delta since last leak and make proportionals leaks
> @@ -91,13 +99,24 @@ int64_t throttle_compute_wait(LeakyBucket *bkt)
>          return 0;
>      }
>  
> -    extra = bkt->level - bkt->max;
> +    /* If the bucket is full then we have to wait */
> +    extra = bkt->level - bkt->max * bkt->burst_length;
> +    if (extra > 0) {
> +        return throttle_do_compute_wait(bkt->avg, extra);
> +    }
>  
> -    if (extra <= 0) {
> -        return 0;
> +    /* If the bucket is not full yet we have to make sure that we
> +     * fulfill the goal of bkt->max units per second. */
> +    if (bkt->burst_length > 1) {
> +        /* We use 1/10 of the max value to smooth the throttling.
> +         * See throttle_fix_bucket() for more details. */
> +        extra = bkt->burst_level - bkt->max / 10;

I don't understand the connection between throttle_fix_bucket() and
this.

throttle_fix_bucket() seems to set a default rate for bursts, which kind
of makes sense to me (but what's the point when this is lower than the
average rate?)

Here we work on a bkt->max that is either supplied by the user and
should therefore be respected, or the default in throttle_fix_bucket()
has already been applied.

What this line does is letting the request wait for more than would be
strictly necessary, or in other words, for the last second before the
bucket runs full, you only allow a tenth of the actual maximum rate.

I understand that having any burst at all helps, so the default that
throttle_fix_bucket() sets used to make sense. I'm not so sure that it
still makes sense with its max < avg setting (max used to be additional
units on top of avg, now it's measured on its own). For the divison by
10 here, however, I'm still puzzled.

What am I missing?

> +        if (extra > 0) {
> +            return throttle_do_compute_wait(bkt->max, extra);
> +        }
>      }
>  
> -    return throttle_do_compute_wait(bkt->avg, extra);
> +    return 0;
>  }

Kevin

Re: [Qemu-devel] [PATCH 08/13] throttle: Add support for burst periods

[Alberto Garcia]

On Tue 16 Feb 2016 11:45:32 AM CET, Kevin Wolf <kwolf@redhat.com> wrote:

>> +    /* If the bucket is not full yet we have to make sure that we
>> +     * fulfill the goal of bkt->max units per second. */
>> +    if (bkt->burst_length > 1) {
>> +        /* We use 1/10 of the max value to smooth the throttling.
>> +         * See throttle_fix_bucket() for more details. */
>> +        extra = bkt->burst_level - bkt->max / 10;
>
> I don't understand the connection between throttle_fix_bucket() and
> this.
>
> throttle_fix_bucket() seems to set a default rate for bursts, which
> kind of makes sense to me (but what's the point when this is lower
> than the average rate?)

The point is to smoothen the throttling: if you request a max rate of
2000 operations per second, what you actually get is a rate of 200
operations per tenth of a second.

With the current code, if you set bkg->avg to 1000 but not bkt->max
you'll get exactly this. I'm just applying the same logic to bursts.

Berto

Re: [Qemu-devel] [Qemu-block] [PATCH 00/13] throttle: Allow I/O bursts for a user-defined period of time

[Alberto Garcia]

On Mon 15 Feb 2016 05:40:29 PM CET, Stefan Hajnoczi wrote:
> On Fri, Feb 05, 2016 at 12:59:10PM +0200, Alberto Garcia wrote:
>>  - With this series we set "a maximum of X operations/second for a
>>    period of T seconds". If would also be possible to make it "a
>>    maximum of X operations/second up to a total of Y operations". It
>>    would be equivalent (Y = X * T) but I thought the current proposal
>>    makes a more clear API.
>
> I find the diagram in the blog post clear.  The QEMU code is a little
> harder to understand, it seems like there are too many variables and
> special cases.  There are 4 core variables:
>
> 1. Refill rate (aka avg), e.g. 30 IOPS
> 2. Max bucket level (aka max * burst_length), e.g. 5.4 million IOPS
> 3. Burst rate (aka max), e.g. 3000 IOPS
> 4. Current bucket level

The blog post uses the token bucket algorithm but QEMU uses the leaky
bucket. They're equivalent, but one is the reverse of the other: in the
former the bucket is full and the performed I/O is the water that leaks
from the bucket; in the latter the bucket is empty and the performed I/O
is the water that goes into the bucket.

We have these 3 variables:

- Leak rate (avg), 30 IOPS
- Fill-up rate (max). 3000 IOPS
- Fill-up time (burst_length), 30 minutes

We allow I/O until the bucket is full and we disallow it afterwards. The
bucket leaks at 30 IOPS so that's the reason why we get that I/O rate.
We need to keep track of the bucket level (bkt->level) and make it leak
at 30 IOPS.

With this series we also want to limit the amount of water per second
that goes into the bucket before it's full. So how do we make sure that
we don't allow more than 3000 IOPS when adding water to the bucket? We
actually have the same problem as in the previous paragraph, except that
the I/O rate here is different. That's why we need bkt->burst_level that
leaks at 3000 IOPS.

It's easier to see if you think about it as a second bucket on top of
the main one that leaks at 3000 IOPS.

Berto

Re: [Qemu-devel] [Qemu-block] [PATCH 00/13] throttle: Allow I/O bursts for a user-defined period of time

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]

On Tue, Feb 16, 2016 at 04:38:04PM +0100, Alberto Garcia wrote:
> On Mon 15 Feb 2016 05:40:29 PM CET, Stefan Hajnoczi wrote:
> > On Fri, Feb 05, 2016 at 12:59:10PM +0200, Alberto Garcia wrote:
> >>  - With this series we set "a maximum of X operations/second for a
> >>    period of T seconds". If would also be possible to make it "a
> >>    maximum of X operations/second up to a total of Y operations". It
> >>    would be equivalent (Y = X * T) but I thought the current proposal
> >>    makes a more clear API.
> >
> > I find the diagram in the blog post clear.  The QEMU code is a little
> > harder to understand, it seems like there are too many variables and
> > special cases.  There are 4 core variables:
> >
> > 1. Refill rate (aka avg), e.g. 30 IOPS
> > 2. Max bucket level (aka max * burst_length), e.g. 5.4 million IOPS
> > 3. Burst rate (aka max), e.g. 3000 IOPS
> > 4. Current bucket level
> 
> The blog post uses the token bucket algorithm but QEMU uses the leaky
> bucket. They're equivalent, but one is the reverse of the other:

Yes, I find the token bucket approach clearer :).

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]