[Qemu-devel] [RFC 00/24] Avocado-based functional tests

[Qemu-devel] [RFC 00/24] Avocado-based functional tests

[Eduardo Habkost]

Background
==========

A few months ago, Cleber started a thread[1] about the system for
Avocado-based tests.  He invited people to take a look at it, we
got some interesting feedback.

However, I saw no feedback on the actual code itself, and I don't
know what needs to be done to get this merged.

To make sure we're having a discussion about the implementation
of avocado_qemu and to figure out what's still necessary to get
it included, I am sending all the commits from Amador's
avocado_qemu branch[2] as a RFC.

Note that I didn't review most of this code yet, and I will
probably send comments and questions as replies to this RFC
later.

Also, many of the patches in this series probably can be squashed
together, but I decided to not do that before getting some
feedback on the changes.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg506859.html
[2] https://github.com/apahim/qemu/commits/avocado_qemu

README
======

Below, I'm copying the raw contents of tests/avocado/README.rst,
for reference:

----------------------------------------------------------------------

========================================
 QEMU tests using the Avocado Framework
========================================

This directory [tests/avocado] hosts functional tests written
using Avocado Testing Framework.

Installation
============

To install Avocado and the dependencies needed for these tests, run::

    pip install --user avocado-framework avocado-framework-plugin-varianter-yaml-to-mux aexpect

Alternatively, follow the instructions on this link::

    http://avocado-framework.readthedocs.io/en/latest/GetStartedGuide.html#installing-avocado

Overview
========

In this directory [tests/avocado], an ``avocado_qemu`` package is
provided, containing the ``test`` module, which inherits from
``avocado.Test`` and provides a builtin and easy-to-use Qemu
virtual machine. Here's a template that can be used as reference
to start writing your own tests::

    from avocado_qemu import test

    class MyTest(test.QemuTest):
        """
        :avocado: enable
        """

        def setUp(self):
            self.vm.args.extend(['-m', '512'])
            self.vm.launch()

        def test_01(self):
            res = self.vm.qmp('human-monitor-command',
                              command_line='info version')
            self.assertIn('v2.9.0', res['return'])

        def tearDown(self):
            self.vm.shutdown()

To execute your test, run::

    avocado run test_my_test.py

To execute all tests, run::

    avocado run .

If you don't specify the Qemu binary to use, the ``avocado_qemu``
package will automatically probe it. The probe will try to use the Qemu
binary from the git tree build directory, using the same architecture as
the local system (if the architecture is not specified). If the Qemu
binary is not available in the git tree build directory, the next try is
to use the system installed Qemu binary.

You can define a number of optional parameters, providing them via YAML
file using the Avocado parameters system:

- ``qemu_bin``: Use a given Qemu binary, skipping the automatic
  probe. Example: ``qemu_bin: /usr/libexec/qemu-kvm``.
- ``qemu_dst_bin``: Use a given Qemu binary to create the destination VM
  when the migration process takes place. If it's not provided, the same
  binary used in the source VM will be used for the destination VM.
  Example: ``qemu_dst_bin: /usr/libexec/qemu-kvm-binary2``.
- ``arch``: Probe the Qemu binary from a given architecture. It has no
  effect if ``qemu_bin`` is specified. If not provided, the binary probe
  will use the system architecture. Example: ``arch: x86_64``
- ``image_path``: When a test requires (usually a bootable) image, this
  parameter is used to define where the image is located. When undefined
  it uses ``$QEMU_ROOT/bootable_image_$arch.qcow2``. The image is added
  to the qemu command __only__ when the test requires an image. By
  default ``,snapshot=on`` is used, but it can be altered by
  ``image_snapshot`` parameter.
- ``image_user`` and ``image_pass``: When using a ``image_path``, if you
  want to get the console from the Guest OS you have to define the Guest
  OS credentials. Example: ``image_user: avocado`` and
  ``image_pass: p4ssw0rd``. Both parameters have defaults to ``avocado``.
- ``machine_type``: Use this option to define a machine type for the VM.
  Example: ``machine_type: pc``
- ``machine_accel``: Use this option to define a machine acceleration
  for the VM. Example: ``machine_accel: kvm``.
- ``machine_kvm_type``: Use this option to select the KVM type when the
  ``accel`` is ``kvm`` and there are more than one KVM types available.
  Example: ``machine_kvm_type: PR``

Run the test with::

    $ avocado run test_my_test.py -m parameters.yaml

Additionally, you can use a variants file to to set different values
for each parameter. Using the YAML tag ``!mux`` Avocado will execute the
tests once per combination of parameters. Example::

    $ cat variants.yaml
    architecture: !mux
        x86_64:
            arch: x86_64
        i386:
            arch: i386

Run it the with::

    $ avocado run test_my_test.py -m variants.yaml

You can use both the parameters file and the variants file in the same
command line::

    $ avocado run test_my_test.py -m parameters.yaml variants.yaml

Avocado will then merge the parameters from both files and create the
proper variants.

See ``avocado run --help`` and ``man avocado`` for several other
options, such as ``--filter-by-tags``, ``--show-job-log``,
``--failfast``, etc.

Uninstallation
==============

If you've followed the installation instructions above, you can easily
uninstall Avocado.  Start by listing the packages you have installed::

    pip list --user

And remove any package you want with::

    pip uninstall <package_name>

Amador Pahim (12):
  qemu.py: Introduce _create_console() method
  Introduce the basic framework to run Avocado tests
  avocado_qemu: Fix exception name in caller
  avocado_qemu: Improve migration error message
  avocado_qemu: Functional test for RHBZ#1431939
  avocado_qemu: Functional test for RHBZ#1447027
  avocado_qemu: Functional test for RHBZ#1436616
  avocado_qemu: Clean unneeded 'pass'
  avocado_qemu: Set QMP log level to INFO
  avocado_qemu: Introduce the add_image() VM API
  avocado_qemu: Tests fixes
  avocado_qemu: Force vmimage distro

Cleber Rosa (3):
  avocado_qemu: Remove duplicate PortTracker implementation
  avocado_qemu: Simplify the installation instructions
  avocado_qemu: Add a few VNC related tests

Lukáš Doktor (9):
  avocado_qemu: Improve handle_prompts to allow login after booted vm
  avocado_qemu: Be lenient towards poluted serial console
  avocado_qemu: Increase the login timeout to 60s
  avocado_qemu: Add " " after the default prompt regexp
  avocado_qemu: Store "arch" in VM
  avocado_qemu: Provide defaults for user and pass
  avocado_qemu: Ignore kernel messages on get_console
  avocado_qemu: Add support to request image for testing
  avocado_qemu: Functional test for RHBZ1473203

 scripts/qemu.py                                    |  59 ++-
 tests/avocado/README.rst                           | 132 +++++++
 tests/avocado/avocado_qemu/__init__.py             |   0
 tests/avocado/avocado_qemu/test.py                 | 418 +++++++++++++++++++++
 tests/avocado/parameters.yaml                      |  19 +
 tests/avocado/test_info_memdev_host_nodes.py       |  66 ++++
 tests/avocado/test_nec-usb-xhci.py                 |  63 ++++
 .../test_nec-usb-xhci.py.data/parameters.yaml      |   4 +
 tests/avocado/test_numa_hotplug.py                 | 120 ++++++
 tests/avocado/test_ovmf_with_240_vcpus.py          |  70 ++++
 .../parameters.yaml                                |   2 +
 tests/avocado/test_vnc.py                          |  58 +++
 tests/avocado/variants.yaml                        |  62 +++
 tests/qemu-iotests/iotests.py                      |  28 +-
 14 files changed, 1077 insertions(+), 24 deletions(-)
 create mode 100644 tests/avocado/README.rst
 create mode 100644 tests/avocado/avocado_qemu/__init__.py
 create mode 100644 tests/avocado/avocado_qemu/test.py
 create mode 100644 tests/avocado/parameters.yaml
 create mode 100644 tests/avocado/test_info_memdev_host_nodes.py
 create mode 100644 tests/avocado/test_nec-usb-xhci.py
 create mode 100644 tests/avocado/test_nec-usb-xhci.py.data/parameters.yaml
 create mode 100644 tests/avocado/test_numa_hotplug.py
 create mode 100644 tests/avocado/test_ovmf_with_240_vcpus.py
 create mode 100644 tests/avocado/test_ovmf_with_240_vcpus.py.data/parameters.yaml
 create mode 100644 tests/avocado/test_vnc.py
 create mode 100644 tests/avocado/variants.yaml

-- 
2.14.3

[Qemu-devel] [RFC 01/24] qemu.py: Introduce _create_console() method

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

This patch adds the QEMUMachine._create_console() method, which
returns a list with the chardev console device arguments to be
used in the qemu command line.

