[Qemu-devel] [PATCH 0/8] Miscellaneous acceptance test and Travis CI improvements

[Qemu-devel] [PATCH 0/8] Miscellaneous acceptance test and Travis CI improvements

[Cleber Rosa]

This is a collection of small improvements to some of the acceptance
tests, and the Travis CI experience.

The main goal was to make tests a bit more robust when run in parallel
(an Avocado feature pending review), and Travis CI diagnostics better
by printing out the full Avocado job log when any error or test
failure occurs.

Cleber Rosa (8):
  Travis: print acceptance tests logs in case of job failure
  tests/requirements.txt: pin paramiko version requirement
  Acceptance tests: drop left over usage of ":avocado: enable"
  Boot Linux Console Test: add a test for ppc64 + pseries
  VNC Acceptance test: use UNIX domain sockets to avoid port collisions
  VNC Acceptance test: simplify test names
  VNC Acceptance test: check protocol version
  Migration acceptance test: reduce the possibility of port collisions

 .travis.yml                            |  4 ++-
 tests/acceptance/boot_linux_console.py | 19 ++++++++++++++
 tests/acceptance/migration.py          |  5 +---
 tests/acceptance/vnc.py                | 36 +++++++++++++++++++++++---
 tests/requirements.txt                 |  2 +-
 5 files changed, 56 insertions(+), 10 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH 1/8] Travis: print acceptance tests logs in case of job failure

[Cleber Rosa]

Because Travis doesn't allow us to keep files produced during tests
(such as log files), let's print the complete job log to the "console"
in case of job failures.

This is a debugging aid, and given that there's been some timeouts
happening on some tests, we absolutely needs the logs to have a proper
action.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 .travis.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index b053a836a3..9f8e73f276 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -226,6 +226,8 @@ matrix:
     - env:
         - CONFIG="--python=/usr/bin/python3 --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu"
         - TEST_CMD="make check-acceptance"
+      after_failure:
+        - cat tests/results/latest/job.log
       addons:
         apt:
           packages:
-- 
2.21.0

[Qemu-devel] [PATCH 2/8] tests/requirements.txt: pin paramiko version requirement

[Cleber Rosa]

It's a good practice (I'd really say a must) to pin as much as
possible of the software versions used during test, so let's apply
that to paramiko.

According to https://pypi.org/project/paramiko/, 2.4.2 is the latest
released version.  It's also easily obtainable on systems such as
Fedora 30.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/requirements.txt b/tests/requirements.txt
index 3ae0e29ad7..bd1f7590ed 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,4 +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==68.0
-paramiko
+paramiko==2.4.2
-- 
2.21.0

[Qemu-devel] [PATCH 3/8] Acceptance tests: drop left over usage of ":avocado: enable"

[Cleber Rosa]

Commit 9531d26c10 removed all of ":avocado: enable" tags, but then
a new entry was added with the introduction of migration.py.

Let's remove it for consistency.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/migration.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index 6115cf6c24..a44c1ae58f 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -17,9 +17,6 @@ from avocado.utils import wait
 
 
 class Migration(Test):
-    """
-    :avocado: enable
-    """
 
     timeout = 10
 
-- 
2.21.0

[Qemu-devel] [PATCH 4/8] Boot Linux Console Test: add a test for ppc64 + pseries

[Cleber Rosa]

Just like the previous tests, boots a Linux kernel on a ppc64 target
using the pseries machine.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Caio Carrara <ccarrara@redhat.com>
CC: Daniel P. Berrangé <berrange@redhat.com>
---
 .travis.yml                            |  2 +-
 tests/acceptance/boot_linux_console.py | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 9f8e73f276..5592e458ab 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -224,7 +224,7 @@ matrix:
 
     # Acceptance (Functional) tests
     - env:
-        - CONFIG="--python=/usr/bin/python3 --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu"
+        - CONFIG="--python=/usr/bin/python3 --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu,ppc64-softmmu"
         - TEST_CMD="make check-acceptance"
       after_failure:
         - cat tests/results/latest/job.log
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index d5c500ea30..a196636367 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -217,3 +217,22 @@ class BootLinuxConsole(Test):
         self.vm.launch()
         console_pattern = 'Kernel command line: %s' % kernel_command_line
         self.wait_for_console_pattern(console_pattern)
