[PATCH 0/2] enhance iotest 223 coverage

Eric Blake posted 2 patches 4 years, 6 months ago
Failed in applying to current master (apply log)
tests/qemu-iotests/223     | 114 ++++++++++++++++++++++++------
tests/qemu-iotests/223.out | 138 +++++++++++++++++++++++++++++++++++++
2 files changed, 231 insertions(+), 21 deletions(-)
[PATCH 0/2] enhance iotest 223 coverage
Posted by Eric Blake 4 years, 6 months ago
Commit 506902c6 dropped non-iothread coverage in order to test iothread,
better is to run things twice.  In doing this, I found it easier to
edit the test when the log shows what commands were triggering various
responses.

Eric Blake (2):
  tests: Make iotest 223 easier to edit
  tests: More iotest 223 improvements

 tests/qemu-iotests/223     | 114 ++++++++++++++++++++++++------
 tests/qemu-iotests/223.out | 138 +++++++++++++++++++++++++++++++++++++
 2 files changed, 231 insertions(+), 21 deletions(-)

-- 
2.21.0


Re: [PATCH 0/2] enhance iotest 223 coverage
Posted by Vladimir Sementsov-Ogievskiy 4 years, 6 months ago
24.09.2019 17:35, Eric Blake wrote:
> Commit 506902c6 dropped non-iothread coverage in order to test iothread,
> better is to run things twice.  In doing this, I found it easier to
> edit the test when the log shows what commands were triggering various
> responses.

Did you consider adding -iothread to tests/qemu-iotests/check instead, to be
able to run any (or some) tests with or without iothread?

> 
> Eric Blake (2):
>    tests: Make iotest 223 easier to edit
>    tests: More iotest 223 improvements
> 
>   tests/qemu-iotests/223     | 114 ++++++++++++++++++++++++------
>   tests/qemu-iotests/223.out | 138 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 231 insertions(+), 21 deletions(-)
> 


-- 
Best regards,
Vladimir
Re: [PATCH 0/2] enhance iotest 223 coverage
Posted by Eric Blake 4 years, 6 months ago
On 9/24/19 2:26 PM, Vladimir Sementsov-Ogievskiy wrote:
> 24.09.2019 17:35, Eric Blake wrote:
>> Commit 506902c6 dropped non-iothread coverage in order to test iothread,
>> better is to run things twice.  In doing this, I found it easier to
>> edit the test when the log shows what commands were triggering various
>> responses.
> 
> Did you consider adding -iothread to tests/qemu-iotests/check instead, to be
> able to run any (or some) tests with or without iothread?

I don't know how many of the existing iotests would benefit from the
ability supply iothread as an option, nor how likely it is that someone
would actually remember to run the testsuite twice to cover the use of
that option.  I also don't know how hard it would be to retrofit the
addition of optional iothread support into all the tests.  Rather, I
addressed the more immediate concern of the fact that my recent addition
to using iothread in 223 lost the previous ability of that test to cover
non-iothread, and where this patch series now makes it cover both
scenarios with a single iotests run.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/2] enhance iotest 223 coverage
Posted by Max Reitz 4 years, 5 months ago
On 24.09.19 21:51, Eric Blake wrote:
> On 9/24/19 2:26 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 24.09.2019 17:35, Eric Blake wrote:
>>> Commit 506902c6 dropped non-iothread coverage in order to test iothread,
>>> better is to run things twice.  In doing this, I found it easier to
>>> edit the test when the log shows what commands were triggering various
>>> responses.
>>
>> Did you consider adding -iothread to tests/qemu-iotests/check instead, to be
>> able to run any (or some) tests with or without iothread?
> 
> I don't know how many of the existing iotests would benefit from the
> ability supply iothread as an option, nor how likely it is that someone
> would actually remember to run the testsuite twice to cover the use of
> that option.

I would, because I already run all qcow2 tests without any creation
options, with compat=0.10, and with refcount_bits=1.  (And plan to add
data_file=$TEST_IMG.data_file in the future.)

(And I let the iotests run through a script from some json description
files, so I won’t forget.)

> I also don't know how hard it would be to retrofit the
> addition of optional iothread support into all the tests.

That I don’t know either.  Though I think the point wouldn’t be to
retrofit iothread support into all tests, but just start with some.
(For example, 262 uses “an iothread just for fun”.  So it could clearly
run with both configurations.)

I do think it’s an interesting idea, but I don’t think it’s important
right now.

Max

> Rather, I
> addressed the more immediate concern of the fact that my recent addition
> to using iothread in 223 lost the previous ability of that test to cover
> non-iothread, and where this patch series now makes it cover both
> scenarios with a single iotests run.
>