[Qemu-devel] [PATCH v2 0/2] Add "boot_linux" acceptance test

[Qemu-devel] [PATCH v2 0/2] Add "boot_linux" acceptance test

[Cleber Rosa]

Some hopefully useful pointers for reviewers:
=============================================

Git Info:
  - URI: https://github.com/clebergnu/qemu/tree/sent/test_boot_linux_v2
  - Remote: https://github.com/clebergnu/qemu
  - Branch: sent/test_boot_linux_v2

Travis CI Info:

  - Build: https://travis-ci.org/clebergnu/qemu/builds/458110642
  - Execution results for the test added here:
    * https://travis-ci.org/clebergnu/qemu/jobs/458110664#L2043

Previous version:
  - http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02530.html

Changes from v1:
================

 * The commit message was adjusted, removing the reference to the
   avocado.utils.vmimage encoding issue on previous Avocado versions
   (<= 64.0) and the fix that would (and was) included in Avocado
   version 65.0.

 * Effectively added pycdlib==1.6.0 to the requirements.txt file,
   added on a56931eef3, and adjusted the commit message was also
   to reflect that.

 * Updated the default version of the guest OS, from Fedora 28 to 29.
   Besides possible improvements in the (virtual) hardware coverage,
   it brings a performance improvement in the order of 20% to the
   test:

   With a Fedora 28 guest:
   ... boot_linux.py:BootLinux.test: PASS (14.23 s)

   With a Fedora 29 guest:
   ...  boot_linux.py:BootLinux.test: PASS (11.87 s)

 * Removed all direct parameters usage from the test.  Because some
   parameters and its default values implemented in the test would
   prevent it from running on some environments.  Example: the "accel"
   parameter had a default value of "kvm", which would prevent this
   test, that boots a x86_64 OS, from running on a host arch different
   than x86_64.  I recognize that it's desirable to make tests
   reusable and parameterized (that was the reason for the first
   version doing so), but the mechanism to be used to define the
   architectures that a given test should support is still an open
   issue, and has been discussed in other threads.  I'll follow up
   those discussions with a proposal, and until then, removing those
   aspects from this test implementation seemed to be the best option.
   A caveat: this test currently adds the same tag (x86_64) and
   follows other assumptions made on "boot_linux_console.py", that is,
   that a x86_64 target binary will be used to run it.  If a user is
   in an environment that does not have a x86_64 target binary, it
   could filter those tests out with: "avocado run
   --filter-by-tags='-x86_64' tests/acceptance".

 * Removed most arguments to the QEMU command line for pretty much the
   same reasons described above, and by following the general
   perception that I could grasp from other discussions that QEMU
   defaults should preferrably be used.  This test, as well as others,
   can and should be extended later to allow for different test
   scenarios by passing well documented parameter values.  That is,
   they should respect well-known parameters such as "accel" mentioned
   above, so that the same test can run with KVM or TCG.

 * Changed the value of the memory argument to 1024, which based on
   my experimentations and observations is the minimum amount of RAM
   for the Fedora 29 cloud image to sucessfully boot on QEMU.  I know
   there's no such thing as a "one size fits all", specially for QEMU,
   but this makes me wonder wether a x86_64 machine type shouldn't
   have its default_ram_size bumped to a number practical enough to
   run modern operating systems.

 * Added a new patch "RFC: Acceptance tests: add the build directory
   to the system PATH", which is supposed to gather feedback on how to
   enable the use of built binaries, such as qemu-img, to code used by
   the test code.  The specific situation here is that the vmimage,
   part of the avocado.utils libraries, makes use of qemu-img to create
   snapshot files.  Even though we could require qemu-img to be installed
   as a dependency of tests, system wide, it actually goes against the
   goal of testing all QEMU things from the source/build tree.  This
   became aparent with tests running on environments such as Travis CI,
   which don't necessarily have qemu-img available elsewhere.

