[Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Fam Zheng]

Previously all test cases in a category, such as check-qtest-y, are
executed in a single long gtester command. This patch separates each
test into its own make target to allow better parallism.

Signed-off-by: Fam Zheng <famz@redhat.com>
---

This saves 50% of the time "make check takes" compared to on master
(I use -j8). RFC because I'm not sure if the new gcov usage is correct
with the now much higher level of parallism compared to before.
---
 tests/Makefile.include | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 14be491..9bf0326 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -691,27 +691,29 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
 # gtester tests, possibly with verbose output
 
 .PHONY: $(patsubst %, check-qtest-%, $(QTEST_TARGETS))
-$(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
+
+qtest-run-%: tests/%
 	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
-	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
+	$(call quiet-command,\
+		$(if $(QTEST_TARGET), \
+			QTEST_QEMU_BINARY=$(QTEST_TARGET)-softmmu/qemu-system-$(QTEST_TARGET)) \
 		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
 		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
-		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER $@")
-	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
-	  echo Gcov report for $$f:;\
-	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
-	done,)
+		gtester $< $(GTESTER_OPTIONS) -m=$(SPEED),"GTESTER $<")
+	$(if $(CONFIG_GCOV), @echo Gcov report for $<:;\
+	  $(GCOV) $(GCOV_OPTIONS) $< -o `dirname $<`; \
+	)
+
+$(foreach target, $(QTEST_TARGETS), \
+	$(eval check-qtest-$(target): QTEST_TARGET := $(target)) \
+	$(eval check-qtest-$(target): $(patsubst tests/%, qtest-run-%, \
+                                             $(check-qtest-y) \
+                                             $(check-qtest-$(target)-y) \
+                                             $(check-qtest-generic-y))) \
+)
 
 .PHONY: $(patsubst %, check-%, $(check-unit-y))
-$(patsubst %, check-%, $(check-unit-y)): check-%: %
-	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
-	$(call quiet-command, \
-		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
-		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
-	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
-	  echo Gcov report for $$f:;\
-	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
-	done,)
+$(patsubst %, check-%, $(check-unit-y)): check-tests/%: qtest-run-%
 
 # gtester tests with XML output
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[John Snow]

On 08/12/2016 05:19 AM, Fam Zheng wrote:
> Previously all test cases in a category, such as check-qtest-y, are
> executed in a single long gtester command. This patch separates each
> test into its own make target to allow better parallism.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>
> This saves 50% of the time "make check takes" compared to on master
> (I use -j8). RFC because I'm not sure if the new gcov usage is correct
> with the now much higher level of parallism compared to before.
> ---
>  tests/Makefile.include | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 14be491..9bf0326 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -691,27 +691,29 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
>  # gtester tests, possibly with verbose output
>
>  .PHONY: $(patsubst %, check-qtest-%, $(QTEST_TARGETS))
> -$(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
> +
> +qtest-run-%: tests/%
>  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
> -	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> +	$(call quiet-command,\
> +		$(if $(QTEST_TARGET), \
> +			QTEST_QEMU_BINARY=$(QTEST_TARGET)-softmmu/qemu-system-$(QTEST_TARGET)) \
>  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
>  		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> -		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER $@")
> -	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
> -	  echo Gcov report for $$f:;\
> -	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> -	done,)
> +		gtester $< $(GTESTER_OPTIONS) -m=$(SPEED),"GTESTER $<")
> +	$(if $(CONFIG_GCOV), @echo Gcov report for $<:;\
> +	  $(GCOV) $(GCOV_OPTIONS) $< -o `dirname $<`; \
> +	)
> +
> +$(foreach target, $(QTEST_TARGETS), \
> +	$(eval check-qtest-$(target): QTEST_TARGET := $(target)) \
> +	$(eval check-qtest-$(target): $(patsubst tests/%, qtest-run-%, \
> +                                             $(check-qtest-y) \
> +                                             $(check-qtest-$(target)-y) \
> +                                             $(check-qtest-generic-y))) \
> +)
>
>  .PHONY: $(patsubst %, check-%, $(check-unit-y))
> -$(patsubst %, check-%, $(check-unit-y)): check-%: %
> -	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
> -	$(call quiet-command, \
> -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> -		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
> -	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
> -	  echo Gcov report for $$f:;\
> -	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> -	done,)
> +$(patsubst %, check-%, $(check-unit-y)): check-tests/%: qtest-run-%
>
>  # gtester tests with XML output
>
>