+
+    def test_ppc64_pseries(self):
+        """
+        :avocado: tags=arch:ppc64
+        :avocado: tags=machine:pseries
+        """
+        kernel_url = ('https://download.fedoraproject.org/pub/fedora-secondary/'
+                      'releases/29/Everything/ppc64le/os/ppc/ppc64/vmlinuz')
+        kernel_hash = '3fe04abfc852b66653b8c3c897a59a689270bc77'
+        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+
+        self.vm.set_machine('pseries')
+        self.vm.set_console()
+        kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=hvc0'
+        self.vm.add_args('-kernel', kernel_path,
+                         '-append', kernel_command_line)
+        self.vm.launch()
+        console_pattern = 'Kernel command line: %s' % kernel_command_line
+        self.wait_for_console_pattern(console_pattern)
-- 
2.21.0

[Qemu-devel] [PATCH 5/8] VNC Acceptance test: use UNIX domain sockets to avoid port collisions

[Cleber Rosa]

While running in parallel, the VNC tests that use a TCP port easily
collide.  There's a number of possibilities to reduce the probability
of collisions, but none that completely prevents it from happening.

So, to avoid those collisions, and given that the scope of the tests
are really not related to nature of the socket type, let's switch to
UNIX domain sockets created in temporary directories.

Note: the amount of boiler plate code is far from the ideal, but it's
related to the fact that a test "workdir"[1] attribute can not be used
here, because of the 108 bytes limitation of the UNIX socket path (see
ad9579aaa16). There's a fair assumption here that the temporary
directory returned by Python's tempfile.mkdtemp() won't be anywhere
close to 100 bytes.

[1] https://avocado-framework.readthedocs.io/en/68.0/api/test/avocado.html#avocado.Test.workdir

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/vnc.py | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
index 064ceabcc1..675fd507ed 100644
--- a/tests/acceptance/vnc.py
+++ b/tests/acceptance/vnc.py
@@ -8,6 +8,10 @@
 # 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
+import tempfile
+import shutil
+
 from avocado_qemu import Test
 
 
@@ -34,8 +38,16 @@ class Vnc(Test):
         self.assertEqual(set_password_response['error']['desc'],
                          'Could not set password')
 
+class VncUnixSocket(Test):
+
+    def setUp(self):
+        super(VncUnixSocket, self).setUp()
+        self.socket_dir = tempfile.mkdtemp()
+        self.socket_path = os.path.join(self.socket_dir, 'vnc-socket')
+
     def test_vnc_change_password_requires_a_password(self):
-        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0')
+        self.vm.add_args('-nodefaults', '-S',
+                         '-vnc', 'unix:%s' % self.socket_path)
         self.vm.launch()
         self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
         set_password_response = self.vm.qmp('change',
@@ -49,7 +61,8 @@ class Vnc(Test):
                          'Could not set password')
 
     def test_vnc_change_password(self):
-        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0,password')
+        self.vm.add_args('-nodefaults', '-S',
+                         '-vnc', 'unix:%s,password' % self.socket_path)
         self.vm.launch()
         self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
         set_password_response = self.vm.qmp('change',
@@ -57,3 +70,6 @@ class Vnc(Test):
                                             target='password',
                                             arg='new_password')
         self.assertEqual(set_password_response['return'], {})
+
+    def tearDown(self):
+        shutil.rmtree(self.socket_dir)
-- 
2.21.0

[Qemu-devel] [PATCH 6/8] VNC Acceptance test: simplify test names

[Cleber Rosa]

The test name is composed of the class name and method name, so it
looks like there's some redundancy here that we can eliminate.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/vnc.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
index 675fd507ed..d32ae46685 100644
--- a/tests/acceptance/vnc.py
+++ b/tests/acceptance/vnc.py
@@ -45,7 +45,7 @@ class VncUnixSocket(Test):
         self.socket_dir = tempfile.mkdtemp()
         self.socket_path = os.path.join(self.socket_dir, 'vnc-socket')
 
-    def test_vnc_change_password_requires_a_password(self):
+    def test_change_password_requires_a_password(self):
         self.vm.add_args('-nodefaults', '-S',
                          '-vnc', 'unix:%s' % self.socket_path)
         self.vm.launch()
@@ -60,7 +60,7 @@ class VncUnixSocket(Test):
         self.assertEqual(set_password_response['error']['desc'],
                          'Could not set password')
 
-    def test_vnc_change_password(self):
+    def test_change_password(self):
         self.vm.add_args('-nodefaults', '-S',
                          '-vnc', 'unix:%s,password' % self.socket_path)
         self.vm.launch()
-- 
2.21.0

[Qemu-devel] [PATCH 7/8] VNC Acceptance test: check protocol version

[Cleber Rosa]

