Re: [Qemu-devel] [PATCH] block/mirror: honor ratelimit again

[Jeff Cody <jcody@redhat.com>]

On Tue, Apr 24, 2018 at 01:35:27PM +0100, Stefan Hajnoczi wrote:
> Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 ("block/mirror: change
> the semantic of 'force' of block-job-cancel") accidentally removed the
> ratelimit in the mirror job.
> 
> Reintroduce the ratelimit but keep the block-job-cancel force=true
> behavior that was added in commit
> b76e4458b1eb3c32e9824fe6aa51f67d2b251748.
> 
> Note that block_job_sleep_ns() returns immediately when the job is
> cancelled.  Therefore it's safe to unconditionally call
> block_job_sleep_ns() - a cancelled job does not sleep.
> 
> This commit fixes the non-deterministic qemu-iotests 185 output.  The
> test relies on the ratelimit to make the job sleep until the 'quit'
> command is processed.  Previously the job could complete before the
> 'quit' command was received since there was no ratelimit.
> 
> Cc: Liang Li <liliang.opensource@gmail.com>
> Cc: Jeff Cody <jcody@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/mirror.c             | 8 +++++---
>  tests/qemu-iotests/185.out | 4 ++--
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 820f512c7b..9436a8d5ee 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -868,12 +868,14 @@ static void coroutine_fn mirror_run(void *opaque)
>          }
>  
>          ret = 0;
> +
> +        if (s->synced && !should_complete) {
> +            delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0);
> +        }
>          trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
> +        block_job_sleep_ns(&s->common, delay_ns);
>          if (block_job_is_cancelled(&s->common) && s->common.force) {
>              break;
> -        } else if (!should_complete) {
> -            delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0);
> -            block_job_sleep_ns(&s->common, delay_ns);
>          }
>          s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
>      }
> diff --git a/tests/qemu-iotests/185.out b/tests/qemu-iotests/185.out
> index 2c4b04de73..992162f418 100644
> --- a/tests/qemu-iotests/185.out
> +++ b/tests/qemu-iotests/185.out
> @@ -36,9 +36,9 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 backing_file=TEST_DIR/t.q
>  {"return": {}}
>  Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 lazy_refcounts=off refcount_bits=16
>  {"return": {}}
> +{"return": {}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}}
> -{"return": {}}
> -{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}}
>  
>  === Start backup job and exit qemu ===
> -- 
> 2.14.3
> 

Thanks,

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc block

-Jeff