Re: [PATCH 0/7] iotests/129: Fix it

[Vladimir Sementsov-Ogievskiy]

13.01.2021 17:06, Max Reitz wrote:
> Hi,
>
> There are some problems with iotests 129 (perhaps more than these, but
> these are the ones I know of):
>
> 1. It checks @busy to see whether a block job is still running; however,
>     block jobs tend to unset @busy all the time (when they yield).
>     [Fixed by patch 3]
>
> 2. It uses blockdev throttling, which quite some time ago has been moved
>     to the BB level; since then, such throttling will no longer affect
>     block jobs.  We can get throttling to work by using a throttle filter
>     node.
>     [Fixed by patch 4]
>
> 3. The mirror job has a large buffer size by default.  A simple drain
>     may lead to it making significant process, which is kind of
>     dangerous, because we don’t want the job to complete.

Not quite clear to me. iotest 129 wants to mirror 128M of data. Mirror by
default will have 1M chunk size and maximum of 16 parallel requests. So with
throttling (even if throttling can't correctly handle parallel requests)
we will not exceed 16M of progress.. Why we need limiting buffer size?

>     To get around this, we can simply limit its buffer size.  (And we
>     should make the commit job an actual commit job instead of an active
>     commit (which is just mirror), because the commit interface does not
>     allow setting a buffer size.)
>     [Fixed by patches 5 and 6]
>
> This series fixes those things, and now 129 seems to reliably pass for
> me.
>
>
> Apart from the major issues above, there are also minor flaws:
>
> - It doesn’t remove the test images.
>    [Fixed by patches 1 and 2]
>
> - pylint and mypy complain.
>    (Running mypy with the options given in 297.)
>    [Patch 4 removes one pylint complaint; patch 7 the rest.]
>
>
> Max Reitz (7):
>    iotests: Move try_remove to iotests.py
>    iotests/129: Remove test images in tearDown()
>    iotests/129: Do not check @busy
>    iotests/129: Use throttle node
>    iotests/129: Actually test a commit job
>    iotests/129: Limit mirror job's buffer size
>    iotests/129: Clean up pylint and mypy complaints
>
>   tests/qemu-iotests/124        |  8 +---
>   tests/qemu-iotests/129        | 76 ++++++++++++++++++++++-------------
>   tests/qemu-iotests/iotests.py | 11 +++--
>   3 files changed, 55 insertions(+), 40 deletions(-)


--
Best regards,
Vladimir