Cleber Rosa (2):
  RFC: Acceptance tests: add the build directory to the system PATH
  Add "boot_linux" acceptance test

 tests/acceptance/avocado_qemu/__init__.py |  9 +++++
 tests/acceptance/boot_linux.py            | 46 +++++++++++++++++++++++
 tests/requirements.txt                    |  1 +
 3 files changed, 56 insertions(+)
 create mode 100644 tests/acceptance/boot_linux.py

-- 
2.19.1

[Qemu-devel] [PATCH v2 1/2] RFC: Acceptance tests: add the build directory to the system PATH

[Cleber Rosa]

So that when binaries such as qemu-img are searched for, those in the
build tree will be favored.  As a clarification, SRC_ROOT_DIR is
dependent on the location from where tests are executed, so they are
equal to the build directory if one is being used.

The original motivation is that Avocado libraries such as
avocado.utils.vmimage.get() may use the matching binaries, but it may
also apply to any other binary that test code may eventually attempt
to execute.

Other competing alternatives would be a more explicit path or binary
registration mechanism, in which we tell the libraries such as
avocado.utils.vmimage, the binaries to use in advance.  I think the
model proposed here is simpler though, and is not inconsistent with
the general approach of favoring the built binaries, and falling back
to binaries available in the system.  I'd love to have comments on
that, though.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 1e54fd5932..3d5190cbab 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -49,6 +49,15 @@ class Test(avocado.Test):
             self.cancel("No QEMU binary defined or found in the source tree")
         self.vm = QEMUMachine(self.qemu_bin)
 
+        # RFC: avocado.utils.vmimage.get() uses qemu-img, from the
+        # system's PATH, to create a snapshot.  This is a transparent,
+        # but implicit way of making sure it finds the qemu-img that
+        # matches the code being tested (as tests it indirectly too).
+        # As for the cleanup, given that in the Avocado test execution
+        # model every test is started in a different process, no
+        # cleanup is needed.
+        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
+
     def tearDown(self):
         if self.vm is not None:
             self.vm.shutdown()
-- 
2.19.1

[Qemu-devel] [PATCH v2 2/2] Add "boot_linux" acceptance test

[Cleber Rosa]

This acceptance test, validates that a full blown Linux guest can
successfully boot in QEMU.  In this specific case, the guest chosen is
Fedora version 29.  By passing parameters, the same test can attempt
to boot different distros, arches, etc.

The method for checking the successful boot is based on "cloudinit"
and its "phone home" feature.  The guest is given an ISO image
with the location of the phone home server, and the information to
post (the instance ID).  Upon receiving the correct information,
from the guest, the test is considered to have PASSed.

This test is currently limited to user mode networking only, and
instructs the guest to connect to the "router" address that is hard
coded in QEMU.

To create the cloudinit ISO image that will be used to configure the
guest, the pycdlib library is also required and has been added as
requirement to the virtual environment created by "check-venv".

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/boot_linux.py | 46 ++++++++++++++++++++++++++++++++++
 tests/requirements.txt         |  1 +
 2 files changed, 47 insertions(+)
 create mode 100644 tests/acceptance/boot_linux.py

diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
new file mode 100644
index 0000000000..dceebcb964
--- /dev/null
+++ b/tests/acceptance/boot_linux.py
@@ -0,0 +1,46 @@
+# Functional test that boots a complete Linux system via a cloud image
+#
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# Author:
+#  Cleber Rosa <crosa@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import os
+
+from avocado_qemu import Test
+
+from avocado.utils import cloudinit
+from avocado.utils import network
+from avocado.utils import vmimage
+
+
+class BootLinux(Test):
+    """
+    Boots a Linux system, checking for a successful initialization
+
+    :avocado: enable
+    :avocado: tags=x86_64
+    """
+
+    timeout = 600
+
+    def test(self):
+        self.vm.add_args('-m', '1024')
+        boot = vmimage.get('fedora', arch='x86_64', version='29',
+                           cache_dir=self.cache_dirs[0],
+                           snapshot_dir=self.workdir)
+        self.vm.add_args('-drive', 'file=%s' % boot.path)
+
+        cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso')
+        phone_home_port = network.find_free_port()
+        cloudinit.iso(cloudinit_iso, self.name,
+                      # QEMU's hard coded usermode router address
+                      phone_home_host='10.0.2.2',
+                      phone_home_port=phone_home_port)
+        self.vm.add_args('-drive', 'file=%s' % cloudinit_iso)
+
+        self.vm.launch()
+        cloudinit.wait_for_phone_home(('0.0.0.0', phone_home_port), self.name)
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 64c6e27a94..fdc2d7d325 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,3 +2,4 @@
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
 avocado-framework==65.0
+pycdlib==1.6.0
-- 
2.19.1

Re: [Qemu-devel] [PATCH v2 1/2] RFC: Acceptance tests: add the build directory to the system PATH

[Wainer dos Santos Moschetta]

On 11/21/2018 07:48 PM, Cleber Rosa wrote:
> So that when binaries such as qemu-img are searched for, those in the
> build tree will be favored.  As a clarification, SRC_ROOT_DIR is
> dependent on the location from where tests are executed, so they are
> equal to the build directory if one is being used.

On avocado's job.log file I can see the full path of the qemu-img which 
was called, but I wonder if it wouldn't be better to log that 
information somewhere more explicitly. It wouldn't prevent this patch 
from being merged though, it's just an improvement suggestion.

>
> The original motivation is that Avocado libraries such as
> avocado.utils.vmimage.get() may use the matching binaries, but it may
> also apply to any other binary that test code may eventually attempt
> to execute.
>
> Other competing alternatives would be a more explicit path or binary
> registration mechanism, in which we tell the libraries such as
> avocado.utils.vmimage, the binaries to use in advance.  I think the
> model proposed here is simpler though, and is not inconsistent with
> the general approach of favoring the built binaries, and falling back
> to binaries available in the system.  I'd love to have comments on
> that, though.

IMHO it makes sense to pick the built binaries, falling back to system's 
otherwise. Keep it simple (and consistent) unless we eventually need 
something robust, I would go for that approach here.

>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 1e54fd5932..3d5190cbab 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -49,6 +49,15 @@ class Test(avocado.Test):
>               self.cancel("No QEMU binary defined or found in the source tree")
>           self.vm = QEMUMachine(self.qemu_bin)
>   
> +        # RFC: avocado.utils.vmimage.get() uses qemu-img, from the
> +        # system's PATH, to create a snapshot.  This is a transparent,
> +        # but implicit way of making sure it finds the qemu-img that
> +        # matches the code being tested (as tests it indirectly too).
> +        # As for the cleanup, given that in the Avocado test execution
> +        # model every test is started in a different process, no
> +        # cleanup is needed.
> +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
> +
>       def tearDown(self):
>           if self.vm is not None:
>               self.vm.shutdown()

The boot Linux test (added on patch 02) exits with error when I ran 
'make check-acceptance'. I am using Fedora 29 x86_64 which don't have 
qemu-img installed system-wide. See below:

------
# make check-acceptance
   AVOCADO tests/acceptance