This goes a bit further than the other tests, and does a basic (read
only) interaction with the VNC protocol.

This is not a enough to perform a handshake, but enough to make sure
that the socket is somewhat operational and that the expected initial
step of the handshake is performed by the server and that the version
matches.

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

diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
index d32ae46685..b000446d7c 100644
--- a/tests/acceptance/vnc.py
+++ b/tests/acceptance/vnc.py
@@ -11,6 +11,7 @@
 import os
 import tempfile
 import shutil
+import socket
 
 from avocado_qemu import Test
 
@@ -71,5 +72,16 @@ class VncUnixSocket(Test):
                                             arg='new_password')
         self.assertEqual(set_password_response['return'], {})
 
+    def test_protocol_version(self):
+        self.vm.add_args('-nodefaults', '-S',
+                         '-vnc', 'unix:%s' % self.socket_path)
+        self.vm.launch()
+        self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
+        client = socket.socket(socket.AF_UNIX)
+        client.connect(self.socket_path)
+        expected = b'RFB 003.008'
+        actual = client.recv(len(expected))
+        self.assertEqual(expected, actual, "Wrong protocol version")
+
     def tearDown(self):
         shutil.rmtree(self.socket_dir)
-- 
2.21.0

[Qemu-devel] [PATCH 8/8] Migration acceptance test: reduce the possibility of port collisions

[Cleber Rosa]

The avocado.utils.network.find_free_port() will by default attempt
to return the first port given in the range, if it seems to be
available.

Let's reduce the probability of collisions by picking a random port
by default.

As a side note, this behavior has changed in a recent Avocado version,
and the default use will then be equivalent to this proposed change.

Reference: https://avocado-framework.readthedocs.io/en/68.0/api/utils/avocado.utils.html#avocado.utils.network.find_free_port
Reference: https://github.com/avocado-framework/avocado/commit/f232e4505f7cfefc513449e0b97790b517275da7
Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/migration.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index a44c1ae58f..b2a5767348 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -25,7 +25,7 @@ class Migration(Test):
         return vm.command('query-migrate')['status'] in ('completed', 'failed')
 
     def _get_free_port(self):
-        port = network.find_free_port()
+        port = network.find_free_port(sequent=False)
         if port is None:
             self.cancel('Failed to find a free port')
         return port
-- 
2.21.0

Re: [Qemu-devel] [PATCH 0/8] Miscellaneous acceptance test and Travis CI improvements

[Cleber Rosa]

On Fri, Jun 07, 2019 at 11:22:15AM -0400, Cleber Rosa wrote:
> This is a collection of small improvements to some of the acceptance
> tests, and the Travis CI experience.
> 
> The main goal was to make tests a bit more robust when run in parallel
> (an Avocado feature pending review), and Travis CI diagnostics better
> by printing out the full Avocado job log when any error or test
> failure occurs.

Forgot to add these hopefully useful pointers:

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

Travis CI Info:
  - Build: https://travis-ci.org/clebergnu/qemu/builds/542815049
  - Job: https://travis-ci.org/clebergnu/qemu/jobs/542815074

Regards,
- Cleber.

> 
> Cleber Rosa (8):
>   Travis: print acceptance tests logs in case of job failure
>   tests/requirements.txt: pin paramiko version requirement
>   Acceptance tests: drop left over usage of ":avocado: enable"
>   Boot Linux Console Test: add a test for ppc64 + pseries
>   VNC Acceptance test: use UNIX domain sockets to avoid port collisions
>   VNC Acceptance test: simplify test names
>   VNC Acceptance test: check protocol version
>   Migration acceptance test: reduce the possibility of port collisions
> 
>  .travis.yml                            |  4 ++-
>  tests/acceptance/boot_linux_console.py | 19 ++++++++++++++
>  tests/acceptance/migration.py          |  5 +---
>  tests/acceptance/vnc.py                | 36 +++++++++++++++++++++++---
>  tests/requirements.txt                 |  2 +-
>  5 files changed, 56 insertions(+), 10 deletions(-)
> 
> -- 
> 2.21.0
> 

Re: [Qemu-devel] [PATCH 7/8] VNC Acceptance test: check protocol version

[Daniel P. Berrangé]

On Fri, Jun 07, 2019 at 11:22:22AM -0400, Cleber Rosa wrote:
> This goes a bit further than the other tests, and does a basic (read
> only) interaction with the VNC protocol.
> 
> This is not a enough to perform a handshake, but enough to make sure
> that the socket is somewhat operational and that the expected initial
> step of the handshake is performed by the server and that the version
> matches.

