[Qemu-devel] [PATCH 0/2] tests: Add check-block to "make check"

[Qemu-devel] [PATCH 0/2] tests: Add check-block to "make check"

[Fam Zheng]

qemu-iotests contains useful tests that have a nice coverage of block layer
code. Adding check-block (which calls tests/qemu-iotests-quick.sh) to "make
check" is good for developers' self-testing.

With the first patch, this set takes a half minute on my laptop. If "-j" option
is used, it only takes a few more seconds than what we have now.

Fam Zheng (2):
  qemu-iotests: Speed up make check-block
  tests/Makefile: Add check-block to make check

 tests/Makefile              | 2 +-
 tests/qemu-iotests-quick.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/2] qemu-iotests: Speed up make check-block

[Fam Zheng]

Using /tmp (usually mounted as tmpfs) and cache=writeback, the quick
group can be quicker.

On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
50s to 30s.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests-quick.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
index 12af731..0b54dbf 100755
--- a/tests/qemu-iotests-quick.sh
+++ b/tests/qemu-iotests-quick.sh
@@ -3,6 +3,6 @@
 cd tests/qemu-iotests
 
 ret=0
-./check -T -qcow2 -g quick || ret=1
+TEST_DIR=/tmp/qemu-iotests-quick-$$ ./check -T -qcow2 -g quick -c writeback || ret=1
 
 exit $ret
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: Speed up make check-block

[Markus Armbruster]

Fam Zheng <famz@redhat.com> writes:

> Using /tmp (usually mounted as tmpfs) and cache=writeback, the quick
> group can be quicker.
>
> On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
> 50s to 30s.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  tests/qemu-iotests-quick.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
> index 12af731..0b54dbf 100755
> --- a/tests/qemu-iotests-quick.sh
> +++ b/tests/qemu-iotests-quick.sh
> @@ -3,6 +3,6 @@
>  cd tests/qemu-iotests
>  
>  ret=0
> -./check -T -qcow2 -g quick || ret=1
> +TEST_DIR=/tmp/qemu-iotests-quick-$$ ./check -T -qcow2 -g quick -c
> writeback || ret=1
>  
>  exit $ret

Should we let users specify another directory in the environment?

Like this:

: ${TEST_DIR=/tmp/qemu-iotests-quick-$$}
ret=0
./check -T -qcow2 -g quick || ret=1

Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: Speed up make check-block

[Max Reitz]

On 2014-10-28 at 07:45, Fam Zheng wrote:
> Using /tmp (usually mounted as tmpfs) and cache=writeback, the quick
> group can be quicker.
>
> On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
> 50s to 30s.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   tests/qemu-iotests-quick.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
> index 12af731..0b54dbf 100755
> --- a/tests/qemu-iotests-quick.sh
> +++ b/tests/qemu-iotests-quick.sh
> @@ -3,6 +3,6 @@
>   cd tests/qemu-iotests
>   
>   ret=0
> -./check -T -qcow2 -g quick || ret=1
> +TEST_DIR=/tmp/qemu-iotests-quick-$$ ./check -T -qcow2 -g quick -c writeback || ret=1

There are (at least) two tests which don't work with -c writeback (026 
and 039), one of them is in the quick group (039). Why not use -c 
writethrough? It doesn't make any difference on tmpfs anyway  (we can't 
omit it because that will break 091).

And thank you, I didn't know about TEST_DIR. I always built the full 
qemu in /tmp. *g*

Max

>   exit $ret

Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: Speed up make check-block

[Kevin Wolf]