I can't vouch for gcov either, but:

Tested-by: John Snow <jsnow@redhat.com>

-j1:

real	1m51.195s
user	1m5.478s
sys	0m21.158s

-j9:

real	0m53.039s
user	1m41.754s
sys	0m31.150s


This seems useful.

--js

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Fam Zheng]

On Wed, 09/21 14:24, John Snow wrote:
> 
> 
> On 08/12/2016 05:19 AM, Fam Zheng wrote:
> > Previously all test cases in a category, such as check-qtest-y, are
> > executed in a single long gtester command. This patch separates each
> > test into its own make target to allow better parallism.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> > 
> > This saves 50% of the time "make check takes" compared to on master
> > (I use -j8). RFC because I'm not sure if the new gcov usage is correct
> > with the now much higher level of parallism compared to before.
> > ---
> >  tests/Makefile.include | 34 ++++++++++++++++++----------------
> >  1 file changed, 18 insertions(+), 16 deletions(-)
> > 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 14be491..9bf0326 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -691,27 +691,29 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
> >  # gtester tests, possibly with verbose output
> > 
> >  .PHONY: $(patsubst %, check-qtest-%, $(QTEST_TARGETS))
> > -$(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
> > +
> > +qtest-run-%: tests/%
> >  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
> > -	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> > +	$(call quiet-command,\
> > +		$(if $(QTEST_TARGET), \
> > +			QTEST_QEMU_BINARY=$(QTEST_TARGET)-softmmu/qemu-system-$(QTEST_TARGET)) \
> >  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> >  		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> > -		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER $@")
> > -	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
> > -	  echo Gcov report for $$f:;\
> > -	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> > -	done,)
> > +		gtester $< $(GTESTER_OPTIONS) -m=$(SPEED),"GTESTER $<")
> > +	$(if $(CONFIG_GCOV), @echo Gcov report for $<:;\
> > +	  $(GCOV) $(GCOV_OPTIONS) $< -o `dirname $<`; \
> > +	)
> > +
> > +$(foreach target, $(QTEST_TARGETS), \
> > +	$(eval check-qtest-$(target): QTEST_TARGET := $(target)) \
> > +	$(eval check-qtest-$(target): $(patsubst tests/%, qtest-run-%, \
> > +                                             $(check-qtest-y) \
> > +                                             $(check-qtest-$(target)-y) \
> > +                                             $(check-qtest-generic-y))) \
> > +)
> > 
> >  .PHONY: $(patsubst %, check-%, $(check-unit-y))
> > -$(patsubst %, check-%, $(check-unit-y)): check-%: %
> > -	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
> > -	$(call quiet-command, \
> > -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> > -		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
> > -	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
> > -	  echo Gcov report for $$f:;\
> > -	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> > -	done,)
> > +$(patsubst %, check-%, $(check-unit-y)): check-tests/%: qtest-run-%
> > 
> >  # gtester tests with XML output
> > 
> > 
> 
> I can't vouch for gcov either, but:

Too bad that this seems to be too big a hammer that breaks the way gcov was
used: 1) the "rm *.gcda" command now runs in each test, so it's harder to
get an overall coverage report; 2) there is a risk that the parallism may
corrupt those files. :(

Fam

> 
> Tested-by: John Snow <jsnow@redhat.com>
> 
> -j1:
> 
> real	1m51.195s
> user	1m5.478s
> sys	0m21.158s
> 
> -j9:
> 
> real	0m53.039s
> user	1m41.754s
> sys	0m31.150s
> 
> 
> This seems useful.
> 
> --js

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Gonglei (Arei)]

Hi Fam,