Signed-off-by: Amador Pahim <apahim@redhat.com>
[ehabkost: reword commit message]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/qemu.py | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 44 insertions(+), 5 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 08a3e9af5a..9e9d502543 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -55,7 +55,7 @@ class QEMUMachine(object):
 
     def __init__(self, binary, args=None, wrapper=None, name=None,
                  test_dir="/var/tmp", monitor_address=None,
-                 socket_scm_helper=None):
+                 socket_scm_helper=None, arch=None):
         '''
         Initialize a QEMUMachine
 
@@ -91,6 +91,10 @@ class QEMUMachine(object):
         self._test_dir = test_dir
         self._temp_dir = None
         self._launched = False
+        if arch is None:
+            arch = binary.split('-')[-1]
+        self._arch = arch
+        self._console_address = None
 
         # just in case logging wasn't configured by the main script:
         logging.basicConfig()
@@ -179,6 +183,39 @@ class QEMUMachine(object):
                 '-mon', 'chardev=mon,mode=control',
                 '-display', 'none', '-vga', 'none']
 
+    def _create_console(self, console_address):
+        for item in self._args:
+            for option in ['isa-serial', 'spapr-vty', 'sclpconsole']:
+                if option in item:
+                    return []
+
+        chardev = 'socket,id=console,{address},server,nowait'
+        if console_address is None:
+            console_address = tempfile.mktemp()
+            chardev = chardev.format(address='path=%s' %
+                                     console_address)
+        elif isinstance(console_address, tuple):
+            chardev = chardev.format(address='host=%s,port=%s' %
+                                     (console_address[0],
+                                     console_address[1]))
+        else:
+            chardev = chardev.format(address='path=%s' % console_address)
+
+        self._console_address = console_address
+
+        device = '{dev_type},chardev=console'
+        if '86' in self._arch:
+            device = device.format(dev_type='isa-serial')
+        elif 'ppc' in self._arch:
+            device = device.format(dev_type='spapr-vty')
+        elif 's390x' in self._arch:
+            device = device.format(dev_type='sclpconsole')
+        else:
+            return []
+
+        return ['-chardev', chardev,
+                '-device', device]
+
     def _pre_launch(self):
         self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
         if self._monitor_address is not None:
@@ -206,7 +243,7 @@ class QEMUMachine(object):
             shutil.rmtree(self._temp_dir)
             self._temp_dir = None
 
-    def launch(self):
+    def launch(self, console_address=None):
         """
         Launch the VM and make sure we cleanup and expose the
         command line/output in case of exception
@@ -218,7 +255,7 @@ class QEMUMachine(object):
         self._iolog = None
         self._qemu_full_args = None
         try:
-            self._launch()
+            self._launch(console_address)
             self._launched = True
         except:
             self.shutdown()
@@ -230,12 +267,14 @@ class QEMUMachine(object):
                 LOG.debug('Output: %r', self._iolog)
             raise
 
-    def _launch(self):
+    def _launch(self, console_address):
         '''Launch the VM and establish a QMP connection'''
         devnull = open(os.path.devnull, 'rb')
         self._pre_launch()
+        bargs = self._base_args()
+        bargs.extend(self._create_console(console_address))
         self._qemu_full_args = (self._wrapper + [self._binary] +
-                                self._base_args() + self._args)
+                                bargs + self.args)
         self._popen = subprocess.Popen(self._qemu_full_args,
                                        stdin=devnull,
                                        stdout=self._qemu_log_file,
-- 
2.14.3

[Qemu-devel] [RFC 02/24] Introduce the basic framework to run Avocado tests

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

Avocado Testing Framework can help with functional tests in
qemu development process. This patch creates the basic infrastructure
to use with Avocado tests. It contains:

- A README file with the initial documentation.
- The test module which inherits from avocado.Test and adds a VM object,
  created from scripts/qemu.py module. The QemuTest class is the test
  API for the actual Qemu tests to inherit from.
- A parameters yaml file with the supported keys.
- A variants yaml file with the Qemu supported architectures.

After this commit, you can expect a series of real-world tests, written
using this new framework.

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/qemu.py                        |  12 +-
 tests/avocado/README.rst               | 111 ++++++++++
 tests/avocado/avocado_qemu/__init__.py |   0
 tests/avocado/avocado_qemu/test.py     | 365 +++++++++++++++++++++++++++++++++
 tests/avocado/parameters.yaml          |  28 +++
 tests/avocado/variants.yaml            |  62 ++++++
 tests/qemu-iotests/iotests.py          |  28 +--
 7 files changed, 586 insertions(+), 20 deletions(-)
 create mode 100644 tests/avocado/README.rst
 create mode 100644 tests/avocado/avocado_qemu/__init__.py
 create mode 100644 tests/avocado/avocado_qemu/test.py
 create mode 100644 tests/avocado/parameters.yaml
 create mode 100644 tests/avocado/variants.yaml

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 9e9d502543..bd66620f45 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -81,7 +81,7 @@ class QEMUMachine(object):
         self._qemu_log_file = None
         self._popen = None
         self._binary = binary
-        self._args = list(args)     # Force copy args in case we modify them
+        self.args = list(args)     # Force copy args in case we modify them
         self._wrapper = wrapper
         self._events = []
         self._iolog = None
@@ -109,8 +109,8 @@ class QEMUMachine(object):
     # This can be used to add an unused monitor instance.
     def add_monitor_telnet(self, ip, port):
         args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port)
-        self._args.append('-monitor')
-        self._args.append(args)
+        self.args.append('-monitor')
+        self.args.append(args)
 
     def add_fd(self, fd, fdset, opaque, opts=''):
         '''Pass a file descriptor to the VM'''
@@ -120,8 +120,8 @@ class QEMUMachine(object):
         if opts:
             options.append(opts)
 
-        self._args.append('-add-fd')
-        self._args.append(','.join(options))
+        self.args.append('-add-fd')
+        self.args.append(','.join(options))
         return self
 
     def send_fd_scm(self, fd_file_path):
@@ -184,7 +184,7 @@ class QEMUMachine(object):
                 '-display', 'none', '-vga', 'none']
 
     def _create_console(self, console_address):
-        for item in self._args:
+        for item in self.args:
             for option in ['isa-serial', 'spapr-vty', 'sclpconsole']:
                 if option in item:
                     return []
diff --git a/tests/avocado/README.rst b/tests/avocado/README.rst
new file mode 100644
index 0000000000..f0e703fe06
--- /dev/null
+++ b/tests/avocado/README.rst
@@ -0,0 +1,111 @@
+This directory is hosting functional tests written using Avocado Testing
+Framework. To install Avocado, follow the instructions from this link::
+
+    http://avocado-framework.readthedocs.io/en/latest/GetStartedGuide.html#installing-avocado
+
+Tests here are written keeping the minimum amount of dependencies. To
+run the tests, you need the Avocado core package (`python-avocado` on
+Fedora, `avocado-framework` on pip). Extra dependencies should be
+documented in this file.
+
+In this directory, an ``avocado_qemu`` package is provided, containing
+the ``test`` module, which inherits from ``avocado.Test`` and provides
+a builtin and easy-to-use Qemu virtual machine. Here's a template that
+can be used as reference to start writing your own tests::
+
+    from avocado_qemu import test
+
+    class MyTest(test.QemuTest):
+        """
+        :avocado: enable
+        """
+
+        def setUp(self):
+            self.vm.args.extend(['-m', '512'])
+            self.vm.launch()
+
+        def test_01(self):
+            res = self.vm.qmp('human-monitor-command',
+                              command_line='info version')
+            self.assertIn('v2.9.0', res['return'])
+
+        def tearDown(self):
+            self.vm.shutdown()
+
+To execute your test, run::
+
+    avocado run test_my_test.py
+
+To execute all tests, run::
+
+    avocado run .
+
+If you don't specify the Qemu binary to use, the ``avocado_qemu``
+package will automatically probe it. The probe will try to use the Qemu
+binary from the git tree build directory, using the same architecture as
+the local system (if the architecture is not specified). If the Qemu
+binary is not available in the git tree build directory, the next try is
+to use the system installed Qemu binary.
+
+You can define a number of optional parameters, providing them via YAML
+file using the Avocado parameters system:
+
+- ``qemu_bin``: Use a given Qemu binary, skipping the automatic
+  probe. Example: ``qemu_bin: /usr/libexec/qemu-kvm``.
+- ``qemu_dst_bin``: Use a given Qemu binary to create the destination VM
+  when the migration process takes place. If it's not provided, the same
+  binary used in the source VM will be used for the destination VM.
+  Example: ``qemu_dst_bin: /usr/libexec/qemu-kvm-binary2``.
+- ``arch``: Probe the Qemu binary from a given architecture. It has no
+  effect if ``qemu_bin`` is specified. If not provided, the binary probe
+  will use the system architecture. Example: ``arch: x86_64``
+- ``image_path``: VMs are defined without image. If the ``image_path``
+  is specified, it will be used as the VM image. The ``-snapshot``
+  option will then be used to avoid writing into the image. Example:
+  ``image_path: /var/lib/images/fedora-25.img``
+- ``image_user`` and ``image_pass``: When using a ``image_path``, if you
+  want to get the console from the Guest OS you have to define the Guest
+  OS credentials. Example: ``image_user: root`` and
+  ``image_pass: p4ssw0rd``
+- ``machine_type``: Use this option to define a machine type for the VM.
+  Example: ``machine_type: pc``
+- ``machine_accel``: Use this option to define a machine acceleration
+  for the VM. Example: ``machine_accel: kvm``.
+- ``machine_kvm_type``: Use this option to select the KVM type when the
+  ``accel`` is ``kvm`` and there are more than one KVM types available.
+  Example: ``machine_kvm_type: PR``
+
+To use a parameters file, you have to install the yaml_to_mux plugin
+(`python2-avocado-plugins-varianter-yaml-to-mux` on Fedora,
+`avocado-framework-plugin-varianter-yaml-to-mux` on pip).
+
+Run the test with::
+
+    $ avocado run test_my_test.py -m parameters.yaml
+
+Additionally, you can use a variants file to to set different values
+for each parameter. Using the YAML tag ``!mux`` Avocado will execute the
+tests once per combination of parameters. Example::
+
+    $ cat variants.yaml
+    architecture: !mux
+        x86_64:
+            arch: x86_64
+        i386:
+            arch: i386
+
+Run it the with::
+
+    $ avocado run test_my_test.py -m variants.yaml
+
+You can use both the parameters file and the variants file in the same
+command line::
+
+    $ avocado run test_my_test.py -m parameters.yaml variants.yaml
+
+Avocado will then merge the parameters from both files and create the
+proper variants.
+
+See ``avocado run --help`` and ``man avocado`` for several other
+options, such as ``--filter-by-tags``, ``--show-job-log``,
+``--failfast``, etc.
diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
new file mode 100644
index 0000000000..e74de97a3e
--- /dev/null
+++ b/tests/avocado/avocado_qemu/test.py
@@ -0,0 +1,365 @@
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+#
+# See LICENSE for more details.
+#
+# Copyright (C) 2017 Red Hat Inc
+#
+# Authors:
+#  Amador Pahim <apahim@redhat.com>
+#
+# Based on code from:
+#   https://github.com/avocado-framework/avocado-virt
+
+
+"""
+Avocado Qemu Test module to extend the Avocado Test module providing
+extra features intended for Qemu testing.
+"""
+
+
+import logging
+import os
+import sys
+import time
+import uuid
+
+import aexpect
+
+from avocado import Test
+from avocado.utils.data_structures import Borg
+from avocado.utils import network
+from avocado.utils import process
+from avocado.utils import path as utils_path
+from avocado.utils import wait
+sys.path.append(os.path.join(os.path.dirname(__file__),
+                             '..', '..', '..', 'scripts'))
+import qemu
+
+
+class QEMULoginTimeoutError(Exception):
+    """
+    If timeout expires
+    """
+    pass
+
+
+class QEMULoginAuthenticationError(Exception):
+    """
+    If authentication fails
+    """
+    pass
+
+
+class QEMULoginProcessTerminatedError(Exception):
+    """
+    If the client terminates during login
+    """
+    pass
+
+
+class QEMULoginError(Exception):
+    """
+    If some other error occurs
+    """
+    pass
+
+
+class QEMUConsoleError(Exception):
+    """
+    If some error with the console access happens
+    """
+    pass
+
+
+class QEMUMigrationError(Exception):
+    """
+    If some error with the migration happens
+    """
+    pass
+
+
+def _get_qemu_bin(arch):
+    git_root = process.system_output('git rev-parse --show-toplevel',
+                                     ignore_status=True,
+                                     verbose=False)
+    qemu_binary = os.path.join(git_root,
+                               "%s-softmmu" % arch,
+                               "qemu-system-%s" % arch)
+    if not os.path.exists(qemu_binary):
+        qemu_binary = utils_path.find_command('qemu-system-%s' % arch)
+    return qemu_binary
+
+
+def _handle_prompts(session, username, password, prompt, timeout=10,
+                    debug=False):
+    """
+    Connect to a remote host (guest) using SSH or Telnet or else.
+
+    Wait for questions and provide answers.  If timeout expires while
+    waiting for output from the child (e.g. a password prompt or
+    a shell prompt) -- fail.
+
+    :param session: An Expect or ShellSession instance to operate on
+    :param username: The username to send in reply to a login prompt
+    :param password: The password to send in reply to a password prompt
+    :param prompt: The shell prompt that indicates a successful login
+    :param timeout: The maximal time duration (in seconds) to wait for each
+            step of the login procedure (i.e. the "Are you sure" prompt, the
+            password prompt, the shell prompt, etc)
+    :raise QEMULoginTimeoutError: If timeout expires
+    :raise QEMULoginAuthenticationError: If authentication fails
+    :raise QEMULoginProcessTerminatedError: If the client terminates during login
+    :raise QEMULoginError: If some other error occurs
+    :return: If connect succeed return the output text to script for further
+             debug.
+    """
+    password_prompt_count = 0
+    login_prompt_count = 0
+    last_chance = False
+
+    output = ""
+    while True:
+        try:
+            match, text = session.read_until_last_line_matches(
+                [r"[Aa]re you sure", r"[Pp]assword:\s*",
+                 # Prompt of rescue mode for Red Hat.
+                 r"\(or (press|type) Control-D to continue\):\s*$",
+                 r"[Gg]ive.*[Ll]ogin:\s*$",  # Prompt of rescue mode for SUSE.
+                 r"(?<![Ll]ast )[Ll]ogin:\s*$",  # Don't match "Last Login:"
+                 r"[Cc]onnection.*closed", r"[Cc]onnection.*refused",
+                 r"[Pp]lease wait", r"[Ww]arning", r"[Ee]nter.*username",
+                 r"[Ee]nter.*password", r"[Cc]onnection timed out", prompt,
+                 r"Escape character is.*"],
+                timeout=timeout, internal_timeout=0.5)
+            output += text
+            if match == 0:  # "Are you sure you want to continue connecting"
+                if debug:
+                    logging.debug("Got 'Are you sure...', sending 'yes'")
+                session.sendline("yes")
+                continue
+            elif match in [1, 2, 3, 10]:  # "password:"
+                if password_prompt_count == 0:
+                    if debug:
+                        logging.debug("Got password prompt, sending '%s'",
+                                      password)
+                    session.sendline(password)
+                    password_prompt_count += 1
+                    continue
+                else:
+                    raise QEMULoginAuthenticationError("Got password prompt "
+                                                       "twice", text)
+            elif match == 4 or match == 9:  # "login:"
+                if login_prompt_count == 0 and password_prompt_count == 0:
+                    if debug:
+                        logging.debug("Got username prompt; sending '%s'",
+                                      username)
+                    session.sendline(username)
+                    login_prompt_count += 1
+                    continue
+                else:
+                    if login_prompt_count > 0:
+                        msg = "Got username prompt twice"
+                    else:
+                        msg = "Got username prompt after password prompt"
+                    raise QEMULoginAuthenticationError(msg, text)
+            elif match == 5:  # "Connection closed"
+                raise QEMULoginError("Client said 'connection closed'", text)
+            elif match == 6:  # "Connection refused"
+                raise QEMULoginError("Client said 'connection refused'", text)
+            elif match == 11:  # Connection timeout
+                raise QEMULoginError("Client said 'connection timeout'", text)
+            elif match == 7:  # "Please wait"
+                if debug:
+                    logging.debug("Got 'Please wait'")
+                timeout = 30
+                continue
+            elif match == 8:  # "Warning added RSA"
+                if debug:
+                    logging.debug("Got 'Warning added RSA to known host list")
+                continue
+            elif match == 12:  # prompt
+                if debug:
+                    logging.debug("Got shell prompt -- logged in")
+                break
+            elif match == 13:  # console prompt
+                logging.debug("Got console prompt, send return to show login")
+                session.sendline()
+        except aexpect.ExpectTimeoutError as details:
+            # sometimes, linux kernel print some message to console
+            # the message maybe impact match login pattern, so send
+            # a empty line to avoid unexpect login timeout
+            if not last_chance:
+                time.sleep(0.5)
+                session.sendline()
+                last_chance = True
+                continue
+            else:
+                raise QEMULoginTimeoutError(details.output)
+        except aexpect.ExpectProcessTerminatedError as details:
+            raise QEMULoginProcessTerminatedError(details.status, details.output)
+
+    return output
+
+
+class _PortTracker(Borg):
+
+    """
+    Tracks ports used in the host machine.
+    """
+
+    def __init__(self):
+        Borg.__init__(self)
+        self.address = 'localhost'
+        self.start_port = 5000
+        if not hasattr(self, 'retained_ports'):
+            self._reset_retained_ports()
+
+    def __str__(self):
+        return 'Ports tracked: %r' % self.retained_ports
+
+    def _reset_retained_ports(self):
+        self.retained_ports = []
+
+    def register_port(self, port):
+        if ((port not in self.retained_ports) and
+                (network.is_port_free(port, self.address))):
+            self.retained_ports.append(port)
+        else:
+            raise ValueError('Port %d in use' % port)
+        return port
+
+    def find_free_port(self, start_port=None):
+        if start_port is None:
+            start_port = self.start_port
+        port = start_port
+        while ((port in self.retained_ports) or
+               (not network.is_port_free(port, self.address))):
+            port += 1
+        self.retained_ports.append(port)
+        return port
+
+    def release_port(self, port):
+        if port in self.retained:
+            self.retained.remove(port)
+
+
+class _VM(qemu.QEMUMachine):
+    '''A QEMU VM'''
+
+    def __init__(self, qemu_bin=None, arch=None, username=None, password=None,
+                 qemu_dst_bin=None):
+        if arch is None:
+            arch = os.uname()[4]
+        self.ports = _PortTracker()
+        self.name = "qemu-%s" % str(uuid.uuid4())[:8]
+        if qemu_bin is None:
+            qemu_bin = _get_qemu_bin(arch)
+        if qemu_dst_bin is None:
+            qemu_dst_bin = qemu_bin
+        self.qemu_bin = qemu_bin
+        self.qemu_dst_bin = qemu_dst_bin
+        self.username = username
+        self.password = password
+        super(_VM, self).__init__(qemu_bin, name=self.name, arch=arch)
+
+    def get_console(self, console_address=None, prompt="[\#\$]"):
+        """
+        :param address: Socket address, can be either a unix socket path
+                        (string) or a tuple in the form (address, port)
+                        for a TCP connection
+        :param prompt: The regex to identify we reached the prompt.
+        """
+
+        if not self.is_running():
+            raise QEMUConsoleError('VM is not running.')
+
+        if console_address is None:
+            if self._console_address is None:
+                raise QEMUConsoleError("Can't determine the console address "
+                                       "to connect to.")
+            else:
+                console_address = self._console_address
+
+        nc_cmd = 'nc'
+        if isinstance(console_address, tuple):
+            nc_cmd += ' %s %s' % (console_address[0], console_address[1])
+        else:
+            nc_cmd += ' -U %s' % console_address
+
+        console = aexpect.ShellSession(nc_cmd)
+        try:
+            logging.info('Console: Waiting login prompt...')
+            _handle_prompts(console, self.username, self.password, "[\#\$]")
+            logging.info('Console: Ready!')
+        except:
+            console.close()
+            raise
+
+        return console
+
+    def migrate(self, console_address=None, timeout=20):
+        def migrate_complete():
+            cmd = 'info migrate'
+            res = self.qmp('human-monitor-command', command_line=cmd)
+            if 'completed' in res['return']:
+                logging.info("Migration successful")
+                return True
+            elif 'failed' in res['return']:
+                raise QEMUMigrateError("Migration of %s failed" % self)
+            return False
+
+        port = self.ports.find_free_port()
+        newvm = _VM(self.qemu_dst_bin, self._arch, self.username, self.password)
+        newvm.args = self.args
+        newvm.args.extend(['-incoming', 'tcp:0:%s' % port])
+
+        newvm.launch(console_address)
+        cmd = 'migrate -d tcp:0:%s' % port
+        self.qmp('human-monitor-command', command_line=cmd)
+        mig_result = wait.wait_for(migrate_complete, timeout=timeout,
+                                   text='Waiting for migration to complete')
+
+        if mig_result is None:
+            raise QEMUMigrateError("Migration of %s did not complete after "
+                                   "%s s" % (self.name, timeout))
+
+        return newvm
+
+
+class QemuTest(Test):
+
+    def __init__(self, methodName=None, name=None, params=None,
+                 base_logdir=None, job=None, runner_queue=None):
+        super(QemuTest, self).__init__(methodName=methodName, name=name,
+                                       params=params, base_logdir=base_logdir,
+                                       job=job, runner_queue=runner_queue)
+        self.vm = _VM(qemu_bin=self.params.get('qemu_bin'),
+                      arch=self.params.get('arch'),
+                      username=self.params.get('image_user'),
+                      password=self.params.get('image_pass'),
+                      qemu_dst_bin=self.params.get('qemu_dst_bin'))
+
+        self.vm.image = self.params.get('image_path')
+        if self.vm.image is not None:
+            self.vm.args.extend(['-drive', 'file=%s' % self.vm.image])
+            self.vm.args.append('-snapshot')
+
+        machine_type = self.params.get('machine_type')
+        machine_accel = self.params.get('machine_accel')
+        machine_kvm_type = self.params.get('machine_kvm_type')
+        machine = ""
+        if machine_type is not None:
+            machine += "%s," % machine_type
+        if machine_accel is not None:
+            machine += "accel=%s," % machine_accel
+        if machine_kvm_type is not None:
+            machine += "kvm-type=%s," % machine_kvm_type
+        if machine:
+            self.vm.args.extend(['-machine', machine])
diff --git a/tests/avocado/parameters.yaml b/tests/avocado/parameters.yaml
new file mode 100644
index 0000000000..3c5a0f92e0
--- /dev/null
+++ b/tests/avocado/parameters.yaml
@@ -0,0 +1,28 @@
+# Probe the Qemu binary from a given architecture. It has no effect if
+# 'qemu_bin' is specified. If not provided, the binary probe will use
+# the local system architecture.
+arch: null
+
+# Use a given Qemu binary, skipping the automatic probe.
+qemu_bin: null
+# Use a given Qemu binary to create the destination VM when the
+# migration process is called. If it's not provided, the same binary
+# used in the source VM will be used for the destination VM.
+qemu_dst_bin: null
+
+# VMs are defined without image. If the 'image_path' is specified, it
+# will be used as the VM image. The '-snapshot' option will then be used
+# to avoid writing data to the image.
+image_path: null
+# Username used to get the console from the Guest OS.
+image_user: null
+# Password used to get the console from the Guest OS.
+image_pass: null
+
+# Use this option to define a machine type for the VM.
+machine_type: null
+# Use this option to define a machine acceleration for the VM.
+machine_accel: null
+# Use this option to select the KVM type when the 'machine_accel' is set
+# to 'kvm' and there are more than one KVM types available.
+machine_kvm_type: null
diff --git a/tests/avocado/variants.yaml b/tests/avocado/variants.yaml
new file mode 100644
index 0000000000..6fc689b3ba
--- /dev/null
+++ b/tests/avocado/variants.yaml
@@ -0,0 +1,62 @@
+architecture: !mux
+    # Set the architecture of the qemu binary to execute tests
+    # with. This setting has no effect if you're using custom
+    # qemu_bin configuration.
+    x86_64:
+        arch: x86_64
+    aarch64:
+        arch: aarch64
+    alpha:
+        arch: alpha
+    arm:
+        arch: arm
+    cris:
+        arch: cris
+    i386:
+        arch: i386
+    lm32:
+        arch: lm32
+    m68k:
+        arch: m68k
+    microblazeel:
+        arch: microblazeel
+    microblaze:
+        arch: microblaze
+    mips64el:
+        arch: mips64el
+    mips64:
+        arch: mips64
+    mipsel:
+        arch: mipsel
+    mips:
+        arch: mips
+    moxie:
+        arch: moxie
+    nios2:
+        arch: nios2
+    or1k:
+        arch: or1k
+    ppc64:
+        arch: ppc64
+    ppcemb:
+        arch: ppcemb
+    ppc:
+        arch: ppc
+    s390x:
+        arch: s390x
+    sh4eb:
+        arch: sh4eb
+    sh4:
+        arch: sh4
+    sparc64:
+        arch: sparc64
+    sparc:
+        arch: sparc
+    tricore:
+        arch: tricore
+    unicore32:
+        arch: unicore32
+    xtensaeb:
+        arch: xtensaeb
+    xtensa:
+        arch: xtensa
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b25d48a91b..a2e4f03743 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -293,18 +293,18 @@ class VM(qtest.QEMUQtestMachine):
         self._num_drives = 0
 
     def add_object(self, opts):
-        self._args.append('-object')
-        self._args.append(opts)
+        self.args.append('-object')
+        self.args.append(opts)
         return self
 
     def add_device(self, opts):
-        self._args.append('-device')
-        self._args.append(opts)
+        self.args.append('-device')
+        self.args.append(opts)
         return self
 
     def add_drive_raw(self, opts):
-        self._args.append('-drive')
-        self._args.append(opts)
+        self.args.append('-drive')
+        self.args.append(opts)
         return self
 
     def add_drive(self, path, opts='', interface='virtio', format=imgfmt):
@@ -322,27 +322,27 @@ class VM(qtest.QEMUQtestMachine):
 
         if format == 'luks' and 'key-secret' not in opts:
             # default luks support
-            if luks_default_secret_object not in self._args:
+            if luks_default_secret_object not in self.args:
                 self.add_object(luks_default_secret_object)
 
             options.append(luks_default_key_secret_opt)
 
-        self._args.append('-drive')
-        self._args.append(','.join(options))
+        self.args.append('-drive')
+        self.args.append(','.join(options))
         self._num_drives += 1
         return self
 
     def add_blockdev(self, opts):
-        self._args.append('-blockdev')
+        self.args.append('-blockdev')
         if isinstance(opts, str):
-            self._args.append(opts)
+            self.args.append(opts)
         else:
-            self._args.append(','.join(opts))
+            self.args.append(','.join(opts))
         return self
 
     def add_incoming(self, addr):
-        self._args.append('-incoming')
-        self._args.append(addr)
+        self.args.append('-incoming')
+        self.args.append(addr)
         return self
 
     def pause_drive(self, drive, event=None):
-- 
2.14.3

[Qemu-devel] [RFC 03/24] avocado_qemu: Improve handle_prompts to allow login after booted vm

[Eduardo Habkost]

From: Lukáš Doktor <ldoktor@redhat.com>

When the VM is booted before calling "vm.get_console" this command fails
to login as there is no new output in the console. Let's just press
enter (which should be relatively harmless as login asks for the user
again and when already logged in we might "only" execute what's already
written.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index e74de97a3e..9cc163b3a8 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -123,7 +123,8 @@ def _handle_prompts(session, username, password, prompt, timeout=10,
     password_prompt_count = 0
     login_prompt_count = 0
     last_chance = False
-
+    # Send enter to refresh output (in case session was attached after boot)
+    session.sendline()
     output = ""
     while True:
         try:
-- 
2.14.3

[Qemu-devel] [RFC 04/24] avocado_qemu: Be lenient towards poluted serial console

[Eduardo Habkost]

From: Lukáš Doktor <ldoktor@redhat.com>

The serial console is frequently spammed kernel messages, let's not be
so strict and match lines also containing other parts.

There is still possibility of failure in case kernel injects message on
a new line after the expected output producing non-matching last_line.
Anyway checking for any-line would be a bit too prone to false-positives
so let's just go with this at this point.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index 9cc163b3a8..fb16785a20 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -131,9 +131,9 @@ def _handle_prompts(session, username, password, prompt, timeout=10,
             match, text = session.read_until_last_line_matches(
                 [r"[Aa]re you sure", r"[Pp]assword:\s*",
                  # Prompt of rescue mode for Red Hat.
-                 r"\(or (press|type) Control-D to continue\):\s*$",
-                 r"[Gg]ive.*[Ll]ogin:\s*$",  # Prompt of rescue mode for SUSE.
-                 r"(?<![Ll]ast )[Ll]ogin:\s*$",  # Don't match "Last Login:"
+                 r"\(or (press|type) Control-D to continue\):\s*",
+                 r"[Gg]ive.*[Ll]ogin:\s*",  # Prompt of rescue mode for SUSE.
+                 r"(?<![Ll]ast )[Ll]ogin:\s*",  # Don't match "Last Login:"
                  r"[Cc]onnection.*closed", r"[Cc]onnection.*refused",
                  r"[Pp]lease wait", r"[Ww]arning", r"[Ee]nter.*username",
                  r"[Ee]nter.*password", r"[Cc]onnection timed out", prompt,
-- 
2.14.3

[Qemu-devel] [RFC 05/24] avocado_qemu: Increase the login timeout to 60s

[Eduardo Habkost]

From: Lukáš Doktor <ldoktor@redhat.com>

The 10s login is quite short, let's use 60s and see whether it fits.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index fb16785a20..e99c7e9e8d 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -97,7 +97,7 @@ def _get_qemu_bin(arch):
     return qemu_binary
 
 
-def _handle_prompts(session, username, password, prompt, timeout=10,
+def _handle_prompts(session, username, password, prompt, timeout=60,
                     debug=False):
     """
     Connect to a remote host (guest) using SSH or Telnet or else.
-- 
2.14.3

[Qemu-devel] [RFC 06/24] avocado_qemu: Add " " after the default prompt regexp

[Eduardo Habkost]

From: Lukáš Doktor <ldoktor@redhat.com>

The prompt regexp is quite basic one (# or $), let's require at least
one additional space afterwards, which is the case in most modern
systems and decreases the probability of false-positives.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index e99c7e9e8d..eeb759ec65 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -270,7 +270,7 @@ class _VM(qemu.QEMUMachine):
         self.password = password
         super(_VM, self).__init__(qemu_bin, name=self.name, arch=arch)
 
-    def get_console(self, console_address=None, prompt="[\#\$]"):
+    def get_console(self, console_address=None, prompt=r"[\#\$] "):
         """
         :param address: Socket address, can be either a unix socket path
                         (string) or a tuple in the form (address, port)
@@ -297,7 +297,7 @@ class _VM(qemu.QEMUMachine):
         console = aexpect.ShellSession(nc_cmd)
         try:
             logging.info('Console: Waiting login prompt...')
-            _handle_prompts(console, self.username, self.password, "[\#\$]")
+            _handle_prompts(console, self.username, self.password, prompt)
             logging.info('Console: Ready!')
         except:
             console.close()
-- 
2.14.3

[Qemu-devel] [RFC 07/24] avocado_qemu: Store "arch" in VM

[Eduardo Habkost]

From: Lukáš Doktor <ldoktor@redhat.com>

The "arch" is quite important info regarding the current VM, let's store
it so tests can query for it.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index eeb759ec65..6bf7825e55 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -258,6 +258,7 @@ class _VM(qemu.QEMUMachine):
                  qemu_dst_bin=None):
         if arch is None:
             arch = os.uname()[4]
+        self.arch = arch
         self.ports = _PortTracker()
         self.name = "qemu-%s" % str(uuid.uuid4())[:8]
         if qemu_bin is None:
-- 
2.14.3

[Qemu-devel] [RFC 08/24] avocado_qemu: Provide defaults for user and pass

[Eduardo Habkost]

From: Lukáš Doktor <ldoktor@redhat.com>

For conveniency let's set default user to root and password to 123456 to
avoid the need to always input those. This password is inspired by the
default password of JeOS available from avocado-vt.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index 6bf7825e55..9dc6c1ef91 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -344,8 +344,8 @@ class QemuTest(Test):
                                        job=job, runner_queue=runner_queue)
         self.vm = _VM(qemu_bin=self.params.get('qemu_bin'),
                       arch=self.params.get('arch'),
-                      username=self.params.get('image_user'),
-                      password=self.params.get('image_pass'),
+                      username=self.params.get('image_user', default="root"),
+                      password=self.params.get('image_pass', default="123456"),
                       qemu_dst_bin=self.params.get('qemu_dst_bin'))
 
         self.vm.image = self.params.get('image_path')
-- 
2.14.3

[Qemu-devel] [RFC 09/24] avocado_qemu: Ignore kernel messages on get_console

[Eduardo Habkost]

From: Lukáš Doktor <ldoktor@redhat.com>

The get_console (and _handle_prompt) uses the last non-empty line to
check what is going on, but when debug is enabled, kernel produces lots
of lines spoiling the output. Let's also ignore the messages that looks
like kernel debugs ([  $float] $msg).

This significantly improves the results on my machine with JeOS and
enabled debug.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index 9dc6c1ef91..966936a52f 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -26,6 +26,7 @@ extra features intended for Qemu testing.
 
 import logging
 import os
+import re
 import sys
 import time
 import uuid
@@ -120,6 +121,17 @@ def _handle_prompts(session, username, password, prompt, timeout=60,
     :return: If connect succeed return the output text to script for further
              debug.
     """
+    re_kernel_message = re.compile(r"^\[\s*\d+.\d+\] ")
+
+    def get_last_nonempty_line(cont):
+        """Return last non-empty non-kernel line"""
+        nonempty_lines = [_ for _ in cont.splitlines()
+                          if _.strip() and not re_kernel_message.match(_)]
+        if nonempty_lines:
+            return nonempty_lines[-1]
+        else:
+            return ""
+
     password_prompt_count = 0
     login_prompt_count = 0
     last_chance = False
@@ -128,7 +140,7 @@ def _handle_prompts(session, username, password, prompt, timeout=60,
     output = ""
     while True:
         try:
-            match, text = session.read_until_last_line_matches(
+            match, text = session.read_until_output_matches(
                 [r"[Aa]re you sure", r"[Pp]assword:\s*",
                  # Prompt of rescue mode for Red Hat.
                  r"\(or (press|type) Control-D to continue\):\s*",
@@ -137,7 +149,7 @@ def _handle_prompts(session, username, password, prompt, timeout=60,
                  r"[Cc]onnection.*closed", r"[Cc]onnection.*refused",
                  r"[Pp]lease wait", r"[Ww]arning", r"[Ee]nter.*username",
                  r"[Ee]nter.*password", r"[Cc]onnection timed out", prompt,
-                 r"Escape character is.*"],
+                 r"Escape character is.*"], get_last_nonempty_line,
                 timeout=timeout, internal_timeout=0.5)
             output += text
             if match == 0:  # "Are you sure you want to continue connecting"
-- 
2.14.3

[Qemu-devel] [RFC 10/24] avocado_qemu: Add support to request image for testing

[Eduardo Habkost]

From: Lukáš Doktor <ldoktor@redhat.com>

Some of the tests require (usually bootable) image to be executed, let's
add a helper function which uses the default params to define the image
or provides useful default and explains what's going on in case the
image is not available.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/README.rst           | 12 ++++++-----
 tests/avocado/avocado_qemu/test.py | 44 ++++++++++++++++++++++++++++++++------
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/tests/avocado/README.rst b/tests/avocado/README.rst
index f0e703fe06..07852e790a 100644
--- a/tests/avocado/README.rst
+++ b/tests/avocado/README.rst
@@ -59,14 +59,16 @@ file using the Avocado parameters system:
 - ``arch``: Probe the Qemu binary from a given architecture. It has no
   effect if ``qemu_bin`` is specified. If not provided, the binary probe
   will use the system architecture. Example: ``arch: x86_64``
-- ``image_path``: VMs are defined without image. If the ``image_path``
-  is specified, it will be used as the VM image. The ``-snapshot``
-  option will then be used to avoid writing into the image. Example:
-  ``image_path: /var/lib/images/fedora-25.img``
+- ``image_path``: When a test requires (usually a bootable) image, this
+  parameter is used to define where the image is located. When undefined
+  it uses ``$QEMU_ROOT/bootable_image_$arch.qcow2``. The image is added
+  to the qemu command __only__ when the test requires an image. By
+  default ``,snapshot=on`` is used, but it can be altered by
+  ``image_snapshot`` parameter.
 - ``image_user`` and ``image_pass``: When using a ``image_path``, if you
   want to get the console from the Guest OS you have to define the Guest
   OS credentials. Example: ``image_user: root`` and
-  ``image_pass: p4ssw0rd``
+  ``image_pass: p4ssw0rd``. By default it uses ``root`` and ``123456``.
 - ``machine_type``: Use this option to define a machine type for the VM.
   Example: ``machine_type: pc``
 - ``machine_accel``: Use this option to define a machine acceleration
diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index 966936a52f..57c63b2853 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -39,8 +39,10 @@ from avocado.utils import network
 from avocado.utils import process
 from avocado.utils import path as utils_path
 from avocado.utils import wait
-sys.path.append(os.path.join(os.path.dirname(__file__),
-                             '..', '..', '..', 'scripts'))
+
+QEMU_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(os.path.dirname(
+    os.path.dirname(__file__)))))
+sys.path.append(os.path.join(QEMU_ROOT, 'scripts'))
 import qemu
 
 
@@ -360,11 +362,6 @@ class QemuTest(Test):
                       password=self.params.get('image_pass', default="123456"),
                       qemu_dst_bin=self.params.get('qemu_dst_bin'))
 
-        self.vm.image = self.params.get('image_path')
-        if self.vm.image is not None:
-            self.vm.args.extend(['-drive', 'file=%s' % self.vm.image])
-            self.vm.args.append('-snapshot')
-
         machine_type = self.params.get('machine_type')
         machine_accel = self.params.get('machine_accel')
         machine_kvm_type = self.params.get('machine_kvm_type')
@@ -377,3 +374,36 @@ class QemuTest(Test):
             machine += "kvm-type=%s," % machine_kvm_type
         if machine:
             self.vm.args.extend(['-machine', machine])
+
+    def request_image(self, path=None, snapshot=None, extra=None):
+        """
+        Add image to the `self.vm` using params or arguments.
+
+        Unless it's overridden by arguments it uses following test params
+        to specify the image:
+
+        * image_path - defines the path to the user-image. If not specified
+                       it uses "QEMU_ROOT/boot_image_$arch.qcow2"
+        * image_snapshot - whether to use "snapshot=on" (snapshot=off is not
+                           supplied)
+        * image_extra - free-form string to extend the "-drive" params
+
+        :param path: Override the path ("image_path" param is used otherwise)
+        :param snapshot: Override the usage of snapshot
+        :param extra: Extra arguments to be added to drive definition
+        """
+        if snapshot is None:
+            snapshot = self.params.get("image_snapshot", default=True)
+        if extra is None:
+            extra = self.params.get("image_extra", default="")
+        if path is None:
+            path = self.params.get("image_path")
+            if path is None:
+                arch = self.vm.arch
+                path = os.path.join(QEMU_ROOT, "boot_image_%s.qcow2" % arch)
+        if not os.path.exists(path):
+            self.error("Require a bootable image, which was not found. "
+                       "Please provide one in '%s'." % path)
+        if snapshot:
+            extra += ",snapshot=on"
+        self.vm.args.extend(['-drive', 'file=%s%s' % (path, extra)])
-- 
2.14.3

[Qemu-devel] [RFC 11/24] avocado_qemu: Fix exception name in caller

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

There's a exception name mismatch when callin `QEMUMigrationError`.
Fixing.

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index 57c63b2853..f3c7091343 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -328,7 +328,7 @@ class _VM(qemu.QEMUMachine):
                 logging.info("Migration successful")
                 return True
             elif 'failed' in res['return']:
-                raise QEMUMigrateError("Migration of %s failed" % self)
+                raise QEMUMigrationError("Migration of %s failed" % self)
             return False
 
         port = self.ports.find_free_port()
@@ -343,8 +343,8 @@ class _VM(qemu.QEMUMachine):
                                    text='Waiting for migration to complete')
 
         if mig_result is None:
-            raise QEMUMigrateError("Migration of %s did not complete after "
-                                   "%s s" % (self.name, timeout))
+            raise QEMUMigrationError("Migration of %s did not complete after "
+                                     "%s s" % (self.name, timeout))
 
         return newvm
 
-- 
2.14.3

[Qemu-devel] [RFC 12/24] avocado_qemu: Improve migration error message

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

Expose the QMP result on migration error.

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index f3c7091343..5e42cac8d0 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -328,7 +328,8 @@ class _VM(qemu.QEMUMachine):
                 logging.info("Migration successful")
                 return True
             elif 'failed' in res['return']:
-                raise QEMUMigrationError("Migration of %s failed" % self)
+                logging.error(res)
+                raise QEMUMigrationError("Migration of %s failed" % self.name)
             return False
 
         port = self.ports.find_free_port()
-- 
2.14.3

[Qemu-devel] [RFC 13/24] avocado_qemu: Functional test for RHBZ#1431939

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

This issue was fixed in commit d81d857.

According to the RHBZ1431939, the issue is 'host nodes' returning '128'.
It should return empty value when using default policy or 0 when using
bind policy.

Test consists in inspect the result of the 'info memdev' QMP command
after hot-adding memory.

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/test_info_memdev_host_nodes.py | 66 ++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 tests/avocado/test_info_memdev_host_nodes.py

diff --git a/tests/avocado/test_info_memdev_host_nodes.py b/tests/avocado/test_info_memdev_host_nodes.py
new file mode 100644
index 0000000000..69891b723d
--- /dev/null
+++ b/tests/avocado/test_info_memdev_host_nodes.py
@@ -0,0 +1,66 @@
+from avocado import main
+from avocado_qemu import test
+
+
+class TestInfoMemdev(test.QemuTest):
+    """
+
+    :avocado: enable
+    :avocado: tags=qmp,object_add,device_add,memdev
+    """
+
+    def setUp(self):
+        self.vm.args.extend(['-m', '4G,slots=32,maxmem=40G'])
+        self.vm.launch()
+
+    def test_hotplug_memory_default_policy(self):
+        """
+        According to the RHBZ1431939, the issue is 'host nodes'
+        returning '128'. It should return empty value when memory
+        hotplug default policy is used.
+
+        Fixed in commit d81d857f4421d205395d55200425daa6591c28a5.
+        :avocado: tags=RHBZ1431939
+        """
+
+        cmd = 'object_add memory-backend-ram,id=mem1,size=1G'
+        res = self.vm.qmp('human-monitor-command', command_line=cmd)
+        self.assertEqual('', res['return'])
+
+        cmd = 'device_add pc-dimm,id=dimm1,memdev=mem1'
+        res = self.vm.qmp('human-monitor-command', command_line=cmd)
+        self.assertEqual('', res['return'])
+
+        cmd = 'info memdev'
+        res = self.vm.qmp('human-monitor-command', command_line=cmd)
+        self.assertIn('policy: default\r', res['return'])
+        self.assertIn('host nodes: \r', res['return'])
+
+    def test_hotplug_memory_bind_policy(self):
+        """
+        According to the RHBZ1431939, the issue is 'host nodes'
+        returning '128'. It should return 0 when memory hotplug
+        bind policy is used.
+
+        Fixed in commit d81d857f4421d205395d55200425daa6591c28a5.
+        :avocado: tags=RHBZ1431939
+        """
+
+        cmd = 'object_add memory-backend-ram,id=mem1,host-nodes=0,size=2G,policy=bind'
+        res = self.vm.qmp('human-monitor-command', command_line=cmd)
+        self.assertEqual('', res['return'])
+
+        cmd = 'device_add pc-dimm,id=dimm1,memdev=mem1'
+        res = self.vm.qmp('human-monitor-command', command_line=cmd)
+        self.assertEqual('', res['return'])
+
+        cmd = 'info memdev'
+        res = self.vm.qmp('human-monitor-command', command_line=cmd)
+        self.assertIn('policy: bind\r', res['return'])
+        self.assertIn('host nodes: 0\r', res['return'])
+
+    def tearDown(self):
+        self.vm.shutdown()
+
+if __name__ == "__main__":
+    avocado.main()
-- 
2.14.3

[Qemu-devel] [RFC 14/24] avocado_qemu: Functional test for RHBZ#1447027

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

Fixed in commit e85c0d1.

According to the RHBZ1447027, the issue is: guest cannot boot with 240
or above vcpus when using ovmf.

Test consists in set the VM with 240 vcpus and the OVMF device to then
check whether the VM can be started.

A parameters.yaml file is provided in order to customize the OVMF files
locations. Execute with:

    $ avocado run test_ovmf_with_240_vcpus.py -m \
     test_ovmf_with_240_vcpus.py.data/parameters.yaml

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/test_ovmf_with_240_vcpus.py          | 70 ++++++++++++++++++++++
 .../parameters.yaml                                |  2 +
 2 files changed, 72 insertions(+)
 create mode 100644 tests/avocado/test_ovmf_with_240_vcpus.py
 create mode 100644 tests/avocado/test_ovmf_with_240_vcpus.py.data/parameters.yaml

diff --git a/tests/avocado/test_ovmf_with_240_vcpus.py b/tests/avocado/test_ovmf_with_240_vcpus.py
new file mode 100644
index 0000000000..da688dbc76
--- /dev/null
+++ b/tests/avocado/test_ovmf_with_240_vcpus.py
@@ -0,0 +1,70 @@
+import os
+import shutil
+import sys
+
+from avocado import main
+from avocado_qemu import test
+
+
+class TestOvmfVcpus(test.QemuTest):
+    """
+    Run with:
+
+        avocado run test_ovmf_with_240_vcpus.py \
+        -m test_ovmf_with_240_vcpus.py.data/parameters.yaml
+
+    :avocado: enable
+    :avocado: tags=ovmf
+    """
+
+    def setUp(self):
+        ovmf_code_path = self.params.get('OVMF_CODE',
+                                         default='/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd')
+        ovmf_vars_path = self.params.get('OVMF_VARS',
+                                         default='/usr/share/edk2/ovmf/OVMF_VARS.fd')
+        if not ovmf_code_path or not os.path.exists(ovmf_code_path):
+            basename = os.path.basename(__file__)
+            self.cancel('OVMF_CODE file not found. Set the correct '
+                        'path on "%s.data/parameters.yaml" and run this test '
+                        'with: "avocado run %s -m %s.data/parameters.yaml"' %
+                        (basename, basename, basename))
+        if not ovmf_vars_path or not os.path.exists(ovmf_vars_path):
+            basename = os.path.basename(__file__)
+            self.cancel('OVMF_VARS file not found. Set the correct '
+                        'path on "%s.data/parameters.yaml" and run this test '
+                        'with: "avocado run %s -m %s.data/parameters.yaml"' %
+                        (basename, basename, basename))
+
+        ovmf_vars_tmp = os.path.join(self.workdir,
+                                     os.path.basename(ovmf_vars_path))
+        if not os.path.exists(ovmf_vars_tmp):
+            shutil.copy(ovmf_vars_path, self.workdir)
+
+        self.vm.args.extend(['-drive',
+                             'file=%s,if=pflash,format=raw,readonly=on,unit=0' %
+                             ovmf_code_path])
+        self.vm.args.extend(['-drive',
+                             'file=%s,if=pflash,format=raw,unit=1' %
+                             ovmf_vars_tmp])
+
+        self.vm.args.extend(['-smp', '240'])
+
+    def test_run_vm(self):
+        """
+        According to the RHBZ1447027, the issue is: Guest cannot boot
+        with 240 or above vcpus when using ovmf.
+        Fixed in commit e85c0d14014514a2f0faeae5b4c23fab5b234de4.
+
+        :avocado: tags=RHBZ1447027
+        """
+
+        try:
+            self.vm.launch()
+        except Exception as details:
+            self.fail(details)
+
+    def tearDown(self):
+        self.vm.shutdown()
+
+if __name__ == "__main__":
+    avocado.main()
diff --git a/tests/avocado/test_ovmf_with_240_vcpus.py.data/parameters.yaml b/tests/avocado/test_ovmf_with_240_vcpus.py.data/parameters.yaml
new file mode 100644
index 0000000000..79f6da1d29
--- /dev/null
+++ b/tests/avocado/test_ovmf_with_240_vcpus.py.data/parameters.yaml
@@ -0,0 +1,2 @@
+OVMF_CODE: /usr/share/edk2/ovmf/OVMF_CODE.secboot.fd
+OVMF_VARS: /usr/share/edk2/ovmf/OVMF_VARS.fd
-- 
2.14.3

[Qemu-devel] [RFC 15/24] avocado_qemu: Functional test for RHBZ#1436616

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

Fixed in commit 243afe858b95765b98d16a1f0dd50dca262858ad.

According to the RHBZ1436616, the issue is: usb-storage device under
pci-bridge is unusable after migration.

Test consists in create a VM with an usb-storage device and check
whether it's still available after a live migration.

A parameters.yaml file is provided in order to customize the guest
image location and credentials. Execute with:

        avocado run test_nec-usb-xhci.py \
        -m test_nec-usb-xhci.py.data/parameters.yaml

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/test_nec-usb-xhci.py                 | 55 ++++++++++++++++++++++
 .../test_nec-usb-xhci.py.data/parameters.yaml      |  4 ++
 2 files changed, 59 insertions(+)
 create mode 100644 tests/avocado/test_nec-usb-xhci.py
 create mode 100644 tests/avocado/test_nec-usb-xhci.py.data/parameters.yaml

diff --git a/tests/avocado/test_nec-usb-xhci.py b/tests/avocado/test_nec-usb-xhci.py
new file mode 100644
index 0000000000..3f0d645032
--- /dev/null
+++ b/tests/avocado/test_nec-usb-xhci.py
@@ -0,0 +1,55 @@
+import copy
+import os
+import tempfile
+
+from avocado_qemu import test
+from avocado.utils import process
+
+class TestNecUsbXhci(test.QemuTest):
+    """
+    Run with:
+
+        avocado run test_nec-usb-xhci.py \
+        -m test_nec-usb-xhci.py.data/parameters.yaml
+
+    :avocado: enable
+    :avocado: tags=usbstorage
+    """
+
+    def setUp(self):
+        usbdevice = os.path.join(self.workdir, 'usb.img')
+        self.request_image()
+        process.run('dd if=/dev/zero of=%s bs=1M count=10' % usbdevice)
+        self.vm.args.extend(['-device', 'pci-bridge,id=bridge1,chassis_nr=1'])
+        self.vm.args.extend(['-device', 'nec-usb-xhci,id=xhci1,bus=bridge1,addr=0x3'])
+        self.vm.args.extend(['-drive', 'file=%s,format=raw,id=drive_usb,if=none' % usbdevice])
+        self.vm.args.extend(['-device', 'usb-storage,drive=drive_usb,id=device_usb,bus=xhci1.0'])
+        self.vm.launch()
+
+    def test_available_after_migration(self):
+        """
+        According to the RHBZ1436616, the issue is: usb-storage device
+        under pci-bridge is unusable after migration.
+
+        Fixed in commit 243afe858b95765b98d16a1f0dd50dca262858ad.
+
+        :avocado: tags=migration,RHBZ1436616
+        """
+        console = self.vm.get_console()
+        console.sendline('fdisk -l')
+        result = console.read_nonblocking()
+        console.close()
+        self.assertIn('Disk /dev/sdb: 10 MiB, 10485760 bytes, 20480 sectors',
+                      result)
+
+        self.vm_dst = self.vm.migrate()
+        console = self.vm_dst.get_console()
+        console.sendline('fdisk -l')
+        result = console.read_nonblocking()
+        console.close()
+        self.assertIn('Disk /dev/sdb: 10 MiB, 10485760 bytes, 20480 sectors',
+                      result)
+
+    def tearDown(self):
+        self.vm.shutdown()
+        self.vm_dst.shutdown()
diff --git a/tests/avocado/test_nec-usb-xhci.py.data/parameters.yaml b/tests/avocado/test_nec-usb-xhci.py.data/parameters.yaml
new file mode 100644
index 0000000000..37a4e9dc37
--- /dev/null
+++ b/tests/avocado/test_nec-usb-xhci.py.data/parameters.yaml
@@ -0,0 +1,4 @@
+machine_accel: kvm
+image_path: /var/lib/images/fedora-25.img
+image_user: root
+image_pass: p4ssw0rd
-- 
2.14.3

[Qemu-devel] [RFC 16/24] avocado_qemu: Functional test for RHBZ1473203

[Eduardo Habkost]

From: Lukáš Doktor <ldoktor@redhat.com>

Adds regresion test for RHBZ1473203 which runs VM with 16 numa nodes and
then verifies memory allocation is accurate before and after hotplugging
memory into default and 13th node.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/test_numa_hotplug.py | 116 +++++++++++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)
 create mode 100644 tests/avocado/test_numa_hotplug.py

diff --git a/tests/avocado/test_numa_hotplug.py b/tests/avocado/test_numa_hotplug.py
new file mode 100644
index 0000000000..a99b8dcebf
--- /dev/null
+++ b/tests/avocado/test_numa_hotplug.py
@@ -0,0 +1,116 @@
+import re
+import time
+
+from avocado_qemu import test
+
+
+class TestNumaHotplug(test.QemuTest):
+    """
+    Verifies that "info numa" and "/sys/devices/system/node/" contains
+    correct values before/after inserting memory devices into default
+    and then into 13th numa node.
+
+    Associated bug trackers: RHBZ1473203
+        https://bugzilla.redhat.com/show_bug.cgi?id=1473203
+
+    Fixed in kernel commit dc421b200f91930c9c6a9586810ff8c232cf10fc.
+
+    :avocado: enable
+    :avocado: tags=RHBZ1473203,requires_linux,numa,memory,ppc64le
+    """
+
+    def setUp(self):
+        self.request_image()
+        self.vm.args.extend(["-m", "4G,slots=208,maxmem=80G"])
+        self.vm.args.extend(["-numa", "node"] * 16)
+        self.vm.launch()
+
+    def check_mem_console(self, console, exp):
+        """
+        Verifies that memory layout is according to exp using console/ssh
+
+        :param console: session
+        :param exp: list of MemTotals per node in MB, tolerance is +-100MB
+        """
+        out = console.cmd_output_safe("echo /sys/devices/system/node/node*")
+        nodes = re.findall(r"/sys/devices/system/node/node\d+", out)
+        self.assertEqual(len(nodes), len(exp), "Number of nodes is not "
+                         "%s:\n%s" % (len(exp), out))
+        for i in xrange(len(exp)):
+            out = console.cmd_output_safe("cat /sys/devices/system/node/"
+                                          "node%s/meminfo" % i)
+            mem = re.search(r"MemTotal:\s*(\d+) kB", out)
+            self.assertTrue(mem, "Failed to obtain node%s MemTotal:\n%s"
+                            % (i, out))
+            _exp = exp[i] * 1024
+            mem = int(mem.group(1))
+            self.assertGreater(mem, _exp - 102400, "TotalMem of node%s is not "
+                               "%s+-51200 kb (%s)" % (i, _exp, mem))
+            self.assertLess(mem, _exp + 102400, "TotalMem of node%s is not "
+                            "%s+-51200 kb (%s)" % (i, _exp, mem))
+
+    def check_mem_monitor(self, monitor, exp):
+        """
+        Verifies that memory layout is according to exp using QMP monitor
+
+        :param console: session
+        :param exp: list of MemTotals per node in MB, tolerance is +-100MB
+        """
+        ret = monitor("human-monitor-command", command_line="info numa")
+        out = ret["return"]
+        self.assertTrue(out.startswith("%s nodes" % len(exp)), "Number of "
+                        "nodes is not %s:\n%s" % (len(exp), out))
+        for i in xrange(len(exp)):
+            _exp = "node %s size: %s MB" % (i, exp[i])
+            self.assertIn(_exp, out, "%s is not in 'info numa' output, "
+                          "probably wrong memory size reported:\n%s"
+                          % (_exp, out))
+
+    @staticmethod
+    def _retry_until_timeout(timeout, func, *args, **kwargs):
+        """
+        Repeat the function until it returns anything ignoring AssertionError.
+        After the deadline repeate the function one more time without ignoring
+        timeout.
+        """
+        end = time.time() + timeout
+        while time.time() < end:
+            try:
+                ret = func(*args, **kwargs)
+            except AssertionError:
+                continue
+            break
+        else:
+            ret = func(*args, **kwargs)
+        return ret
+
+    def test_hotplug_mem_into_node(self):
+        console = self.vm.get_console()
+        exp = [256] * 16
+        self.check_mem_monitor(self.vm.qmp, exp)
+        self.check_mem_console(console, exp)
+        cmd = "object_add memory-backend-ram,id=mem2,size=1G"
+        res = self.vm.qmp("human-monitor-command", command_line=cmd)
+        self.assertEqual(res["return"], "")
+        cmd = "device_add pc-dimm,id=dimm2,memdev=mem2"
+        res = self.vm.qmp("human-monitor-command", command_line=cmd)
+        self.assertEqual(res["return"], "")
+        exp = [1280] + [256] * 15
+        self.check_mem_monitor(self.vm.qmp, exp)
+        # Wait up to 10s to propagate the changes
+        self._retry_until_timeout(10, self.check_mem_console, console, exp)
+        cmd = "object_add memory-backend-ram,id=mem8,size=1G"
+        res = self.vm.qmp("human-monitor-command", command_line=cmd)
+        self.assertEqual(res["return"], "")
+        cmd = "device_add pc-dimm,id=dimm8,memdev=mem8,node=13"
+        res = self.vm.qmp("human-monitor-command", command_line=cmd)
+        self.assertEqual(res["return"], "")
+        time.sleep(5)
+        exp = [1280] + [256] * 12 + [1280] + [256] * 2
+        self.check_mem_monitor(self.vm.qmp, exp)
+        # Wait up to 10s to propagate the changes
+        self._retry_until_timeout(10, self.check_mem_console, console, exp)
+        console.close()
+
+    def tearDown(self):
+        self.vm.shutdown()
-- 
2.14.3

[Qemu-devel] [RFC 17/24] avocado_qemu: Remove duplicate PortTracker implementation

[Eduardo Habkost]

From: Cleber Rosa <crosa@redhat.com>

The PortTracker class was introduced on Avocado's own utility
libraries, starting with version 54.0, and it actually received fixes
after that.

Let's avoid this duplicate implementation and rely on the standard
one.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/README.rst           |  2 +-
 tests/avocado/avocado_qemu/test.py | 45 +-------------------------------------
 2 files changed, 2 insertions(+), 45 deletions(-)

diff --git a/tests/avocado/README.rst b/tests/avocado/README.rst
index 07852e790a..50bc865fc1 100644
--- a/tests/avocado/README.rst
+++ b/tests/avocado/README.rst
@@ -6,7 +6,7 @@ Framework. To install Avocado, follow the instructions from this link::
 Tests here are written keeping the minimum amount of dependencies. To
 run the tests, you need the Avocado core package (`python-avocado` on
 Fedora, `avocado-framework` on pip). Extra dependencies should be
-documented in this file.
+documented in this file.  The current minimum required version is 54.0.
 
 In this directory, an ``avocado_qemu`` package is provided, containing
 the ``test`` module, which inherits from ``avocado.Test`` and provides
diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index 5e42cac8d0..dfffa2a03b 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -34,7 +34,6 @@ import uuid
 import aexpect
 
 from avocado import Test
-from avocado.utils.data_structures import Borg
 from avocado.utils import network
 from avocado.utils import process
 from avocado.utils import path as utils_path
@@ -223,48 +222,6 @@ def _handle_prompts(session, username, password, prompt, timeout=60,
     return output
 
 
-class _PortTracker(Borg):
-
-    """
-    Tracks ports used in the host machine.
-    """
-
-    def __init__(self):
-        Borg.__init__(self)
-        self.address = 'localhost'
-        self.start_port = 5000
-        if not hasattr(self, 'retained_ports'):
-            self._reset_retained_ports()
-
-    def __str__(self):
-        return 'Ports tracked: %r' % self.retained_ports
-
-    def _reset_retained_ports(self):
-        self.retained_ports = []
-
-    def register_port(self, port):
-        if ((port not in self.retained_ports) and
-                (network.is_port_free(port, self.address))):
-            self.retained_ports.append(port)
-        else:
-            raise ValueError('Port %d in use' % port)
-        return port
-
-    def find_free_port(self, start_port=None):
-        if start_port is None:
-            start_port = self.start_port
-        port = start_port
-        while ((port in self.retained_ports) or
-               (not network.is_port_free(port, self.address))):
-            port += 1
-        self.retained_ports.append(port)
-        return port
-
-    def release_port(self, port):
-        if port in self.retained:
-            self.retained.remove(port)
-
-
 class _VM(qemu.QEMUMachine):
     '''A QEMU VM'''
 
@@ -273,7 +230,7 @@ class _VM(qemu.QEMUMachine):
         if arch is None:
             arch = os.uname()[4]
         self.arch = arch
-        self.ports = _PortTracker()
+        self.ports = network.PortTracker()
         self.name = "qemu-%s" % str(uuid.uuid4())[:8]
         if qemu_bin is None:
             qemu_bin = _get_qemu_bin(arch)
-- 
2.14.3

[Qemu-devel] [RFC 18/24] avocado_qemu: Simplify the installation instructions

[Eduardo Habkost]

From: Cleber Rosa <crosa@redhat.com>

Let's just give a single command line that can be reliably executed to
install avocado and the dependencies we're aware for the existing
tests.

The installation approach chosen is the most universal one (pip) and
one that impacts the least the developer's system, being limited to
the user's home directory.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/README.rst | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/tests/avocado/README.rst b/tests/avocado/README.rst
index 50bc865fc1..e2aa993501 100644
--- a/tests/avocado/README.rst
+++ b/tests/avocado/README.rst
@@ -1,12 +1,23 @@
-This directory is hosting functional tests written using Avocado Testing
-Framework. To install Avocado, follow the instructions from this link::
+========================================
+ QEMU tests using the Avocado Framework
+========================================
+
+This directory hosts functional tests written using Avocado Testing
+Framework.
+
+Installation
+============
+
+To install Avocado and the dependencies needed for these tests, run::
+
+    pip install --user avocado-framework avocado-framework-plugin-varianter-yaml-to-mux aexpect
+
+Alternatively, follow the instructions on this link::
 
     http://avocado-framework.readthedocs.io/en/latest/GetStartedGuide.html#installing-avocado
 
-Tests here are written keeping the minimum amount of dependencies. To
-run the tests, you need the Avocado core package (`python-avocado` on
-Fedora, `avocado-framework` on pip). Extra dependencies should be
-documented in this file.  The current minimum required version is 54.0.
+Overview
+========
 
 In this directory, an ``avocado_qemu`` package is provided, containing
 the ``test`` module, which inherits from ``avocado.Test`` and provides
@@ -77,10 +88,6 @@ file using the Avocado parameters system:
   ``accel`` is ``kvm`` and there are more than one KVM types available.
   Example: ``machine_kvm_type: PR``
 
-To use a parameters file, you have to install the yaml_to_mux plugin
-(`python2-avocado-plugins-varianter-yaml-to-mux` on Fedora,
-`avocado-framework-plugin-varianter-yaml-to-mux` on pip).
-
 Run the test with::
 
     $ avocado run test_my_test.py -m parameters.yaml
@@ -111,3 +118,15 @@ proper variants.
 See ``avocado run --help`` and ``man avocado`` for several other
 options, such as ``--filter-by-tags``, ``--show-job-log``,
 ``--failfast``, etc.
+
+Uninstallation
+==============
+
+If you've followed the installation instructions above, you can easily
+uninstall Avocado.  Start by listing the packages you have installed::
+
+    pip list --user
+
+And remove any package you want with::
+
+    pip uninstall <package_name>
-- 
2.14.3

[Qemu-devel] [RFC 19/24] avocado_qemu: Clean unneeded 'pass'

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

The docstrings are enough. Let's remove the 'pass' instructions.

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index dfffa2a03b..b921151bae 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -49,42 +49,36 @@ class QEMULoginTimeoutError(Exception):
     """
     If timeout expires
     """
-    pass
 
 
 class QEMULoginAuthenticationError(Exception):
     """
     If authentication fails
     """
-    pass
 
 
 class QEMULoginProcessTerminatedError(Exception):
     """
     If the client terminates during login
     """
-    pass
 
 
 class QEMULoginError(Exception):
     """
     If some other error occurs
     """
-    pass
 
 
 class QEMUConsoleError(Exception):
     """
     If some error with the console access happens
     """
-    pass
 
 
 class QEMUMigrationError(Exception):
     """
     If some error with the migration happens
     """
-    pass
 
 
 def _get_qemu_bin(arch):
-- 
2.14.3

[Qemu-devel] [RFC 20/24] avocado_qemu: Set QMP log level to INFO

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

Otherwise we will see the debug messages in our test logs.

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/avocado_qemu/test.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index b921151bae..308fdfa514 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -235,6 +235,7 @@ class _VM(qemu.QEMUMachine):
         self.username = username
         self.password = password
         super(_VM, self).__init__(qemu_bin, name=self.name, arch=arch)
+        logging.getLogger('QMP').setLevel(logging.INFO)
 
     def get_console(self, console_address=None, prompt=r"[\#\$] "):
         """
-- 
2.14.3

[Qemu-devel] [RFC 21/24] avocado_qemu: Introduce the add_image() VM API

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

Uses can not add an image to the virtual machine with the option to
configure the user/password using cloudinit.

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/README.rst           |   4 +-
 tests/avocado/avocado_qemu/test.py | 136 ++++++++++++++++++++++++++-----------
 tests/avocado/parameters.yaml      |   9 ---
 tests/avocado/test_nec-usb-xhci.py |  14 ++--
 tests/avocado/test_numa_hotplug.py |   5 +-
 5 files changed, 111 insertions(+), 57 deletions(-)

diff --git a/tests/avocado/README.rst b/tests/avocado/README.rst
index e2aa993501..a33c4a2577 100644
--- a/tests/avocado/README.rst
+++ b/tests/avocado/README.rst
@@ -78,8 +78,8 @@ file using the Avocado parameters system:
   ``image_snapshot`` parameter.
 - ``image_user`` and ``image_pass``: When using a ``image_path``, if you
   want to get the console from the Guest OS you have to define the Guest
-  OS credentials. Example: ``image_user: root`` and
-  ``image_pass: p4ssw0rd``. By default it uses ``root`` and ``123456``.
+  OS credentials. Example: ``image_user: avocado`` and
+  ``image_pass: p4ssw0rd``. Both parameters have defaults to ``avocado``.
 - ``machine_type``: Use this option to define a machine type for the VM.
   Example: ``machine_type: pc``
 - ``machine_accel``: Use this option to define a machine acceleration
diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
index 308fdfa514..5a08dace45 100644
--- a/tests/avocado/avocado_qemu/test.py
+++ b/tests/avocado/avocado_qemu/test.py
@@ -28,6 +28,7 @@ import logging
 import os
 import re
 import sys
+import tempfile
 import time
 import uuid
 
@@ -81,6 +82,12 @@ class QEMUMigrationError(Exception):
     """
 
 
+class QEMUCloudinitError(Exception):
+    """
+    If some error with the cloudinit happens
+    """
+
+
 def _get_qemu_bin(arch):
     git_root = process.system_output('git rev-parse --show-toplevel',
                                      ignore_status=True,
@@ -219,8 +226,8 @@ def _handle_prompts(session, username, password, prompt, timeout=60,
 class _VM(qemu.QEMUMachine):
     '''A QEMU VM'''
 
-    def __init__(self, qemu_bin=None, arch=None, username=None, password=None,
-                 qemu_dst_bin=None):
+    def __init__(self, qemu_bin=None, arch=None, qemu_dst_bin=None,
+                 username=None, password=None):
         if arch is None:
             arch = os.uname()[4]
         self.arch = arch
@@ -245,8 +252,11 @@ class _VM(qemu.QEMUMachine):
         :param prompt: The regex to identify we reached the prompt.
         """
 
+        if not all((self.username, self.password)):
+            raise QEMULoginError('Username or password not set.')
+
         if not self.is_running():
-            raise QEMUConsoleError('VM is not running.')
+            raise QEMULoginError('VM is not running.')
 
         if console_address is None:
             if self._console_address is None:
@@ -285,9 +295,12 @@ class _VM(qemu.QEMUMachine):
             return False
 
         port = self.ports.find_free_port()
-        newvm = _VM(self.qemu_dst_bin, self._arch, self.username, self.password)
+        newvm = _VM(self.qemu_dst_bin, self._arch, username=self.username,
+                    password=self.password)
         newvm.args = self.args
         newvm.args.extend(['-incoming', 'tcp:0:%s' % port])
+        newvm.username = self.username
+        newvm.password = self.password
 
         newvm.launch(console_address)
         cmd = 'migrate -d tcp:0:%s' % port
@@ -301,6 +314,80 @@ class _VM(qemu.QEMUMachine):
 
         return newvm
 
+    def add_image(self, path, username=None, password=None, cloudinit=False,
+                  snapshot=True, extra=None):
+        """
+        Adds the '-drive' command line option and its parameters to
+        the Qemu VM
+
+        :param path: Image path (i.e. /var/lib/images/guestos.qcow2)
+        :param username: The username to log into the Guest OS with
+        :param password: The password to log into the Guest OS with
+        :param cloudinit: Whether the cloudinit cdrom will be attached to
+                          the image
+        :param snapshot: Whether the parameter snapshot=on will be used
+        :param extra: Extra parameters to the -drive option
+        """
+        file_option = 'file=%s' % path
+        for item in self.args:
+            if file_option in item:
+                logging.error('Image %s already present', path)
+                return
+
+        if extra is not None:
+            file_option += ',%s' % extra
+
+        if snapshot:
+            file_option += ',snapshot=on'
+
+        self.args.extend(['-drive', file_option])
+
+        if username is not None:
+            self.username = username
+
+        if password is not None:
+            self.password = password
+
+        if cloudinit:
+            self._cloudinit()
+
+    def _cloudinit(self):
+        """
+        Creates a CDROM Iso Image with the required cloudinit files
+        (meta-data and user-data) to make the initial Cloud Image
+        configuration, attaching the CDROM to the VM.
+        """
+        try:
+            geniso_bin = utils_path.find_command('genisoimage')
+        except:
+            raise QEMUCloudinitError('Command not found (genisoimage)')
+
+        data_dir = tempfile.mkdtemp()
+
+        metadata_path = os.path.join(data_dir, 'meta-data')
+        metadata_content = ("instance-id: %s\n"
+                            "local-hostname: %s\n" % (self.name, self.name))
+        with open(metadata_path, 'w') as metadata_file:
+            metadata_file.write(metadata_content)
+
+        userdata_path = os.path.join(data_dir, 'user-data')
+        userdata_content = ("#cloud-config\n"
+                            "password: %s\n"
+                            "ssh_pwauth: True\n"
+                            "chpasswd: { expire: False }\n"
+                            "system_info:\n"
+                            "    default_user:\n"
+                            "        name: %s\n" %
+                            (self.password, self.username))
+
+        with open(userdata_path, 'w') as userdata_file:
+            userdata_file.write(userdata_content)
+
+        iso_path = os.path.join(data_dir, 'cdrom.iso')
+        process.run("%s -output %s -volid cidata -joliet -rock %s %s" %
+                    (geniso_bin, iso_path, metadata_path, userdata_path))
+
+        self.args.extend(['-cdrom', iso_path])
 
 class QemuTest(Test):
 
@@ -311,9 +398,11 @@ class QemuTest(Test):
                                        job=job, runner_queue=runner_queue)
         self.vm = _VM(qemu_bin=self.params.get('qemu_bin'),
                       arch=self.params.get('arch'),
-                      username=self.params.get('image_user', default="root"),
-                      password=self.params.get('image_pass', default="123456"),
-                      qemu_dst_bin=self.params.get('qemu_dst_bin'))
+                      qemu_dst_bin=self.params.get('qemu_dst_bin'),
+                      username=self.params.get('image_user',
+                                               default='avocado'),
+                      password=self.params.get('image_pass',
+                                               default='avocado'))
 
         machine_type = self.params.get('machine_type')
         machine_accel = self.params.get('machine_accel')
@@ -327,36 +416,3 @@ class QemuTest(Test):
             machine += "kvm-type=%s," % machine_kvm_type
         if machine:
             self.vm.args.extend(['-machine', machine])
-
-    def request_image(self, path=None, snapshot=None, extra=None):
-        """
-        Add image to the `self.vm` using params or arguments.
-
-        Unless it's overridden by arguments it uses following test params
-        to specify the image:
-
-        * image_path - defines the path to the user-image. If not specified
-                       it uses "QEMU_ROOT/boot_image_$arch.qcow2"
-        * image_snapshot - whether to use "snapshot=on" (snapshot=off is not
-                           supplied)
-        * image_extra - free-form string to extend the "-drive" params
-
-        :param path: Override the path ("image_path" param is used otherwise)
-        :param snapshot: Override the usage of snapshot
-        :param extra: Extra arguments to be added to drive definition
-        """
-        if snapshot is None:
-            snapshot = self.params.get("image_snapshot", default=True)
-        if extra is None:
-            extra = self.params.get("image_extra", default="")
-        if path is None:
-            path = self.params.get("image_path")
-            if path is None:
-                arch = self.vm.arch
-                path = os.path.join(QEMU_ROOT, "boot_image_%s.qcow2" % arch)
-        if not os.path.exists(path):
-            self.error("Require a bootable image, which was not found. "
-                       "Please provide one in '%s'." % path)
-        if snapshot:
-            extra += ",snapshot=on"
-        self.vm.args.extend(['-drive', 'file=%s%s' % (path, extra)])
diff --git a/tests/avocado/parameters.yaml b/tests/avocado/parameters.yaml
index 3c5a0f92e0..03c4ed1416 100644
--- a/tests/avocado/parameters.yaml
+++ b/tests/avocado/parameters.yaml
@@ -10,15 +10,6 @@ qemu_bin: null
 # used in the source VM will be used for the destination VM.
 qemu_dst_bin: null
 
-# VMs are defined without image. If the 'image_path' is specified, it
-# will be used as the VM image. The '-snapshot' option will then be used
-# to avoid writing data to the image.
-image_path: null
-# Username used to get the console from the Guest OS.
-image_user: null
-# Password used to get the console from the Guest OS.
-image_pass: null
-
 # Use this option to define a machine type for the VM.
 machine_type: null
 # Use this option to define a machine acceleration for the VM.
diff --git a/tests/avocado/test_nec-usb-xhci.py b/tests/avocado/test_nec-usb-xhci.py
index 3f0d645032..1c4822544e 100644
--- a/tests/avocado/test_nec-usb-xhci.py
+++ b/tests/avocado/test_nec-usb-xhci.py
@@ -4,6 +4,7 @@ import tempfile
 
 from avocado_qemu import test
 from avocado.utils import process
+from avocado.utils import vmimage
 
 class TestNecUsbXhci(test.QemuTest):
     """
@@ -17,8 +18,10 @@ class TestNecUsbXhci(test.QemuTest):
     """
 
     def setUp(self):
+        self.image = vmimage.get()
+        self.vm.add_image(self.image.path, cloudinit=True, snapshot=False)
+
         usbdevice = os.path.join(self.workdir, 'usb.img')
-        self.request_image()
         process.run('dd if=/dev/zero of=%s bs=1M count=10' % usbdevice)
         self.vm.args.extend(['-device', 'pci-bridge,id=bridge1,chassis_nr=1'])
         self.vm.args.extend(['-device', 'nec-usb-xhci,id=xhci1,bus=bridge1,addr=0x3'])
@@ -35,17 +38,18 @@ class TestNecUsbXhci(test.QemuTest):
 
         :avocado: tags=migration,RHBZ1436616
         """
+
         console = self.vm.get_console()
-        console.sendline('fdisk -l')
-        result = console.read_nonblocking()
+        console.sendline('sudo fdisk -l')
+        result = console.read_up_to_prompt()
         console.close()
         self.assertIn('Disk /dev/sdb: 10 MiB, 10485760 bytes, 20480 sectors',
                       result)
 
         self.vm_dst = self.vm.migrate()
         console = self.vm_dst.get_console()
-        console.sendline('fdisk -l')
-        result = console.read_nonblocking()
+        console.sendline('sudo fdisk -l')
+        result = console.read_up_to_prompt()
         console.close()
         self.assertIn('Disk /dev/sdb: 10 MiB, 10485760 bytes, 20480 sectors',
                       result)
diff --git a/tests/avocado/test_numa_hotplug.py b/tests/avocado/test_numa_hotplug.py
index a99b8dcebf..ee43e60089 100644
--- a/tests/avocado/test_numa_hotplug.py
+++ b/tests/avocado/test_numa_hotplug.py
@@ -2,6 +2,7 @@ import re
 import time
 
 from avocado_qemu import test
+from avocado.utils import vmimage
 
 
 class TestNumaHotplug(test.QemuTest):
@@ -20,7 +21,9 @@ class TestNumaHotplug(test.QemuTest):
     """
 
     def setUp(self):
-        self.request_image()
+        self.image = vmimage.get()
+        self.vm.add_image(self.image.path, cloudinit=True, snapshot=False)
+
         self.vm.args.extend(["-m", "4G,slots=208,maxmem=80G"])
         self.vm.args.extend(["-numa", "node"] * 16)
         self.vm.launch()
-- 
2.14.3

[Qemu-devel] [RFC 22/24] avocado_qemu: Tests fixes

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

- Using acell=KVM when we have a guest.
- Better resilience for the cases where live-migration fails.

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/test_nec-usb-xhci.py | 6 +++++-
 tests/avocado/test_numa_hotplug.py | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/avocado/test_nec-usb-xhci.py b/tests/avocado/test_nec-usb-xhci.py
index 1c4822544e..d3a2716eb4 100644
--- a/tests/avocado/test_nec-usb-xhci.py
+++ b/tests/avocado/test_nec-usb-xhci.py
@@ -18,8 +18,10 @@ class TestNecUsbXhci(test.QemuTest):
     """
 
     def setUp(self):
+        self.vm_dst = None
         self.image = vmimage.get()
         self.vm.add_image(self.image.path, cloudinit=True, snapshot=False)
+        self.vm.args.extend(['-machine', 'accel=kvm'])
 
         usbdevice = os.path.join(self.workdir, 'usb.img')
         process.run('dd if=/dev/zero of=%s bs=1M count=10' % usbdevice)
@@ -56,4 +58,6 @@ class TestNecUsbXhci(test.QemuTest):
 
     def tearDown(self):
         self.vm.shutdown()
-        self.vm_dst.shutdown()
+        if self.vm_dst is not None:
+            self.vm_dst.shutdown()
+        os.remove(self.image.path)
diff --git a/tests/avocado/test_numa_hotplug.py b/tests/avocado/test_numa_hotplug.py
index ee43e60089..1993f9b159 100644
--- a/tests/avocado/test_numa_hotplug.py
+++ b/tests/avocado/test_numa_hotplug.py
@@ -24,6 +24,7 @@ class TestNumaHotplug(test.QemuTest):
         self.image = vmimage.get()
         self.vm.add_image(self.image.path, cloudinit=True, snapshot=False)
 
+        self.vm.args.extend(['-machine', 'accel=kvm'])
         self.vm.args.extend(["-m", "4G,slots=208,maxmem=80G"])
         self.vm.args.extend(["-numa", "node"] * 16)
         self.vm.launch()
-- 
2.14.3

[Qemu-devel] [RFC 23/24] avocado_qemu: Force vmimage distro

[Eduardo Habkost]

From: Amador Pahim <apahim@redhat.com>

Not all distros will support cloudinit. Let's use Fedora, which is
proven to work.

Signed-off-by: Amador Pahim <apahim@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/test_nec-usb-xhci.py | 2 +-
 tests/avocado/test_numa_hotplug.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/avocado/test_nec-usb-xhci.py b/tests/avocado/test_nec-usb-xhci.py
index d3a2716eb4..c29b5ebaa1 100644
--- a/tests/avocado/test_nec-usb-xhci.py
+++ b/tests/avocado/test_nec-usb-xhci.py
@@ -19,7 +19,7 @@ class TestNecUsbXhci(test.QemuTest):
 
     def setUp(self):
         self.vm_dst = None
-        self.image = vmimage.get()
+        self.image = vmimage.get('Fedora')
         self.vm.add_image(self.image.path, cloudinit=True, snapshot=False)
         self.vm.args.extend(['-machine', 'accel=kvm'])
 
diff --git a/tests/avocado/test_numa_hotplug.py b/tests/avocado/test_numa_hotplug.py
index 1993f9b159..256ec0f49f 100644
--- a/tests/avocado/test_numa_hotplug.py
+++ b/tests/avocado/test_numa_hotplug.py
@@ -21,7 +21,7 @@ class TestNumaHotplug(test.QemuTest):
     """
 
     def setUp(self):
-        self.image = vmimage.get()
+        self.image = vmimage.get('Fedora')
         self.vm.add_image(self.image.path, cloudinit=True, snapshot=False)
 
         self.vm.args.extend(['-machine', 'accel=kvm'])
-- 
2.14.3

[Qemu-devel] [RFC 24/24] avocado_qemu: Add a few VNC related tests

[Eduardo Habkost]

From: Cleber Rosa <crosa@redhat.com>

These exercise the basics of firing up QEMU with and without
a VNC device, and attempting to set the password when a VNC
device and password is given (or not).

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/avocado/test_vnc.py | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 tests/avocado/test_vnc.py

diff --git a/tests/avocado/test_vnc.py b/tests/avocado/test_vnc.py
new file mode 100644
index 0000000000..f5fe26ead6
--- /dev/null
+++ b/tests/avocado/test_vnc.py
@@ -0,0 +1,58 @@
+from avocado import main
+from avocado_qemu import test
+
+
+class Vnc(test.QemuTest):
+    """
+    :avocado: enable
+    :avocado: tags=vnc,quick
+    """
+    def test_no_vnc(self):
+        self.vm.args = ['-nodefaults', '-S']
+        self.vm.launch()
+        self.assertFalse(self.vm.qmp('query-vnc')['return']['enabled'])
+
+    def test_no_vnc_change_password(self):
+        self.vm.args = ['-nodefaults', '-S']
+        self.vm.launch()
+        self.assertFalse(self.vm.qmp('query-vnc')['return']['enabled'])
+        set_password_response = self.vm.qmp('change',
+                                            device='vnc',
+                                            target='password',
+                                            arg='new_password')
+        self.assertIn('error', set_password_response)
+        self.assertEqual(set_password_response['error']['class'],
+                         'GenericError')
+        self.assertEqual(set_password_response['error']['desc'],
+                         'Could not set password')
+
+    def test_vnc_change_password_requires_a_password(self):
+        self.vm.args = ['-nodefaults', '-S', '-vnc', ':0']
+        self.vm.launch()
+        self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
+        set_password_response = self.vm.qmp('change',
+                                            device='vnc',
+                                            target='password',
+                                            arg='new_password')
+        self.assertIn('error', set_password_response)
+        self.assertEqual(set_password_response['error']['class'],
+                         'GenericError')
+        self.assertEqual(set_password_response['error']['desc'],
+                         'Could not set password')
+
+    def test_vnc_change_password(self):
+        self.vm.args = ['-nodefaults', '-S', '-vnc', ':0,password']
+        self.vm.launch()
+        self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
+        set_password_response = self.vm.qmp('change',
+                                            device='vnc',
+                                            target='password',
+                                            arg='new_password')
+        self.assertEqual(set_password_response['return'], {})
+
+    def tearDown(self):
+        self.vm.shutdown()
+
+
+if __name__ == '__main__':
+    main()
-- 
2.14.3

Re: [Qemu-devel] [RFC 00/24] Avocado-based functional tests

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180420181951.7252-1-ehabkost@redhat.com
Subject: [Qemu-devel] [RFC 00/24] Avocado-based functional tests

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20180420181951.7252-1-ehabkost@redhat.com -> patchew/20180420181951.7252-1-ehabkost@redhat.com
Switched to a new branch 'test'
ec49778cf4 avocado_qemu: Add a few VNC related tests
1f46f502ab avocado_qemu: Force vmimage distro
3c652684e4 avocado_qemu: Tests fixes
c4e2d71898 avocado_qemu: Introduce the add_image() VM API
35b9856b7d avocado_qemu: Set QMP log level to INFO
3d81ee79d0 avocado_qemu: Clean unneeded 'pass'
3fea592db2 avocado_qemu: Simplify the installation instructions
d1b9960835 avocado_qemu: Remove duplicate PortTracker implementation
030c3444fa avocado_qemu: Functional test for RHBZ1473203
4452f51949 avocado_qemu: Functional test for RHBZ#1436616
a791819b84 avocado_qemu: Functional test for RHBZ#1447027
1b525617c2 avocado_qemu: Functional test for RHBZ#1431939
8cb3174c7d avocado_qemu: Improve migration error message
800fdc0407 avocado_qemu: Fix exception name in caller
487b76eeea avocado_qemu: Add support to request image for testing
2349952ea0 avocado_qemu: Ignore kernel messages on get_console
ee1ba766ab avocado_qemu: Provide defaults for user and pass
2151fc0647 avocado_qemu: Store "arch" in VM
27d897fc64 avocado_qemu: Add " " after the default prompt regexp
1b31385780 avocado_qemu: Increase the login timeout to 60s
f964227931 avocado_qemu: Be lenient towards poluted serial console
903c5ccd71 avocado_qemu: Improve handle_prompts to allow login after booted vm
46e5bdc04f Introduce the basic framework to run Avocado tests
9a629a9861 qemu.py: Introduce _create_console() method

=== OUTPUT BEGIN ===
Checking PATCH 1/24: qemu.py: Introduce _create_console() method...
Checking PATCH 2/24: Introduce the basic framework to run Avocado tests...
WARNING: line over 80 characters
#311: FILE: tests/avocado/avocado_qemu/test.py:118:
+    :raise QEMULoginProcessTerminatedError: If the client terminates during login

WARNING: line over 80 characters
#399: FILE: tests/avocado/avocado_qemu/test.py:206:
+            raise QEMULoginProcessTerminatedError(details.status, details.output)

total: 0 errors, 2 warnings, 661 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/24: avocado_qemu: Improve handle_prompts to allow login after booted vm...
Checking PATCH 4/24: avocado_qemu: Be lenient towards poluted serial console...
Checking PATCH 5/24: avocado_qemu: Increase the login timeout to 60s...
Checking PATCH 6/24: avocado_qemu: Add " " after the default prompt regexp...
Checking PATCH 7/24: avocado_qemu: Store "arch" in VM...
Checking PATCH 8/24: avocado_qemu: Provide defaults for user and pass...
Checking PATCH 9/24: avocado_qemu: Ignore kernel messages on get_console...
Checking PATCH 10/24: avocado_qemu: Add support to request image for testing...
Checking PATCH 11/24: avocado_qemu: Fix exception name in caller...
Checking PATCH 12/24: avocado_qemu: Improve migration error message...
Checking PATCH 13/24: avocado_qemu: Functional test for RHBZ#1431939...
WARNING: line over 80 characters
#73: FILE: tests/avocado/test_info_memdev_host_nodes.py:49:
+        cmd = 'object_add memory-backend-ram,id=mem1,host-nodes=0,size=2G,policy=bind'

total: 0 errors, 1 warnings, 66 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 14/24: avocado_qemu: Functional test for RHBZ#1447027...
ERROR: line over 90 characters
#51: FILE: tests/avocado/test_ovmf_with_240_vcpus.py:22:
+                                         default='/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd')

WARNING: line over 80 characters
#53: FILE: tests/avocado/test_ovmf_with_240_vcpus.py:24:
+                                         default='/usr/share/edk2/ovmf/OVMF_VARS.fd')

total: 1 errors, 1 warnings, 72 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 15/24: avocado_qemu: Functional test for RHBZ#1436616...
WARNING: line over 80 characters
#53: FILE: tests/avocado/test_nec-usb-xhci.py:24:
+        self.vm.args.extend(['-device', 'nec-usb-xhci,id=xhci1,bus=bridge1,addr=0x3'])

ERROR: line over 90 characters
#54: FILE: tests/avocado/test_nec-usb-xhci.py:25:
+        self.vm.args.extend(['-drive', 'file=%s,format=raw,id=drive_usb,if=none' % usbdevice])

ERROR: line over 90 characters
#55: FILE: tests/avocado/test_nec-usb-xhci.py:26:
+        self.vm.args.extend(['-device', 'usb-storage,drive=drive_usb,id=device_usb,bus=xhci1.0'])

total: 2 errors, 1 warnings, 59 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 16/24: avocado_qemu: Functional test for RHBZ1473203...
Checking PATCH 17/24: avocado_qemu: Remove duplicate PortTracker implementation...
Checking PATCH 18/24: avocado_qemu: Simplify the installation instructions...
Checking PATCH 19/24: avocado_qemu: Clean unneeded 'pass'...
Checking PATCH 20/24: avocado_qemu: Set QMP log level to INFO...
Checking PATCH 21/24: avocado_qemu: Introduce the add_image() VM API...
Checking PATCH 22/24: avocado_qemu: Tests fixes...
Checking PATCH 23/24: avocado_qemu: Force vmimage distro...
Checking PATCH 24/24: avocado_qemu: Add a few VNC related tests...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [RFC 01/24] qemu.py: Introduce _create_console() method

[Eduardo Habkost]

On Fri, Apr 20, 2018 at 03:19:28PM -0300, Eduardo Habkost wrote:
> From: Amador Pahim <apahim@redhat.com>
> 
> This patch adds the QEMUMachine._create_console() method, which
> returns a list with the chardev console device arguments to be
> used in the qemu command line.
> 
> Signed-off-by: Amador Pahim <apahim@redhat.com>
> [ehabkost: reword commit message]
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  scripts/qemu.py | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 08a3e9af5a..9e9d502543 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -55,7 +55,7 @@ class QEMUMachine(object):
>  
>      def __init__(self, binary, args=None, wrapper=None, name=None,
>                   test_dir="/var/tmp", monitor_address=None,
> -                 socket_scm_helper=None):
> +                 socket_scm_helper=None, arch=None):
>          '''
>          Initialize a QEMUMachine
>  
> @@ -91,6 +91,10 @@ class QEMUMachine(object):
>          self._test_dir = test_dir
>          self._temp_dir = None
>          self._launched = False
> +        if arch is None:
> +            arch = binary.split('-')[-1]

This is not good.  We don't want a test case to break only
because we renamed a QEMU binary (e.g. RHEL uses
/usr/libexec/qemu-kvm, and I can imagine people using qemu.py to
write test scripts for it).


> +        self._arch = arch
> +        self._console_address = None
>  
>          # just in case logging wasn't configured by the main script:
>          logging.basicConfig()
> @@ -179,6 +183,39 @@ class QEMUMachine(object):
>                  '-mon', 'chardev=mon,mode=control',
>                  '-display', 'none', '-vga', 'none']
>  
> +    def _create_console(self, console_address):
> +        for item in self._args:
> +            for option in ['isa-serial', 'spapr-vty', 'sclpconsole']:
> +                if option in item:
> +                    return []

This is very fragile.  What's the goal of this chunk of code?


> +
> +        chardev = 'socket,id=console,{address},server,nowait'
> +        if console_address is None:
> +            console_address = tempfile.mktemp()
> +            chardev = chardev.format(address='path=%s' %
> +                                     console_address)
> +        elif isinstance(console_address, tuple):
> +            chardev = chardev.format(address='host=%s,port=%s' %
> +                                     (console_address[0],
> +                                     console_address[1]))
> +        else:
> +            chardev = chardev.format(address='path=%s' % console_address)
> +
> +        self._console_address = console_address
> +
> +        device = '{dev_type},chardev=console'
> +        if '86' in self._arch:
> +            device = device.format(dev_type='isa-serial')
> +        elif 'ppc' in self._arch:
> +            device = device.format(dev_type='spapr-vty')
> +        elif 's390x' in self._arch:
> +            device = device.format(dev_type='sclpconsole')

