Re: [PATCH 3/6] blockjob: let ratelimit handle a speed of 0

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

10.05.2021 11:59, Emanuele Giuseppe Esposito wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   blockjob.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index dc1d9e0e46..046c1bcd66 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -300,10 +300,6 @@ bool block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
>   
>   int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n)
>   {
> -    if (!job->speed) {
> -        return 0;
> -    }
> -
>       return ratelimit_calculate_delay(&job->limit, n);
>   }
>   
> @@ -473,11 +469,9 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
>       blk_set_allow_aio_context_change(blk, true);
>   
>       /* Only set speed when necessary to avoid NotSupported error */

strange comment, I'd drop it.

> -    if (speed != 0) {
> -        if (!block_job_set_speed(job, speed, errp)) {
> -            job_early_fail(&job->job);
> -            return NULL;
> -        }
> +    if (!block_job_set_speed(job, speed, errp)) {
> +        job_early_fail(&job->job);
> +        return NULL;
>       }
>   
>       return job;
> 

Probably, we should store speed into RateLimit, and add ratelimit_get_speed() call. Then, drop job->speed field.


with dropped "Only set" comment:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir