qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Re-enabling tests/avocado/boot_linux.py for PPC64
@ 2023-03-27 11:50 Kautuk Consul
  2023-03-27 11:50 ` [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0 Kautuk Consul
  2023-03-27 11:50 ` [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64 Kautuk Consul
  0 siblings, 2 replies; 12+ messages in thread
From: Kautuk Consul @ 2023-03-27 11:50 UTC (permalink / raw)
  To: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal
  Cc: qemu-devel, Kautuk Consul

The tests/avocado/boot_linux.py was disabled because it would take
too long to execute due to which it would timeout. On investigation
of this it was found that:
1)	The avocado module was downloading the Fedora 31 qcow2 image 2
	times due to checksum algorithm mismatch. The first download
	was computing the checksum with the sha1 algorithm whereas the
	second time the sha256 algorithm checksum was being passed by the
	avocado_qemu module due to which the 2nd download was being
	triggered.
2)	The boot_linux.py test-case was including the image download time
	for the 2nd download (as mentioned in point 1) in the test-case
	timeout time.

This patchset aims to solve the above problems by:
1)	Bumping up the avocado-framework version used by qemu to 101.0.
	This version of avocado includes a fix that re-computes the
	checksum of the already downloaded file using sha256 and then checks
	the checksum string being passed by avocado_qemu. This fix will
	also update the *-CHECKSUM file with a new line for the sha256
	checksum.
2)	Separating the download timeout from the actual test-case
	execution timeout in boot_linux.py.

Kautuk Consul (2):
  tests/requirements.txt: bump up avocado-framework version to 101.0
  tests/avocado/boot_linux.py: re-enable test-case for ppc64

 tests/avocado/boot_linux.py | 6 +++++-
 tests/requirements.txt      | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.39.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0
  2023-03-27 11:50 [PATCH 0/2] Re-enabling tests/avocado/boot_linux.py for PPC64 Kautuk Consul
