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

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

[Alberto Garcia]

Hi,

here's a new version of the series that adds support for performing
I/O bursts for a user-defined period of time. Please follow the link
to the first version of the series for a complete description.

There are two important changes in this version:

a) The previous series was broken because the new parameters were
   missing from qmp-commands.hx. This is fixed now. [patch 10]
b) This series has new tests and documentation [patches 14 and 16]

I also added myself as maintainer of the throttling code [patch 17].

Regards,

Berto

v2:
- Patch 10: Add the new parameters to qmp-commands.hx
- Patch 14: New iotest for this feature
- Patch 15: Fix typo in the API documentation
- Patch 16: New document that explains the throttling infrastructure
- Patch 17: Add myself as maintainer of the throttling code

v1: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00210.html
- Initial version

Alberto Garcia (17):
  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
  qemu-iotests: Extend iotest 093 to test bursts
  qapi: Correct the name of the iops_rd parameter
  docs: Document the throttling infrastructure
  MAINTAINERS: Add myself as maintainer of the throttling code

 MAINTAINERS                |   9 ++
 block/qapi.c               |  20 ++++
 blockdev.c                 |  99 +++++++++++++-----
 docs/throttle.txt          | 252 +++++++++++++++++++++++++++++++++++++++++++++
 hmp.c                      |  12 +++
 include/qemu/throttle.h    |  55 +++++++---
 qapi/block-core.json       |  92 ++++++++++++++---
 qmp-commands.hx            |  25 +++--
 tests/qemu-iotests/093     |  65 +++++++++---
 tests/qemu-iotests/093.out |   4 +-
 tests/test-throttle.c      |  88 ++++++++++++----
 util/throttle.c            | 132 ++++++++++++++++--------
 12 files changed, 708 insertions(+), 145 deletions(-)
 create mode 100644 docs/throttle.txt

-- 
2.7.0

[Qemu-devel] [PATCH v2 01/17] 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.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 v2 02/17] 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.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 1f73478..95bc2fa 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 858f1aa..579b8af 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -255,31 +255,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 v2 03/17] 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.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 95bc2fa..94a9c73 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 579b8af..49bd3bc 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -338,15 +338,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 v2 04/17] 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.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 94a9c73..693967c 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 49bd3bc..0e7c7e0 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -315,7 +315,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 v2 05/17] 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/test-throttle.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 0e7c7e0..3e208a8 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -222,6 +222,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 v2 06/17] 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.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 693967c..5aa9a63 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 3e208a8..a0c17ac 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -257,31 +257,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)
@@ -340,15 +340,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 v2 07/17] 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.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 5aa9a63..c288efa 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 a0c17ac..34f1f9e 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -35,6 +35,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;
@@ -64,6 +67,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;
@@ -233,17 +239,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));
     }
@@ -256,29 +262,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));
@@ -315,7 +321,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);
         }
@@ -337,7 +343,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));
@@ -552,7 +558,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 v2 08/17] 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>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.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 v2 09/17] 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>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index c288efa..e8871fc 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 v2 10/17] 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 +++++++++++++++++++++++++++++++++++++++++++++------
 qmp-commands.hx      | 25 ++++++++++++++++---------
 4 files changed, 104 insertions(+), 15 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e8871fc..a5523ec 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 996cb91..1f93181 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1414,6 +1414,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' } }
 
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9fb0d78..085dc7d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2006,7 +2006,7 @@ EQMP
 
     {
         .name       = "block_set_io_throttle",
-        .args_type  = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?,iops_size:l?,group:s?",
+        .args_type  = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?,bps_max_length:l?,bps_rd_max_length:l?,bps_wr_max_length:l?,iops_max_length:l?,iops_rd_max_length:l?,iops_wr_max_length:l?,iops_size:l?,group:s?",
         .mhandler.cmd_new = qmp_marshal_block_set_io_throttle,
     },
 
@@ -2025,14 +2025,20 @@ Arguments:
 - "iops": total I/O operations per second (json-int)
 - "iops_rd": read I/O operations per second (json-int)
 - "iops_wr": write I/O operations per second (json-int)