> -----Original Message-----
> From: Qemu-devel
> [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On
> Behalf Of Fam Zheng
> Sent: Friday, September 23, 2016 3:58 PM
> To: John Snow
> Cc: pbonzini@redhat.com; qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel
> 
> On Wed, 09/21 14:24, John Snow wrote:
> >
> >
> > On 08/12/2016 05:19 AM, Fam Zheng wrote:
> > > Previously all test cases in a category, such as check-qtest-y, are
> > > executed in a single long gtester command. This patch separates each
> > > test into its own make target to allow better parallism.
> > >

That's will be great if we can specify a test to run, especially for the scenario
which add one use qtest case.

For example: 

 # make check test-crypto-cipher

then only run the tests/ test-crypto-cipher. 

Do you think it makes sense?

Regards,
-Gonglei

> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > ---
> > >
> > > This saves 50% of the time "make check takes" compared to on master
> > > (I use -j8). RFC because I'm not sure if the new gcov usage is correct
> > > with the now much higher level of parallism compared to before.
> > > ---
> > >  tests/Makefile.include | 34 ++++++++++++++++++----------------
> > >  1 file changed, 18 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > > index 14be491..9bf0326 100644
> > > --- a/tests/Makefile.include
> > > +++ b/tests/Makefile.include
> > > @@ -691,27 +691,29 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
> > >  # gtester tests, possibly with verbose output
> > >
> > >  .PHONY: $(patsubst %, check-qtest-%, $(QTEST_TARGETS))
> > > -$(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%:
> $(check-qtest-y)
> > > +
> > > +qtest-run-%: tests/%
> > >  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda
> */*/*/*.gcda,)
> > > -	$(call
> quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> > > +	$(call quiet-command,\
> > > +		$(if $(QTEST_TARGET), \
> > > +
> 	QTEST_QEMU_BINARY=$(QTEST_TARGET)-softmmu/qemu-system-$(QTE
> ST_TARGET)) \
> > >  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> > >  		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM %
> 255 + 1))} \
> > > -		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y)
> $(check-qtest-generic-y),"GTESTER $@")
> > > -	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y);
> do \
> > > -	  echo Gcov report for $$f:;\
> > > -	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> > > -	done,)
> > > +		gtester $< $(GTESTER_OPTIONS) -m=$(SPEED),"GTESTER $<")
> > > +	$(if $(CONFIG_GCOV), @echo Gcov report for $<:;\
> > > +	  $(GCOV) $(GCOV_OPTIONS) $< -o `dirname $<`; \
> > > +	)
> > > +
> > > +$(foreach target, $(QTEST_TARGETS), \
> > > +	$(eval check-qtest-$(target): QTEST_TARGET := $(target)) \
> > > +	$(eval check-qtest-$(target): $(patsubst tests/%, qtest-run-%, \
> > > +                                             $(check-qtest-y) \
> > > +
> $(check-qtest-$(target)-y) \
> > > +
> $(check-qtest-generic-y))) \
> > > +)
> > >
> > >  .PHONY: $(patsubst %, check-%, $(check-unit-y))
> > > -$(patsubst %, check-%, $(check-unit-y)): check-%: %
> > > -	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
> > > -	$(call quiet-command, \
> > > -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 +
> 1))} \
> > > -		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
> > > -	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y)
> $(gcov-files-generic-y); do \
> > > -	  echo Gcov report for $$f:;\
> > > -	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> > > -	done,)
> > > +$(patsubst %, check-%, $(check-unit-y)): check-tests/%: qtest-run-%
> > >
> > >  # gtester tests with XML output
> > >
> > >
> >
> > I can't vouch for gcov either, but:
> 
> Too bad that this seems to be too big a hammer that breaks the way gcov was
> used: 1) the "rm *.gcda" command now runs in each test, so it's harder to
> get an overall coverage report; 2) there is a risk that the parallism may
> corrupt those files. :(
> 
> Fam
> 
> >
> > Tested-by: John Snow <jsnow@redhat.com>
> >
> > -j1:
> >
> > real	1m51.195s
> > user	1m5.478s
> > sys	0m21.158s
> >
> > -j9:
> >
> > real	0m53.039s
> > user	1m41.754s
> > sys	0m31.150s
> >
> >
> > This seems useful.
> >
> > --js

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Fam Zheng]