@ 2023-03-27 11:50 ` Kautuk Consul
  2023-03-30  6:11   ` Kautuk Consul
  2023-04-21  3:42   ` Cleber Rosa
  2023-03-27 11:50 ` [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64 Kautuk Consul
  1 sibling, 2 replies; 12+ messages in thread
From: Kautuk Consul @ 2023-03-27 11:50 UTC (permalink / raw)
  To: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal
  Cc: qemu-devel, Kautuk Consul, Hariharan T S

Avocado version 101.0 has a fix to re-compute the checksum
of an asset file if the algorithm used in the *-CHECKSUM
file isn't the same as the one being passed to it by the
avocado user (i.e. the avocado_qemu python module).
In the earlier avocado versions this fix wasn't there due
to which if the checksum wouldn't match the earlier
checksum (calculated by a different algorithm), the avocado
code would start downloading a fresh image from the internet
URL thus making the test-cases take longer to execute.

Bump up the avocado-framework version to 101.0.

Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
---
 tests/requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..a6f73da681 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,5 +2,5 @@
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
 # Note that qemu.git/python/ is always implicitly installed.
-avocado-framework==88.1
+avocado-framework==101.0
 pycdlib==1.11.0
-- 
2.39.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64
  2023-03-27 11:50 [PATCH 0/2] Re-enabling tests/avocado/boot_linux.py for PPC64 Kautuk Consul
  2023-03-27 11:50 ` [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0 Kautuk Consul
@ 2023-03-27 11:50 ` Kautuk Consul
  2023-03-27 16:07   ` Alex Bennée
  1 sibling, 1 reply; 12+ messages in thread
From: Kautuk Consul @ 2023-03-27 11:50 UTC (permalink / raw)
  To: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal
  Cc: qemu-devel, Kautuk Consul, Alex Bennée

Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").

Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
for PPC64. On investigation, this turns out to be an issue with the
time taken for downloading the Fedora 31 qcow2 image being included
within the test-case timeout.
Re-enable this test-case by setting the timeout to 360 seconds just
before launching the downloaded VM image.

Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
Reported-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Hariharan T S hariharan.ts@linux.vnet.ibm.com
---
 tests/avocado/boot_linux.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
index be30dcbd58..c3869a987c 100644
--- a/tests/avocado/boot_linux.py
+++ b/tests/avocado/boot_linux.py
@@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
     :avocado: tags=arch:ppc64
     """
 
+    # timeout for downloading new VM image.
     timeout = 360
 
-    @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
     def test_pseries_tcg(self):
         """
         :avocado: tags=machine:pseries
@@ -101,6 +101,10 @@ def test_pseries_tcg(self):
         """
         self.require_accelerator("tcg")
         self.vm.add_args("-accel", "tcg")
+
+        # timeout for actual Linux PPC boot test
+        self.timeout = 360
+
         self.launch_and_wait(set_up_ssh_connection=False)
 
 
-- 
2.39.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64
  2023-03-27 11:50 ` [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64 Kautuk Consul
@ 2023-03-27 16:07   ` Alex Bennée
  2023-03-28 11:24     ` Kautuk Consul
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Bennée @ 2023-03-27 16:07 UTC (permalink / raw)
  To: Kautuk Consul
  Cc: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal, qemu-devel


Kautuk Consul <kconsul@linux.vnet.ibm.com> writes:

> Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").
>
> Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
> for PPC64. On investigation, this turns out to be an issue with the
> time taken for downloading the Fedora 31 qcow2 image being included
> within the test-case timeout.
> Re-enable this test-case by setting the timeout to 360 seconds just
> before launching the downloaded VM image.
>
> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
> Reported-by: Alex Bennée <alex.bennee@linaro.org>
> Tested-by: Hariharan T S hariharan.ts@linux.vnet.ibm.com

It doesn't really address the principle problem that the
boot_linux.py:BootLinuxPPC64.test_pseries_tcg is super heavyweight for
only 2% extra coverage of the executed lines.

What we really need is a script so we can compare the output between the
two jsons:

  gcovr --json --exclude-unreachable-branches --print-summary -o coverage.json --root ../../ . *.p

because I suspect we could make up that missing few % noodling the
baseline test a bit more.

> ---
>  tests/avocado/boot_linux.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
> index be30dcbd58..c3869a987c 100644
> --- a/tests/avocado/boot_linux.py
> +++ b/tests/avocado/boot_linux.py
> @@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
>      :avocado: tags=arch:ppc64
>      """
>  
> +    # timeout for downloading new VM image.
>      timeout = 360
>  
> -    @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
>      def test_pseries_tcg(self):
>          """
>          :avocado: tags=machine:pseries
> @@ -101,6 +101,10 @@ def test_pseries_tcg(self):
>          """
>          self.require_accelerator("tcg")
>          self.vm.add_args("-accel", "tcg")
> +
> +        # timeout for actual Linux PPC boot test
> +        self.timeout = 360
> +
>          self.launch_and_wait(set_up_ssh_connection=False)


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64
  2023-03-27 16:07   ` Alex Bennée
@ 2023-03-28 11:24     ` Kautuk Consul
  2023-03-28 12:21       ` Alex Bennée
  2023-03-31 11:19       ` Cédric Le Goater
  0 siblings, 2 replies; 12+ messages in thread
From: Kautuk Consul @ 2023-03-28 11:24 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal, qemu-devel

On 2023-03-27 17:07:30, Alex Bennée wrote:
> 
> Kautuk Consul <kconsul@linux.vnet.ibm.com> writes:
> 
> > Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").
> >
> > Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
> > for PPC64. On investigation, this turns out to be an issue with the
> > time taken for downloading the Fedora 31 qcow2 image being included
> > within the test-case timeout.
> > Re-enable this test-case by setting the timeout to 360 seconds just
> > before launching the downloaded VM image.
> >
> > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
> > Reported-by: Alex Bennée <alex.bennee@linaro.org>
> > Tested-by: Hariharan T S hariharan.ts@linux.vnet.ibm.com
> 
> It doesn't really address the principle problem that the
> boot_linux.py:BootLinuxPPC64.test_pseries_tcg is super heavyweight for
> only 2% extra coverage of the executed lines.
By re-enabling this test-case we will ensure that PPC64 part of qemu
works okay in terms of basic linux boot. Without this we will have
a regression in the sense that there won't be any way to test out
basic linux boot for PPC64.
> 
> What we really need is a script so we can compare the output between the
> two jsons:
> 
>   gcovr --json --exclude-unreachable-branches --print-summary -o coverage.json --root ../../ . *.p
> 
> because I suspect we could make up that missing few % noodling the
> baseline test a bit more.
Can you tell me how you check code coverage with and without this
test-case ? I am kind of new to qemu so it would be nice to know how you
do this. And I am trying to increase the code coverage by improving
the baseline test by including more devices in the qemu-system-ppc64
command line so I would appreciate any tips on how to do that also.
> 
> > ---
> >  tests/avocado/boot_linux.py | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
> > index be30dcbd58..c3869a987c 100644
> > --- a/tests/avocado/boot_linux.py
> > +++ b/tests/avocado/boot_linux.py
> > @@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
> >      :avocado: tags=arch:ppc64
> >      """
> >  
> > +    # timeout for downloading new VM image.
> >      timeout = 360
> >  
> > -    @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
> >      def test_pseries_tcg(self):
> >          """
> >          :avocado: tags=machine:pseries
> > @@ -101,6 +101,10 @@ def test_pseries_tcg(self):
> >          """
> >          self.require_accelerator("tcg")
> >          self.vm.add_args("-accel", "tcg")
> > +
> > +        # timeout for actual Linux PPC boot test
> > +        self.timeout = 360
> > +
> >          self.launch_and_wait(set_up_ssh_connection=False)
> 
> 
> -- 
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64
  2023-03-28 11:24     ` Kautuk Consul
@ 2023-03-28 12:21       ` Alex Bennée
  2023-03-31 11:19       ` Cédric Le Goater
  1 sibling, 0 replies; 12+ messages in thread
From: Alex Bennée @ 2023-03-28 12:21 UTC (permalink / raw)
  To: Kautuk Consul
  Cc: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal, qemu-devel


Kautuk Consul <kconsul@linux.vnet.ibm.com> writes:

> On 2023-03-27 17:07:30, Alex Bennée wrote:
>> 
>> Kautuk Consul <kconsul@linux.vnet.ibm.com> writes:
>> 
>> > Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").
>> >
>> > Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
>> > for PPC64. On investigation, this turns out to be an issue with the
>> > time taken for downloading the Fedora 31 qcow2 image being included
>> > within the test-case timeout.
>> > Re-enable this test-case by setting the timeout to 360 seconds just
>> > before launching the downloaded VM image.
>> >
>> > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>> > Reported-by: Alex Bennée <alex.bennee@linaro.org>
>> > Tested-by: Hariharan T S hariharan.ts@linux.vnet.ibm.com
>> 
>> It doesn't really address the principle problem that the
>> boot_linux.py:BootLinuxPPC64.test_pseries_tcg is super heavyweight for
>> only 2% extra coverage of the executed lines.
> By re-enabling this test-case we will ensure that PPC64 part of qemu
> works okay in terms of basic linux boot. Without this we will have
> a regression in the sense that there won't be any way to test out
> basic linux boot for PPC64.

Sure we do:

  ➜  ./tests/venv/bin/avocado list ./tests/avocado/tuxrun_baselines.py:TuxRunBaselineTest.test_p
  INSTRUMENTED ./tests/avocado/tuxrun_baselines.py:TuxRunBaselineTest.test_ppc32
  INSTRUMENTED ./tests/avocado/tuxrun_baselines.py:TuxRunBaselineTest.test_ppc64
  INSTRUMENTED ./tests/avocado/tuxrun_baselines.py:TuxRunBaselineTest.test_ppc64le

boot 3 different ppc configurations.

>> 
>> What we really need is a script so we can compare the output between the
>> two jsons:
>> 
>>   gcovr --json --exclude-unreachable-branches --print-summary -o coverage.json --root ../../ . *.p
>> 
>> because I suspect we could make up that missing few % noodling the
>> baseline test a bit more.
> Can you tell me how you check code coverage with and without this
> test-case ?

I use two build directories, both configured with --enable-gcov. e.g.:

 ../../configure' '--disable-docs' '--enable-gcov' '--target-list=ppc64-softmmu'

and run a different set of tests in each build dir. You can then run:

  make coverage-html V=1

for the initial report. See:

  https://qemu.readthedocs.io/en/latest/devel/testing.html#gcc-gcov-support
  
> I am kind of new to qemu so it would be nice to know how you
> do this. And I am trying to increase the code coverage by improving
> the baseline test by including more devices in the qemu-system-ppc64
> command line so I would appreciate any tips on how to do that also.

The only problem is eyeballing the html reports is a very fuzzy way of
comparing coverage. However the gcovr report generates some useful
machine readable json which could be compared with a script.

>> 
>> > ---
>> >  tests/avocado/boot_linux.py | 6 +++++-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
>> > index be30dcbd58..c3869a987c 100644
>> > --- a/tests/avocado/boot_linux.py
>> > +++ b/tests/avocado/boot_linux.py
>> > @@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
>> >      :avocado: tags=arch:ppc64
>> >      """
>> >  
>> > +    # timeout for downloading new VM image.
>> >      timeout = 360
>> >  
>> > -    @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
>> >      def test_pseries_tcg(self):
>> >          """
>> >          :avocado: tags=machine:pseries
>> > @@ -101,6 +101,10 @@ def test_pseries_tcg(self):
>> >          """
>> >          self.require_accelerator("tcg")
>> >          self.vm.add_args("-accel", "tcg")
>> > +
>> > +        # timeout for actual Linux PPC boot test
>> > +        self.timeout = 360
>> > +
>> >          self.launch_and_wait(set_up_ssh_connection=False)
>> 
>> 
>> -- 
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro
>> 


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0
  2023-03-27 11:50 ` [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0 Kautuk Consul
@ 2023-03-30  6:11   ` Kautuk Consul
  2023-03-30  9:18     ` Alex Bennée
  2023-03-31 10:19     ` Alex Bennée
  2023-04-21  3:42   ` Cleber Rosa
  1 sibling, 2 replies; 12+ messages in thread
From: Kautuk Consul @ 2023-03-30  6:11 UTC (permalink / raw)
  To: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal
  Cc: qemu-devel, Hariharan T S

Hi,
On 2023-03-27 07:50:29, Kautuk Consul wrote:
> Avocado version 101.0 has a fix to re-compute the checksum
> of an asset file if the algorithm used in the *-CHECKSUM
> file isn't the same as the one being passed to it by the
> avocado user (i.e. the avocado_qemu python module).
> In the earlier avocado versions this fix wasn't there due
> to which if the checksum wouldn't match the earlier
> checksum (calculated by a different algorithm), the avocado
> code would start downloading a fresh image from the internet
> URL thus making the test-cases take longer to execute.
> 
> Bump up the avocado-framework version to 101.0.
Any comments on this ? I have tested this patch and it seems to work
fine with the avocado test-cases.
> 
> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
> Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
> ---
>  tests/requirements.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/requirements.txt b/tests/requirements.txt
> index 0ba561b6bd..a6f73da681 100644
> --- a/tests/requirements.txt
> +++ b/tests/requirements.txt
> @@ -2,5 +2,5 @@
>  # in the tests/venv Python virtual environment. For more info,
>  # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>  # Note that qemu.git/python/ is always implicitly installed.
> -avocado-framework==88.1
> +avocado-framework==101.0
>  pycdlib==1.11.0
> -- 
> 2.39.2
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0
  2023-03-30  6:11   ` Kautuk Consul
@ 2023-03-30  9:18     ` Alex Bennée
  2023-03-31 10:19     ` Alex Bennée
  1 sibling, 0 replies; 12+ messages in thread
From: Alex Bennée @ 2023-03-30  9:18 UTC (permalink / raw)
  To: Kautuk Consul
  Cc: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal, Hariharan T S, qemu-devel


Kautuk Consul <kconsul@linux.vnet.ibm.com> writes:

> Hi,
> On 2023-03-27 07:50:29, Kautuk Consul wrote:
>> Avocado version 101.0 has a fix to re-compute the checksum
>> of an asset file if the algorithm used in the *-CHECKSUM
>> file isn't the same as the one being passed to it by the
>> avocado user (i.e. the avocado_qemu python module).
>> In the earlier avocado versions this fix wasn't there due
>> to which if the checksum wouldn't match the earlier
>> checksum (calculated by a different algorithm), the avocado
>> code would start downloading a fresh image from the internet
>> URL thus making the test-cases take longer to execute.
>> 
>> Bump up the avocado-framework version to 101.0.
> Any comments on this ? I have tested this patch and it seems to work
> fine with the avocado test-cases.

I've queued up just this patch to for-8.0/more-misc-fixes, thanks.

>> 
>> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>> Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
>> ---
>>  tests/requirements.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tests/requirements.txt b/tests/requirements.txt
>> index 0ba561b6bd..a6f73da681 100644
>> --- a/tests/requirements.txt
>> +++ b/tests/requirements.txt
>> @@ -2,5 +2,5 @@
>>  # in the tests/venv Python virtual environment. For more info,
>>  # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>>  # Note that qemu.git/python/ is always implicitly installed.
>> -avocado-framework==88.1
>> +avocado-framework==101.0
>>  pycdlib==1.11.0
>> -- 
>> 2.39.2
>> 
>> 


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0
  2023-03-30  6:11   ` Kautuk Consul
  2023-03-30  9:18     ` Alex Bennée
@ 2023-03-31 10:19     ` Alex Bennée
  2023-03-31 10:47       ` Kautuk Consul
  1 sibling, 1 reply; 12+ messages in thread
From: Alex Bennée @ 2023-03-31 10:19 UTC (permalink / raw)
  To: Kautuk Consul
  Cc: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal, Hariharan T S, qemu-devel


Kautuk Consul <kconsul@linux.vnet.ibm.com> writes:

> Hi,
> On 2023-03-27 07:50:29, Kautuk Consul wrote:
>> Avocado version 101.0 has a fix to re-compute the checksum
>> of an asset file if the algorithm used in the *-CHECKSUM
>> file isn't the same as the one being passed to it by the
>> avocado user (i.e. the avocado_qemu python module).
>> In the earlier avocado versions this fix wasn't there due
>> to which if the checksum wouldn't match the earlier
>> checksum (calculated by a different algorithm), the avocado
>> code would start downloading a fresh image from the internet
>> URL thus making the test-cases take longer to execute.
>> 
>> Bump up the avocado-framework version to 101.0.
> Any comments on this ? I have tested this patch and it seems to work
> fine with the avocado test-cases.

I'm dropping this from the for-8.0 series as it causes a bunch of
failures in tests. I'll keep it in testing/next for when the tree
re-opens.

>> 
>> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>> Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
>> ---
>>  tests/requirements.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tests/requirements.txt b/tests/requirements.txt
>> index 0ba561b6bd..a6f73da681 100644
>> --- a/tests/requirements.txt
>> +++ b/tests/requirements.txt
>> @@ -2,5 +2,5 @@
>>  # in the tests/venv Python virtual environment. For more info,
>>  # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>>  # Note that qemu.git/python/ is always implicitly installed.
>> -avocado-framework==88.1
>> +avocado-framework==101.0
>>  pycdlib==1.11.0
>> -- 
>> 2.39.2
>> 
>> 


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0
  2023-03-31 10:19     ` Alex Bennée
@ 2023-03-31 10:47       ` Kautuk Consul
  0 siblings, 0 replies; 12+ messages in thread
From: Kautuk Consul @ 2023-03-31 10:47 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal, Hariharan T S, qemu-devel

On 2023-03-31 11:19:18, Alex Bennée wrote:
> 
> Kautuk Consul <kconsul@linux.vnet.ibm.com> writes:
> 
> > Hi,
> > On 2023-03-27 07:50:29, Kautuk Consul wrote:
> >> Avocado version 101.0 has a fix to re-compute the checksum
> >> of an asset file if the algorithm used in the *-CHECKSUM
> >> file isn't the same as the one being passed to it by the
> >> avocado user (i.e. the avocado_qemu python module).
> >> In the earlier avocado versions this fix wasn't there due
> >> to which if the checksum wouldn't match the earlier
> >> checksum (calculated by a different algorithm), the avocado
> >> code would start downloading a fresh image from the internet
> >> URL thus making the test-cases take longer to execute.
> >> 
> >> Bump up the avocado-framework version to 101.0.
> > Any comments on this ? I have tested this patch and it seems to work
> > fine with the avocado test-cases.
> 
> I'm dropping this from the for-8.0 series as it causes a bunch of
> failures in tests. I'll keep it in testing/next for when the tree
> re-opens.
Sure, sounds good. Thanks.
> 
> >> 
> >> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
> >> Tested-by: Hariharan T S <hariharan.ts@linux.vnet.ibm.com>
> >> ---
> >>  tests/requirements.txt | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/tests/requirements.txt b/tests/requirements.txt
> >> index 0ba561b6bd..a6f73da681 100644
> >> --- a/tests/requirements.txt
> >> +++ b/tests/requirements.txt
> >> @@ -2,5 +2,5 @@
> >>  # in the tests/venv Python virtual environment. For more info,
> >>  # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
> >>  # Note that qemu.git/python/ is always implicitly installed.
> >> -avocado-framework==88.1
> >> +avocado-framework==101.0
> >>  pycdlib==1.11.0
> >> -- 
> >> 2.39.2
> >> 
> >> 
> 
> 
> -- 
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64
  2023-03-28 11:24     ` Kautuk Consul
  2023-03-28 12:21       ` Alex Bennée
@ 2023-03-31 11:19       ` Cédric Le Goater
  1 sibling, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2023-03-31 11:19 UTC (permalink / raw)
  To: Kautuk Consul, Alex Bennée
  Cc: Paolo Bonzini, John Snow, Cleber Rosa,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal, qemu-devel, Daniel Henrique Barboza,
	list@suse.de:PowerPC

Hello,

[ Copying qemu-ppc@ and Daniel ]

On 3/28/23 13:24, Kautuk Consul wrote:
> On 2023-03-27 17:07:30, Alex Bennée wrote:
>>
>> Kautuk Consul <kconsul@linux.vnet.ibm.com> writes:
>>
>>> Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").
>>>
>>> Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
>>> for PPC64. On investigation, this turns out to be an issue with the
>>> time taken for downloading the Fedora 31 qcow2 image being included
>>> within the test-case timeout.
>>> Re-enable this test-case by setting the timeout to 360 seconds just
>>> before launching the downloaded VM image.
>>>
>>> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com>
>>> Reported-by: Alex Bennée <alex.bennee@linaro.org>
>>> Tested-by: Hariharan T S hariharan.ts@linux.vnet.ibm.com
>>
>> It doesn't really address the principle problem that the
>> boot_linux.py:BootLinuxPPC64.test_pseries_tcg is super heavyweight for
>> only 2% extra coverage of the executed lines.
> By re-enabling this test-case we will ensure that PPC64 part of qemu
> works okay in terms of basic linux boot. Without this we will have
> a regression in the sense that there won't be any way to test out
> basic linux boot for PPC64.

There are ways and pseries is not only PPC64 machine. There is more
to it. See :

   https://github.com/legoater/qemu-ppc-boot/tree/main/buildroot
   https://github.com/legoater/buildroot/tree/qemu-ppc/board/qemu

QEMU PPC maintainers have external tools for regressions which are
run regularly, at least before sending a PR for upstream.

Thanks,

C.

>>
>> What we really need is a script so we can compare the output between the
>> two jsons:
>>
>>    gcovr --json --exclude-unreachable-branches --print-summary -o coverage.json --root ../../ . *.p
>>
>> because I suspect we could make up that missing few % noodling the
>> baseline test a bit more.
> Can you tell me how you check code coverage with and without this
> test-case ? I am kind of new to qemu so it would be nice to know how you
> do this. And I am trying to increase the code coverage by improving
> the baseline test by including more devices in the qemu-system-ppc64
> command line so I would appreciate any tips on how to do that also.
>>
>>> ---
>>>   tests/avocado/boot_linux.py | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
>>> index be30dcbd58..c3869a987c 100644
>>> --- a/tests/avocado/boot_linux.py
>>> +++ b/tests/avocado/boot_linux.py
>>> @@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
>>>       :avocado: tags=arch:ppc64
>>>       """
>>>   
>>> +    # timeout for downloading new VM image.
>>>       timeout = 360
>>>   
>>> -    @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
>>>       def test_pseries_tcg(self):
>>>           """
>>>           :avocado: tags=machine:pseries
>>> @@ -101,6 +101,10 @@ def test_pseries_tcg(self):
>>>           """
>>>           self.require_accelerator("tcg")
>>>           self.vm.add_args("-accel", "tcg")
>>> +
>>> +        # timeout for actual Linux PPC boot test
>>> +        self.timeout = 360
>>> +
>>>           self.launch_and_wait(set_up_ssh_connection=False)
>>
>>
>> -- 
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro
>>
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0
  2023-03-27 11:50 ` [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0 Kautuk Consul
  2023-03-30  6:11   ` Kautuk Consul
@ 2023-04-21  3:42   ` Cleber Rosa
  1 sibling, 0 replies; 12+ messages in thread
From: Cleber Rosa @ 2023-04-21  3:42 UTC (permalink / raw)
  To: Kautuk Consul, Paolo Bonzini, John Snow,
	Philippe Mathieu-Daudé, Wainer dos Santos Moschetta,
	Beraldo Leal
  Cc: qemu-devel, Hariharan T S


On 3/27/23 07:50, Kautuk Consul wrote:
> Avocado version 101.0 has a fix to re-compute the checksum
> of an asset file if the algorithm used in the *-CHECKSUM
> file isn't the same as the one being passed to it by the
> avocado user (i.e. the avocado_qemu python module).
> In the earlier avocado versions this fix wasn't there due
> to which if the checksum wouldn't match the earlier
> checksum (calculated by a different algorithm), the avocado
> code would start downloading a fresh image from the internet
> URL thus making the test-cases take longer to execute.
>
> Bump up the avocado-framework version to 101.0.

Hi Kautuk,

First of all, thanks for working on this, and thanks to Hariharan for 
testing it.

I'd like to give some context which not everyone may be aware of.  
Avocado 101.0 is a very different when compared with 88.1. Everything 
related to the execution of tests is brand new.  To be more precise, on 
version 91.0[1], this new runner[2] became the default. On version 97.0, 
the old runner implementation (currently in use in QEMU) was finally 
removed.

On most releases since then, I've been running the QEMU tests with the 
latest Avocado, and finding issues that are (as resources allow) 
addressed in later versions.   As you probably noticed, Avocado 101.0 
runs the QEMU tests without much (or any) visible issues for most 
people.  But, I'm aware of two pending issues that may or may not be a 
big deal to users:

I) The logging behavior is a bit different since Avocado 88.1. At a 
given point it was considered that Avocado should not mess around 
inadvertently with Python's root logger, and should be more picky about 
it includes in logs.  For most cases, a simple workaround[4] does the 
trick.  But, for some other use cases (say for 3rd party libraries' logs 
you want logged alongside Avocado's logs) there's a pending PR[5] that 
will take care of all known limitations.

II) The support for killing tests (internally in Avocado represented as 
more generic "tasks") and all its children is a bit lacking.  This is an 
issue I'm actively working on[6].  This may leave some processes (such 
as "qemu-system-*") running even after a test was interrupted.

Fixes for both of these issues are due to be in version 102.0. The ETA 
for version 102.0 is 1-2 weeks.

With that being said, I'm more than OK with this patch (alongside PATCH 
2, without which havoc ensues :) provided people understand the two 
pending issues above.  If this patch is taken before Avocado 102.0 is 
released, the delta from 101.0 would be much smaller, so it should be an 
easier change to test.

Cheers,

- Cleber.


[1] - https://avocado-framework.readthedocs.io/en/101.0/releases/91_0.html

[2] - The new runner is called "nrunner" and I am to be blamed for the 
naming lacking any originality

[3] - 
https://avocado-framework.readthedocs.io/en/101.0/releases/97_0.html#users-test-writers

[4] - 
https://gitlab.com/cleber.gnu/qemu/-/commit/a9f39c4f6671b756196a185c7275eb7ebd13e588

[5] - https://github.com/avocado-framework/avocado/pull/5645

[6] - https://github.com/avocado-framework/avocado/issues/4994




^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-04-21  3:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-27 11:50 [PATCH 0/2] Re-enabling tests/avocado/boot_linux.py for PPC64 Kautuk Consul
2023-03-27 11:50 ` [PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0 Kautuk Consul
2023-03-30  6:11   ` Kautuk Consul
2023-03-30  9:18     ` Alex Bennée
2023-03-31 10:19     ` Alex Bennée
2023-03-31 10:47       ` Kautuk Consul
2023-04-21  3:42   ` Cleber Rosa
2023-03-27 11:50 ` [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64 Kautuk Consul
2023-03-27 16:07   ` Alex Bennée
2023-03-28 11:24     ` Kautuk Consul
2023-03-28 12:21       ` Alex Bennée
2023-03-31 11:19       ` Cédric Le Goater

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).