Re: [Qemu-devel] [PATCH for-2.12 v2] qemu-iotests: update 185 output

[Stefan Hajnoczi]

On Wed, Apr 04, 2018 at 06:16:12PM +0200, Max Reitz wrote:
> On 2018-04-04 17:01, Stefan Hajnoczi wrote:
>  === Start mirror job and exit qemu ===
> 
> This seems to be independent of whether there is actually data on
> TEST_IMG (the commit source), so something doesn't seem quite right with
> the block job throttling here...?

I've been playing around with this test failure.  Let's leave it broken
(!) in QEMU 2.12 because this test has uncovered a block job ratelimit
issue that's not a regression.  The fix shouldn't be rushed.

block/mirror.c calculates delay_ns but then discards it:

  static void coroutine_fn mirror_run(void *opaque)
  {
      ...

      for (;;) {
          ...delay_ns is calculated based on job speed...

          ret = 0;
          trace_mirror_before_sleep(s, cnt, s->synced, 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);
                ^--- we discard it!
              block_job_sleep_ns(&s->common, delay_ns);
          }
          s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
      }

      ...
  }

This is why the mirror-based tests are completing even though we'd
expect them to only reach the "next" iteration due to the job speed.

Increasing the 4 MB write operation in the test to >16 MB (the mirror
buffer size) doesn't solve the problem because the next QMP command will
race with the job's 0 ns sleep.  It would just make the test unreliable.

I'll work on the following for QEMU 2.13:

1. Avoid spurious block_job_enter() from block_job_drain().  This
   eliminates the extra block job iteration that changed the output in
   the first place.

2. Honor the job speed in block/mirror.c.

The end result will be that the test output will not require changes.

Stefan

signature.asc
Description: PGP signature