[Qemu-devel] [PATCH 0/4] iotests: Make 245 faster and more reliable

Max Reitz posted 4 patches 9 weeks ago
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190515201503.19069-1-mreitz@redhat.com
Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>
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(-)

[Qemu-devel] [PATCH 0/4] iotests: Make 245 faster and more reliable

Posted by Max Reitz 9 weeks ago
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


Re: [Qemu-devel] [PATCH 0/4] iotests: Make 245 faster and more reliable

Posted by Kevin Wolf 9 weeks ago
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