Why do we need this? Why isn't -serial enough?


> +        else:
> +            return []
> +
> +        return ['-chardev', chardev,
> +                '-device', device]
> +
>      def _pre_launch(self):
>          self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
>          if self._monitor_address is not None:
> @@ -206,7 +243,7 @@ class QEMUMachine(object):
>              shutil.rmtree(self._temp_dir)
>              self._temp_dir = None
>  
> -    def launch(self):
> +    def launch(self, console_address=None):
>          """
>          Launch the VM and make sure we cleanup and expose the
>          command line/output in case of exception
> @@ -218,7 +255,7 @@ class QEMUMachine(object):
>          self._iolog = None
>          self._qemu_full_args = None
>          try:
> -            self._launch()
> +            self._launch(console_address)
>              self._launched = True
>          except:
>              self.shutdown()
> @@ -230,12 +267,14 @@ class QEMUMachine(object):
>                  LOG.debug('Output: %r', self._iolog)
>              raise
>  
> -    def _launch(self):
> +    def _launch(self, console_address):
>          '''Launch the VM and establish a QMP connection'''
>          devnull = open(os.path.devnull, 'rb')
>          self._pre_launch()
> +        bargs = self._base_args()
> +        bargs.extend(self._create_console(console_address))
>          self._qemu_full_args = (self._wrapper + [self._binary] +
> -                                self._base_args() + self._args)
> +                                bargs + self.args)
>          self._popen = subprocess.Popen(self._qemu_full_args,
>                                         stdin=devnull,
>                                         stdout=self._qemu_log_file,
> -- 
> 2.14.3
> 

-- 
Eduardo

Re: [Qemu-devel] [RFC 02/24] Introduce the basic framework to run Avocado tests

[Eduardo Habkost]

On Fri, Apr 20, 2018 at 03:19:29PM -0300, Eduardo Habkost wrote:
> From: Amador Pahim <apahim@redhat.com>
> 
> Avocado Testing Framework can help with functional tests in
> qemu development process. This patch creates the basic infrastructure
> to use with Avocado tests. It contains:
> 
> - A README file with the initial documentation.
> - The test module which inherits from avocado.Test and adds a VM object,
>   created from scripts/qemu.py module. The QemuTest class is the test
>   API for the actual Qemu tests to inherit from.
> - A parameters yaml file with the supported keys.
> - A variants yaml file with the Qemu supported architectures.
> 
> After this commit, you can expect a series of real-world tests, written
> using this new framework.
> 
> Signed-off-by: Amador Pahim <apahim@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  scripts/qemu.py                        |  12 +-
>  tests/avocado/README.rst               | 111 ++++++++++
>  tests/avocado/avocado_qemu/__init__.py |   0
>  tests/avocado/avocado_qemu/test.py     | 365 +++++++++++++++++++++++++++++++++
>  tests/avocado/parameters.yaml          |  28 +++
>  tests/avocado/variants.yaml            |  62 ++++++
>  tests/qemu-iotests/iotests.py          |  28 +--
>  7 files changed, 586 insertions(+), 20 deletions(-)
>  create mode 100644 tests/avocado/README.rst
>  create mode 100644 tests/avocado/avocado_qemu/__init__.py
>  create mode 100644 tests/avocado/avocado_qemu/test.py
>  create mode 100644 tests/avocado/parameters.yaml
>  create mode 100644 tests/avocado/variants.yaml
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 9e9d502543..bd66620f45 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -81,7 +81,7 @@ class QEMUMachine(object):
>          self._qemu_log_file = None
>          self._popen = None
>          self._binary = binary
> -        self._args = list(args)     # Force copy args in case we modify them
> +        self.args = list(args)     # Force copy args in case we modify them

This should go to a separate patch.

Also, I would prefer a self.add_args(...) interface, instead of
exposing self.args to the outside directly.  I remember some past
discussions about this, but I don't remember what was the
conclusion.

I will review tests/avocado/* later.

>          self._wrapper = wrapper
>          self._events = []
>          self._iolog = None
> @@ -109,8 +109,8 @@ class QEMUMachine(object):
>      # This can be used to add an unused monitor instance.
>      def add_monitor_telnet(self, ip, port):
>          args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port)
> -        self._args.append('-monitor')
> -        self._args.append(args)
> +        self.args.append('-monitor')
> +        self.args.append(args)
>  
>      def add_fd(self, fd, fdset, opaque, opts=''):
>          '''Pass a file descriptor to the VM'''
> @@ -120,8 +120,8 @@ class QEMUMachine(object):
>          if opts:
>              options.append(opts)
>  
> -        self._args.append('-add-fd')
> -        self._args.append(','.join(options))
> +        self.args.append('-add-fd')
> +        self.args.append(','.join(options))
>          return self
>  
>      def send_fd_scm(self, fd_file_path):
> @@ -184,7 +184,7 @@ class QEMUMachine(object):
>                  '-display', 'none', '-vga', 'none']
>  
>      def _create_console(self, console_address):
> -        for item in self._args:
> +        for item in self.args:
>              for option in ['isa-serial', 'spapr-vty', 'sclpconsole']:
>                  if option in item:
>                      return []
[...]

-- 
Eduardo

Re: [Qemu-devel] [RFC 20/24] avocado_qemu: Set QMP log level to INFO

[Eduardo Habkost]

On Fri, Apr 20, 2018 at 03:19:47PM -0300, Eduardo Habkost wrote:
> From: Amador Pahim <apahim@redhat.com>
> 
> Otherwise we will see the debug messages in our test logs.

Why don't we want to see the QMP debug messages when debugging is
enabled?

> 
> Signed-off-by: Amador Pahim <apahim@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  tests/avocado/avocado_qemu/test.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py
> index b921151bae..308fdfa514 100644
> --- a/tests/avocado/avocado_qemu/test.py
> +++ b/tests/avocado/avocado_qemu/test.py
> @@ -235,6 +235,7 @@ class _VM(qemu.QEMUMachine):
>          self.username = username
>          self.password = password
>          super(_VM, self).__init__(qemu_bin, name=self.name, arch=arch)
> +        logging.getLogger('QMP').setLevel(logging.INFO)
>  
>      def get_console(self, console_address=None, prompt=r"[\#\$] "):
>          """
> -- 
> 2.14.3
> 