-- "bps_max":  total max in bytes (json-int)
-- "bps_rd_max":  read max in bytes (json-int)
-- "bps_wr_max":  write max in bytes (json-int)
-- "iops_max":  total I/O operations max (json-int)
-- "iops_rd_max":  read I/O operations max (json-int)
-- "iops_wr_max":  write I/O operations max (json-int)
-- "iops_size":  I/O size in bytes when limiting (json-int)
-- "group": throttle group name (json-string)
+- "bps_max": total throughput limit during bursts, in bytes (json-int, optional)
+- "bps_rd_max": read throughput limit during bursts, in bytes (json-int, optional)
+- "bps_wr_max": write throughput limit during bursts, in bytes (json-int, optional)
+- "iops_max": total I/O operations per second during bursts (json-int, optional)
+- "iops_rd_max": read I/O operations per second during bursts (json-int, optional)
+- "iops_wr_max": write I/O operations per second during bursts (json-int, optional)
+- "bps_max_length": maximum length of the @bps_max burst period, in seconds (json-int, optional)
+- "bps_rd_max_length": maximum length of the @bps_rd_max burst period, in seconds (json-int, optional)
+- "bps_wr_max_length": maximum length of the @bps_wr_max burst period, in seconds (json-int, optional)
+- "iops_max_length": maximum length of the @iops_max burst period, in seconds (json-int, optional)
+- "iops_rd_max_length": maximum length of the @iops_rd_max burst period, in seconds (json-int, optional)
+- "iops_wr_max_length": maximum length of the @iops_wr_max burst period, in seconds (json-int, optional)
+- "iops_size":  I/O size in bytes when limiting (json-int, optional)
+- "group": throttle group name (json-string, optional)
 
 Example:
 
@@ -2049,6 +2055,7 @@ Example:
                                                "iops_max": 0,
                                                "iops_rd_max": 0,
                                                "iops_wr_max": 0,
+                                               "bps_max_length": 60,
                                                "iops_size": 0 } }
 <- { "return": {} }
 
-- 
2.7.0

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

[Eric Blake]

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

On 02/18/2016 03:27 AM, Alberto Garcia wrote:
> 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 +++++++++++++++++++++++++++++++++++++++++++++------
>  qmp-commands.hx      | 25 ++++++++++++++++---------
>  4 files changed, 104 insertions(+), 15 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index e8871fc..a5523ec 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,

Not a problem with this patch, but your argument list is getting
painfully long; so we really want to simplify this once my boxed
parameters for commands lands:
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04394.html


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Alberto Garcia]

On Mon 22 Feb 2016 05:41:27 PM CET, Eric Blake <eblake@redhat.com> wrote:
>> @@ -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,
>
> Not a problem with this patch, but your argument list is getting
> painfully long; so we really want to simplify this once my boxed
> parameters for commands lands:
> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04394.html

I will be more than happy to do it :-) Thanks!

Berto

[Qemu-devel] [PATCH v2 11/17] 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>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.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 67891b7..db2d3fb 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 v2 12/17] 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>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/test-throttle.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 34f1f9e..145ba08 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -60,6 +60,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 v2 13/17] 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>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/test-throttle.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 145ba08..59675fa 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -80,6 +80,7 @@ static void test_leak_bucket(void)
 
 static void test_compute_wait(void)
 {
+    unsigned i;
     int64_t wait;
     int64_t result;
 
@@ -115,6 +116,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

[Qemu-devel] [PATCH v2 14/17] qemu-iotests: Extend iotest 093 to test bursts

[Alberto Garcia]

This patch adds a new test that checks that the burst settings
('iops_max', 'iops_max_length', etc.) of the throttling code work as
expected.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 tests/qemu-iotests/093     | 65 ++++++++++++++++++++++++++++++++++++----------
 tests/qemu-iotests/093.out |  4 +--
 2 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index c0e9e2b..ce8e13c 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -3,7 +3,7 @@
 # Tests for IO throttling
 #
 # Copyright (C) 2015 Red Hat, Inc.
-# Copyright (C) 2015 Igalia, S.L.
+# Copyright (C) 2015-2016 Igalia, S.L.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -21,6 +21,8 @@
 
 import iotests
 
+nsec_per_sec = 1000000000
+
 class ThrottleTestCase(iotests.QMPTestCase):
     test_img = "null-aio://"
     max_drives = 3
@@ -42,6 +44,15 @@ class ThrottleTestCase(iotests.QMPTestCase):
     def tearDown(self):
         self.vm.shutdown()
 
+    def configure_throttle(self, ndrives, params):
+        params['group'] = 'test'
+
+        # Set the I/O throttling parameters to all drives
+        for i in range(0, ndrives):
+            params['device'] = 'drive%d' % i
+            result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params)
+            self.assert_qmp(result, 'return', {})
+
     def do_test_throttle(self, ndrives, seconds, params):
         def check_limit(limit, num):
             # IO throttling algorithm is discrete, allow 10% error so the test