The GTK-VNC project provides a low level library libgvnc that can
be used to talk the RFB protocol in a fairly fine grained manner.
This is built using GObject, so is accessible from Python thanks
to GObject Introspection.

We could use libgvnc for exercising the VNC server instead of writing
custom RFB code. For your simple test just sending/receiving the
version it won't save much, but if we ever want to test TLS or
SASL integration, it would save alot of work dealing wth the auth
handshake and subsequent encryption needs.

The main limitation it would have is that it would only work well
for sending "well formed" RFB protocol messages. If we ever want to
send intentionally evil/bad RFB data to check QEMU's VNC server
security hardening it would be harder.

As the maintainer of GTK-VNC though, I would be open to adding more
APIs to the low level gvnc library to facilitate QEMU's testing
needs if we want.

> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/vnc.py | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
> index d32ae46685..b000446d7c 100644
> --- a/tests/acceptance/vnc.py
> +++ b/tests/acceptance/vnc.py
> @@ -11,6 +11,7 @@
>  import os
>  import tempfile
>  import shutil
> +import socket
>  
>  from avocado_qemu import Test
>  
> @@ -71,5 +72,16 @@ class VncUnixSocket(Test):
>                                              arg='new_password')
>          self.assertEqual(set_password_response['return'], {})
>  
> +    def test_protocol_version(self):
> +        self.vm.add_args('-nodefaults', '-S',
> +                         '-vnc', 'unix:%s' % self.socket_path)
> +        self.vm.launch()
> +        self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
> +        client = socket.socket(socket.AF_UNIX)
> +        client.connect(self.socket_path)
> +        expected = b'RFB 003.008'
> +        actual = client.recv(len(expected))
> +        self.assertEqual(expected, actual, "Wrong protocol version")
> +
>      def tearDown(self):
>          shutil.rmtree(self.socket_dir)
> -- 
> 2.21.0
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 7/8] VNC Acceptance test: check protocol version

[Cleber Rosa]

On Fri, Jun 07, 2019 at 06:29:15PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jun 07, 2019 at 11:22:22AM -0400, Cleber Rosa wrote:
> > This goes a bit further than the other tests, and does a basic (read
> > only) interaction with the VNC protocol.
> > 
> > This is not a enough to perform a handshake, but enough to make sure
> > that the socket is somewhat operational and that the expected initial
> > step of the handshake is performed by the server and that the version
> > matches.
> 
> The GTK-VNC project provides a low level library libgvnc that can
> be used to talk the RFB protocol in a fairly fine grained manner.
> This is built using GObject, so is accessible from Python thanks
> to GObject Introspection.
>

Interesting.

> We could use libgvnc for exercising the VNC server instead of writing
> custom RFB code. For your simple test just sending/receiving the
> version it won't save much, but if we ever want to test TLS or
> SASL integration, it would save alot of work dealing wth the auth
> handshake and subsequent encryption needs.
>

Absolutely.

> The main limitation it would have is that it would only work well
> for sending "well formed" RFB protocol messages. If we ever want to
> send intentionally evil/bad RFB data to check QEMU's VNC server
> security hardening it would be harder.
>

Right.  Still, there's a lot that can be done until we eventually
exaust all possibilities and look into sending bad messages.

> As the maintainer of GTK-VNC though, I would be open to adding more
> APIs to the low level gvnc library to facilitate QEMU's testing
> needs if we want.
>

I personally need to get acquainted with the currently available APIs
first, but it looks like you alread have ideas for extensions that
would come in handy.

Also, the one concern I have is how to deploy the library and Python
bindings so that we can host those more advanced tests and still allow
for a "make check-acceptance"-like experience.  What I mean is, I
expect the Python bindings to be easily installed by pip, by I'd be
(positively) surprised if the libgvnc would also have such an easy
bootstrap.

Any ideas on this?

Thanks!
- Cleber.

> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  tests/acceptance/vnc.py | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
> > index d32ae46685..b000446d7c 100644
> > --- a/tests/acceptance/vnc.py
> > +++ b/tests/acceptance/vnc.py
> > @@ -11,6 +11,7 @@
> >  import os
> >  import tempfile
> >  import shutil
> > +import socket
> >  
> >  from avocado_qemu import Test
> >  
> > @@ -71,5 +72,16 @@ class VncUnixSocket(Test):
> >                                              arg='new_password')
> >          self.assertEqual(set_password_response['return'], {})
> >  
> > +    def test_protocol_version(self):
> > +        self.vm.add_args('-nodefaults', '-S',
> > +                         '-vnc', 'unix:%s' % self.socket_path)
> > +        self.vm.launch()
> > +        self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
> > +        client = socket.socket(socket.AF_UNIX)
> > +        client.connect(self.socket_path)
> > +        expected = b'RFB 003.008'
> > +        actual = client.recv(len(expected))
> > +        self.assertEqual(expected, actual, "Wrong protocol version")
> > +
> >      def tearDown(self):
> >          shutil.rmtree(self.socket_dir)
> > -- 
> > 2.21.0
> > 
> > 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 7/8] VNC Acceptance test: check protocol version

[Daniel P. Berrangé]

On Fri, Jun 07, 2019 at 02:12:07PM -0400, Cleber Rosa wrote:
> On Fri, Jun 07, 2019 at 06:29:15PM +0100, Daniel P. Berrangé wrote:
> > On Fri, Jun 07, 2019 at 11:22:22AM -0400, Cleber Rosa wrote:
> > > This goes a bit further than the other tests, and does a basic (read
> > > only) interaction with the VNC protocol.
> > > 
> > > This is not a enough to perform a handshake, but enough to make sure
> > > that the socket is somewhat operational and that the expected initial
> > > step of the handshake is performed by the server and that the version
> > > matches.
> > 
> > The GTK-VNC project provides a low level library libgvnc that can
> > be used to talk the RFB protocol in a fairly fine grained manner.
> > This is built using GObject, so is accessible from Python thanks
> > to GObject Introspection.
> >
> 
> Interesting.
> 
> > We could use libgvnc for exercising the VNC server instead of writing
> > custom RFB code. For your simple test just sending/receiving the
> > version it won't save much, but if we ever want to test TLS or
> > SASL integration, it would save alot of work dealing wth the auth
> > handshake and subsequent encryption needs.
> >
> 
> Absolutely.
> 
> > The main limitation it would have is that it would only work well
> > for sending "well formed" RFB protocol messages. If we ever want to
> > send intentionally evil/bad RFB data to check QEMU's VNC server
> > security hardening it would be harder.
> >
> 
> Right.  Still, there's a lot that can be done until we eventually
> exaust all possibilities and look into sending bad messages.
> 
> > As the maintainer of GTK-VNC though, I would be open to adding more
> > APIs to the low level gvnc library to facilitate QEMU's testing
> > needs if we want.
> >
> 
> I personally need to get acquainted with the currently available APIs
> first, but it looks like you alread have ideas for extensions that
> would come in handy.
> 
> Also, the one concern I have is how to deploy the library and Python
> bindings so that we can host those more advanced tests and still allow
> for a "make check-acceptance"-like experience.  What I mean is, I
> expect the Python bindings to be easily installed by pip, by I'd be
> (positively) surprised if the libgvnc would also have such an easy
> bootstrap.
> 
> Any ideas on this?

IMHO we shouldn't try to install anything from pip. It should all be
available from distro repos already. In fact if you have "virt-install"
or "virt-manager" installed, you should already have the required
python bits.

On RHEL/Fedora you need python3-gobject to get the GObject introspection
support and then all you need is the native gvnc library. There is no
python library for gvnc because GObject introspection makes this
redundant - the python API is dynamically created at runtime using
FFI to access the native library APIs.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/8] Travis: print acceptance tests logs in case of job failure

[Wainer dos Santos Moschetta]

On 06/07/2019 12:22 PM, Cleber Rosa wrote:
> Because Travis doesn't allow us to keep files produced during tests
> (such as log files), let's print the complete job log to the "console"
> in case of job failures.
>
> This is a debugging aid, and given that there's been some timeouts
> happening on some tests, we absolutely needs the logs to have a proper
> action.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   .travis.yml | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/.travis.yml b/.travis.yml
> index b053a836a3..9f8e73f276 100644
> --- a/.travis.yml
> +++ b/.travis.yml

It's handy. Unfortunately you cannot archive the log files in Travis, 
otherwise that would be a nice option.

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

> @@ -226,6 +226,8 @@ matrix:
>       - env:
>           - CONFIG="--python=/usr/bin/python3 --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu"
>           - TEST_CMD="make check-acceptance"
> +      after_failure:
> +        - cat tests/results/latest/job.log
>         addons:
>           apt:
>             packages:

Re: [Qemu-devel] [PATCH 2/8] tests/requirements.txt: pin paramiko version requirement

[Wainer dos Santos Moschetta]