make: *** [/root/qemu/tests/Makefile.include:940: check-acceptance] Error 9
# cat tests/results/latest/results.tap
1..8
not ok 1 /root/qemu/tests/acceptance/boot_linux.py:BootLinux.test
# grep -e 'ERROR.*CmdNotFoundError' tests/results/latest/job.log
2018-11-22 14:51:30,540 stacktrace       L0047 ERROR|     raise 
CmdNotFoundError(cmd, path_paths)
2018-11-22 14:51:30,540 stacktrace       L0047 ERROR| 
avocado.utils.path.CmdNotFoundError: Command 'qemu-img' could not be 
found in any of the PATH dirs: ['/usr/local/bin', '/bin', '/root/bin', 
'/sbin', '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin', 
'/usr/sbin']
2018-11-22 14:51:30,572 test             L0984 ERROR| 
avocado.utils.path.CmdNotFoundError: Command 'qemu-img' could not be 
found in any of the PATH dirs: ['/usr/local/bin', '/bin', '/root/bin', 
'/sbin', '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin', 
'/usr/sbin']
2018-11-22 14:51:30,572 test             L0999 ERROR| ERROR 
1-/root/qemu/tests/acceptance/boot_linux.py:BootLinux.test -> 
CmdNotFoundError: Command 'qemu-img' could not be found in any of the 
PATH dirs: ['/usr/local/bin', '/bin', '/root/bin', '/sbin', 
'/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin', '/usr/sbin']
------

The same test finished successfully when I ran with 'avocado run (...)' 
though.

- Wainer

Re: [Qemu-devel] [PATCH v2 1/2] RFC: Acceptance tests: add the build directory to the system PATH

[Cleber Rosa]

On 11/22/18 3:15 PM, Wainer dos Santos Moschetta wrote:
> 
> On 11/21/2018 07:48 PM, Cleber Rosa wrote:
>> So that when binaries such as qemu-img are searched for, those in the
>> build tree will be favored.  As a clarification, SRC_ROOT_DIR is
>> dependent on the location from where tests are executed, so they are
>> equal to the build directory if one is being used.
> 
> On avocado's job.log file I can see the full path of the qemu-img which
> was called, but I wonder if it wouldn't be better to log that
> information somewhere more explicitly. It wouldn't prevent this patch
> from being merged though, it's just an improvement suggestion.
> 
>>
>> The original motivation is that Avocado libraries such as
>> avocado.utils.vmimage.get() may use the matching binaries, but it may
>> also apply to any other binary that test code may eventually attempt
>> to execute.
>>
>> Other competing alternatives would be a more explicit path or binary
>> registration mechanism, in which we tell the libraries such as
>> avocado.utils.vmimage, the binaries to use in advance.  I think the
>> model proposed here is simpler though, and is not inconsistent with
>> the general approach of favoring the built binaries, and falling back
>> to binaries available in the system.  I'd love to have comments on
>> that, though.
> 
> IMHO it makes sense to pick the built binaries, falling back to system's
> otherwise. Keep it simple (and consistent) unless we eventually need
> something robust, I would go for that approach here.
> 

Agreed.

