[Qemu-devel] [PATCH] throttle: refuse iops-size without iops-total/read/write

[Qemu-devel] [PATCH] throttle: refuse iops-size without iops-total/read/write

[Stefan Hajnoczi]

In a similar vein to commit ee2bdc33c913b7d765baa5aa338c29fb30a05c9a
("throttle: refuse bps_max/iops_max without bps/iops") it is likely that
the user made a configuration error if iops-size has been set but no
iops limit has been set.

Print an error message so the user can check their throttling
configuration.  They should either remove iops-size if they don't want
any throttling or specify one of iops-total, iops-read, or iops-write.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/test-throttle.c | 10 ++++++++++
 util/throttle.c       |  8 ++++++++
 2 files changed, 18 insertions(+)

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index c02be80..d584870 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -398,6 +398,14 @@ static void test_max_is_missing_limit(void)
     }
 }
 
+static void test_iops_size_is_missing_limit(void)
+{
+    /* A total/read/write iops limit is required */
+    throttle_config_init(&cfg);
+    cfg.op_size = 4096;
+    g_assert(!throttle_is_valid(&cfg, NULL));
+}
+
 static void test_have_timer(void)
 {
     /* zero structures */
@@ -653,6 +661,8 @@ int main(int argc, char **argv)
     g_test_add_func("/throttle/config/conflicting", test_conflicting_config);
     g_test_add_func("/throttle/config/is_valid",    test_is_valid);
     g_test_add_func("/throttle/config/max",         test_max_is_missing_limit);
+    g_test_add_func("/throttle/config/iops_size",
+                    test_iops_size_is_missing_limit);
     g_test_add_func("/throttle/config_functions",   test_config_functions);
     g_test_add_func("/throttle/accounting",         test_accounting);
     g_test_add_func("/throttle/groups",             test_groups);
diff --git a/util/throttle.c b/util/throttle.c
index 71246b2..654f95c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -315,6 +315,14 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
         return false;
     }
 
+    if (cfg->op_size &&
+        !cfg->buckets[THROTTLE_OPS_TOTAL].avg &&
+        !cfg->buckets[THROTTLE_OPS_READ].avg &&
+        !cfg->buckets[THROTTLE_OPS_WRITE].avg) {
+        error_setg(errp, "iops size requires an iops value to be set");
+        return false;
+    }
+
     for (i = 0; i < BUCKETS_COUNT; i++) {
         if (cfg->buckets[i].avg < 0 ||
             cfg->buckets[i].max < 0 ||
-- 
2.5.5

Re: [Qemu-devel] [PATCH] throttle: refuse iops-size without iops-total/read/write

[Alberto Garcia]

On Thu 02 Jun 2016 02:40:31 AM CEST, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> In a similar vein to commit ee2bdc33c913b7d765baa5aa338c29fb30a05c9a
> ("throttle: refuse bps_max/iops_max without bps/iops") it is likely that
> the user made a configuration error if iops-size has been set but no
> iops limit has been set.
>
> Print an error message so the user can check their throttling
> configuration.  They should either remove iops-size if they don't want
> any throttling or specify one of iops-total, iops-read, or iops-write.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH] throttle: refuse iops-size without iops-total/read/write

[Stefan Hajnoczi]

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

On Wed, Jun 01, 2016 at 05:40:31PM -0700, Stefan Hajnoczi wrote:
> In a similar vein to commit ee2bdc33c913b7d765baa5aa338c29fb30a05c9a
> ("throttle: refuse bps_max/iops_max without bps/iops") it is likely that
> the user made a configuration error if iops-size has been set but no
> iops limit has been set.
> 
> Print an error message so the user can check their throttling
> configuration.  They should either remove iops-size if they don't want
> any throttling or specify one of iops-total, iops-read, or iops-write.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/test-throttle.c | 10 ++++++++++
>  util/throttle.c       |  8 ++++++++
>  2 files changed, 18 insertions(+)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

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