@@ -50,23 +61,13 @@ class ThrottleTestCase(iotests.QMPTestCase):
                    (num < seconds * limit * 1.1 / ndrives
                    and num > seconds * limit * 0.9 / ndrives)
 
-        nsec_per_sec = 1000000000
-
-        params['group'] = 'test'
-
-        # Set the I/O throttling parameters to all drives
-        for i in range(0, ndrives):
-            params['device'] = 'drive%d' % i
-            result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params)
-            self.assert_qmp(result, 'return', {})
-
         # Set vm clock to a known value
         ns = seconds * nsec_per_sec
         self.vm.qtest("clock_step %d" % ns)
 
-        # Submit enough requests. They will drain bps_max and iops_max, but the
-        # rest requests won't get executed until we advance the virtual clock
-        # with qtest interface
+        # Submit enough requests so the throttling mechanism kicks
+        # in. The throttled requests won't be executed until we
+        # advance the virtual clock.
         rq_size = 512
         rd_nr = max(params['bps'] / rq_size / 2,
                     params['bps_rd'] / rq_size,
@@ -142,8 +143,44 @@ class ThrottleTestCase(iotests.QMPTestCase):
             for tk in params:
                 limits = dict([(k, 0) for k in params])
                 limits[tk] = params[tk] * ndrives
+                self.configure_throttle(ndrives, limits)
                 self.do_test_throttle(ndrives, 5, limits)
 
+    def test_burst(self):
+        params = {"bps": 4096,
+                  "bps_rd": 4096,
+                  "bps_wr": 4096,
+                  "iops": 10,
+                  "iops_rd": 10,
+                  "iops_wr": 10,
+                 }
+        ndrives = 1
+        # Pick each out of all possible params and test
+        for tk in params:
+            rate = params[tk] * ndrives
+            burst_rate = rate * 7
+            burst_length = 4
+
+            # Configure the throttling settings
+            settings = dict([(k, 0) for k in params])
+            settings[tk] = rate
+            settings['%s_max' % tk] = burst_rate
+            settings['%s_max_length' % tk] = burst_length
+            self.configure_throttle(ndrives, settings)
+
+            # Wait for the bucket to empty so we can do bursts
+            wait_ns = nsec_per_sec * burst_length * burst_rate / rate
+            self.vm.qtest("clock_step %d" % wait_ns)
+
+            # Test I/O at the max burst rate
+            limits = dict([(k, 0) for k in params])
+            limits[tk] = burst_rate
+            self.do_test_throttle(ndrives, burst_length, limits)
+
+            # Now test I/O at the normal rate
+            limits[tk] = rate
+            self.do_test_throttle(ndrives, 5, limits)
+
 class ThrottleTestCoroutine(ThrottleTestCase):
     test_img = "null-co://"
 
diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out
index fbc63e6..89968f3 100644
--- a/tests/qemu-iotests/093.out
+++ b/tests/qemu-iotests/093.out
@@ -1,5 +1,5 @@
-..
+....
 ----------------------------------------------------------------------
-Ran 2 tests
+Ran 4 tests
 
 OK
-- 
2.7.0

[Qemu-devel] [PATCH v2 15/17] qapi: Correct the name of the iops_rd parameter

[Alberto Garcia]

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 qapi/block-core.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index fbbc709..9bf1b22 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1321,7 +1321,7 @@
 #
 # @iops: total I/O operations per second
 #
-# @ops_rd: read I/O operations per second
+# @iops_rd: read I/O operations per second
 #
 # @iops_wr: write I/O operations per second
 #
-- 
2.7.0

[Qemu-devel] [PATCH v2 16/17] docs: Document the throttling infrastructure

[Alberto Garcia]

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 docs/throttle.txt | 252 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 252 insertions(+)
 create mode 100644 docs/throttle.txt

diff --git a/docs/throttle.txt b/docs/throttle.txt
new file mode 100644
index 0000000..28204e4
--- /dev/null
+++ b/docs/throttle.txt
@@ -0,0 +1,252 @@
+The QEMU throttling infrastructure
+==================================
+Copyright (C) 2016 Igalia, S.L.
+Author: Alberto Garcia <berto@igalia.com>
+
+This work is licensed under the terms of the GNU GPL, version 2 or
+later. See the COPYING file in the top-level directory.
+
+Introduction
+------------
+QEMU includes a throttling module that can be used to set limits to
+I/O operations. The code itself is generic and independent of the I/O
+units, but it is currenly used to limit the number of bytes per second
+and operations per second (IOPS) when performing disk I/O.
+
+This document explains how to use the throttling code in QEMU, and how
+it works internally. The implementation is in throttle.c.
+
+
+Using throttling to limit disk I/O
+----------------------------------
+Two aspects of the disk I/O can be limited: the number of bytes per
+second and the number of operations per second (IOPS). For each one of
+them the user can set a global limit or separate limits for read and
+write operations. This gives us a total of six different parameters.
+
+I/O limits can be set using the throttling.* parameters of -drive, or
+using the QMP 'block_set_io_throttle' command. These are the names of
+the parameters for both cases:
+
+|-----------------------+-----------------------|
+| -drive                | block_set_io_throttle |
+|-----------------------+-----------------------|
+| throttling.iops-total | iops                  |
+| throttling.iops-read  | iops_rd               |
+| throttling.iops-write | iops_wr               |
+| throttling.bps-total  | bps                   |
+| throttling.bps-read   | bps_rd                |
+| throttling.bps-write  | bps_wr                |
+|-----------------------+-----------------------|
+
+It is possible to set limits for both IOPS and bps and the same time,
+and for each case we can decide whether to have separate read and
+write limits or not, but note that if iops-total is set then neither
+iops-read nor iops-write can be set. The same applies to bps-total and
+bps-read/write.
+
+The default value of these parameters is 0, and it means 'unlimited'.
+
+In its most basic usage, the user can add a drive to QEMU with a limit
+of 100 IOPS with the following -drive line:
+
+   -drive file=hd0.qcow2,throttling.iops-total=100
+
+We can do the same using QMP. In this case all these parameters are
+mandatory, so we must set to 0 the ones that we don't want to limit:
+
+   { "execute": "block_set_io_throttle",
+     "arguments": {
+        "device": "virtio0",
+        "iops": 100,
+        "iops_rd": 0,
+        "iops_wr": 0,
+        "bps": 0,
+        "bps_rd": 0,
+        "bps_wr": 0
+     }
+   }
+
+
+I/O bursts
+----------
+In addition to the basic limits we have just seen, QEMU allows the
+user to do bursts of I/O for a configurable amount of time. A burst is
+an amount of I/O that can exceed the basic limit. Bursts are useful to
+allow better performance when there are peaks of activity (the OS
+boots, a service needs to be restarted) while keeping the average
+limits lower the rest of the time.
+
+Two parameters control bursts: their length and the maximum amount of
+I/O they allow. These two can be configured separately for each one of
+the six basic parameters described in the previous section, but in
+this section we'll use 'iops-total' as an example.
+
+The I/O limit during bursts is set using 'iops-total-max', and the
+maximum length (in seconds) is set with 'iops-total-max-length'. So if
+we want to configure a drive with a basic limit of 100 IOPS and allow
+bursts of 2000 IOPS for 60 seconds, we would do it like this (the line
+is split for clarity):
+
+   -drive file=hd0.qcow2,
+          throttling.iops-total=100,
+          throttling.iops-total-max=2000,
+          throttling.iops-total-max-length=60
+
+Or, with QMP:
+
+   { "execute": "block_set_io_throttle",
+     "arguments": {
+        "device": "virtio0",
+        "iops": 100,
+        "iops_rd": 0,
+        "iops_wr": 0,
+        "bps": 0,
+        "bps_rd": 0,
+        "bps_wr": 0,
+        "iops_max": 2000,
+        "iops_max_length": 60,
+     }
+   }
+
+With this, the user can perform I/O on hd0.qcow2 at a rate of 2000
+IOPS for 1 minute before it's throttled down to 100 IOPS.
+
+The user will be able to do bursts again if there's a sufficiently
+long period of time with unused I/O (see below for details).
+
+The default value for 'iops-total-max' is 0 and it means that bursts
+are not allowed. 'iops-total-max-length' can only be set if
+'iops-total-max' is set as well, and its default value is 1 second.
+
+Here's the complete list of parameters for configuring bursts:
+
+|----------------------------------+-----------------------|
+| -drive                           | block_set_io_throttle |
+|----------------------------------+-----------------------|
+| throttling.iops-total-max        | iops_max              |
+| throttling.iops-total-max-length | iops_max_length       |
+| throttling.iops-read-max         | iops_rd_max           |
+| throttling.iops-read-max-length  | iops_rd_max_length    |
+| throttling.iops-write-max        | iops_wr_max           |
+| throttling.iops-write-max-length | iops_wr_max_length    |
+| throttling.bps-total-max         | bps_max               |
+| throttling.bps-total-max-length  | bps_max_length        |
+| throttling.bps-read-max          | bps_rd_max            |
+| throttling.bps-read-max-length   | bps_rd_max_length     |
+| throttling.bps-write-max         | bps_wr_max            |
+| throttling.bps-write-max-length  | bps_wr_max_length     |
+|----------------------------------+-----------------------|
+
+
+Controlling the size of I/O operations
+--------------------------------------
+When applying IOPS limits all I/O operations are treated equally
+regardless of their size. This means that the user can take advantage
+of this in order to circumvent the limits and submit one huge I/O
+request instead of several smaller ones.
+
+QEMU provides a setting called throttling.iops-size to prevent this
+from happening. This setting specifies the size (in bytes) of an I/O
+request for accounting purposes. Larger requests will be counted
+proportionally to this size.
+
+For example, if iops-size is set to 4096 then an 8KB request will be
+counted as two, and a 6KB request will be counted as one and a
+half. This only applies to requests larger than iops-size: smaller
+requests will be always counted as one, no matter their size.
+
+The default value of iops-size is 0 and it means that the size of the
+requests is never taken into account when applying IOPS limits.
+
+
+Applying I/O limits to groups of disks
+--------------------------------------
+In all the examples so far we have seen how to apply limits to the I/O
+performed on individual drives, but QEMU allows grouping drives so
+they all share the same limits.
+
+The way it works is that each drive with I/O limits is assigned to a
+group named using the throttling.group parameter. If this parameter is
+not specified, then the device name (i.e. 'virtio0', 'ide0-hd0') will
+be used as the group name.
+
+Limits set using the throttling.* parameters discussed earlier in this
+document apply to the combined I/O of all members of a group.
+
+Consider this example:
+
+   -drive file=hd1.qcow2,throttling.iops-total=6000,throttling.group=foo
+   -drive file=hd2.qcow2,throttling.iops-total=6000,throttling.group=foo
+   -drive file=hd3.qcow2,throttling.iops-total=3000,throttling.group=bar
+   -drive file=hd4.qcow2,throttling.iops-total=6000,throttling.group=foo
+   -drive file=hd5.qcow2,throttling.iops-total=3000,throttling.group=bar
+   -drive file=hd6.qcow2,throttling.iops-total=5000
+
+Here hd1, hd2 and hd4 are all members of a group named 'foo' with a
+combined IOPS limit of 6000, and hd3 and hd5 are members of 'bar'. hd6
+is left alone (technically it is part of a 1-member group).
+
+Limits are applied in a round-robin fashion so if there are concurrent
+I/O requests on several drives of the same group they will be
+distributed evenly.
+
+When I/O limits are applied to an existing drive using the QMP command
+'block_set_io_throttle', the following things need to be taken into
+account:
+
+   - I/O limits are shared within the same group, so new values will
+     affect all members and overwrite the previous settings. In other
+     words: if different limits are applied to members of the same
+     group, the last one wins.
+
+   - If 'group' is unset it is assumed to be the current group of that
+     drive. If the drive is not in a group yet, it will be added to a
+     group named after the device name.
+
+   - If 'group' is set then the drive will be moved to that group if
+     it was member of a different one. In this case the limits
+     specified in the parameters will be applied to the new group
+     only.
+
+   - I/O limits can be disabled by setting all of them to 0. In this
+     case the device will be removed from its group and the rest of
+     its members will not be affected. The 'group' parameter is
+     ignored.
+
+
+The Leaky Bucket algorithm
+--------------------------
+I/O limits in QEMU are implemented using the leaky bucket algorithm
+(specifically the "Leaky bucket as a meter" variant).
+
+This algorithm uses the analogy of a bucket that leaks water
+constantly. The water that gets into the bucket represents the I/O
+that has been performed, and no more I/O is allowed once the bucket is
+full.
+
+To see the way this corresponds to the throttling parameters in QEMU,
+consider the following values:
+
+  iops-total=100
+  iops-total-max=2000
+  iops-total-max-length=60
+
+  - Water leaks from the bucket at a rate of 100 IOPS.
+  - Water can be added to the bucket at a rate of 2000 IOPS.
+  - The size of the bucket is 2000 x 60 = 120000
+  - If 'iops-total-max-length' is unset then the bucket size is 100.
+
+The bucket is initially empty, therefore water can be added until it's
+full at a rate of 2000 IOPS (the burst rate). Once the bucket is full
+we can only add as much water as it leaks, therefore the I/O rate is
+reduced to 100 IOPS. If we add less water than it leaks then the
+bucket will start to empty, allowing for bursts again.
+
+Note that since water is leaking from the bucket even during bursts,
+it will take a bit more than 60 seconds at 2000 IOPS to fill it
+up. After those 60 seconds the bucket will have leaked 60 x 100 =
+6000, allowing for 3 more seconds of I/O at 2000 IOPS.
+
+Also, due to the way the algorithm works, longer burst can be done at
+a lower I/O rate, e.g. 1000 IOPS during 120 seconds.
-- 
2.7.0

[Qemu-devel] [PATCH v2 17/17] MAINTAINERS: Add myself as maintainer of the throttling code

[Alberto Garcia]

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 02710f8..c2f2dd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1276,6 +1276,15 @@ S: Maintained
 F: include/qemu/sockets.h
 F: util/qemu-sockets.c
 
+Throttling infrastructure
+M: Alberto Garcia <berto@igalia.com>
+S: Supported
+F: block/throttle-groups.c
+F: include/block/throttle-groups.h
+F: include/qemu/throttle.h
+F: util/throttle.c
+L: qemu-block@nongnu.org
+
 Usermode Emulation
 ------------------
 Overall
-- 
2.7.0

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

[Kevin Wolf]

Am 18.02.2016 um 11:26 hat Alberto Garcia geschrieben:
> Hi,
> 
> here's a new version of the series that adds support for performing
> I/O bursts for a user-defined period of time. Please follow the link
> to the first version of the series for a complete description.
> 
> There are two important changes in this version:
> 
> a) The previous series was broken because the new parameters were
>    missing from qmp-commands.hx. This is fixed now. [patch 10]
> b) This series has new tests and documentation [patches 14 and 16]
> 
> I also added myself as maintainer of the throttling code [patch 17].

Thanks, applied to the block branch.

Kevin