>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>   tests/acceptance/avocado_qemu/__init__.py | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py
>> b/tests/acceptance/avocado_qemu/__init__.py
>> index 1e54fd5932..3d5190cbab 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -49,6 +49,15 @@ class Test(avocado.Test):
>>               self.cancel("No QEMU binary defined or found in the
>> source tree")
>>           self.vm = QEMUMachine(self.qemu_bin)
>>   +        # RFC: avocado.utils.vmimage.get() uses qemu-img, from the
>> +        # system's PATH, to create a snapshot.  This is a transparent,
>> +        # but implicit way of making sure it finds the qemu-img that
>> +        # matches the code being tested (as tests it indirectly too).
>> +        # As for the cleanup, given that in the Avocado test execution
>> +        # model every test is started in a different process, no
>> +        # cleanup is needed.
>> +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR,
>> os.environ['PATH'])
>> +
>>       def tearDown(self):
>>           if self.vm is not None:
>>               self.vm.shutdown()
> 
> The boot Linux test (added on patch 02) exits with error when I ran
> 'make check-acceptance'. I am using Fedora 29 x86_64 which don't have
> qemu-img installed system-wide. See below:
> 
> ------
> # make check-acceptance
>   AVOCADO tests/acceptance
> make: *** [/root/qemu/tests/Makefile.include:940: check-acceptance] Error 9
> # cat tests/results/latest/results.tap
> 1..8
> not ok 1 /root/qemu/tests/acceptance/boot_linux.py:BootLinux.test
> # grep -e 'ERROR.*CmdNotFoundError' tests/results/latest/job.log
> 2018-11-22 14:51:30,540 stacktrace       L0047 ERROR|     raise
> CmdNotFoundError(cmd, path_paths)
> 2018-11-22 14:51:30,540 stacktrace       L0047 ERROR|
> avocado.utils.path.CmdNotFoundError: Command 'qemu-img' could not be
> found in any of the PATH dirs: ['/usr/local/bin', '/bin', '/root/bin',
> '/sbin', '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin',
> '/usr/sbin']
> 2018-11-22 14:51:30,572 test             L0984 ERROR|
> avocado.utils.path.CmdNotFoundError: Command 'qemu-img' could not be
> found in any of the PATH dirs: ['/usr/local/bin', '/bin', '/root/bin',
> '/sbin', '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin',
> '/usr/sbin']
> 2018-11-22 14:51:30,572 test             L0999 ERROR| ERROR
> 1-/root/qemu/tests/acceptance/boot_linux.py:BootLinux.test ->
> CmdNotFoundError: Command 'qemu-img' could not be found in any of the
> PATH dirs: ['/usr/local/bin', '/bin', '/root/bin', '/sbin',
> '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin', '/usr/sbin']
> ------
> 
> The same test finished successfully when I ran with 'avocado run (...)'
> though.
> 
> - Wainer
> 

This is interesting, because:

 1) I can't reproduce it
 2) I do see '/root/qemu' listed in the PATH, which I assume is the QEMU
build directory

The only thing that comes to my mind is that you may have not built QEMU
before running `make check-acceptance`, and perhaps, did so before
running "avocado run ..." ?

Anyway, I'll be sending a v3 shortly, and I'll try further to reproduce
it.  I'll, of course, appreciate if you try this again! :)

Thanks!
- Cleber.

Re: [Qemu-devel] [PATCH v2 1/2] RFC: Acceptance tests: add the build directory to the system PATH

[Philippe Mathieu-Daudé]

Hi Cleber,