On 06/07/2019 12:22 PM, Cleber Rosa wrote:
> It's a good practice (I'd really say a must) to pin as much as
> possible of the software versions used during test, so let's apply
> that to paramiko.
>
> According to https://pypi.org/project/paramiko/, 2.4.2 is the latest
> released version.  It's also easily obtainable on systems such as
> Fedora 30.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/requirements.txt | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
> diff --git a/tests/requirements.txt b/tests/requirements.txt
> index 3ae0e29ad7..bd1f7590ed 100644
> --- a/tests/requirements.txt
> +++ b/tests/requirements.txt
> @@ -2,4 +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==68.0
> -paramiko
> +paramiko==2.4.2

Re: [Qemu-devel] [PATCH 3/8] Acceptance tests: drop left over usage of ":avocado: enable"

[Wainer dos Santos Moschetta]

On 06/07/2019 12:22 PM, Cleber Rosa wrote:
> Commit 9531d26c10 removed all of ":avocado: enable" tags, but then
> a new entry was added with the introduction of migration.py.
>
> Let's remove it for consistency.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/migration.py | 3 ---
>   1 file changed, 3 deletions(-)

I was about to send a patch to remove it as well. :)

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 6115cf6c24..a44c1ae58f 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -17,9 +17,6 @@ from avocado.utils import wait
>   
>   
>   class Migration(Test):
> -    """
> -    :avocado: enable
> -    """
>   
>       timeout = 10
>   

Re: [Qemu-devel] [PATCH 4/8] Boot Linux Console Test: add a test for ppc64 + pseries

[Wainer dos Santos Moschetta]

Hi Cleber,

On 06/07/2019 12:22 PM, Cleber Rosa wrote:
> Just like the previous tests, boots a Linux kernel on a ppc64 target
> using the pseries machine.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> Reviewed-by: Caio Carrara <ccarrara@redhat.com>
> CC: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   .travis.yml                            |  2 +-
>   tests/acceptance/boot_linux_console.py | 19 +++++++++++++++++++
>   2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 9f8e73f276..5592e458ab 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -224,7 +224,7 @@ matrix:
>   
>       # Acceptance (Functional) tests
>       - env:
> -        - CONFIG="--python=/usr/bin/python3 --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu"
> +        - CONFIG="--python=/usr/bin/python3 --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu,ppc64-softmmu"
>           - TEST_CMD="make check-acceptance"
>         after_failure:
>           - cat tests/results/latest/job.log
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index d5c500ea30..a196636367 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -217,3 +217,22 @@ class BootLinuxConsole(Test):
>           self.vm.launch()
>           console_pattern = 'Kernel command line: %s' % kernel_command_line
>           self.wait_for_console_pattern(console_pattern)
> +
> +    def test_ppc64_pseries(self):
> +        """
> +        :avocado: tags=arch:ppc64
> +        :avocado: tags=machine:pseries
> +        """
> +        kernel_url = ('https://download.fedoraproject.org/pub/fedora-secondary/'
> +                      'releases/29/Everything/ppc64le/os/ppc/ppc64/vmlinuz')
> +        kernel_hash = '3fe04abfc852b66653b8c3c897a59a689270bc77'
> +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> +
> +        self.vm.set_machine('pseries')
> +        self.vm.set_console()
> +        kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=hvc0'
> +        self.vm.add_args('-kernel', kernel_path,
> +                         '-append', kernel_command_line)
> +        self.vm.launch()
> +        console_pattern = 'Kernel command line: %s' % kernel_command_line
> +        self.wait_for_console_pattern(console_pattern)

I'm refactoring the acceptance tests which boot a Linux Kernel so that 
they can share common functions. My latest implementation [1] was 
re-based on this patch, and it would help manage my contribution if we 
could merge this very soon.

Anyway, tested this patch on my Fedora 30 x86_64 machine. Passed.

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

[1]  https://github.com/wainersm/qemu/tree/acceptance_boot_linux

Re: [Qemu-devel] [PATCH 5/8] VNC Acceptance test: use UNIX domain sockets to avoid port collisions

[Wainer dos Santos Moschetta]

