Don't allow burst limits to be lower than the normal limits

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

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

---
 util/throttle.c | 5 +++++
 1 file changed, 5 insertions(+)

index 654f95c..3817d9b 100644
@@ -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

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.

---
 tests/test-throttle.c | 8 ++++++++
 1 file changed, 8 insertions(+)

index afe094b..363b59a 100644
@@ -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

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

Stefan