This patch reuses the code to set throttle limits.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
blockdev.c | 53 +++--------------------------------------------------
1 file changed, 3 insertions(+), 50 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 43f706d..364df4d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2570,6 +2570,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
BlockDriverState *bs;
BlockBackend *blk;
AioContext *aio_context;
+ ThrottleLimits *tlimit;
blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
arg->has_id ? arg->id : NULL,
@@ -2587,56 +2588,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
goto out;
}
- throttle_config_init(&cfg);
- cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
- cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd;
- cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
-
- cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
- cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd;
- cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
-
- if (arg->has_bps_max) {
- cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
- }
- if (arg->has_bps_rd_max) {
- cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
- }
- if (arg->has_bps_wr_max) {
- cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
- }
- if (arg->has_iops_max) {
- cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
- }
- if (arg->has_iops_rd_max) {
- cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
- }
- if (arg->has_iops_wr_max) {
- cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
- }
-
- if (arg->has_bps_max_length) {
- cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
- }
- if (arg->has_bps_rd_max_length) {
- cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
- }
- if (arg->has_bps_wr_max_length) {
- cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length;
- }
- if (arg->has_iops_max_length) {
- cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
- }
- if (arg->has_iops_rd_max_length) {
- cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
- }
- if (arg->has_iops_wr_max_length) {
- cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
- }
-
- if (arg->has_iops_size) {
- cfg.op_size = arg->iops_size;
- }
+ tlimit = qapi_BlockIOThrottle_base(arg);
+ throttle_config_to_limits(&cfg, tlimit);
if (!throttle_is_valid(&cfg, errp)) {
goto out;
--
1.8.3.1
On Mon 02 Oct 2017 04:33:29 PM CEST, Pradeep Jagadeesh wrote:
> - if (arg->has_iops_rd_max_length) {
> - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
> - }
> - if (arg->has_iops_wr_max_length) {
> - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
> - }
> -
> - if (arg->has_iops_size) {
> - cfg.op_size = arg->iops_size;
> - }
The old code takes an empty ThrottleConfig, and initializes it using the
values from a BlockIOThrottle structure...
> + tlimit = qapi_BlockIOThrottle_base(arg);
> + throttle_config_to_limits(&cfg, tlimit);
...but the new code does the exact opposite (?).
Berto
On 10/13/2017 4:29 PM, Alberto Garcia wrote:
> On Mon 02 Oct 2017 04:33:29 PM CEST, Pradeep Jagadeesh wrote:
>
>> - if (arg->has_iops_rd_max_length) {
>> - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
>> - }
>> - if (arg->has_iops_wr_max_length) {
>> - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
>> - }
>> -
>> - if (arg->has_iops_size) {
>> - cfg.op_size = arg->iops_size;
>> - }
>
> The old code takes an empty ThrottleConfig, and initializes it using the
> values from a BlockIOThrottle structure...
>
>> + tlimit = qapi_BlockIOThrottle_base(arg);
>> + throttle_config_to_limits(&cfg, tlimit);
>
> ...but the new code does the exact opposite (?).
Its a mistake of a call. It should be throttle_limits_to_config()
I will change it.
Regards,
Pradeep
>
> Berto
>
On Thu 02 Nov 2017 11:55:26 AM CET, Pradeep Jagadeesh wrote:
>>> - if (arg->has_iops_rd_max_length) {
>>> - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
>>> - }
>>> - if (arg->has_iops_wr_max_length) {
>>> - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
>>> - }
>>> -
>>> - if (arg->has_iops_size) {
>>> - cfg.op_size = arg->iops_size;
>>> - }
>>
>> The old code takes an empty ThrottleConfig, and initializes it using the
>> values from a BlockIOThrottle structure...
>>
>>> + tlimit = qapi_BlockIOThrottle_base(arg);
>>> + throttle_config_to_limits(&cfg, tlimit);
>>
>> ...but the new code does the exact opposite (?).
>
> Its a mistake of a call. It should be throttle_limits_to_config()
> I will change it.
Thanks! It would be nice if the series had a few tests to verify that
this all is working as expected :-)
Berto
On 11/2/2017 12:00 PM, Alberto Garcia wrote:
> On Thu 02 Nov 2017 11:55:26 AM CET, Pradeep Jagadeesh wrote:
>
>>>> - if (arg->has_iops_rd_max_length) {
>>>> - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
>>>> - }
>>>> - if (arg->has_iops_wr_max_length) {
>>>> - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
>>>> - }
>>>> -
>>>> - if (arg->has_iops_size) {
>>>> - cfg.op_size = arg->iops_size;
>>>> - }
>>>
>>> The old code takes an empty ThrottleConfig, and initializes it using the
>>> values from a BlockIOThrottle structure...
>>>
>>>> + tlimit = qapi_BlockIOThrottle_base(arg);
>>>> + throttle_config_to_limits(&cfg, tlimit);
>>>
>>> ...but the new code does the exact opposite (?).
>>
>> Its a mistake of a call. It should be throttle_limits_to_config()
>> I will change it.
>
> Thanks! It would be nice if the series had a few tests to verify that
> this all is working as expected :-)
Sure!. Next time on!.
-Pradeep
>
> Berto
>
© 2016 - 2026 Red Hat, Inc.