-- 
Eduardo

Re: [Qemu-devel] [RFC 01/24] qemu.py: Introduce _create_console() method

[Thomas Huth]

On 20.04.2018 21:56, Eduardo Habkost wrote:
> On Fri, Apr 20, 2018 at 03:19:28PM -0300, Eduardo Habkost wrote:
>> From: Amador Pahim <apahim@redhat.com>
>>
>> This patch adds the QEMUMachine._create_console() method, which
>> returns a list with the chardev console device arguments to be
>> used in the qemu command line.
>>
>> Signed-off-by: Amador Pahim <apahim@redhat.com>
>> [ehabkost: reword commit message]
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>  scripts/qemu.py | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 44 insertions(+), 5 deletions(-)
>>
>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>> index 08a3e9af5a..9e9d502543 100644
>> --- a/scripts/qemu.py
>> +++ b/scripts/qemu.py
>> @@ -55,7 +55,7 @@ class QEMUMachine(object):
[...]
>> +        chardev = 'socket,id=console,{address},server,nowait'
>> +        if console_address is None:
>> +            console_address = tempfile.mktemp()
>> +            chardev = chardev.format(address='path=%s' %
>> +                                     console_address)
>> +        elif isinstance(console_address, tuple):
>> +            chardev = chardev.format(address='host=%s,port=%s' %
>> +                                     (console_address[0],
>> +                                     console_address[1]))
>> +        else:
>> +            chardev = chardev.format(address='path=%s' % console_address)
>> +
>> +        self._console_address = console_address
>> +
>> +        device = '{dev_type},chardev=console'
>> +        if '86' in self._arch:
>> +            device = device.format(dev_type='isa-serial')
>> +        elif 'ppc' in self._arch:
>> +            device = device.format(dev_type='spapr-vty')
>> +        elif 's390x' in self._arch:
>> +            device = device.format(dev_type='sclpconsole')
> 
> Why do we need this? Why isn't -serial enough?

AFAIK, at least on s390x, -serial is not implemented and you need to
specify a sclpconsole device.

 Thomas

Re: [Qemu-devel] [RFC 01/24] qemu.py: Introduce _create_console() method

[Eduardo Habkost]

On Mon, Apr 23, 2018 at 05:26:13AM +0200, Thomas Huth wrote:
> On 20.04.2018 21:56, Eduardo Habkost wrote:
> > On Fri, Apr 20, 2018 at 03:19:28PM -0300, Eduardo Habkost wrote:
> >> From: Amador Pahim <apahim@redhat.com>
> >>
> >> This patch adds the QEMUMachine._create_console() method, which
> >> returns a list with the chardev console device arguments to be
> >> used in the qemu command line.
> >>
> >> Signed-off-by: Amador Pahim <apahim@redhat.com>
> >> [ehabkost: reword commit message]
> >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >> ---
> >>  scripts/qemu.py | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
> >>  1 file changed, 44 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/scripts/qemu.py b/scripts/qemu.py
> >> index 08a3e9af5a..9e9d502543 100644
> >> --- a/scripts/qemu.py
> >> +++ b/scripts/qemu.py
> >> @@ -55,7 +55,7 @@ class QEMUMachine(object):
> [...]
> >> +        chardev = 'socket,id=console,{address},server,nowait'
> >> +        if console_address is None:
> >> +            console_address = tempfile.mktemp()
> >> +            chardev = chardev.format(address='path=%s' %
> >> +                                     console_address)
> >> +        elif isinstance(console_address, tuple):
> >> +            chardev = chardev.format(address='host=%s,port=%s' %
> >> +                                     (console_address[0],
> >> +                                     console_address[1]))
> >> +        else:
> >> +            chardev = chardev.format(address='path=%s' % console_address)
> >> +
> >> +        self._console_address = console_address
> >> +
> >> +        device = '{dev_type},chardev=console'
> >> +        if '86' in self._arch:
> >> +            device = device.format(dev_type='isa-serial')
> >> +        elif 'ppc' in self._arch:
> >> +            device = device.format(dev_type='spapr-vty')
> >> +        elif 's390x' in self._arch:
> >> +            device = device.format(dev_type='sclpconsole')
> > 
> > Why do we need this? Why isn't -serial enough?
> 
> AFAIK, at least on s390x, -serial is not implemented and you need to
> specify a sclpconsole device.