Am 25.11.2014 um 10:12 hat Max Reitz geschrieben:
> On 2014-10-28 at 07:45, Fam Zheng wrote:
> >Using /tmp (usually mounted as tmpfs) and cache=writeback, the quick
> >group can be quicker.
> >
> >On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
> >50s to 30s.
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >---
> >  tests/qemu-iotests-quick.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
> >index 12af731..0b54dbf 100755
> >--- a/tests/qemu-iotests-quick.sh
> >+++ b/tests/qemu-iotests-quick.sh
> >@@ -3,6 +3,6 @@
> >  cd tests/qemu-iotests
> >  ret=0
> >-./check -T -qcow2 -g quick || ret=1
> >+TEST_DIR=/tmp/qemu-iotests-quick-$$ ./check -T -qcow2 -g quick -c writeback || ret=1
> 
> There are (at least) two tests which don't work with -c writeback
> (026 and 039), one of them is in the quick group (039). Why not use
> -c writethrough? It doesn't make any difference on tmpfs anyway  (we
> can't omit it because that will break 091).

Why use any -c? The default is the fast option writeback, and for those
test cases that don't support writeback, something working is chosen
instead.

Kevin

Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: Speed up make check-block

[Max Reitz]

On 2014-11-25 at 10:21, Kevin Wolf wrote:
> Am 25.11.2014 um 10:12 hat Max Reitz geschrieben:
>> On 2014-10-28 at 07:45, Fam Zheng wrote:
>>> Using /tmp (usually mounted as tmpfs) and cache=writeback, the quick
>>> group can be quicker.
>>>
>>> On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
>>> 50s to 30s.
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> ---
>>>   tests/qemu-iotests-quick.sh | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
>>> index 12af731..0b54dbf 100755
>>> --- a/tests/qemu-iotests-quick.sh
>>> +++ b/tests/qemu-iotests-quick.sh
>>> @@ -3,6 +3,6 @@
>>>   cd tests/qemu-iotests
>>>   ret=0
>>> -./check -T -qcow2 -g quick || ret=1
>>> +TEST_DIR=/tmp/qemu-iotests-quick-$$ ./check -T -qcow2 -g quick -c writeback || ret=1
>> There are (at least) two tests which don't work with -c writeback
>> (026 and 039), one of them is in the quick group (039). Why not use
>> -c writethrough? It doesn't make any difference on tmpfs anyway  (we
>> can't omit it because that will break 091).
> Why use any -c? The default is the fast option writeback, and for those
> test cases that don't support writeback, something working is chosen
> instead.

Because that breaks 091.

Max

Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: Speed up make check-block

[Kevin Wolf]

Am 25.11.2014 um 10:21 hat Max Reitz geschrieben:
> On 2014-11-25 at 10:21, Kevin Wolf wrote:
> >Am 25.11.2014 um 10:12 hat Max Reitz geschrieben:
> >>On 2014-10-28 at 07:45, Fam Zheng wrote:
> >>>Using /tmp (usually mounted as tmpfs) and cache=writeback, the quick
> >>>group can be quicker.
> >>>
> >>>On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
> >>>50s to 30s.
> >>>
> >>>Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>---
> >>>  tests/qemu-iotests-quick.sh | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
> >>>index 12af731..0b54dbf 100755
> >>>--- a/tests/qemu-iotests-quick.sh
> >>>+++ b/tests/qemu-iotests-quick.sh
> >>>@@ -3,6 +3,6 @@
> >>>  cd tests/qemu-iotests
> >>>  ret=0
> >>>-./check -T -qcow2 -g quick || ret=1
> >>>+TEST_DIR=/tmp/qemu-iotests-quick-$$ ./check -T -qcow2 -g quick -c writeback || ret=1
> >>There are (at least) two tests which don't work with -c writeback
> >>(026 and 039), one of them is in the quick group (039). Why not use
> >>-c writethrough? It doesn't make any difference on tmpfs anyway  (we
> >>can't omit it because that will break 091).
> >Why use any -c? The default is the fast option writeback, and for those
> >test cases that don't support writeback, something working is chosen
> >instead.
> 
> Because that breaks 091.

That's unfortunate. I wish tmpfs supported O_DIRECT...

