245 is a bit flakey for me, because it uses block jobs that copy 1 MB of
data but have a buffer size of 512 kB, so they may be done before the
test gets to do the things it wants to do while the check is running.
(Rate limiting doesn’t change this.)

The boring way to fix this would be to increase the amount of data.

The interesting way to fix this is to make use of auto_finalize=false
and thus keep the jobs around until the test is done with them.
However, this has one problem: In one case, 245 tries to make the target
node of a stream job read-only.  If the job is still copying data, doing
so will fail because the target node is in COR mode.  Otherwise, we get
a cryptic “Block node is read-only” message.

What the message means is “After reopening, the node will be read-only,
and that won’t work, because there is a writer on it.”  It doesn’t say
that, though, but it should.  So patch 1 makes it say something to that
effect (“Cannot make block node read-only, there is a writer on it”).

245 doesn’t care about the actual error message, both reflect that qemu
correctly detects that this node cannot be made read-only at this time.
So the other thing we have to do is let assert_qmp() accept an array of
valid error messages and choose the one that matches (if any).  Then we
can just pass both error messages to it and everything works.


Nice side effect: For me, the test duration goes down from about 12 s to
about 6 s.
(That’s because the test forgot to disable rate limiting on the jobs
before waiting for their completion.)


Max Reitz (4):
  block: Improve "Block node is read-only" message
  iotests.py: Let assert_qmp() accept an array
  iotests.py: Fix VM.run_job
  iotests: Make 245 faster and more reliable

 block.c                       | 17 ++++++++++++++++-
 tests/qemu-iotests/245        | 22 ++++++++++++++--------
 tests/qemu-iotests/245.out    | 12 ++++++++++++
 tests/qemu-iotests/iotests.py | 20 +++++++++++++++++---
 4 files changed, 59 insertions(+), 12 deletions(-)

-- 
2.21.0

Am 15.05.2019 um 22:14 hat Max Reitz geschrieben:
> 245 is a bit flakey for me, because it uses block jobs that copy 1 MB of
> data but have a buffer size of 512 kB, so they may be done before the
> test gets to do the things it wants to do while the check is running.
> (Rate limiting doesn’t change this.)
> 
> The boring way to fix this would be to increase the amount of data.
> 
> The interesting way to fix this is to make use of auto_finalize=false
> and thus keep the jobs around until the test is done with them.
> However, this has one problem: In one case, 245 tries to make the target
> node of a stream job read-only.  If the job is still copying data, doing
> so will fail because the target node is in COR mode.  Otherwise, we get
> a cryptic “Block node is read-only” message.
> 
> What the message means is “After reopening, the node will be read-only,
> and that won’t work, because there is a writer on it.”  It doesn’t say
> that, though, but it should.  So patch 1 makes it say something to that
> effect (“Cannot make block node read-only, there is a writer on it”).
> 
> 245 doesn’t care about the actual error message, both reflect that qemu
> correctly detects that this node cannot be made read-only at this time.
> So the other thing we have to do is let assert_qmp() accept an array of
> valid error messages and choose the one that matches (if any).  Then we
> can just pass both error messages to it and everything works.
> 
> 
> Nice side effect: For me, the test duration goes down from about 12 s to
> about 6 s.
> (That’s because the test forgot to disable rate limiting on the jobs
> before waiting for their completion.)

Thanks, applied to the block branch.

Kevin