On Fri, 09/23 09:39, Gonglei (Arei) wrote:
> 
> Hi Fam,
> 
> 
> > -----Original Message-----
> > From: Qemu-devel
> > [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On
> > Behalf Of Fam Zheng
> > Sent: Friday, September 23, 2016 3:58 PM
> > To: John Snow
> > Cc: pbonzini@redhat.com; qemu-devel@nongnu.org
> > Subject: Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel
> > 
> > On Wed, 09/21 14:24, John Snow wrote:
> > >
> > >
> > > On 08/12/2016 05:19 AM, Fam Zheng wrote:
> > > > Previously all test cases in a category, such as check-qtest-y, are
> > > > executed in a single long gtester command. This patch separates each
> > > > test into its own make target to allow better parallism.
> > > >
> 
> That's will be great if we can specify a test to run, especially for the scenario
> which add one use qtest case.
> 
> For example: 
> 
>  # make check test-crypto-cipher
> 
> then only run the tests/ test-crypto-cipher. 
> 
> Do you think it makes sense?

Or more likely:

    # make check TESTS="test-crypto-cipher test-crypto-hash ..."

Usually I just extract the gtester command line with V=1 and run it from my
shell prompt.  Feel free to send a patch, though.

Fam

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Gonglei (Arei)]

> -----Original Message-----
> From: Fam Zheng [mailto:famz@redhat.com]
> Sent: Friday, September 23, 2016 5:59 PM
> To: Gonglei (Arei)
> Cc: John Snow; pbonzini@redhat.com; qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel
> 
> On Fri, 09/23 09:39, Gonglei (Arei) wrote:
> >
> > Hi Fam,
> >
> >
> > > -----Original Message-----
> > > From: Qemu-devel
> > > [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On
> > > Behalf Of Fam Zheng
> > > Sent: Friday, September 23, 2016 3:58 PM
> > > To: John Snow
> > > Cc: pbonzini@redhat.com; qemu-devel@nongnu.org
> > > Subject: Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel
> > >
> > > On Wed, 09/21 14:24, John Snow wrote:
> > > >
> > > >
> > > > On 08/12/2016 05:19 AM, Fam Zheng wrote:
> > > > > Previously all test cases in a category, such as check-qtest-y, are
> > > > > executed in a single long gtester command. This patch separates each
> > > > > test into its own make target to allow better parallism.
> > > > >
> >
> > That's will be great if we can specify a test to run, especially for the scenario
> > which add one use qtest case.
> >
> > For example:
> >
> >  # make check test-crypto-cipher
> >
> > then only run the tests/ test-crypto-cipher.
> >
> > Do you think it makes sense?
> 
> Or more likely:
> 
>     # make check TESTS="test-crypto-cipher test-crypto-hash ..."
> 
> Usually I just extract the gtester command line with V=1 and run it from my
> shell prompt.  Feel free to send a patch, though.
> 
Sorry, I have no patch for this, it's just my idea ;)
Appreciate it if you can realize it. 

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Daniel P. Berrange]

On Fri, Sep 23, 2016 at 05:59:05PM +0800, Fam Zheng wrote:
> On Fri, 09/23 09:39, Gonglei (Arei) wrote:
> > 
> > Hi Fam,
> > 
> > 
> > > -----Original Message-----
> > > From: Qemu-devel
> > > [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On
> > > Behalf Of Fam Zheng
> > > Sent: Friday, September 23, 2016 3:58 PM
> > > To: John Snow
> > > Cc: pbonzini@redhat.com; qemu-devel@nongnu.org
> > > Subject: Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel
> > > 
> > > On Wed, 09/21 14:24, John Snow wrote:
> > > >
> > > >
> > > > On 08/12/2016 05:19 AM, Fam Zheng wrote:
> > > > > Previously all test cases in a category, such as check-qtest-y, are
> > > > > executed in a single long gtester command. This patch separates each
> > > > > test into its own make target to allow better parallism.
> > > > >
> > 
> > That's will be great if we can specify a test to run, especially for the scenario
> > which add one use qtest case.
> > 
> > For example: 
> > 
> >  # make check test-crypto-cipher
> > 
> > then only run the tests/ test-crypto-cipher. 
> > 
> > Do you think it makes sense?
> 
> Or more likely:
> 
>     # make check TESTS="test-crypto-cipher test-crypto-hash ..."
> 
> Usually I just extract the gtester command line with V=1 and run it from my
> shell prompt.  Feel free to send a patch, though.

Shouldn't even need todo that in most cases - I tend to just do

  make tests/test-crypto-cipher && ./tess/test-crypto-cipher

If there are tests which rely on some environment set by the Makefile,
then really they should be fixed to have sensible defaults so that they
can be directly executed.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Daniel P. Berrange]