On 06/07/2019 12:22 PM, Cleber Rosa wrote:
> While running in parallel, the VNC tests that use a TCP port easily
> collide.  There's a number of possibilities to reduce the probability
> of collisions, but none that completely prevents it from happening.
>
> So, to avoid those collisions, and given that the scope of the tests
> are really not related to nature of the socket type, let's switch to
> UNIX domain sockets created in temporary directories.
>
> Note: the amount of boiler plate code is far from the ideal, but it's
> related to the fact that a test "workdir"[1] attribute can not be used
> here, because of the 108 bytes limitation of the UNIX socket path (see
> ad9579aaa16). There's a fair assumption here that the temporary
> directory returned by Python's tempfile.mkdtemp() won't be anywhere
> close to 100 bytes.
>
> [1] https://avocado-framework.readthedocs.io/en/68.0/api/test/avocado.html#avocado.Test.workdir
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/vnc.py | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
> index 064ceabcc1..675fd507ed 100644
> --- a/tests/acceptance/vnc.py
> +++ b/tests/acceptance/vnc.py
> @@ -8,6 +8,10 @@
>   # 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
> +import tempfile
> +import shutil
> +
>   from avocado_qemu import Test
>   
>   
> @@ -34,8 +38,16 @@ class Vnc(Test):
>           self.assertEqual(set_password_response['error']['desc'],
>                            'Could not set password')
>   
> +class VncUnixSocket(Test):
> +
> +    def setUp(self):
> +        super(VncUnixSocket, self).setUp()
> +        self.socket_dir = tempfile.mkdtemp()
> +        self.socket_path = os.path.join(self.socket_dir, 'vnc-socket')
> +
>       def test_vnc_change_password_requires_a_password(self):
> -        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0')
> +        self.vm.add_args('-nodefaults', '-S',
> +                         '-vnc', 'unix:%s' % self.socket_path)
>           self.vm.launch()
>           self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
>           set_password_response = self.vm.qmp('change',
> @@ -49,7 +61,8 @@ class Vnc(Test):
>                            'Could not set password')
>   
>       def test_vnc_change_password(self):
> -        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0,password')
> +        self.vm.add_args('-nodefaults', '-S',
> +                         '-vnc', 'unix:%s,password' % self.socket_path)
>           self.vm.launch()
>           self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
>           set_password_response = self.vm.qmp('change',
> @@ -57,3 +70,6 @@ class Vnc(Test):
>                                               target='password',
>                                               arg='new_password')
>           self.assertEqual(set_password_response['return'], {})
> +
> +    def tearDown(self):
> +        shutil.rmtree(self.socket_dir)

You missed to call super's tearDown in order to gently shutdown all VM 
created in by the tests. Other than that, it looks good to me.

- Wainer

Re: [Qemu-devel] [PATCH 6/8] VNC Acceptance test: simplify test names

[Wainer dos Santos Moschetta]

On 06/07/2019 12:22 PM, Cleber Rosa wrote:
> The test name is composed of the class name and method name, so it
> looks like there's some redundancy here that we can eliminate.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/vnc.py | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
> diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
> index 675fd507ed..d32ae46685 100644
> --- a/tests/acceptance/vnc.py
> +++ b/tests/acceptance/vnc.py
> @@ -45,7 +45,7 @@ class VncUnixSocket(Test):
>           self.socket_dir = tempfile.mkdtemp()
>           self.socket_path = os.path.join(self.socket_dir, 'vnc-socket')
>   
> -    def test_vnc_change_password_requires_a_password(self):
> +    def test_change_password_requires_a_password(self):
>           self.vm.add_args('-nodefaults', '-S',
>                            '-vnc', 'unix:%s' % self.socket_path)
>           self.vm.launch()
> @@ -60,7 +60,7 @@ class VncUnixSocket(Test):
>           self.assertEqual(set_password_response['error']['desc'],
>                            'Could not set password')
>   
> -    def test_vnc_change_password(self):
> +    def test_change_password(self):
>           self.vm.add_args('-nodefaults', '-S',
>                            '-vnc', 'unix:%s,password' % self.socket_path)
>           self.vm.launch()

Re: [Qemu-devel] [PATCH 7/8] VNC Acceptance test: check protocol version

[Wainer dos Santos Moschetta]

On 06/07/2019 12:22 PM, Cleber Rosa wrote:
> This goes a bit further than the other tests, and does a basic (read
> only) interaction with the VNC protocol.
>
> This is not a enough to perform a handshake, but enough to make sure
> that the socket is somewhat operational and that the expected initial
> step of the handshake is performed by the server and that the version
> matches.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/vnc.py | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
> index d32ae46685..b000446d7c 100644
> --- a/tests/acceptance/vnc.py
> +++ b/tests/acceptance/vnc.py
> @@ -11,6 +11,7 @@
>   import os
>   import tempfile
>   import shutil
> +import socket
>   
>   from avocado_qemu import Test
>   
> @@ -71,5 +72,16 @@ class VncUnixSocket(Test):
>                                               arg='new_password')
>           self.assertEqual(set_password_response['return'], {})
>   
> +    def test_protocol_version(self):
> +        self.vm.add_args('-nodefaults', '-S',
> +                         '-vnc', 'unix:%s' % self.socket_path)
> +        self.vm.launch()
> +        self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])