On 2/6/19 8:30 PM, Cleber Rosa wrote:
> On 11/22/18 3:15 PM, Wainer dos Santos Moschetta wrote:
>>
>> On 11/21/2018 07:48 PM, Cleber Rosa wrote:
>>> So that when binaries such as qemu-img are searched for, those in the
>>> build tree will be favored.  As a clarification, SRC_ROOT_DIR is
>>> dependent on the location from where tests are executed, so they are
>>> equal to the build directory if one is being used.
>>
>> On avocado's job.log file I can see the full path of the qemu-img which
>> was called, but I wonder if it wouldn't be better to log that
>> information somewhere more explicitly. It wouldn't prevent this patch
>> from being merged though, it's just an improvement suggestion.
>>
>>>
>>> The original motivation is that Avocado libraries such as
>>> avocado.utils.vmimage.get() may use the matching binaries, but it may
>>> also apply to any other binary that test code may eventually attempt
>>> to execute.
>>>
>>> Other competing alternatives would be a more explicit path or binary
>>> registration mechanism, in which we tell the libraries such as
>>> avocado.utils.vmimage, the binaries to use in advance.  I think the
>>> model proposed here is simpler though, and is not inconsistent with
>>> the general approach of favoring the built binaries, and falling back
>>> to binaries available in the system.  I'd love to have comments on
>>> that, though.
>>
>> IMHO it makes sense to pick the built binaries, falling back to system's
>> otherwise. Keep it simple (and consistent) unless we eventually need
>> something robust, I would go for that approach here.
>>
> 
> Agreed.
> 
>>>
>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>> ---
>>>   tests/acceptance/avocado_qemu/__init__.py | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/tests/acceptance/avocado_qemu/__init__.py
>>> b/tests/acceptance/avocado_qemu/__init__.py
>>> index 1e54fd5932..3d5190cbab 100644
>>> --- a/tests/acceptance/avocado_qemu/__init__.py
>>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>>> @@ -49,6 +49,15 @@ class Test(avocado.Test):
>>>               self.cancel("No QEMU binary defined or found in the
>>> source tree")
>>>           self.vm = QEMUMachine(self.qemu_bin)
>>>   +        # RFC: avocado.utils.vmimage.get() uses qemu-img, from the
>>> +        # system's PATH, to create a snapshot.  This is a transparent,
>>> +        # but implicit way of making sure it finds the qemu-img that
>>> +        # matches the code being tested (as tests it indirectly too).
>>> +        # As for the cleanup, given that in the Avocado test execution
>>> +        # model every test is started in a different process, no
>>> +        # cleanup is needed.
>>> +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR,
>>> os.environ['PATH'])
>>> +
>>>       def tearDown(self):
>>>           if self.vm is not None:
>>>               self.vm.shutdown()
>>
>> The boot Linux test (added on patch 02) exits with error when I ran
>> 'make check-acceptance'. I am using Fedora 29 x86_64 which don't have
>> qemu-img installed system-wide. See below:
>>
>> ------
>> # make check-acceptance
>>   AVOCADO tests/acceptance
>> make: *** [/root/qemu/tests/Makefile.include:940: check-acceptance] Error 9
>> # cat tests/results/latest/results.tap
>> 1..8
>> not ok 1 /root/qemu/tests/acceptance/boot_linux.py:BootLinux.test
>> # grep -e 'ERROR.*CmdNotFoundError' tests/results/latest/job.log
>> 2018-11-22 14:51:30,540 stacktrace       L0047 ERROR|     raise
>> CmdNotFoundError(cmd, path_paths)
>> 2018-11-22 14:51:30,540 stacktrace       L0047 ERROR|
>> avocado.utils.path.CmdNotFoundError: Command 'qemu-img' could not be
>> found in any of the PATH dirs: ['/usr/local/bin', '/bin', '/root/bin',
>> '/sbin', '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin',
>> '/usr/sbin']
>> 2018-11-22 14:51:30,572 test             L0984 ERROR|
>> avocado.utils.path.CmdNotFoundError: Command 'qemu-img' could not be
>> found in any of the PATH dirs: ['/usr/local/bin', '/bin', '/root/bin',
>> '/sbin', '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin',
>> '/usr/sbin']
>> 2018-11-22 14:51:30,572 test             L0999 ERROR| ERROR
>> 1-/root/qemu/tests/acceptance/boot_linux.py:BootLinux.test ->
>> CmdNotFoundError: Command 'qemu-img' could not be found in any of the
>> PATH dirs: ['/usr/local/bin', '/bin', '/root/bin', '/sbin',
>> '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin', '/usr/sbin']
>> ------
>>
>> The same test finished successfully when I ran with 'avocado run (...)'
>> though.
>>
>> - Wainer
>>
> 
> This is interesting, because:
> 
>  1) I can't reproduce it
>  2) I do see '/root/qemu' listed in the PATH, which I assume is the QEMU
> build directory
> 
> The only thing that comes to my mind is that you may have not built QEMU
> before running `make check-acceptance`, and perhaps, did so before
> running "avocado run ..." ?

I can reproduce Wainer's failure. I didn't test your v3 but I expect
this problem to persist.

If an avocado test requires some external tools, we could check they are
available and SKIP the test. But then this reduce the tests covered...

Similarly to:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00951.html

This snippet helps:
-- >8 --
@@ -1117,3 +1117,3 @@ check-venv: $(TESTS_VENV_DIR)

-check-acceptance: check-venv $(TESTS_RESULTS_DIR)
+check-acceptance: check-venv $(TESTS_RESULTS_DIR) qemu-img$(EXESUF)
        $(call quiet-command, \
---

Build qemu-img once previous to run any test doesn't seem that bad.

What do you think? At least it is a cheap patch ;)