On Fri, Sep 23, 2016 at 03:58:07PM +0800, Fam Zheng wrote:
> On Wed, 09/21 14:24, John Snow wrote:
> > 
> > 
> > On 08/12/2016 05:19 AM, Fam Zheng wrote:
> > > Previously all test cases in a category, such as check-qtest-y, are
> > > executed in a single long gtester command. This patch separates each
> > > test into its own make target to allow better parallism.
> > > 
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > ---
> > > 
> > > This saves 50% of the time "make check takes" compared to on master
> > > (I use -j8). RFC because I'm not sure if the new gcov usage is correct
> > > with the now much higher level of parallism compared to before.
> > > ---
> > >  tests/Makefile.include | 34 ++++++++++++++++++----------------
> > >  1 file changed, 18 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > > index 14be491..9bf0326 100644
> > > --- a/tests/Makefile.include
> > > +++ b/tests/Makefile.include
> > > @@ -691,27 +691,29 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
> > >  # gtester tests, possibly with verbose output
> > > 
> > >  .PHONY: $(patsubst %, check-qtest-%, $(QTEST_TARGETS))
> > > -$(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
> > > +
> > > +qtest-run-%: tests/%
> > >  	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
> > > -	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> > > +	$(call quiet-command,\
> > > +		$(if $(QTEST_TARGET), \
> > > +			QTEST_QEMU_BINARY=$(QTEST_TARGET)-softmmu/qemu-system-$(QTEST_TARGET)) \
> > >  		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> > >  		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> > > -		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER $@")
> > > -	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
> > > -	  echo Gcov report for $$f:;\
> > > -	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> > > -	done,)
> > > +		gtester $< $(GTESTER_OPTIONS) -m=$(SPEED),"GTESTER $<")
> > > +	$(if $(CONFIG_GCOV), @echo Gcov report for $<:;\
> > > +	  $(GCOV) $(GCOV_OPTIONS) $< -o `dirname $<`; \
> > > +	)
> > > +
> > > +$(foreach target, $(QTEST_TARGETS), \
> > > +	$(eval check-qtest-$(target): QTEST_TARGET := $(target)) \
> > > +	$(eval check-qtest-$(target): $(patsubst tests/%, qtest-run-%, \
> > > +                                             $(check-qtest-y) \
> > > +                                             $(check-qtest-$(target)-y) \
> > > +                                             $(check-qtest-generic-y))) \
> > > +)
> > > 
> > >  .PHONY: $(patsubst %, check-%, $(check-unit-y))
> > > -$(patsubst %, check-%, $(check-unit-y)): check-%: %
> > > -	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
> > > -	$(call quiet-command, \
> > > -		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
> > > -		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
> > > -	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
> > > -	  echo Gcov report for $$f:;\
> > > -	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> > > -	done,)
> > > +$(patsubst %, check-%, $(check-unit-y)): check-tests/%: qtest-run-%
> > > 
> > >  # gtester tests with XML output
> > > 
> > > 
> > 
> > I can't vouch for gcov either, but:
> 
> Too bad that this seems to be too big a hammer that breaks the way gcov was
> used: 1) the "rm *.gcda" command now runs in each test, so it's harder to
> get an overall coverage report; 2) there is a risk that the parallism may
> corrupt those files. :(

Almost no one will ever build with gcov support, so how about just
making the "normal" case run in parallel and add a special target
for running under gcov which serializes.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Fam Zheng]

On Tue, 09/27 11:17, Daniel P. Berrange wrote:
> > Too bad that this seems to be too big a hammer that breaks the way gcov was
> > used: 1) the "rm *.gcda" command now runs in each test, so it's harder to
> > get an overall coverage report; 2) there is a risk that the parallism may
> > corrupt those files. :(
> 
> Almost no one will ever build with gcov support, so how about just
> making the "normal" case run in parallel and add a special target
> for running under gcov which serializes.

This sounds good to me. I'll take a look. Thanks.

Fam

> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Gonglei (Arei)]

> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Tuesday, September 27, 2016 6:15 PM
> To: Fam Zheng
> Cc: Gonglei (Arei); pbonzini@redhat.com; John Snow; qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel
> 
> On Fri, Sep 23, 2016 at 05:59:05PM +0800, Fam Zheng wrote:
> > On Fri, 09/23 09:39, Gonglei (Arei) wrote:
> > >
> > > Hi Fam,
> > >
> > >
> > > > -----Original Message-----
> > > > From: Qemu-devel
> > > > [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On
> > > > Behalf Of Fam Zheng
> > > > Sent: Friday, September 23, 2016 3:58 PM
> > > > To: John Snow
> > > > Cc: pbonzini@redhat.com; qemu-devel@nongnu.org
> > > > Subject: Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel
> > > >
> > > > On Wed, 09/21 14:24, John Snow wrote:
> > > > >
> > > > >
> > > > > On 08/12/2016 05:19 AM, Fam Zheng wrote:
> > > > > > Previously all test cases in a category, such as check-qtest-y, are
> > > > > > executed in a single long gtester command. This patch separates each
> > > > > > test into its own make target to allow better parallism.
> > > > > >
> > >
> > > That's will be great if we can specify a test to run, especially for the
> scenario
> > > which add one use qtest case.
> > >
> > > For example:
> > >
> > >  # make check test-crypto-cipher
> > >
> > > then only run the tests/ test-crypto-cipher.
> > >
> > > Do you think it makes sense?
> >
> > Or more likely:
> >
> >     # make check TESTS="test-crypto-cipher test-crypto-hash ..."
> >
> > Usually I just extract the gtester command line with V=1 and run it from my
> > shell prompt.  Feel free to send a patch, though.
> 
> Shouldn't even need todo that in most cases - I tend to just do
> 
>   make tests/test-crypto-cipher && ./tess/test-crypto-cipher
> 
> If there are tests which rely on some environment set by the Makefile,
> then really they should be fixed to have sensible defaults so that they
> can be directly executed.
> 
Thanks for your reminding! It works fine.

# ./tests/test-crypto-cipher 
/crypto/cipher/aes-ecb-128: OK
/crypto/cipher/aes-ecb-192: OK
/crypto/cipher/aes-ecb-256: OK
/crypto/cipher/aes-cbc-128: OK
/crypto/cipher/aes-cbc-192: OK
/crypto/cipher/aes-cbc-256: OK
/crypto/cipher/des-rfb-ecb-56: OK
/crypto/cipher/cast5-128: OK
/crypto/cipher/serpent-128: OK
/crypto/cipher/serpent-192: OK
/crypto/cipher/serpent-256a: OK
/crypto/cipher/serpent-256b: OK
/crypto/cipher/twofish-128: OK
/crypto/cipher/twofish-256: OK
/crypto/cipher/aes-xts-128-1: OK
/crypto/cipher/aes-xts-128-2: OK
/crypto/cipher/aes-xts-128-3: OK
/crypto/cipher/aes-xts-128-4: OK
/crypto/cipher/cast5-xts-128: OK
/crypto/cipher/aes-ctr-128: OK
/crypto/cipher/aes-ctr-192: OK
/crypto/cipher/aes-ctr-256: OK
/crypto/cipher/null-iv: OK
/crypto/cipher/short-plaintext: OK

# ./tests/virtio-net-test
**
ERROR:tests/libqtest.c:561:qtest_get_arch: assertion failed: (qemu != NULL)
Aborted (core dumped)

# ./tests/virtio-blk-test
**
ERROR:tests/libqtest.c:561:qtest_get_arch: assertion failed: (qemu != NULL)
Aborted (core dumped)

But they work after I set the environment variable to specify architecture:
 
# QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 ./tests/virtio-net-test
/x86_64/virtio/net/pci/basic: OK
/x86_64/virtio/net/pci/rx_stop_cont: OK
/x86_64/virtio/net/pci/hotplug: OK

# QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 ./tests/virtio-blk-test
/x86_64/virtio/blk/pci/basic: OK
/x86_64/virtio/blk/pci/indirect: OK
/x86_64/virtio/blk/pci/config: OK
/x86_64/virtio/blk/pci/msix: OK
/x86_64/virtio/blk/pci/idx: OK
/x86_64/virtio/blk/pci/hotplug: OK

So, Maybe we should add check if the environment relied on is set
before executing specific operations in this kind of tests. Right?

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Fam Zheng]