But let's just remove it from quick then - it doesn't really matter if
it doesn't run because of the cache mode or because we didn't include it
in the group.

-c writethrough is okay as long as you really have tmpfs on your /tmp,
but it really hurts when you don't (and I for one don't, standard RHEL 7
installation).

Kevin

Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: Speed up make check-block

[Max Reitz]

On 2014-11-25 at 10:30, Kevin Wolf wrote:
> Am 25.11.2014 um 10:21 hat Max Reitz geschrieben:
>> On 2014-11-25 at 10:21, Kevin Wolf wrote:
>>> Am 25.11.2014 um 10:12 hat Max Reitz geschrieben:
>>>> On 2014-10-28 at 07:45, Fam Zheng wrote:
>>>>> Using /tmp (usually mounted as tmpfs) and cache=writeback, the quick
>>>>> group can be quicker.
>>>>>
>>>>> On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
>>>>> 50s to 30s.
>>>>>
>>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>>> ---
>>>>>   tests/qemu-iotests-quick.sh | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
>>>>> index 12af731..0b54dbf 100755
>>>>> --- a/tests/qemu-iotests-quick.sh
>>>>> +++ b/tests/qemu-iotests-quick.sh
>>>>> @@ -3,6 +3,6 @@
>>>>>   cd tests/qemu-iotests
>>>>>   ret=0
>>>>> -./check -T -qcow2 -g quick || ret=1
>>>>> +TEST_DIR=/tmp/qemu-iotests-quick-$$ ./check -T -qcow2 -g quick -c writeback || ret=1
>>>> There are (at least) two tests which don't work with -c writeback
>>>> (026 and 039), one of them is in the quick group (039). Why not use
>>>> -c writethrough? It doesn't make any difference on tmpfs anyway  (we
>>>> can't omit it because that will break 091).
>>> Why use any -c? The default is the fast option writeback, and for those
>>> test cases that don't support writeback, something working is chosen
>>> instead.
>> Because that breaks 091.
> That's unfortunate. I wish tmpfs supported O_DIRECT...
>
> But let's just remove it from quick then - it doesn't really matter if
> it doesn't run because of the cache mode or because we didn't include it
> in the group.

Fine with me.

Max

> -c writethrough is okay as long as you really have tmpfs on your /tmp,
> but it really hurts when you don't (and I for one don't, standard RHEL 7
> installation).

Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: Speed up make check-block

[Fam Zheng]

On Tue, 11/25 10:31, Max Reitz wrote:
> On 2014-11-25 at 10:30, Kevin Wolf wrote:
> >Am 25.11.2014 um 10:21 hat Max Reitz geschrieben:
> >>On 2014-11-25 at 10:21, Kevin Wolf wrote:
> >>>Am 25.11.2014 um 10:12 hat Max Reitz geschrieben:
> >>>>On 2014-10-28 at 07:45, Fam Zheng wrote:
> >>>>>Using /tmp (usually mounted as tmpfs) and cache=writeback, the quick
> >>>>>group can be quicker.
> >>>>>
> >>>>>On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
> >>>>>50s to 30s.
> >>>>>
> >>>>>Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>>>---
> >>>>>  tests/qemu-iotests-quick.sh | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>>diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
> >>>>>index 12af731..0b54dbf 100755
> >>>>>--- a/tests/qemu-iotests-quick.sh
> >>>>>+++ b/tests/qemu-iotests-quick.sh
> >>>>>@@ -3,6 +3,6 @@
> >>>>>  cd tests/qemu-iotests
> >>>>>  ret=0
> >>>>>-./check -T -qcow2 -g quick || ret=1
> >>>>>+TEST_DIR=/tmp/qemu-iotests-quick-$$ ./check -T -qcow2 -g quick -c writeback || ret=1
> >>>>There are (at least) two tests which don't work with -c writeback
> >>>>(026 and 039), one of them is in the quick group (039). Why not use
> >>>>-c writethrough? It doesn't make any difference on tmpfs anyway  (we
> >>>>can't omit it because that will break 091).
> >>>Why use any -c? The default is the fast option writeback, and for those
> >>>test cases that don't support writeback, something working is chosen
> >>>instead.
> >>Because that breaks 091.
> >That's unfortunate. I wish tmpfs supported O_DIRECT...