Regards,

Phil.

> Anyway, I'll be sending a v3 shortly, and I'll try further to reproduce
> it.  I'll, of course, appreciate if you try this again! :)
> 
> Thanks!
> - Cleber.
> 

Re: [Qemu-devel] [PATCH v2 1/2] RFC: Acceptance tests: add the build directory to the system PATH

[Cleber Rosa]

On 2/6/19 5:55 PM, Philippe Mathieu-Daudé wrote:
> Hi Cleber,
> 
> On 2/6/19 8:30 PM, Cleber Rosa wrote:
>> On 11/22/18 3:15 PM, Wainer dos Santos Moschetta wrote:
>>>
>>> On 11/21/2018 07:48 PM, Cleber Rosa wrote:
>>>> So that when binaries such as qemu-img are searched for, those in the
>>>> build tree will be favored.  As a clarification, SRC_ROOT_DIR is
>>>> dependent on the location from where tests are executed, so they are
>>>> equal to the build directory if one is being used.
>>>
>>> On avocado's job.log file I can see the full path of the qemu-img which
>>> was called, but I wonder if it wouldn't be better to log that
>>> information somewhere more explicitly. It wouldn't prevent this patch
>>> from being merged though, it's just an improvement suggestion.
>>>
>>>>
>>>> The original motivation is that Avocado libraries such as
>>>> avocado.utils.vmimage.get() may use the matching binaries, but it may
>>>> also apply to any other binary that test code may eventually attempt
>>>> to execute.
>>>>
>>>> Other competing alternatives would be a more explicit path or binary
>>>> registration mechanism, in which we tell the libraries such as
>>>> avocado.utils.vmimage, the binaries to use in advance.  I think the
>>>> model proposed here is simpler though, and is not inconsistent with
>>>> the general approach of favoring the built binaries, and falling back
>>>> to binaries available in the system.  I'd love to have comments on
>>>> that, though.
>>>
>>> IMHO it makes sense to pick the built binaries, falling back to system's
>>> otherwise. Keep it simple (and consistent) unless we eventually need
>>> something robust, I would go for that approach here.
>>>
>>
>> Agreed.
>>
>>>>
>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>> ---
>>>>   tests/acceptance/avocado_qemu/__init__.py | 9 +++++++++
>>>>   1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/tests/acceptance/avocado_qemu/__init__.py
>>>> b/tests/acceptance/avocado_qemu/__init__.py
>>>> index 1e54fd5932..3d5190cbab 100644
>>>> --- a/tests/acceptance/avocado_qemu/__init__.py
>>>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>>>> @@ -49,6 +49,15 @@ class Test(avocado.Test):
>>>>               self.cancel("No QEMU binary defined or found in the
>>>> source tree")
>>>>           self.vm = QEMUMachine(self.qemu_bin)
>>>>   +        # RFC: avocado.utils.vmimage.get() uses qemu-img, from the
>>>> +        # system's PATH, to create a snapshot.  This is a transparent,
>>>> +        # but implicit way of making sure it finds the qemu-img that
>>>> +        # matches the code being tested (as tests it indirectly too).
>>>> +        # As for the cleanup, given that in the Avocado test execution
>>>> +        # model every test is started in a different process, no
>>>> +        # cleanup is needed.
>>>> +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR,
>>>> os.environ['PATH'])
>>>> +
>>>>       def tearDown(self):
>>>>           if self.vm is not None:
>>>>               self.vm.shutdown()
>>>
>>> The boot Linux test (added on patch 02) exits with error when I ran
>>> 'make check-acceptance'. I am using Fedora 29 x86_64 which don't have
>>> qemu-img installed system-wide. See below:
>>>
>>> ------
>>> # make check-acceptance
>>>   AVOCADO tests/acceptance
>>> make: *** [/root/qemu/tests/Makefile.include:940: check-acceptance] Error 9
>>> # cat tests/results/latest/results.tap
>>> 1..8
>>> not ok 1 /root/qemu/tests/acceptance/boot_linux.py:BootLinux.test
>>> # grep -e 'ERROR.*CmdNotFoundError' tests/results/latest/job.log
>>> 2018-11-22 14:51:30,540 stacktrace       L0047 ERROR|     raise
>>> CmdNotFoundError(cmd, path_paths)
>>> 2018-11-22 14:51:30,540 stacktrace       L0047 ERROR|
>>> avocado.utils.path.CmdNotFoundError: Command 'qemu-img' could not be
>>> found in any of the PATH dirs: ['/usr/local/bin', '/bin', '/root/bin',
>>> '/sbin', '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin',
>>> '/usr/sbin']
>>> 2018-11-22 14:51:30,572 test             L0984 ERROR|
>>> avocado.utils.path.CmdNotFoundError: Command 'qemu-img' could not be
>>> found in any of the PATH dirs: ['/usr/local/bin', '/bin', '/root/bin',
>>> '/sbin', '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin',
>>> '/usr/sbin']
>>> 2018-11-22 14:51:30,572 test             L0999 ERROR| ERROR
>>> 1-/root/qemu/tests/acceptance/boot_linux.py:BootLinux.test ->
>>> CmdNotFoundError: Command 'qemu-img' could not be found in any of the
>>> PATH dirs: ['/usr/local/bin', '/bin', '/root/bin', '/sbin',
>>> '/usr/libexec', '/usr/local/sbin', '/root/qemu', '/usr/bin', '/usr/sbin']
>>> ------
>>>
>>> The same test finished successfully when I ran with 'avocado run (...)'
>>> though.
>>>
>>> - Wainer
>>>
>>
>> This is interesting, because:
>>
>>  1) I can't reproduce it
>>  2) I do see '/root/qemu' listed in the PATH, which I assume is the QEMU
>> build directory
>>
>> The only thing that comes to my mind is that you may have not built QEMU
>> before running `make check-acceptance`, and perhaps, did so before
>> running "avocado run ..." ?
> 
> I can reproduce Wainer's failure. I didn't test your v3 but I expect
> this problem to persist.
> 
> If an avocado test requires some external tools, we could check they are
> available and SKIP the test. But then this reduce the tests covered...
> 
> Similarly to:
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00951.html
> 
> This snippet helps:
> -- >8 --
> @@ -1117,3 +1117,3 @@ check-venv: $(TESTS_VENV_DIR)
> 
> -check-acceptance: check-venv $(TESTS_RESULTS_DIR)
> +check-acceptance: check-venv $(TESTS_RESULTS_DIR) qemu-img$(EXESUF)
>         $(call quiet-command, \
> ---
> 
> Build qemu-img once previous to run any test doesn't seem that bad.
> 

Oh, now I see your point.  I had the understanding that one should
guarantee that a build had been completed before attempting to run `make
check-acceptance`.  So, I think your patch makes sense at this point.
My hope is that we'll soon be able to leverage the KConfig work and
automatically select tests based on the build environment (when in one).

Now, can you reproduce Wainer's experience when it comes to different
behavior when running "make check-acceptance" versus "avocado run ..."?
 This is what I was most puzzled about.

PS: I've tested your suggestion with `--disable-tools`, but the Makefile
doesn't care about it and builds qemu-img nonetheless.

> What do you think? At least it is a cheap patch ;)
> 
> Regards,
> 
> Phil.
> 

Thanks a lot for the feedback and suggestions!
- Cleber.

>> Anyway, I'll be sending a v3 shortly, and I'll try further to reproduce
>> it.  I'll, of course, appreciate if you try this again! :)
>>
>> Thanks!
>> - Cleber.
>>