It sounds like it would be useful if it did implement it.  Does
anybody see any drawbacks?

-- 
Eduardo

Re: [Qemu-devel] [RFC 13/24] avocado_qemu: Functional test for RHBZ#1431939

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 895 bytes --]

On Fri, Apr 20, 2018 at 03:19:40PM -0300, Eduardo Habkost wrote:
> +    def test_hotplug_memory_default_policy(self):
> +        """
> +        According to the RHBZ1431939, the issue is 'host nodes'
> +        returning '128'. It should return empty value when memory
> +        hotplug default policy is used.
> +
> +        Fixed in commit d81d857f4421d205395d55200425daa6591c28a5.
> +        :avocado: tags=RHBZ1431939
> +        """
> +
> +        cmd = 'object_add memory-backend-ram,id=mem1,size=1G'
> +        res = self.vm.qmp('human-monitor-command', command_line=cmd)
> +        self.assertEqual('', res['return'])

General question about QMP test coding style:

What happens if res['return'] does not exist because the QMP command
failed?

I tend to use dict.get() to prevent KeyError.  That way the
assertEqual() will fail instead of an unhandled KeyError in the test
code.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [RFC 13/24] avocado_qemu: Functional test for RHBZ#1431939

[Eduardo Habkost]

On Mon, Apr 30, 2018 at 02:02:46PM +0100, Stefan Hajnoczi wrote:
> On Fri, Apr 20, 2018 at 03:19:40PM -0300, Eduardo Habkost wrote:
> > +    def test_hotplug_memory_default_policy(self):
> > +        """
> > +        According to the RHBZ1431939, the issue is 'host nodes'
> > +        returning '128'. It should return empty value when memory
> > +        hotplug default policy is used.
> > +
> > +        Fixed in commit d81d857f4421d205395d55200425daa6591c28a5.
> > +        :avocado: tags=RHBZ1431939
> > +        """
> > +
> > +        cmd = 'object_add memory-backend-ram,id=mem1,size=1G'
> > +        res = self.vm.qmp('human-monitor-command', command_line=cmd)
> > +        self.assertEqual('', res['return'])
> 
> General question about QMP test coding style:
> 
> What happens if res['return'] does not exist because the QMP command
> failed?
> 
> I tend to use dict.get() to prevent KeyError.  That way the
> assertEqual() will fail instead of an unhandled KeyError in the test
> code.