Indeed unfortunate!

> >
> >But let's just remove it from quick then - it doesn't really matter if
> >it doesn't run because of the cache mode or because we didn't include it
> >in the group.

Will do it when respin.

> >-c writethrough is okay as long as you really have tmpfs on your /tmp,
> >but it really hurts when you don't (and I for one don't, standard RHEL 7
> >installation).
> 

Oh, do you have any better idea than hardcoding to /tmp then?

Fam

Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: Speed up make check-block

[Kevin Wolf]

Am 25.11.2014 um 10:44 hat Fam Zheng geschrieben:
> On Tue, 11/25 10:31, Max Reitz wrote:
> > On 2014-11-25 at 10:30, Kevin Wolf wrote:
> > >Am 25.11.2014 um 10:21 hat Max Reitz geschrieben:
> > >>On 2014-11-25 at 10:21, Kevin Wolf wrote:
> > >>>Am 25.11.2014 um 10:12 hat Max Reitz geschrieben:
> > >>>>On 2014-10-28 at 07:45, Fam Zheng wrote:
> > >>>>>Using /tmp (usually mounted as tmpfs) and cache=writeback, the quick
> > >>>>>group can be quicker.
> > >>>>>
> > >>>>>On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
> > >>>>>50s to 30s.
> > >>>>>
> > >>>>>Signed-off-by: Fam Zheng <famz@redhat.com>
> > >>>>>---
> > >>>>>  tests/qemu-iotests-quick.sh | 2 +-
> > >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>>
> > >>>>>diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
> > >>>>>index 12af731..0b54dbf 100755
> > >>>>>--- a/tests/qemu-iotests-quick.sh
> > >>>>>+++ b/tests/qemu-iotests-quick.sh
> > >>>>>@@ -3,6 +3,6 @@
> > >>>>>  cd tests/qemu-iotests
> > >>>>>  ret=0
> > >>>>>-./check -T -qcow2 -g quick || ret=1
> > >>>>>+TEST_DIR=/tmp/qemu-iotests-quick-$$ ./check -T -qcow2 -g quick -c writeback || ret=1
> > >>>>There are (at least) two tests which don't work with -c writeback
> > >>>>(026 and 039), one of them is in the quick group (039). Why not use
> > >>>>-c writethrough? It doesn't make any difference on tmpfs anyway  (we
> > >>>>can't omit it because that will break 091).
> > >>>Why use any -c? The default is the fast option writeback, and for those
> > >>>test cases that don't support writeback, something working is chosen
> > >>>instead.
> > >>Because that breaks 091.
> > >That's unfortunate. I wish tmpfs supported O_DIRECT...
> 
> Indeed unfortunate!
> 
> > >
> > >But let's just remove it from quick then - it doesn't really matter if
> > >it doesn't run because of the cache mode or because we didn't include it
> > >in the group.
> 
> Will do it when respin.
> 
> > >-c writethrough is okay as long as you really have tmpfs on your /tmp,
> > >but it really hurts when you don't (and I for one don't, standard RHEL 7
> > >installation).
> > 
> 
> Oh, do you have any better idea than hardcoding to /tmp then?

I think /tmp is fine. It helps those of us who do have a tmpfs there,
and it shouldn't hurt the rest at least. Just don't combine it with
writethrough, but leave out -c.

Kevin

