[Qemu-devel] [PATCH for-2.7 v2 0/2] Don't allow burst limits to be lower than the normal limits

[Qemu-devel] [PATCH for-2.7 v2 0/2] Don't allow burst limits to be lower than the normal limits

[Alberto Garcia]

Hello,

Gu Nini found this problem and reported it in

https://bugzilla.redhat.com/show_bug.cgi?id=1355665

When setting the throttling configuration, the burst limits can be
lower than the normal limits. This does not making any sense and
behaves oddly, so let's forbid it.

Berto

v2:
- Simplify error message [Eric]

Alberto Garcia (2):
  throttle: Don't allow burst limits to be lower than the normal limits
  throttle: Test burst limits lower than the normal limits

 tests/test-throttle.c | 8 ++++++++
 util/throttle.c       | 5 +++++
 2 files changed, 13 insertions(+)

-- 
2.8.1

[Qemu-devel] [PATCH for-2.7 v2 1/2] throttle: Don't allow burst limits to be lower than the normal limits

[Alberto Garcia]

Setting FOO_max to a value that is lower than FOO does not make
sense, and it produces odd results depending on the value of
FOO_max_length. Although the user should not set that configuration
in the first place it's better to reject it explicitly.

https://bugzilla.redhat.com/show_bug.cgi?id=1355665

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reported-by: Gu Nini <ngu@redhat.com>
---
 util/throttle.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/util/throttle.c b/util/throttle.c
index 654f95c..3817d9b 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -348,6 +348,11 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
                        " bps/iops values");
             return false;
         }
+
+        if (cfg->buckets[i].max && cfg->buckets[i].max < cfg->buckets[i].avg) {
+            error_setg(errp, "bps_max/iops_max cannot be lower than bps/iops");
+            return false;
+        }
     }
 
     return true;
-- 
2.8.1

[Qemu-devel] [PATCH for-2.7 v2 2/2] throttle: Test burst limits lower than the normal limits

[Alberto Garcia]

This checks that making FOO_max lower than FOO is not allowed.

We could also forbid having FOO_max == FOO, but that doesn't have
any odd side effects and it would require us to update several other
tests, so let's keep it simple.

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

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index afe094b..363b59a 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -394,6 +394,14 @@ static void test_max_is_missing_limit(void)
         cfg.buckets[i].max = 0;
         cfg.buckets[i].avg = 100;
         g_assert(throttle_is_valid(&cfg, NULL));
+
+        cfg.buckets[i].max = 30;
+        cfg.buckets[i].avg = 100;
+        g_assert(!throttle_is_valid(&cfg, NULL));
+
+        cfg.buckets[i].max = 100;
+        cfg.buckets[i].avg = 100;
+        g_assert(throttle_is_valid(&cfg, NULL));
     }
 }
 
-- 
2.8.1

Re: [Qemu-devel] [PATCH for-2.7 v2 0/2] Don't allow burst limits to be lower than the normal limits

[Eric Blake]

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

On 07/28/2016 02:08 AM, Alberto Garcia wrote:
> Hello,
> 
> Gu Nini found this problem and reported it in
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1355665
> 
> When setting the throttling configuration, the burst limits can be
> lower than the normal limits. This does not making any sense and
> behaves oddly, so let's forbid it.
> 
> Berto
> 
> v2:
> - Simplify error message [Eric]
> 
> Alberto Garcia (2):
>   throttle: Don't allow burst limits to be lower than the normal limits
>   throttle: Test burst limits lower than the normal limits

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
>  tests/test-throttle.c | 8 ++++++++
>  util/throttle.c       | 5 +++++
>  2 files changed, 13 insertions(+)
> 

-- 
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 for-2.7 v2 0/2] Don't allow burst limits to be lower than the normal limits

[Stefan Hajnoczi]

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

On Thu, Jul 28, 2016 at 11:08:11AM +0300, Alberto Garcia wrote:
> Hello,
> 
> Gu Nini found this problem and reported it in
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1355665
> 
> When setting the throttling configuration, the burst limits can be
> lower than the normal limits. This does not making any sense and
> behaves oddly, so let's forbid it.
> 
> Berto
> 
> v2:
> - Simplify error message [Eric]
> 
> Alberto Garcia (2):
>   throttle: Don't allow burst limits to be lower than the normal limits
>   throttle: Test burst limits lower than the normal limits
> 
>  tests/test-throttle.c | 8 ++++++++
>  util/throttle.c       | 5 +++++
>  2 files changed, 13 insertions(+)
> 
> -- 
> 2.8.1
> 

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 --]