On Wed, 09/28 01:31, Gonglei (Arei) wrote:
> # ./tests/virtio-net-test
> **
> ERROR:tests/libqtest.c:561:qtest_get_arch: assertion failed: (qemu != NULL)
> Aborted (core dumped)
> 
> # ./tests/virtio-blk-test
> **
> ERROR:tests/libqtest.c:561:qtest_get_arch: assertion failed: (qemu != NULL)
> Aborted (core dumped)
> 
> But they work after I set the environment variable to specify architecture:
>  
> # QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 ./tests/virtio-net-test
> /x86_64/virtio/net/pci/basic: OK
> /x86_64/virtio/net/pci/rx_stop_cont: OK
> /x86_64/virtio/net/pci/hotplug: OK
> 
> # QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 ./tests/virtio-blk-test
> /x86_64/virtio/blk/pci/basic: OK
> /x86_64/virtio/blk/pci/indirect: OK
> /x86_64/virtio/blk/pci/config: OK
> /x86_64/virtio/blk/pci/msix: OK
> /x86_64/virtio/blk/pci/idx: OK
> /x86_64/virtio/blk/pci/hotplug: OK
> 
> So, Maybe we should add check if the environment relied on is set
> before executing specific operations in this kind of tests. Right?

Or make a guess based on $(realpath $0]) (in this case, print the found path to
avoid testing against wrong binary by mistake).

Fam

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Gonglei (Arei)]

> -----Original Message-----
> From: Fam Zheng [mailto:famz@redhat.com]
> Sent: Wednesday, September 28, 2016 9:45 AM
> To: Gonglei (Arei)
> Cc: Daniel P. Berrange; pbonzini@redhat.com; John Snow;
> qemu-devel@nongnu.org; Wubin (H)
> Subject: Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel
> 
> On Wed, 09/28 01:31, Gonglei (Arei) wrote:
> > # ./tests/virtio-net-test
> > **
> > ERROR:tests/libqtest.c:561:qtest_get_arch: assertion failed: (qemu != NULL)
> > Aborted (core dumped)
> >
> > # ./tests/virtio-blk-test
> > **
> > ERROR:tests/libqtest.c:561:qtest_get_arch: assertion failed: (qemu != NULL)
> > Aborted (core dumped)
> >
> > But they work after I set the environment variable to specify architecture:
> >
> > #
> QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 ./tests/virtio-n
> et-test
> > /x86_64/virtio/net/pci/basic: OK
> > /x86_64/virtio/net/pci/rx_stop_cont: OK
> > /x86_64/virtio/net/pci/hotplug: OK
> >
> > #
> QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 ./tests/virtio-b
> lk-test
> > /x86_64/virtio/blk/pci/basic: OK
> > /x86_64/virtio/blk/pci/indirect: OK
> > /x86_64/virtio/blk/pci/config: OK
> > /x86_64/virtio/blk/pci/msix: OK
> > /x86_64/virtio/blk/pci/idx: OK
> > /x86_64/virtio/blk/pci/hotplug: OK
> >
> > So, Maybe we should add check if the environment relied on is set
> > before executing specific operations in this kind of tests. Right?
> 
> Or make a guess based on $(realpath $0]) (in this case, print the found path to
> avoid testing against wrong binary by mistake).
> 
What do you mean about the realpath? Can we use it to set the environment variable?

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH RFC] tests: Run qtest cases in parallel

[Fam Zheng]

On Wed, 09/28 01:54, Gonglei (Arei) wrote:
> What do you mean about the realpath? Can we use it to set the environment variable?

Basically just guess where QTEST_QEMU_BINARY is based on argv[0].

Fam