[Qemu-devel] [PATCH 2/2] tests/Makefile: Add check-block to make check

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index 16f0e4c..f430b18 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -463,7 +463,7 @@ check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y))
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-unit: $(patsubst %,check-%, $(check-unit-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
-check: check-qapi-schema check-unit check-qtest
+check: check-qapi-schema check-unit check-qtest check-block
 check-clean:
 	$(MAKE) -C tests/tcg clean
 	rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
-- 
1.9.3

Re: [Qemu-devel] [PATCH 2/2] tests/Makefile: Add check-block to make check

[Max Reitz]

On 2014-10-28 at 07:45, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   tests/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 16f0e4c..f430b18 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -463,7 +463,7 @@ check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y))
>   check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
>   check-unit: $(patsubst %,check-%, $(check-unit-y))
>   check-block: $(patsubst %,check-%, $(check-block-y))
> -check: check-qapi-schema check-unit check-qtest
> +check: check-qapi-schema check-unit check-qtest check-block
>   check-clean:
>   	$(MAKE) -C tests/tcg clean
>   	rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)

Even regardless of patch 1:

Reviewed-by: Max Reitz <mreitz@redhat.com>

But we must fix 039 first (I remember some other test failing sometimes, 
but I can't remember which).

Re: [Qemu-devel] [PATCH 0/2] tests: Add check-block to "make check"

[Markus Armbruster]

Fam Zheng <famz@redhat.com> writes:

> qemu-iotests contains useful tests that have a nice coverage of block layer
> code. Adding check-block (which calls tests/qemu-iotests-quick.sh) to "make
> check" is good for developers' self-testing.
>
> With the first patch, this set takes a half minute on my laptop. If "-j" option
> is used, it only takes a few more seconds than what we have now.

Different data point: elderly machine, spinning rust, /tmp is tmpfs, no
-j: elapsed time increases from ~2 to ~3 minutes.

Re: [Qemu-devel] [PATCH 0/2] tests: Add check-block to "make check"

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Fam Zheng <famz@redhat.com> writes:
>
>> qemu-iotests contains useful tests that have a nice coverage of block layer
>> code. Adding check-block (which calls tests/qemu-iotests-quick.sh) to "make
>> check" is good for developers' self-testing.
>>
>> With the first patch, this set takes a half minute on my laptop. If
>> "-j" option
>> is used, it only takes a few more seconds than what we have now.
>
> Different data point: elderly machine, spinning rust, /tmp is tmpfs, no
> -j: elapsed time increases from ~2 to ~3 minutes.

I'm very much in favour of actually running the tests we have.

Running all the block tests for all the formats would be too slow, and
that's why you run just the "quick" group, and only for qcow2.  Quick
enough?

Any ideas on speeding it up further?  Trimming image sizes, perhaps?

What are the slowest tests in the quick group?  Why are they slow?  How
are tests selected for the quick group anyway?

Re: [Qemu-devel] [PATCH 0/2] tests: Add check-block to "make check"

[Max Reitz]

On 2014-11-25 at 08:30, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> Fam Zheng <famz@redhat.com> writes:
>>
>>> qemu-iotests contains useful tests that have a nice coverage of block layer
>>> code. Adding check-block (which calls tests/qemu-iotests-quick.sh) to "make
>>> check" is good for developers' self-testing.
>>>
>>> With the first patch, this set takes a half minute on my laptop. If
>>> "-j" option
>>> is used, it only takes a few more seconds than what we have now.
>> Different data point: elderly machine, spinning rust, /tmp is tmpfs, no
>> -j: elapsed time increases from ~2 to ~3 minutes.
> I'm very much in favour of actually running the tests we have.
>
> Running all the block tests for all the formats would be too slow, and
> that's why you run just the "quick" group, and only for qcow2.  Quick
> enough?
>
> Any ideas on speeding it up further?  Trimming image sizes, perhaps?
>
> What are the slowest tests in the quick group?  Why are they slow?  How
> are tests selected for the quick group anyway?

Last time I updated the associations it was "Whatever runs in under five 
seconds on my HDD".

Max