It looks like vm.command() would be appropriate on most cases, as
it will check for errors and return res['result'] automatically.

vm.qmp() seems to be useful only if you really don't want an
exception to be raised in case of QMP errors.

Maybe we should rename .qmp() to .raw_qmp() to discourage people
from using it.

-- 
Eduardo

Re: [Qemu-devel] [RFC 13/24] avocado_qemu: Functional test for RHBZ#1431939

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]

On Mon, May 07, 2018 at 11:03:29AM -0300, Eduardo Habkost wrote:
> On Mon, Apr 30, 2018 at 02:02:46PM +0100, Stefan Hajnoczi wrote:
> > On Fri, Apr 20, 2018 at 03:19:40PM -0300, Eduardo Habkost wrote:
> > > +    def test_hotplug_memory_default_policy(self):
> > > +        """
> > > +        According to the RHBZ1431939, the issue is 'host nodes'
> > > +        returning '128'. It should return empty value when memory
> > > +        hotplug default policy is used.
> > > +
> > > +        Fixed in commit d81d857f4421d205395d55200425daa6591c28a5.
> > > +        :avocado: tags=RHBZ1431939
> > > +        """
> > > +
> > > +        cmd = 'object_add memory-backend-ram,id=mem1,size=1G'
> > > +        res = self.vm.qmp('human-monitor-command', command_line=cmd)
> > > +        self.assertEqual('', res['return'])
> > 
> > General question about QMP test coding style:
> > 
> > What happens if res['return'] does not exist because the QMP command
> > failed?
> > 
> > I tend to use dict.get() to prevent KeyError.  That way the
> > assertEqual() will fail instead of an unhandled KeyError in the test
> > code.
> 
> It looks like vm.command() would be appropriate on most cases, as
> it will check for errors and return res['result'] automatically.
> 
> vm.qmp() seems to be useful only if you really don't want an
> exception to be raised in case of QMP errors.
> 
> Maybe we should rename .qmp() to .raw_qmp() to discourage people
> from using it.

Sounds good.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [RFC 01/24] qemu.py: Introduce _create_console() method

[Cleber Rosa]

On 04/20/2018 03:56 PM, Eduardo Habkost wrote:
> On Fri, Apr 20, 2018 at 03:19:28PM -0300, Eduardo Habkost wrote:
>> From: Amador Pahim <apahim@redhat.com>
>>
>> This patch adds the QEMUMachine._create_console() method, which
>> returns a list with the chardev console device arguments to be
>> used in the qemu command line.
>>
>> Signed-off-by: Amador Pahim <apahim@redhat.com>
>> [ehabkost: reword commit message]
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>  scripts/qemu.py | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 44 insertions(+), 5 deletions(-)
>>
>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>> index 08a3e9af5a..9e9d502543 100644
>> --- a/scripts/qemu.py
>> +++ b/scripts/qemu.py
>> @@ -55,7 +55,7 @@ class QEMUMachine(object):
>>  
>>      def __init__(self, binary, args=None, wrapper=None, name=None,
>>                   test_dir="/var/tmp", monitor_address=None,
>> -                 socket_scm_helper=None):
>> +                 socket_scm_helper=None, arch=None):
>>          '''
>>          Initialize a QEMUMachine
>>  
>> @@ -91,6 +91,10 @@ class QEMUMachine(object):
>>          self._test_dir = test_dir
>>          self._temp_dir = None
>>          self._launched = False
>> +        if arch is None:
>> +            arch = binary.split('-')[-1]
> 
> This is not good.  We don't want a test case to break only
> because we renamed a QEMU binary (e.g. RHEL uses
> /usr/libexec/qemu-kvm, and I can imagine people using qemu.py to
> write test scripts for it).
> 
> 

Agreed.  This is a simple but fragile way of getting the arch for a
binary.  How about getting this from the QEMU binary itself using a
"query-target" QMP command?  This would, for the sake of simplicity,
require a separate QEMU execution, though.

But an even more important usability issue is this "automagic" behavior.
 While it may be useful for higher level APIs, such as
"avocado_qemu.Test", IMO it should be explicitly enabled, and current
behavior should be preserved.  Only by doing something like

   m = QEMUMachine("/path/to/binary", automatic_devices=True)

Devices based on the architecture (and machine type?) would be added by
default.  Naming is hard, so suggestions for the option name are
appreciated.

>> +        self._arch = arch
>> +        self._console_address = None
>>  
>>          # just in case logging wasn't configured by the main script:
>>          logging.basicConfig()
>> @@ -179,6 +183,39 @@ class QEMUMachine(object):
>>                  '-mon', 'chardev=mon,mode=control',
>>                  '-display', 'none', '-vga', 'none']
>>  
>> +    def _create_console(self, console_address):
>> +        for item in self._args:
>> +            for option in ['isa-serial', 'spapr-vty', 'sclpconsole']:
>> +                if option in item:
>> +                    return []
> 
> This is very fragile.  What's the goal of this chunk of code?
> 
> 

Agreed again.

>> +
>> +        chardev = 'socket,id=console,{address},server,nowait'
>> +        if console_address is None:
>> +            console_address = tempfile.mktemp()
>> +            chardev = chardev.format(address='path=%s' %
>> +                                     console_address)
>> +        elif isinstance(console_address, tuple):
>> +            chardev = chardev.format(address='host=%s,port=%s' %
>> +                                     (console_address[0],
>> +                                     console_address[1]))
>> +        else:
>> +            chardev = chardev.format(address='path=%s' % console_address)
>> +
>> +        self._console_address = console_address
>> +
>> +        device = '{dev_type},chardev=console'
>> +        if '86' in self._arch:
>> +            device = device.format(dev_type='isa-serial')
>> +        elif 'ppc' in self._arch:
>> +            device = device.format(dev_type='spapr-vty')

In my executions with both ppc and ppc64, it failed with ppc not
supporting spapr-vty.  I'd go with exact arch matches, and not
approximations.

>> +        elif 's390x' in self._arch:
>> +            device = device.format(dev_type='sclpconsole')

And this is fragile because s390x won't allow for more than one operator
console.  Exact qemu-system-s390x output is:

   Multiple VT220 operator consoles are not supported
   SCLP event initialization failed.

> 
> Why do we need this? Why isn't -serial enough?
> 
> 

No, some arches need special handling, but this is indeed too fragile.

>> +        else:
>> +            return []
>> +
>> +        return ['-chardev', chardev,
>> +                '-device', device]
>> +
>>      def _pre_launch(self):
>>          self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
>>          if self._monitor_address is not None:
>> @@ -206,7 +243,7 @@ class QEMUMachine(object):
>>              shutil.rmtree(self._temp_dir)
>>              self._temp_dir = None
>>  
>> -    def launch(self):
>> +    def launch(self, console_address=None):

Querying the console address should be available to users, but I don't
see the reason for making it a launch() argument.

>>          """
>>          Launch the VM and make sure we cleanup and expose the
>>          command line/output in case of exception
>> @@ -218,7 +255,7 @@ class QEMUMachine(object):
>>          self._iolog = None
>>          self._qemu_full_args = None
>>          try:
>> -            self._launch()
>> +            self._launch(console_address)
>>              self._launched = True
>>          except:
>>              self.shutdown()
>> @@ -230,12 +267,14 @@ class QEMUMachine(object):
>>                  LOG.debug('Output: %r', self._iolog)
>>              raise
>>  
>> -    def _launch(self):
>> +    def _launch(self, console_address):
>>          '''Launch the VM and establish a QMP connection'''
>>          devnull = open(os.path.devnull, 'rb')
>>          self._pre_launch()
>> +        bargs = self._base_args()
>> +        bargs.extend(self._create_console(console_address))
>>          self._qemu_full_args = (self._wrapper + [self._binary] +
>> -                                self._base_args() + self._args)
>> +                                bargs + self.args)


With the addition of "automagic" devices, I'd propose three internal
argument categories:

 1) base args (self._base_args()), which will always be necessary and
created by QEMUMachine.
 2) auto args, always empty if QEMUMachine() is not given
automatic_devices=True.
 3) user args (self._args)

- Cleber.

>>          self._popen = subprocess.Popen(self._qemu_full_args,
>>                                         stdin=devnull,
>>                                         stdout=self._qemu_log_file,
>> -- 
>> 2.14.3
>>
>