I will advocate for the use QEMUMachine.command() instead of qmp(). The 
former do some checks on the qmp response and raises a more gently 
exception if something went wrong. This patch looks good to me though.

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

> +        client = socket.socket(socket.AF_UNIX)
> +        client.connect(self.socket_path)
> +        expected = b'RFB 003.008'
> +        actual = client.recv(len(expected))
> +        self.assertEqual(expected, actual, "Wrong protocol version")
> +
>       def tearDown(self):
>           shutil.rmtree(self.socket_dir)

Re: [Qemu-devel] [PATCH 1/8] Travis: print acceptance tests logs in case of job failure

[Alex Bennée]

Cleber Rosa <crosa@redhat.com> writes:

> Because Travis doesn't allow us to keep files produced during tests
> (such as log files), let's print the complete job log to the "console"
> in case of job failures.
>
> This is a debugging aid, and given that there's been some timeouts
> happening on some tests, we absolutely needs the logs to have a proper
> action.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Acked-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  .travis.yml | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/.travis.yml b/.travis.yml
> index b053a836a3..9f8e73f276 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -226,6 +226,8 @@ matrix:
>      - env:
>          - CONFIG="--python=/usr/bin/python3 --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu"
>          - TEST_CMD="make check-acceptance"
> +      after_failure:
> +        - cat tests/results/latest/job.log
>        addons:
>          apt:
>            packages:


--
Alex Bennée

Re: [Qemu-devel] [PATCH 2/8] tests/requirements.txt: pin paramiko version requirement

[Philippe Mathieu-Daudé]

On 6/7/19 5:22 PM, Cleber Rosa wrote:
> It's a good practice (I'd really say a must) to pin as much as
> possible of the software versions used during test, so let's apply
> that to paramiko.
> 
> According to https://pypi.org/project/paramiko/, 2.4.2 is the latest
> released version.  It's also easily obtainable on systems such as
> Fedora 30.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/requirements.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/requirements.txt b/tests/requirements.txt
> index 3ae0e29ad7..bd1f7590ed 100644
> --- a/tests/requirements.txt
> +++ b/tests/requirements.txt
> @@ -2,4 +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==68.0
> -paramiko
> +paramiko==2.4.2
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH 3/8] Acceptance tests: drop left over usage of ":avocado: enable"

[Philippe Mathieu-Daudé]

On 6/7/19 5:22 PM, Cleber Rosa wrote:
> Commit 9531d26c10 removed all of ":avocado: enable" tags, but then
> a new entry was added with the introduction of migration.py.
> 
> Let's remove it for consistency.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/migration.py | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 6115cf6c24..a44c1ae58f 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -17,9 +17,6 @@ from avocado.utils import wait
>  
>  
>  class Migration(Test):
> -    """
> -    :avocado: enable
> -    """
>  
>      timeout = 10
>  
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH 6/8] VNC Acceptance test: simplify test names

[Philippe Mathieu-Daudé]

On 6/7/19 5:22 PM, Cleber Rosa wrote:
> The test name is composed of the class name and method name, so it
> looks like there's some redundancy here that we can eliminate.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/vnc.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
> index 675fd507ed..d32ae46685 100644
> --- a/tests/acceptance/vnc.py
> +++ b/tests/acceptance/vnc.py
> @@ -45,7 +45,7 @@ class VncUnixSocket(Test):
>          self.socket_dir = tempfile.mkdtemp()
>          self.socket_path = os.path.join(self.socket_dir, 'vnc-socket')
>  
> -    def test_vnc_change_password_requires_a_password(self):
> +    def test_change_password_requires_a_password(self):
>          self.vm.add_args('-nodefaults', '-S',
>                           '-vnc', 'unix:%s' % self.socket_path)
>          self.vm.launch()
> @@ -60,7 +60,7 @@ class VncUnixSocket(Test):
>          self.assertEqual(set_password_response['error']['desc'],
>                           'Could not set password')
>  
> -    def test_vnc_change_password(self):
> +    def test_change_password(self):
>          self.vm.add_args('-nodefaults', '-S',
>                           '-vnc', 'unix:%s,password' % self.socket_path)
>          self.vm.launch()
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>