Re: [Qemu-block] [Qemu-devel] [PATCH 05/42] blockjob: Introduce block_job_ratelimit_get_delay()

[John Snow]

On 05/09/2018 12:26 PM, Kevin Wolf wrote:
> This gets us rid of more direct accesses to BlockJob fields from the
> job drivers.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>

Reviewed-by: John Snow <address@hidden>

> ---
>  include/block/blockjob_int.h |  8 ++++++++
>  block/backup.c               | 18 +++++++-----------
>  block/commit.c               |  4 ++--
>  block/mirror.c               |  5 +----
>  block/stream.c               |  4 ++--
>  blockjob.c                   |  9 +++++++++
>  6 files changed, 29 insertions(+), 19 deletions(-)
> 

[...]

> diff --git a/block/backup.c b/block/backup.c
> index 8468fd9f94..cfdb6ecdf5 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -325,21 +325,17 @@ static void backup_complete(BlockJob *job, void *opaque)
>  
>  static bool coroutine_fn yield_and_check(BackupBlockJob *job)
>  {
> +    uint64_t delay_ns;
> +
>      if (block_job_is_cancelled(&job->common)) {
>          return true;
>      }
>  
> -    /* we need to yield so that bdrv_drain_all() returns.
> -     * (without, VM does not reboot)
> -     */

I'll kind of miss this terse comment. "without, VM does not reboot" is
very direct and to the point. :)

> -    if (job->common.speed) {
> -        uint64_t delay_ns = ratelimit_calculate_delay(&job->common.limit,
> -                                                      job->bytes_read);
> -        job->bytes_read = 0;
> -        block_job_sleep_ns(&job->common, delay_ns);
> -    } else {
> -        block_job_sleep_ns(&job->common, 0);
> -    }
> +    /* We need to yield even for delay_ns = 0 so that bdrv_drain_all() can
> +     * return. Without a yield, the VM would not reboot. */
> +    delay_ns = block_job_ratelimit_get_delay(&job->common, job->bytes_read);
> +    job->bytes_read = 0;
> +    block_job_sleep_ns(&job->common, delay_ns);
>  
>      if (block_job_is_cancelled(&job->common)) {
>          return true;