[Qemu-devel] [RFC PATCH v2 0/3] python: refactor qemu/__init__.py

[Qemu-devel] [RFC PATCH v2 0/3] python: refactor qemu/__init__.py

[John Snow]

There's a lot of code hiding in what is ostensibly a package
configuration file. Let's break that out into something more visible.

V2:
 - Retain python 2.7+ compatibility :(
 - Refactor shutdown a little more aggressively

John Snow (3):
  python/qemu: split QEMUMachine out from underneath __init__.py
  machine.py: minor delinting
  QEMUMachine: Don't suppress stack traces on close

 python/qemu/__init__.py                   | 502 +-------------------
 python/qemu/machine.py                    | 536 ++++++++++++++++++++++
 python/qemu/qtest.py                      |   2 +-
 scripts/device-crash-test                 |   2 +-
 scripts/render_block_graph.py             |   2 +-
 tests/acceptance/avocado_qemu/__init__.py |   2 +-
 tests/acceptance/virtio_version.py        |   2 +-
 tests/migration/guestperf/engine.py       |  22 +-
 tests/qemu-iotests/235                    |   2 +-
 tests/vm/basevm.py                        |   3 +-
 10 files changed, 556 insertions(+), 519 deletions(-)
 create mode 100644 python/qemu/machine.py

-- 
2.21.0

[Qemu-devel] [RFC PATCH v2 1/3] python/qemu: split QEMUMachine out from underneath __init__.py

[John Snow]

It's not obvious that something named __init__.py actually houses
important code that isn't relevant to python packaging glue. Move the
QEMUMachine and related error classes out into their own module.

Adjust users to the new import location.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/__init__.py                   | 502 +--------------------
 python/qemu/machine.py                    | 520 ++++++++++++++++++++++
 python/qemu/qtest.py                      |   2 +-
 scripts/device-crash-test                 |   2 +-
 scripts/render_block_graph.py             |   2 +-
 tests/acceptance/avocado_qemu/__init__.py |   2 +-
 tests/acceptance/virtio_version.py        |   2 +-
 tests/migration/guestperf/engine.py       |  22 +-
 tests/qemu-iotests/235                    |   2 +-
 tests/vm/basevm.py                        |   3 +-
 10 files changed, 540 insertions(+), 519 deletions(-)
 create mode 100644 python/qemu/machine.py

diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
index dbaf8a5311..6c919a3d56 100644
--- a/python/qemu/__init__.py
+++ b/python/qemu/__init__.py
@@ -12,17 +12,11 @@
 # Based on qmp.py.
 #
 
-import errno
 import logging
 import os
-import subprocess
-import re
-import shutil
-import socket
-import tempfile
 
 from . import qmp
-
+from . import machine
 
 LOG = logging.getLogger(__name__)
 
@@ -39,497 +33,3 @@ def kvm_available(target_arch=None):
         if target_arch != ADDITIONAL_ARCHES.get(host_arch):
             return False
     return os.access("/dev/kvm", os.R_OK | os.W_OK)
-
-
-class QEMUMachineError(Exception):
-    """
-    Exception called when an error in QEMUMachine happens.
-    """
-
-
-class QEMUMachineAddDeviceError(QEMUMachineError):
-    """
-    Exception raised when a request to add a device can not be fulfilled
-
-    The failures are caused by limitations, lack of information or conflicting
-    requests on the QEMUMachine methods.  This exception does not represent
-    failures reported by the QEMU binary itself.
-    """
-
-class MonitorResponseError(qmp.QMPError):
-    """
-    Represents erroneous QMP monitor reply
-    """
-    def __init__(self, reply):
-        try:
-            desc = reply["error"]["desc"]
-        except KeyError:
-            desc = reply
-        super(MonitorResponseError, self).__init__(desc)
-        self.reply = reply
-
-
-class QEMUMachine(object):
-    """
-    A QEMU VM
-
-    Use this object as a context manager to ensure the QEMU process terminates::
-
-        with VM(binary) as vm:
-            ...
-        # vm is guaranteed to be shut down here
-    """
-
-    def __init__(self, binary, args=None, wrapper=None, name=None,
-                 test_dir="/var/tmp", monitor_address=None,
-                 socket_scm_helper=None):
-        '''
-        Initialize a QEMUMachine
-
-        @param binary: path to the qemu binary
-        @param args: list of extra arguments
-        @param wrapper: list of arguments used as prefix to qemu binary
-        @param name: prefix for socket and log file names (default: qemu-PID)
-        @param test_dir: where to create socket and log file
-        @param monitor_address: address for QMP monitor
-        @param socket_scm_helper: helper program, required for send_fd_scm()
-        @note: Qemu process is not started until launch() is used.
-        '''
-        if args is None:
-            args = []
-        if wrapper is None:
-            wrapper = []
-        if name is None:
-            name = "qemu-%d" % os.getpid()
-        self._name = name
-        self._monitor_address = monitor_address
-        self._vm_monitor = None
-        self._qemu_log_path = None
-        self._qemu_log_file = None
-        self._popen = None
-        self._binary = binary
-        self._args = list(args)     # Force copy args in case we modify them
-        self._wrapper = wrapper
-        self._events = []
-        self._iolog = None
-        self._socket_scm_helper = socket_scm_helper
-        self._qmp = None
-        self._qemu_full_args = None
-        self._test_dir = test_dir
-        self._temp_dir = None
-        self._launched = False
-        self._machine = None
-        self._console_set = False
-        self._console_device_type = None
-        self._console_address = None
-        self._console_socket = None
-
-        # just in case logging wasn't configured by the main script:
-        logging.basicConfig()
-
-    def __enter__(self):
-        return self
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        self.shutdown()
-        return False
-
-    # This can be used to add an unused monitor instance.
-    def add_monitor_null(self):
-        self._args.append('-monitor')
-        self._args.append('null')
-
-    def add_fd(self, fd, fdset, opaque, opts=''):
-        """
-        Pass a file descriptor to the VM
-        """
-        options = ['fd=%d' % fd,
-                   'set=%d' % fdset,
-                   'opaque=%s' % opaque]
-        if opts:
-            options.append(opts)
-
-        # This did not exist before 3.4, but since then it is
-        # mandatory for our purpose
-        if hasattr(os, 'set_inheritable'):
-            os.set_inheritable(fd, True)
-
-        self._args.append('-add-fd')
-        self._args.append(','.join(options))
-        return self
-
-    # Exactly one of fd and file_path must be given.
-    # (If it is file_path, the helper will open that file and pass its
-    # own fd)
-    def send_fd_scm(self, fd=None, file_path=None):
-        # In iotest.py, the qmp should always use unix socket.
-        assert self._qmp.is_scm_available()
-        if self._socket_scm_helper is None:
-            raise QEMUMachineError("No path to socket_scm_helper set")
-        if not os.path.exists(self._socket_scm_helper):
-            raise QEMUMachineError("%s does not exist" %
-                                   self._socket_scm_helper)
-
-        # This did not exist before 3.4, but since then it is
-        # mandatory for our purpose
-        if hasattr(os, 'set_inheritable'):
-            os.set_inheritable(self._qmp.get_sock_fd(), True)
-            if fd is not None:
-                os.set_inheritable(fd, True)
-
-        fd_param = ["%s" % self._socket_scm_helper,
-                    "%d" % self._qmp.get_sock_fd()]
-
-        if file_path is not None:
-            assert fd is None
-            fd_param.append(file_path)
-        else:
-            assert fd is not None
-            fd_param.append(str(fd))
-
-        devnull = open(os.path.devnull, 'rb')
-        proc = subprocess.Popen(fd_param, stdin=devnull, stdout=subprocess.PIPE,
-                                stderr=subprocess.STDOUT, close_fds=False)
-        output = proc.communicate()[0]
-        if output:
-            LOG.debug(output)
-
-        return proc.returncode
-
-    @staticmethod
-    def _remove_if_exists(path):
-        """
-        Remove file object at path if it exists
-        """
-        try:
-            os.remove(path)
-        except OSError as exception:
-            if exception.errno == errno.ENOENT:
-                return
-            raise
-
-    def is_running(self):
-        return self._popen is not None and self._popen.poll() is None
-
-    def exitcode(self):
-        if self._popen is None:
-            return None
-        return self._popen.poll()
-
-    def get_pid(self):
-        if not self.is_running():
-            return None
-        return self._popen.pid
-
-    def _load_io_log(self):
-        if self._qemu_log_path is not None:
-            with open(self._qemu_log_path, "r") as iolog:
-                self._iolog = iolog.read()
-
-    def _base_args(self):
-        if isinstance(self._monitor_address, tuple):
-            moncdev = "socket,id=mon,host=%s,port=%s" % (
-                self._monitor_address[0],
-                self._monitor_address[1])
-        else:
-            moncdev = 'socket,id=mon,path=%s' % self._vm_monitor
-        args = ['-chardev', moncdev,
-                '-mon', 'chardev=mon,mode=control',
-                '-display', 'none', '-vga', 'none']
-        if self._machine is not None:
-            args.extend(['-machine', self._machine])
-        if self._console_set:
-            self._console_address = os.path.join(self._temp_dir,
-                                                 self._name + "-console.sock")
-            chardev = ('socket,id=console,path=%s,server,nowait' %
-                       self._console_address)
-            args.extend(['-chardev', chardev])
-            if self._console_device_type is None:
-                args.extend(['-serial', 'chardev:console'])
-            else:
-                device = '%s,chardev=console' % self._console_device_type
-                args.extend(['-device', device])
-        return args
-
-    def _pre_launch(self):
-        self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
-        if self._monitor_address is not None:
-            self._vm_monitor = self._monitor_address
-        else:
-            self._vm_monitor = os.path.join(self._temp_dir,
-                                            self._name + "-monitor.sock")
-        self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
-        self._qemu_log_file = open(self._qemu_log_path, 'wb')
-
-        self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
-                                            server=True)
-
-    def _post_launch(self):
-        self._qmp.accept()
-
-    def _post_shutdown(self):
-        if self._qemu_log_file is not None:
-            self._qemu_log_file.close()
-            self._qemu_log_file = None
-
-        self._qemu_log_path = None
-
-        if self._console_socket is not None:
-            self._console_socket.close()
-            self._console_socket = None
-
-        if self._temp_dir is not None:
-            shutil.rmtree(self._temp_dir)
-            self._temp_dir = None
-
-    def launch(self):
-        """
-        Launch the VM and make sure we cleanup and expose the
-        command line/output in case of exception
-        """
-
-        if self._launched:
-            raise QEMUMachineError('VM already launched')
-
-        self._iolog = None
-        self._qemu_full_args = None
-        try:
-            self._launch()
-            self._launched = True
-        except:
-            self.shutdown()
-
-            LOG.debug('Error launching VM')
-            if self._qemu_full_args:
-                LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
-            if self._iolog:
-                LOG.debug('Output: %r', self._iolog)
-            raise
-
-    def _launch(self):
-        """
-        Launch the VM and establish a QMP connection
-        """
-        devnull = open(os.path.devnull, 'rb')
-        self._pre_launch()
-        self._qemu_full_args = (self._wrapper + [self._binary] +
-                                self._base_args() + self._args)
-        LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
-        self._popen = subprocess.Popen(self._qemu_full_args,
-                                       stdin=devnull,
-                                       stdout=self._qemu_log_file,
-                                       stderr=subprocess.STDOUT,
-                                       shell=False,
-                                       close_fds=False)
-        self._post_launch()
-
-    def wait(self):
-        """
-        Wait for the VM to power off
-        """
-        self._popen.wait()
-        self._qmp.close()
-        self._load_io_log()
-        self._post_shutdown()
-
-    def shutdown(self):
-        """
-        Terminate the VM and clean up
-        """
-        if self.is_running():
-            try:
-                self._qmp.cmd('quit')
-                self._qmp.close()
-            except:
-                self._popen.kill()
-            self._popen.wait()
-
-        self._load_io_log()
-        self._post_shutdown()
-
-        exitcode = self.exitcode()
-        if exitcode is not None and exitcode < 0:
-            msg = 'qemu received signal %i: %s'
-            if self._qemu_full_args:
-                command = ' '.join(self._qemu_full_args)
-            else:
-                command = ''
-            LOG.warn(msg, -exitcode, command)
-
-        self._launched = False
-
-    def qmp(self, cmd, conv_keys=True, **args):
-        """
-        Invoke a QMP command and return the response dict
-        """
-        qmp_args = dict()
-        for key, value in args.items():
-            if conv_keys:
-                qmp_args[key.replace('_', '-')] = value
-            else:
-                qmp_args[key] = value
-
-        return self._qmp.cmd(cmd, args=qmp_args)
-
-    def command(self, cmd, conv_keys=True, **args):
-        """
-        Invoke a QMP command.
-        On success return the response dict.
-        On failure raise an exception.
-        """
-        reply = self.qmp(cmd, conv_keys, **args)
-        if reply is None:
-            raise qmp.QMPError("Monitor is closed")
-        if "error" in reply:
-            raise MonitorResponseError(reply)
-        return reply["return"]
-
-    def get_qmp_event(self, wait=False):
-        """
-        Poll for one queued QMP events and return it
-        """
-        if len(self._events) > 0:
-            return self._events.pop(0)
-        return self._qmp.pull_event(wait=wait)
-
-    def get_qmp_events(self, wait=False):
-        """
-        Poll for queued QMP events and return a list of dicts
-        """
-        events = self._qmp.get_events(wait=wait)
-        events.extend(self._events)
-        del self._events[:]
-        self._qmp.clear_events()
-        return events
-
-    @staticmethod
-    def event_match(event, match=None):
-        """
-        Check if an event matches optional match criteria.
-
-        The match criteria takes the form of a matching subdict. The event is
-        checked to be a superset of the subdict, recursively, with matching
-        values whenever the subdict values are not None.
-
-        This has a limitation that you cannot explicitly check for None values.
-
-        Examples, with the subdict queries on the left:
-         - None matches any object.
-         - {"foo": None} matches {"foo": {"bar": 1}}
-         - {"foo": None} matches {"foo": 5}
-         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
-         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
-        """
-        if match is None:
-            return True
-
-        try:
-            for key in match:
-                if key in event:
-                    if not QEMUMachine.event_match(event[key], match[key]):
-                        return False
-                else:
-                    return False
-            return True
-        except TypeError:
-            # either match or event wasn't iterable (not a dict)
-            return match == event
-
-    def event_wait(self, name, timeout=60.0, match=None):
-        """
-        event_wait waits for and returns a named event from QMP with a timeout.
-
-        name: The event to wait for.
-        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
-        match: Optional match criteria. See event_match for details.
-        """
-        return self.events_wait([(name, match)], timeout)
-
-    def events_wait(self, events, timeout=60.0):
-        """
-        events_wait waits for and returns a named event from QMP with a timeout.
-
-        events: a sequence of (name, match_criteria) tuples.
-                The match criteria are optional and may be None.
-                See event_match for details.
-        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
-        """
-        def _match(event):
-            for name, match in events:
-                if (event['event'] == name and
-                    self.event_match(event, match)):
-                    return True
-            return False
-
-        # Search cached events
-        for event in self._events:
-            if _match(event):
-                self._events.remove(event)
-                return event
-
-        # Poll for new events
-        while True:
-            event = self._qmp.pull_event(wait=timeout)
-            if _match(event):
-                return event
-            self._events.append(event)
-
-        return None
-
-    def get_log(self):
-        """
-        After self.shutdown or failed qemu execution, this returns the output
-        of the qemu process.
-        """
-        return self._iolog
-
-    def add_args(self, *args):
-        """
-        Adds to the list of extra arguments to be given to the QEMU binary
-        """
-        self._args.extend(args)
-
-    def set_machine(self, machine_type):
-        """
-        Sets the machine type
-
-        If set, the machine type will be added to the base arguments
-        of the resulting QEMU command line.
-        """
-        self._machine = machine_type
-
-    def set_console(self, device_type=None):
-        """
-        Sets the device type for a console device
-
-        If set, the console device and a backing character device will
-        be added to the base arguments of the resulting QEMU command
-        line.
-
-        This is a convenience method that will either use the provided
-        device type, or default to a "-serial chardev:console" command
-        line argument.
-
-        The actual setting of command line arguments will be be done at
-        machine launch time, as it depends on the temporary directory
-        to be created.
-
-        @param device_type: the device type, such as "isa-serial".  If
-                            None is given (the default value) a "-serial
-                            chardev:console" command line argument will
-                            be used instead, resorting to the machine's
-                            default device type.
-        """
-        self._console_set = True
-        self._console_device_type = device_type
-
-    @property
-    def console_socket(self):
-        """
-        Returns a socket connected to the console
-        """
-        if self._console_socket is None:
-            self._console_socket = socket.socket(socket.AF_UNIX,
-                                                 socket.SOCK_STREAM)
-            self._console_socket.connect(self._console_address)
-        return self._console_socket
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
new file mode 100644
index 0000000000..a8a311b035
--- /dev/null
+++ b/python/qemu/machine.py
@@ -0,0 +1,520 @@
+# QEMU library
+#
+# Copyright (C) 2015-2016 Red Hat Inc.
+# Copyright (C) 2012 IBM Corp.
+#
+# Authors:
+#  Fam Zheng <famz@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+# Based on qmp.py.
+#
+
+import errno
+import logging
+import os
+import subprocess
+import re
+import shutil
+import socket
+import tempfile
+
+from . import qmp
+
+LOG = logging.getLogger(__name__)
+
+class QEMUMachineError(Exception):
+    """
+    Exception called when an error in QEMUMachine happens.
+    """
+
+
+class QEMUMachineAddDeviceError(QEMUMachineError):
+    """
+    Exception raised when a request to add a device can not be fulfilled
+
+    The failures are caused by limitations, lack of information or conflicting
+    requests on the QEMUMachine methods.  This exception does not represent
+    failures reported by the QEMU binary itself.
+    """
+
+
+class MonitorResponseError(qmp.QMPError):
+    """
+    Represents erroneous QMP monitor reply
+    """
+    def __init__(self, reply):
+        try:
+            desc = reply["error"]["desc"]
+        except KeyError:
+            desc = reply
+        super(MonitorResponseError, self).__init__(desc)
+        self.reply = reply
+
+
+class QEMUMachine(object):
+    """
+    A QEMU VM
+
+    Use this object as a context manager to ensure the QEMU process terminates::
+
+        with VM(binary) as vm:
+            ...
+        # vm is guaranteed to be shut down here
+    """
+
+    def __init__(self, binary, args=None, wrapper=None, name=None,
+                 test_dir="/var/tmp", monitor_address=None,
+                 socket_scm_helper=None):
+        '''
+        Initialize a QEMUMachine
+
+        @param binary: path to the qemu binary
+        @param args: list of extra arguments
+        @param wrapper: list of arguments used as prefix to qemu binary
+        @param name: prefix for socket and log file names (default: qemu-PID)
+        @param test_dir: where to create socket and log file
+        @param monitor_address: address for QMP monitor
+        @param socket_scm_helper: helper program, required for send_fd_scm()
+        @note: Qemu process is not started until launch() is used.
+        '''
+        if args is None:
+            args = []
+        if wrapper is None:
+            wrapper = []
+        if name is None:
+            name = "qemu-%d" % os.getpid()
+        self._name = name
+        self._monitor_address = monitor_address
+        self._vm_monitor = None
+        self._qemu_log_path = None
+        self._qemu_log_file = None
+        self._popen = None
+        self._binary = binary
+        self._args = list(args)     # Force copy args in case we modify them
+        self._wrapper = wrapper
+        self._events = []
+        self._iolog = None
+        self._socket_scm_helper = socket_scm_helper
+        self._qmp = None
+        self._qemu_full_args = None
+        self._test_dir = test_dir
+        self._temp_dir = None
+        self._launched = False
+        self._machine = None
+        self._console_set = False
+        self._console_device_type = None
+        self._console_address = None
+        self._console_socket = None
+
+        # just in case logging wasn't configured by the main script:
+        logging.basicConfig()
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        self.shutdown()
+        return False
+
+    # This can be used to add an unused monitor instance.
+    def add_monitor_null(self):
+        self._args.append('-monitor')
+        self._args.append('null')
+
+    def add_fd(self, fd, fdset, opaque, opts=''):
+        """
+        Pass a file descriptor to the VM
+        """
+        options = ['fd=%d' % fd,
+                   'set=%d' % fdset,
+                   'opaque=%s' % opaque]
+        if opts:
+            options.append(opts)
+
+        # This did not exist before 3.4, but since then it is
+        # mandatory for our purpose
+        if hasattr(os, 'set_inheritable'):
+            os.set_inheritable(fd, True)
+
+        self._args.append('-add-fd')
+        self._args.append(','.join(options))
+        return self
+
+    # Exactly one of fd and file_path must be given.
+    # (If it is file_path, the helper will open that file and pass its
+    # own fd)
+    def send_fd_scm(self, fd=None, file_path=None):
+        # In iotest.py, the qmp should always use unix socket.
+        assert self._qmp.is_scm_available()
+        if self._socket_scm_helper is None:
+            raise QEMUMachineError("No path to socket_scm_helper set")
+        if not os.path.exists(self._socket_scm_helper):
+            raise QEMUMachineError("%s does not exist" %
+                                   self._socket_scm_helper)
+
+        # This did not exist before 3.4, but since then it is
+        # mandatory for our purpose
+        if hasattr(os, 'set_inheritable'):
+            os.set_inheritable(self._qmp.get_sock_fd(), True)
+            if fd is not None:
+                os.set_inheritable(fd, True)
+
+        fd_param = ["%s" % self._socket_scm_helper,
+                    "%d" % self._qmp.get_sock_fd()]
+
+        if file_path is not None:
+            assert fd is None
+            fd_param.append(file_path)
+        else:
+            assert fd is not None
+            fd_param.append(str(fd))
+
+        devnull = open(os.path.devnull, 'rb')
+        proc = subprocess.Popen(fd_param, stdin=devnull, stdout=subprocess.PIPE,
+                                stderr=subprocess.STDOUT, close_fds=False)
+        output = proc.communicate()[0]
+        if output:
+            LOG.debug(output)
+
+        return proc.returncode
+
+    @staticmethod
+    def _remove_if_exists(path):
+        """
+        Remove file object at path if it exists
+        """
+        try:
+            os.remove(path)
+        except OSError as exception:
+            if exception.errno == errno.ENOENT:
+                return
+            raise
+
+    def is_running(self):
+        return self._popen is not None and self._popen.poll() is None
+
+    def exitcode(self):
+        if self._popen is None:
+            return None
+        return self._popen.poll()
+
+    def get_pid(self):
+        if not self.is_running():
+            return None
+        return self._popen.pid
+
+    def _load_io_log(self):
+        if self._qemu_log_path is not None:
+            with open(self._qemu_log_path, "r") as iolog:
+                self._iolog = iolog.read()
+
+    def _base_args(self):
+        if isinstance(self._monitor_address, tuple):
+            moncdev = "socket,id=mon,host=%s,port=%s" % (
+                self._monitor_address[0],
+                self._monitor_address[1])
+        else:
+            moncdev = 'socket,id=mon,path=%s' % self._vm_monitor
+        args = ['-chardev', moncdev,
+                '-mon', 'chardev=mon,mode=control',
+                '-display', 'none', '-vga', 'none']
+        if self._machine is not None:
+            args.extend(['-machine', self._machine])
+        if self._console_set:
+            self._console_address = os.path.join(self._temp_dir,
+                                                 self._name + "-console.sock")
+            chardev = ('socket,id=console,path=%s,server,nowait' %
+                       self._console_address)
+            args.extend(['-chardev', chardev])
+            if self._console_device_type is None:
+                args.extend(['-serial', 'chardev:console'])
+            else:
+                device = '%s,chardev=console' % self._console_device_type
+                args.extend(['-device', device])
+        return args
+
+    def _pre_launch(self):
+        self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
+        if self._monitor_address is not None:
+            self._vm_monitor = self._monitor_address
+        else:
+            self._vm_monitor = os.path.join(self._temp_dir,
+                                            self._name + "-monitor.sock")
+        self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
+        self._qemu_log_file = open(self._qemu_log_path, 'wb')
+
+        self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
+                                            server=True)
+
+    def _post_launch(self):
+        self._qmp.accept()
+
+    def _post_shutdown(self):
+        if self._qemu_log_file is not None:
+            self._qemu_log_file.close()
+            self._qemu_log_file = None
+
+        self._qemu_log_path = None
+
+        if self._console_socket is not None:
+            self._console_socket.close()
+            self._console_socket = None
+
+        if self._temp_dir is not None:
+            shutil.rmtree(self._temp_dir)
+            self._temp_dir = None
+
+    def launch(self):
+        """
+        Launch the VM and make sure we cleanup and expose the
+        command line/output in case of exception
+        """
+
+        if self._launched:
+            raise QEMUMachineError('VM already launched')
+
+        self._iolog = None
+        self._qemu_full_args = None
+        try:
+            self._launch()
+            self._launched = True
+        except:
+            self.shutdown()
+
+            LOG.debug('Error launching VM')
+            if self._qemu_full_args:
+                LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
+            if self._iolog:
+                LOG.debug('Output: %r', self._iolog)
+            raise
+
+    def _launch(self):
+        """
+        Launch the VM and establish a QMP connection
+        """
+        devnull = open(os.path.devnull, 'rb')
+        self._pre_launch()
+        self._qemu_full_args = (self._wrapper + [self._binary] +
+                                self._base_args() + self._args)
+        LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
+        self._popen = subprocess.Popen(self._qemu_full_args,
+                                       stdin=devnull,
+                                       stdout=self._qemu_log_file,
+                                       stderr=subprocess.STDOUT,
+                                       shell=False,
+                                       close_fds=False)
+        self._post_launch()
+
+    def wait(self):
+        """
+        Wait for the VM to power off
+        """
+        self._popen.wait()
+        self._qmp.close()
+        self._load_io_log()
+        self._post_shutdown()
+
+    def shutdown(self):
+        """
+        Terminate the VM and clean up
+        """
+        if self.is_running():
+            try:
+                self._qmp.cmd('quit')
+                self._qmp.close()
+            except:
+                self._popen.kill()
+            self._popen.wait()
+
+        self._load_io_log()
+        self._post_shutdown()
+
+        exitcode = self.exitcode()
+        if exitcode is not None and exitcode < 0:
+            msg = 'qemu received signal %i: %s'
+            if self._qemu_full_args:
+                command = ' '.join(self._qemu_full_args)
+            else:
+                command = ''
+            LOG.warn(msg, -exitcode, command)
+
+        self._launched = False
+
+    def qmp(self, cmd, conv_keys=True, **args):
+        """
+        Invoke a QMP command and return the response dict
+        """
+        qmp_args = dict()
+        for key, value in args.items():
+            if conv_keys:
+                qmp_args[key.replace('_', '-')] = value
+            else:
+                qmp_args[key] = value
+
+        return self._qmp.cmd(cmd, args=qmp_args)
+
+    def command(self, cmd, conv_keys=True, **args):
+        """
+        Invoke a QMP command.
+        On success return the response dict.
+        On failure raise an exception.
+        """
+        reply = self.qmp(cmd, conv_keys, **args)
+        if reply is None:
+            raise qmp.QMPError("Monitor is closed")
+        if "error" in reply:
+            raise MonitorResponseError(reply)
+        return reply["return"]
+
+    def get_qmp_event(self, wait=False):
+        """
+        Poll for one queued QMP events and return it
+        """
+        if len(self._events) > 0:
+            return self._events.pop(0)
+        return self._qmp.pull_event(wait=wait)
+
+    def get_qmp_events(self, wait=False):
+        """
+        Poll for queued QMP events and return a list of dicts
+        """
+        events = self._qmp.get_events(wait=wait)
+        events.extend(self._events)
+        del self._events[:]
+        self._qmp.clear_events()
+        return events
+
+    @staticmethod
+    def event_match(event, match=None):
+        """
+        Check if an event matches optional match criteria.
+
+        The match criteria takes the form of a matching subdict. The event is
+        checked to be a superset of the subdict, recursively, with matching
+        values whenever the subdict values are not None.
+
+        This has a limitation that you cannot explicitly check for None values.
+
+        Examples, with the subdict queries on the left:
+         - None matches any object.
+         - {"foo": None} matches {"foo": {"bar": 1}}
+         - {"foo": None} matches {"foo": 5}
+         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
+         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
+        """
+        if match is None:
+            return True
+
+        try:
+            for key in match:
+                if key in event:
+                    if not QEMUMachine.event_match(event[key], match[key]):
+                        return False
+                else:
+                    return False
+            return True
+        except TypeError:
+            # either match or event wasn't iterable (not a dict)
+            return match == event
+
+    def event_wait(self, name, timeout=60.0, match=None):
+        """
+        event_wait waits for and returns a named event from QMP with a timeout.
+
+        name: The event to wait for.
+        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
+        match: Optional match criteria. See event_match for details.
+        """
+        return self.events_wait([(name, match)], timeout)
+
+    def events_wait(self, events, timeout=60.0):
+        """
+        events_wait waits for and returns a named event from QMP with a timeout.
+
+        events: a sequence of (name, match_criteria) tuples.
+                The match criteria are optional and may be None.
+                See event_match for details.
+        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
+        """
+        def _match(event):
+            for name, match in events:
+                if (event['event'] == name and
+                    self.event_match(event, match)):
+                    return True
+            return False
+
+        # Search cached events
+        for event in self._events:
+            if _match(event):
+                self._events.remove(event)
+                return event
+
+        # Poll for new events
+        while True:
+            event = self._qmp.pull_event(wait=timeout)
+            if _match(event):
+                return event
+            self._events.append(event)
+
+        return None
+
+    def get_log(self):
+        """
+        After self.shutdown or failed qemu execution, this returns the output
+        of the qemu process.
+        """
+        return self._iolog
+
+    def add_args(self, *args):
+        """
+        Adds to the list of extra arguments to be given to the QEMU binary
+        """
+        self._args.extend(args)
+
+    def set_machine(self, machine_type):
+        """
+        Sets the machine type
+
+        If set, the machine type will be added to the base arguments
+        of the resulting QEMU command line.
+        """
+        self._machine = machine_type
+
+    def set_console(self, device_type=None):
+        """
+        Sets the device type for a console device
+
+        If set, the console device and a backing character device will
+        be added to the base arguments of the resulting QEMU command
+        line.
+
+        This is a convenience method that will either use the provided
+        device type, or default to a "-serial chardev:console" command
+        line argument.
+
+        The actual setting of command line arguments will be be done at
+        machine launch time, as it depends on the temporary directory
+        to be created.
+
+        @param device_type: the device type, such as "isa-serial".  If
+                            None is given (the default value) a "-serial
+                            chardev:console" command line argument will
+                            be used instead, resorting to the machine's
+                            default device type.
+        """
+        self._console_set = True
+        self._console_device_type = device_type
+
+    @property
+    def console_socket(self):
+        """
+        Returns a socket connected to the console
+        """
+        if self._console_socket is None:
+            self._console_socket = socket.socket(socket.AF_UNIX,
+                                                 socket.SOCK_STREAM)
+            self._console_socket.connect(self._console_address)
+        return self._console_socket
diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index eb45824dd0..eebcc233ed 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -14,7 +14,7 @@
 import socket
 import os
 
-from . import QEMUMachine
+from .machine import QEMUMachine
 
 
 class QEMUQtestProtocol(object):
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index a6748910ad..15f213a6cd 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -36,7 +36,7 @@ import argparse
 from itertools import chain
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
-from qemu import QEMUMachine
+from qemu.machine import QEMUMachine
 
 logger = logging.getLogger('device-crash-test')
 dbg = logger.debug
diff --git a/scripts/render_block_graph.py b/scripts/render_block_graph.py
index 3e9d282a49..656f0388ad 100755
--- a/scripts/render_block_graph.py
+++ b/scripts/render_block_graph.py
@@ -25,7 +25,7 @@ import json
 from graphviz import Digraph
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
-from qemu import MonitorResponseError
+from qemu.machine import MonitorResponseError
 
 
 def perm(arr):
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 2b236a1cf0..aee5d820ed 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -17,7 +17,7 @@ import avocado
 SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
 sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
 
-from qemu import QEMUMachine
+from qemu.machine import QEMUMachine
 
 def is_readable_executable_file(path):
     return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
diff --git a/tests/acceptance/virtio_version.py b/tests/acceptance/virtio_version.py
index 8b97453ff8..33593c29dd 100644
--- a/tests/acceptance/virtio_version.py
+++ b/tests/acceptance/virtio_version.py
@@ -12,7 +12,7 @@ import sys
 import os
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu import QEMUMachine
+from qemu.machine import QEMUMachine
 from avocado_qemu import Test
 
 # Virtio Device IDs:
diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
index 0e304660b8..f13dbea800 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -30,7 +30,7 @@ from guestperf.timings import TimingRecord, Timings
 
 sys.path.append(os.path.join(os.path.dirname(__file__),
                              '..', '..', '..', 'python'))
-import qemu
+from qemu.machine import QEMUMachine
 
 
 class Engine(object):
@@ -386,17 +386,17 @@ class Engine(object):
             dstmonaddr = "/var/tmp/qemu-dst-%d-monitor.sock" % os.getpid()
         srcmonaddr = "/var/tmp/qemu-src-%d-monitor.sock" % os.getpid()
 
-        src = qemu.QEMUMachine(self._binary,
-                               args=self._get_src_args(hardware),
-                               wrapper=self._get_src_wrapper(hardware),
-                               name="qemu-src-%d" % os.getpid(),
-                               monitor_address=srcmonaddr)
+        src = QEMUMachine(self._binary,
+                          args=self._get_src_args(hardware),
+                          wrapper=self._get_src_wrapper(hardware),
+                          name="qemu-src-%d" % os.getpid(),
+                          monitor_address=srcmonaddr)
 
-        dst = qemu.QEMUMachine(self._binary,
-                               args=self._get_dst_args(hardware, uri),
-                               wrapper=self._get_dst_wrapper(hardware),
-                               name="qemu-dst-%d" % os.getpid(),
-                               monitor_address=dstmonaddr)
+        dst = QEMUMachine(self._binary,
+                          args=self._get_dst_args(hardware, uri),
+                          wrapper=self._get_dst_wrapper(hardware),
+                          name="qemu-dst-%d" % os.getpid(),
+                          monitor_address=dstmonaddr)
 
         try:
             src.launch()
diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
index 2b6a8c13be..fedd111fd4 100755
--- a/tests/qemu-iotests/235
+++ b/tests/qemu-iotests/235
@@ -25,7 +25,7 @@ from iotests import qemu_img_create, qemu_io, file_path, log
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 
-from qemu import QEMUMachine
+from qemu.machine import QEMUMachine
 
 # Note:
 # This test was added to check that mirror dead-lock was fixed (see previous
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 0556bdcf9e..4b496f1551 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -18,7 +18,8 @@ import logging
 import time
 import datetime
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu import QEMUMachine, kvm_available
+from qemu import kvm_available
+from qemu.machine import QEMUMachine
 import subprocess
 import hashlib
 import optparse
-- 
2.21.0

Re: [Qemu-devel] [RFC PATCH v2 1/3] python/qemu: split QEMUMachine out from underneath __init__.py

[Wainer dos Santos Moschetta]

On 06/27/2019 06:28 PM, John Snow wrote:
> It's not obvious that something named __init__.py actually houses
> important code that isn't relevant to python packaging glue. Move the
> QEMUMachine and related error classes out into their own module.
>
> Adjust users to the new import location.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/qemu/__init__.py                   | 502 +--------------------
>   python/qemu/machine.py                    | 520 ++++++++++++++++++++++
>   python/qemu/qtest.py                      |   2 +-
>   scripts/device-crash-test                 |   2 +-
>   scripts/render_block_graph.py             |   2 +-
>   tests/acceptance/avocado_qemu/__init__.py |   2 +-
>   tests/acceptance/virtio_version.py        |   2 +-
>   tests/migration/guestperf/engine.py       |  22 +-
>   tests/qemu-iotests/235                    |   2 +-
>   tests/vm/basevm.py                        |   3 +-
>   10 files changed, 540 insertions(+), 519 deletions(-)
>   create mode 100644 python/qemu/machine.py
>
> diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
> index dbaf8a5311..6c919a3d56 100644
> --- a/python/qemu/__init__.py
> +++ b/python/qemu/__init__.py
> @@ -12,17 +12,11 @@
>   # Based on qmp.py.
>   #
>   
> -import errno
>   import logging
>   import os
> -import subprocess
> -import re
> -import shutil
> -import socket
> -import tempfile
>   
>   from . import qmp
> -
> +from . import machine
>   
>   LOG = logging.getLogger(__name__)
>   
> @@ -39,497 +33,3 @@ def kvm_available(target_arch=None):
>           if target_arch != ADDITIONAL_ARCHES.get(host_arch):
>               return False
>       return os.access("/dev/kvm", os.R_OK | os.W_OK)
> -
> -
> -class QEMUMachineError(Exception):
> -    """
> -    Exception called when an error in QEMUMachine happens.
> -    """
> -
> -
> -class QEMUMachineAddDeviceError(QEMUMachineError):
> -    """
> -    Exception raised when a request to add a device can not be fulfilled
> -
> -    The failures are caused by limitations, lack of information or conflicting
> -    requests on the QEMUMachine methods.  This exception does not represent
> -    failures reported by the QEMU binary itself.
> -    """
> -
> -class MonitorResponseError(qmp.QMPError):
> -    """
> -    Represents erroneous QMP monitor reply
> -    """
> -    def __init__(self, reply):
> -        try:
> -            desc = reply["error"]["desc"]
> -        except KeyError:
> -            desc = reply
> -        super(MonitorResponseError, self).__init__(desc)
> -        self.reply = reply
> -
> -
> -class QEMUMachine(object):
> -    """
> -    A QEMU VM
> -
> -    Use this object as a context manager to ensure the QEMU process terminates::
> -
> -        with VM(binary) as vm:
> -            ...
> -        # vm is guaranteed to be shut down here
> -    """
> -
> -    def __init__(self, binary, args=None, wrapper=None, name=None,
> -                 test_dir="/var/tmp", monitor_address=None,
> -                 socket_scm_helper=None):
> -        '''
> -        Initialize a QEMUMachine
> -
> -        @param binary: path to the qemu binary
> -        @param args: list of extra arguments
> -        @param wrapper: list of arguments used as prefix to qemu binary
> -        @param name: prefix for socket and log file names (default: qemu-PID)
> -        @param test_dir: where to create socket and log file
> -        @param monitor_address: address for QMP monitor
> -        @param socket_scm_helper: helper program, required for send_fd_scm()
> -        @note: Qemu process is not started until launch() is used.
> -        '''
> -        if args is None:
> -            args = []
> -        if wrapper is None:
> -            wrapper = []
> -        if name is None:
> -            name = "qemu-%d" % os.getpid()
> -        self._name = name
> -        self._monitor_address = monitor_address
> -        self._vm_monitor = None
> -        self._qemu_log_path = None
> -        self._qemu_log_file = None
> -        self._popen = None
> -        self._binary = binary
> -        self._args = list(args)     # Force copy args in case we modify them
> -        self._wrapper = wrapper
> -        self._events = []
> -        self._iolog = None
> -        self._socket_scm_helper = socket_scm_helper
> -        self._qmp = None
> -        self._qemu_full_args = None
> -        self._test_dir = test_dir
> -        self._temp_dir = None
> -        self._launched = False
> -        self._machine = None
> -        self._console_set = False
> -        self._console_device_type = None
> -        self._console_address = None
> -        self._console_socket = None
> -
> -        # just in case logging wasn't configured by the main script:
> -        logging.basicConfig()
> -
> -    def __enter__(self):
> -        return self
> -
> -    def __exit__(self, exc_type, exc_val, exc_tb):
> -        self.shutdown()
> -        return False
> -
> -    # This can be used to add an unused monitor instance.
> -    def add_monitor_null(self):
> -        self._args.append('-monitor')
> -        self._args.append('null')
> -
> -    def add_fd(self, fd, fdset, opaque, opts=''):
> -        """
> -        Pass a file descriptor to the VM
> -        """
> -        options = ['fd=%d' % fd,
> -                   'set=%d' % fdset,
> -                   'opaque=%s' % opaque]
> -        if opts:
> -            options.append(opts)
> -
> -        # This did not exist before 3.4, but since then it is
> -        # mandatory for our purpose
> -        if hasattr(os, 'set_inheritable'):
> -            os.set_inheritable(fd, True)
> -
> -        self._args.append('-add-fd')
> -        self._args.append(','.join(options))
> -        return self
> -
> -    # Exactly one of fd and file_path must be given.
> -    # (If it is file_path, the helper will open that file and pass its
> -    # own fd)
> -    def send_fd_scm(self, fd=None, file_path=None):
> -        # In iotest.py, the qmp should always use unix socket.
> -        assert self._qmp.is_scm_available()
> -        if self._socket_scm_helper is None:
> -            raise QEMUMachineError("No path to socket_scm_helper set")
> -        if not os.path.exists(self._socket_scm_helper):
> -            raise QEMUMachineError("%s does not exist" %
> -                                   self._socket_scm_helper)
> -
> -        # This did not exist before 3.4, but since then it is
> -        # mandatory for our purpose
> -        if hasattr(os, 'set_inheritable'):
> -            os.set_inheritable(self._qmp.get_sock_fd(), True)
> -            if fd is not None:
> -                os.set_inheritable(fd, True)
> -
> -        fd_param = ["%s" % self._socket_scm_helper,
> -                    "%d" % self._qmp.get_sock_fd()]
> -
> -        if file_path is not None:
> -            assert fd is None
> -            fd_param.append(file_path)
> -        else:
> -            assert fd is not None
> -            fd_param.append(str(fd))
> -
> -        devnull = open(os.path.devnull, 'rb')
> -        proc = subprocess.Popen(fd_param, stdin=devnull, stdout=subprocess.PIPE,
> -                                stderr=subprocess.STDOUT, close_fds=False)
> -        output = proc.communicate()[0]
> -        if output:
> -            LOG.debug(output)
> -
> -        return proc.returncode
> -
> -    @staticmethod
> -    def _remove_if_exists(path):
> -        """
> -        Remove file object at path if it exists
> -        """
> -        try:
> -            os.remove(path)
> -        except OSError as exception:
> -            if exception.errno == errno.ENOENT:
> -                return
> -            raise
> -
> -    def is_running(self):
> -        return self._popen is not None and self._popen.poll() is None
> -
> -    def exitcode(self):
> -        if self._popen is None:
> -            return None
> -        return self._popen.poll()
> -
> -    def get_pid(self):
> -        if not self.is_running():
> -            return None
> -        return self._popen.pid
> -
> -    def _load_io_log(self):
> -        if self._qemu_log_path is not None:
> -            with open(self._qemu_log_path, "r") as iolog:
> -                self._iolog = iolog.read()
> -
> -    def _base_args(self):
> -        if isinstance(self._monitor_address, tuple):
> -            moncdev = "socket,id=mon,host=%s,port=%s" % (
> -                self._monitor_address[0],
> -                self._monitor_address[1])
> -        else:
> -            moncdev = 'socket,id=mon,path=%s' % self._vm_monitor
> -        args = ['-chardev', moncdev,
> -                '-mon', 'chardev=mon,mode=control',
> -                '-display', 'none', '-vga', 'none']
> -        if self._machine is not None:
> -            args.extend(['-machine', self._machine])
> -        if self._console_set:
> -            self._console_address = os.path.join(self._temp_dir,
> -                                                 self._name + "-console.sock")
> -            chardev = ('socket,id=console,path=%s,server,nowait' %
> -                       self._console_address)
> -            args.extend(['-chardev', chardev])
> -            if self._console_device_type is None:
> -                args.extend(['-serial', 'chardev:console'])
> -            else:
> -                device = '%s,chardev=console' % self._console_device_type
> -                args.extend(['-device', device])
> -        return args
> -
> -    def _pre_launch(self):
> -        self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
> -        if self._monitor_address is not None:
> -            self._vm_monitor = self._monitor_address
> -        else:
> -            self._vm_monitor = os.path.join(self._temp_dir,
> -                                            self._name + "-monitor.sock")
> -        self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
> -        self._qemu_log_file = open(self._qemu_log_path, 'wb')
> -
> -        self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
> -                                            server=True)
> -
> -    def _post_launch(self):
> -        self._qmp.accept()
> -
> -    def _post_shutdown(self):
> -        if self._qemu_log_file is not None:
> -            self._qemu_log_file.close()
> -            self._qemu_log_file = None
> -
> -        self._qemu_log_path = None
> -
> -        if self._console_socket is not None:
> -            self._console_socket.close()
> -            self._console_socket = None
> -
> -        if self._temp_dir is not None:
> -            shutil.rmtree(self._temp_dir)
> -            self._temp_dir = None
> -
> -    def launch(self):
> -        """
> -        Launch the VM and make sure we cleanup and expose the
> -        command line/output in case of exception
> -        """
> -
> -        if self._launched:
> -            raise QEMUMachineError('VM already launched')
> -
> -        self._iolog = None
> -        self._qemu_full_args = None
> -        try:
> -            self._launch()
> -            self._launched = True
> -        except:
> -            self.shutdown()
> -
> -            LOG.debug('Error launching VM')
> -            if self._qemu_full_args:
> -                LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
> -            if self._iolog:
> -                LOG.debug('Output: %r', self._iolog)
> -            raise
> -
> -    def _launch(self):
> -        """
> -        Launch the VM and establish a QMP connection
> -        """
> -        devnull = open(os.path.devnull, 'rb')
> -        self._pre_launch()
> -        self._qemu_full_args = (self._wrapper + [self._binary] +
> -                                self._base_args() + self._args)
> -        LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
> -        self._popen = subprocess.Popen(self._qemu_full_args,
> -                                       stdin=devnull,
> -                                       stdout=self._qemu_log_file,
> -                                       stderr=subprocess.STDOUT,
> -                                       shell=False,
> -                                       close_fds=False)
> -        self._post_launch()
> -
> -    def wait(self):
> -        """
> -        Wait for the VM to power off
> -        """
> -        self._popen.wait()
> -        self._qmp.close()
> -        self._load_io_log()
> -        self._post_shutdown()
> -
> -    def shutdown(self):
> -        """
> -        Terminate the VM and clean up
> -        """
> -        if self.is_running():
> -            try:
> -                self._qmp.cmd('quit')
> -                self._qmp.close()
> -            except:
> -                self._popen.kill()
> -            self._popen.wait()
> -
> -        self._load_io_log()
> -        self._post_shutdown()
> -
> -        exitcode = self.exitcode()
> -        if exitcode is not None and exitcode < 0:
> -            msg = 'qemu received signal %i: %s'
> -            if self._qemu_full_args:
> -                command = ' '.join(self._qemu_full_args)
> -            else:
> -                command = ''
> -            LOG.warn(msg, -exitcode, command)
> -
> -        self._launched = False
> -
> -    def qmp(self, cmd, conv_keys=True, **args):
> -        """
> -        Invoke a QMP command and return the response dict
> -        """
> -        qmp_args = dict()
> -        for key, value in args.items():
> -            if conv_keys:
> -                qmp_args[key.replace('_', '-')] = value
> -            else:
> -                qmp_args[key] = value
> -
> -        return self._qmp.cmd(cmd, args=qmp_args)
> -
> -    def command(self, cmd, conv_keys=True, **args):
> -        """
> -        Invoke a QMP command.
> -        On success return the response dict.
> -        On failure raise an exception.
> -        """
> -        reply = self.qmp(cmd, conv_keys, **args)
> -        if reply is None:
> -            raise qmp.QMPError("Monitor is closed")
> -        if "error" in reply:
> -            raise MonitorResponseError(reply)
> -        return reply["return"]
> -
> -    def get_qmp_event(self, wait=False):
> -        """
> -        Poll for one queued QMP events and return it
> -        """
> -        if len(self._events) > 0:
> -            return self._events.pop(0)
> -        return self._qmp.pull_event(wait=wait)
> -
> -    def get_qmp_events(self, wait=False):
> -        """
> -        Poll for queued QMP events and return a list of dicts
> -        """
> -        events = self._qmp.get_events(wait=wait)
> -        events.extend(self._events)
> -        del self._events[:]
> -        self._qmp.clear_events()
> -        return events
> -
> -    @staticmethod
> -    def event_match(event, match=None):
> -        """
> -        Check if an event matches optional match criteria.
> -
> -        The match criteria takes the form of a matching subdict. The event is
> -        checked to be a superset of the subdict, recursively, with matching
> -        values whenever the subdict values are not None.
> -
> -        This has a limitation that you cannot explicitly check for None values.
> -
> -        Examples, with the subdict queries on the left:
> -         - None matches any object.
> -         - {"foo": None} matches {"foo": {"bar": 1}}
> -         - {"foo": None} matches {"foo": 5}
> -         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
> -         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
> -        """
> -        if match is None:
> -            return True
> -
> -        try:
> -            for key in match:
> -                if key in event:
> -                    if not QEMUMachine.event_match(event[key], match[key]):
> -                        return False
> -                else:
> -                    return False
> -            return True
> -        except TypeError:
> -            # either match or event wasn't iterable (not a dict)
> -            return match == event
> -
> -    def event_wait(self, name, timeout=60.0, match=None):
> -        """
> -        event_wait waits for and returns a named event from QMP with a timeout.
> -
> -        name: The event to wait for.
> -        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
> -        match: Optional match criteria. See event_match for details.
> -        """
> -        return self.events_wait([(name, match)], timeout)
> -
> -    def events_wait(self, events, timeout=60.0):
> -        """
> -        events_wait waits for and returns a named event from QMP with a timeout.
> -
> -        events: a sequence of (name, match_criteria) tuples.
> -                The match criteria are optional and may be None.
> -                See event_match for details.
> -        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
> -        """
> -        def _match(event):
> -            for name, match in events:
> -                if (event['event'] == name and
> -                    self.event_match(event, match)):
> -                    return True
> -            return False
> -
> -        # Search cached events
> -        for event in self._events:
> -            if _match(event):
> -                self._events.remove(event)
> -                return event
> -
> -        # Poll for new events
> -        while True:
> -            event = self._qmp.pull_event(wait=timeout)
> -            if _match(event):
> -                return event
> -            self._events.append(event)
> -
> -        return None
> -
> -    def get_log(self):
> -        """
> -        After self.shutdown or failed qemu execution, this returns the output
> -        of the qemu process.
> -        """
> -        return self._iolog
> -
> -    def add_args(self, *args):
> -        """
> -        Adds to the list of extra arguments to be given to the QEMU binary
> -        """
> -        self._args.extend(args)
> -
> -    def set_machine(self, machine_type):
> -        """
> -        Sets the machine type
> -
> -        If set, the machine type will be added to the base arguments
> -        of the resulting QEMU command line.
> -        """
> -        self._machine = machine_type
> -
> -    def set_console(self, device_type=None):
> -        """
> -        Sets the device type for a console device
> -
> -        If set, the console device and a backing character device will
> -        be added to the base arguments of the resulting QEMU command
> -        line.
> -
> -        This is a convenience method that will either use the provided
> -        device type, or default to a "-serial chardev:console" command
> -        line argument.
> -
> -        The actual setting of command line arguments will be be done at
> -        machine launch time, as it depends on the temporary directory
> -        to be created.
> -
> -        @param device_type: the device type, such as "isa-serial".  If
> -                            None is given (the default value) a "-serial
> -                            chardev:console" command line argument will
> -                            be used instead, resorting to the machine's
> -                            default device type.
> -        """
> -        self._console_set = True
> -        self._console_device_type = device_type
> -
> -    @property
> -    def console_socket(self):
> -        """
> -        Returns a socket connected to the console
> -        """
> -        if self._console_socket is None:
> -            self._console_socket = socket.socket(socket.AF_UNIX,
> -                                                 socket.SOCK_STREAM)
> -            self._console_socket.connect(self._console_address)
> -        return self._console_socket
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> new file mode 100644
> index 0000000000..a8a311b035
> --- /dev/null
> +++ b/python/qemu/machine.py
> @@ -0,0 +1,520 @@
> +# QEMU library
> +#
> +# Copyright (C) 2015-2016 Red Hat Inc.
> +# Copyright (C) 2012 IBM Corp.
> +#
> +# Authors:
> +#  Fam Zheng <famz@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.  See
> +# the COPYING file in the top-level directory.
> +#
> +# Based on qmp.py.
> +#
> +
> +import errno
> +import logging
> +import os
> +import subprocess
> +import re
> +import shutil
> +import socket
> +import tempfile
> +
> +from . import qmp
> +
> +LOG = logging.getLogger(__name__)
> +
> +class QEMUMachineError(Exception):
> +    """
> +    Exception called when an error in QEMUMachine happens.
> +    """
> +
> +
> +class QEMUMachineAddDeviceError(QEMUMachineError):
> +    """
> +    Exception raised when a request to add a device can not be fulfilled
> +
> +    The failures are caused by limitations, lack of information or conflicting
> +    requests on the QEMUMachine methods.  This exception does not represent
> +    failures reported by the QEMU binary itself.
> +    """
> +
> +
> +class MonitorResponseError(qmp.QMPError):
> +    """
> +    Represents erroneous QMP monitor reply
> +    """
> +    def __init__(self, reply):
> +        try:
> +            desc = reply["error"]["desc"]
> +        except KeyError:
> +            desc = reply
> +        super(MonitorResponseError, self).__init__(desc)
> +        self.reply = reply
> +
> +
> +class QEMUMachine(object):
> +    """
> +    A QEMU VM
> +
> +    Use this object as a context manager to ensure the QEMU process terminates::
> +
> +        with VM(binary) as vm:
> +            ...
> +        # vm is guaranteed to be shut down here
> +    """
> +
> +    def __init__(self, binary, args=None, wrapper=None, name=None,
> +                 test_dir="/var/tmp", monitor_address=None,
> +                 socket_scm_helper=None):
> +        '''
> +        Initialize a QEMUMachine
> +
> +        @param binary: path to the qemu binary
> +        @param args: list of extra arguments
> +        @param wrapper: list of arguments used as prefix to qemu binary
> +        @param name: prefix for socket and log file names (default: qemu-PID)
> +        @param test_dir: where to create socket and log file
> +        @param monitor_address: address for QMP monitor
> +        @param socket_scm_helper: helper program, required for send_fd_scm()
> +        @note: Qemu process is not started until launch() is used.
> +        '''
> +        if args is None:
> +            args = []
> +        if wrapper is None:
> +            wrapper = []
> +        if name is None:
> +            name = "qemu-%d" % os.getpid()
> +        self._name = name
> +        self._monitor_address = monitor_address
> +        self._vm_monitor = None
> +        self._qemu_log_path = None
> +        self._qemu_log_file = None
> +        self._popen = None
> +        self._binary = binary
> +        self._args = list(args)     # Force copy args in case we modify them
> +        self._wrapper = wrapper
> +        self._events = []
> +        self._iolog = None
> +        self._socket_scm_helper = socket_scm_helper
> +        self._qmp = None
> +        self._qemu_full_args = None
> +        self._test_dir = test_dir
> +        self._temp_dir = None
> +        self._launched = False
> +        self._machine = None
> +        self._console_set = False
> +        self._console_device_type = None
> +        self._console_address = None
> +        self._console_socket = None
> +
> +        # just in case logging wasn't configured by the main script:
> +        logging.basicConfig()
> +
> +    def __enter__(self):
> +        return self
> +
> +    def __exit__(self, exc_type, exc_val, exc_tb):
> +        self.shutdown()
> +        return False
> +
> +    # This can be used to add an unused monitor instance.
> +    def add_monitor_null(self):
> +        self._args.append('-monitor')
> +        self._args.append('null')
> +
> +    def add_fd(self, fd, fdset, opaque, opts=''):
> +        """
> +        Pass a file descriptor to the VM
> +        """
> +        options = ['fd=%d' % fd,
> +                   'set=%d' % fdset,
> +                   'opaque=%s' % opaque]
> +        if opts:
> +            options.append(opts)
> +
> +        # This did not exist before 3.4, but since then it is
> +        # mandatory for our purpose
> +        if hasattr(os, 'set_inheritable'):
> +            os.set_inheritable(fd, True)
> +
> +        self._args.append('-add-fd')
> +        self._args.append(','.join(options))
> +        return self
> +
> +    # Exactly one of fd and file_path must be given.
> +    # (If it is file_path, the helper will open that file and pass its
> +    # own fd)
> +    def send_fd_scm(self, fd=None, file_path=None):
> +        # In iotest.py, the qmp should always use unix socket.
> +        assert self._qmp.is_scm_available()
> +        if self._socket_scm_helper is None:
> +            raise QEMUMachineError("No path to socket_scm_helper set")
> +        if not os.path.exists(self._socket_scm_helper):
> +            raise QEMUMachineError("%s does not exist" %
> +                                   self._socket_scm_helper)
> +
> +        # This did not exist before 3.4, but since then it is
> +        # mandatory for our purpose
> +        if hasattr(os, 'set_inheritable'):
> +            os.set_inheritable(self._qmp.get_sock_fd(), True)
> +            if fd is not None:
> +                os.set_inheritable(fd, True)
> +
> +        fd_param = ["%s" % self._socket_scm_helper,
> +                    "%d" % self._qmp.get_sock_fd()]
> +
> +        if file_path is not None:
> +            assert fd is None
> +            fd_param.append(file_path)
> +        else:
> +            assert fd is not None
> +            fd_param.append(str(fd))
> +
> +        devnull = open(os.path.devnull, 'rb')
> +        proc = subprocess.Popen(fd_param, stdin=devnull, stdout=subprocess.PIPE,
> +                                stderr=subprocess.STDOUT, close_fds=False)
> +        output = proc.communicate()[0]
> +        if output:
> +            LOG.debug(output)
> +
> +        return proc.returncode
> +
> +    @staticmethod
> +    def _remove_if_exists(path):
> +        """
> +        Remove file object at path if it exists
> +        """
> +        try:
> +            os.remove(path)
> +        except OSError as exception:
> +            if exception.errno == errno.ENOENT:
> +                return
> +            raise
> +
> +    def is_running(self):
> +        return self._popen is not None and self._popen.poll() is None
> +
> +    def exitcode(self):
> +        if self._popen is None:
> +            return None
> +        return self._popen.poll()
> +
> +    def get_pid(self):
> +        if not self.is_running():
> +            return None
> +        return self._popen.pid
> +
> +    def _load_io_log(self):
> +        if self._qemu_log_path is not None:
> +            with open(self._qemu_log_path, "r") as iolog:
> +                self._iolog = iolog.read()
> +
> +    def _base_args(self):
> +        if isinstance(self._monitor_address, tuple):
> +            moncdev = "socket,id=mon,host=%s,port=%s" % (
> +                self._monitor_address[0],
> +                self._monitor_address[1])
> +        else:
> +            moncdev = 'socket,id=mon,path=%s' % self._vm_monitor
> +        args = ['-chardev', moncdev,
> +                '-mon', 'chardev=mon,mode=control',
> +                '-display', 'none', '-vga', 'none']
> +        if self._machine is not None:
> +            args.extend(['-machine', self._machine])
> +        if self._console_set:
> +            self._console_address = os.path.join(self._temp_dir,
> +                                                 self._name + "-console.sock")
> +            chardev = ('socket,id=console,path=%s,server,nowait' %
> +                       self._console_address)
> +            args.extend(['-chardev', chardev])
> +            if self._console_device_type is None:
> +                args.extend(['-serial', 'chardev:console'])
> +            else:
> +                device = '%s,chardev=console' % self._console_device_type
> +                args.extend(['-device', device])
> +        return args
> +
> +    def _pre_launch(self):
> +        self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
> +        if self._monitor_address is not None:
> +            self._vm_monitor = self._monitor_address
> +        else:
> +            self._vm_monitor = os.path.join(self._temp_dir,
> +                                            self._name + "-monitor.sock")
> +        self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
> +        self._qemu_log_file = open(self._qemu_log_path, 'wb')
> +
> +        self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
> +                                            server=True)
> +
> +    def _post_launch(self):
> +        self._qmp.accept()
> +
> +    def _post_shutdown(self):
> +        if self._qemu_log_file is not None:
> +            self._qemu_log_file.close()
> +            self._qemu_log_file = None
> +
> +        self._qemu_log_path = None
> +
> +        if self._console_socket is not None:
> +            self._console_socket.close()
> +            self._console_socket = None
> +
> +        if self._temp_dir is not None:
> +            shutil.rmtree(self._temp_dir)
> +            self._temp_dir = None
> +
> +    def launch(self):
> +        """
> +        Launch the VM and make sure we cleanup and expose the
> +        command line/output in case of exception
> +        """
> +
> +        if self._launched:
> +            raise QEMUMachineError('VM already launched')
> +
> +        self._iolog = None
> +        self._qemu_full_args = None
> +        try:
> +            self._launch()
> +            self._launched = True
> +        except:
> +            self.shutdown()
> +
> +            LOG.debug('Error launching VM')
> +            if self._qemu_full_args:
> +                LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
> +            if self._iolog:
> +                LOG.debug('Output: %r', self._iolog)
> +            raise
> +
> +    def _launch(self):
> +        """
> +        Launch the VM and establish a QMP connection
> +        """
> +        devnull = open(os.path.devnull, 'rb')
> +        self._pre_launch()
> +        self._qemu_full_args = (self._wrapper + [self._binary] +
> +                                self._base_args() + self._args)
> +        LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
> +        self._popen = subprocess.Popen(self._qemu_full_args,
> +                                       stdin=devnull,
> +                                       stdout=self._qemu_log_file,
> +                                       stderr=subprocess.STDOUT,
> +                                       shell=False,
> +                                       close_fds=False)
> +        self._post_launch()
> +
> +    def wait(self):
> +        """
> +        Wait for the VM to power off
> +        """
> +        self._popen.wait()
> +        self._qmp.close()
> +        self._load_io_log()
> +        self._post_shutdown()
> +
> +    def shutdown(self):
> +        """
> +        Terminate the VM and clean up
> +        """
> +        if self.is_running():
> +            try:
> +                self._qmp.cmd('quit')
> +                self._qmp.close()
> +            except:
> +                self._popen.kill()
> +            self._popen.wait()
> +
> +        self._load_io_log()
> +        self._post_shutdown()
> +
> +        exitcode = self.exitcode()
> +        if exitcode is not None and exitcode < 0:
> +            msg = 'qemu received signal %i: %s'
> +            if self._qemu_full_args:
> +                command = ' '.join(self._qemu_full_args)
> +            else:
> +                command = ''
> +            LOG.warn(msg, -exitcode, command)
> +
> +        self._launched = False
> +
> +    def qmp(self, cmd, conv_keys=True, **args):
> +        """
> +        Invoke a QMP command and return the response dict
> +        """
> +        qmp_args = dict()
> +        for key, value in args.items():
> +            if conv_keys:
> +                qmp_args[key.replace('_', '-')] = value
> +            else:
> +                qmp_args[key] = value
> +
> +        return self._qmp.cmd(cmd, args=qmp_args)
> +
> +    def command(self, cmd, conv_keys=True, **args):
> +        """
> +        Invoke a QMP command.
> +        On success return the response dict.
> +        On failure raise an exception.
> +        """
> +        reply = self.qmp(cmd, conv_keys, **args)
> +        if reply is None:
> +            raise qmp.QMPError("Monitor is closed")
> +        if "error" in reply:
> +            raise MonitorResponseError(reply)
> +        return reply["return"]
> +
> +    def get_qmp_event(self, wait=False):
> +        """
> +        Poll for one queued QMP events and return it
> +        """
> +        if len(self._events) > 0:
> +            return self._events.pop(0)
> +        return self._qmp.pull_event(wait=wait)
> +
> +    def get_qmp_events(self, wait=False):
> +        """
> +        Poll for queued QMP events and return a list of dicts
> +        """
> +        events = self._qmp.get_events(wait=wait)
> +        events.extend(self._events)
> +        del self._events[:]
> +        self._qmp.clear_events()
> +        return events
> +
> +    @staticmethod
> +    def event_match(event, match=None):
> +        """
> +        Check if an event matches optional match criteria.
> +
> +        The match criteria takes the form of a matching subdict. The event is
> +        checked to be a superset of the subdict, recursively, with matching
> +        values whenever the subdict values are not None.
> +
> +        This has a limitation that you cannot explicitly check for None values.
> +
> +        Examples, with the subdict queries on the left:
> +         - None matches any object.
> +         - {"foo": None} matches {"foo": {"bar": 1}}
> +         - {"foo": None} matches {"foo": 5}
> +         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
> +         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
> +        """
> +        if match is None:
> +            return True
> +
> +        try:
> +            for key in match:
> +                if key in event:
> +                    if not QEMUMachine.event_match(event[key], match[key]):
> +                        return False
> +                else:
> +                    return False
> +            return True
> +        except TypeError:
> +            # either match or event wasn't iterable (not a dict)
> +            return match == event
> +
> +    def event_wait(self, name, timeout=60.0, match=None):
> +        """
> +        event_wait waits for and returns a named event from QMP with a timeout.
> +
> +        name: The event to wait for.
> +        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
> +        match: Optional match criteria. See event_match for details.
> +        """
> +        return self.events_wait([(name, match)], timeout)
> +
> +    def events_wait(self, events, timeout=60.0):
> +        """
> +        events_wait waits for and returns a named event from QMP with a timeout.
> +
> +        events: a sequence of (name, match_criteria) tuples.
> +                The match criteria are optional and may be None.
> +                See event_match for details.
> +        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
> +        """
> +        def _match(event):
> +            for name, match in events:
> +                if (event['event'] == name and
> +                    self.event_match(event, match)):
> +                    return True
> +            return False
> +
> +        # Search cached events
> +        for event in self._events:
> +            if _match(event):
> +                self._events.remove(event)
> +                return event
> +
> +        # Poll for new events
> +        while True:
> +            event = self._qmp.pull_event(wait=timeout)
> +            if _match(event):
> +                return event
> +            self._events.append(event)
> +
> +        return None
> +
> +    def get_log(self):
> +        """
> +        After self.shutdown or failed qemu execution, this returns the output
> +        of the qemu process.
> +        """
> +        return self._iolog
> +
> +    def add_args(self, *args):
> +        """
> +        Adds to the list of extra arguments to be given to the QEMU binary
> +        """
> +        self._args.extend(args)
> +
> +    def set_machine(self, machine_type):
> +        """
> +        Sets the machine type
> +
> +        If set, the machine type will be added to the base arguments
> +        of the resulting QEMU command line.
> +        """
> +        self._machine = machine_type
> +
> +    def set_console(self, device_type=None):
> +        """
> +        Sets the device type for a console device
> +
> +        If set, the console device and a backing character device will
> +        be added to the base arguments of the resulting QEMU command
> +        line.
> +
> +        This is a convenience method that will either use the provided
> +        device type, or default to a "-serial chardev:console" command
> +        line argument.
> +
> +        The actual setting of command line arguments will be be done at
> +        machine launch time, as it depends on the temporary directory
> +        to be created.
> +
> +        @param device_type: the device type, such as "isa-serial".  If
> +                            None is given (the default value) a "-serial
> +                            chardev:console" command line argument will
> +                            be used instead, resorting to the machine's
> +                            default device type.
> +        """
> +        self._console_set = True
> +        self._console_device_type = device_type
> +
> +    @property
> +    def console_socket(self):
> +        """
> +        Returns a socket connected to the console
> +        """
> +        if self._console_socket is None:
> +            self._console_socket = socket.socket(socket.AF_UNIX,
> +                                                 socket.SOCK_STREAM)
> +            self._console_socket.connect(self._console_address)
> +        return self._console_socket
> diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
> index eb45824dd0..eebcc233ed 100644
> --- a/python/qemu/qtest.py
> +++ b/python/qemu/qtest.py
> @@ -14,7 +14,7 @@
>   import socket
>   import os
>   
> -from . import QEMUMachine
> +from .machine import QEMUMachine
>   
>   
>   class QEMUQtestProtocol(object):
> diff --git a/scripts/device-crash-test b/scripts/device-crash-test
> index a6748910ad..15f213a6cd 100755
> --- a/scripts/device-crash-test
> +++ b/scripts/device-crash-test
> @@ -36,7 +36,7 @@ import argparse
>   from itertools import chain
>   
>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
> -from qemu import QEMUMachine
> +from qemu.machine import QEMUMachine
>   
>   logger = logging.getLogger('device-crash-test')
>   dbg = logger.debug
> diff --git a/scripts/render_block_graph.py b/scripts/render_block_graph.py
> index 3e9d282a49..656f0388ad 100755
> --- a/scripts/render_block_graph.py
> +++ b/scripts/render_block_graph.py
> @@ -25,7 +25,7 @@ import json
>   from graphviz import Digraph
>   
>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
> -from qemu import MonitorResponseError
> +from qemu.machine import MonitorResponseError
>   
>   
>   def perm(arr):
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 2b236a1cf0..aee5d820ed 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -17,7 +17,7 @@ import avocado
>   SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
>   sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
>   
> -from qemu import QEMUMachine
> +from qemu.machine import QEMUMachine
>   
>   def is_readable_executable_file(path):
>       return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
> diff --git a/tests/acceptance/virtio_version.py b/tests/acceptance/virtio_version.py
> index 8b97453ff8..33593c29dd 100644
> --- a/tests/acceptance/virtio_version.py
> +++ b/tests/acceptance/virtio_version.py
> @@ -12,7 +12,7 @@ import sys
>   import os
>   
>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> -from qemu import QEMUMachine
> +from qemu.machine import QEMUMachine
>   from avocado_qemu import Test
>   
>   # Virtio Device IDs:
> diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
> index 0e304660b8..f13dbea800 100644
> --- a/tests/migration/guestperf/engine.py
> +++ b/tests/migration/guestperf/engine.py
> @@ -30,7 +30,7 @@ from guestperf.timings import TimingRecord, Timings
>   
>   sys.path.append(os.path.join(os.path.dirname(__file__),
>                                '..', '..', '..', 'python'))
> -import qemu
> +from qemu.machine import QEMUMachine
>   
>   
>   class Engine(object):
> @@ -386,17 +386,17 @@ class Engine(object):
>               dstmonaddr = "/var/tmp/qemu-dst-%d-monitor.sock" % os.getpid()
>           srcmonaddr = "/var/tmp/qemu-src-%d-monitor.sock" % os.getpid()
>   
> -        src = qemu.QEMUMachine(self._binary,
> -                               args=self._get_src_args(hardware),
> -                               wrapper=self._get_src_wrapper(hardware),
> -                               name="qemu-src-%d" % os.getpid(),
> -                               monitor_address=srcmonaddr)
> +        src = QEMUMachine(self._binary,
> +                          args=self._get_src_args(hardware),
> +                          wrapper=self._get_src_wrapper(hardware),
> +                          name="qemu-src-%d" % os.getpid(),
> +                          monitor_address=srcmonaddr)
>   
> -        dst = qemu.QEMUMachine(self._binary,
> -                               args=self._get_dst_args(hardware, uri),
> -                               wrapper=self._get_dst_wrapper(hardware),
> -                               name="qemu-dst-%d" % os.getpid(),
> -                               monitor_address=dstmonaddr)
> +        dst = QEMUMachine(self._binary,
> +                          args=self._get_dst_args(hardware, uri),
> +                          wrapper=self._get_dst_wrapper(hardware),
> +                          name="qemu-dst-%d" % os.getpid(),
> +                          monitor_address=dstmonaddr)
>   
>           try:
>               src.launch()
> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
> index 2b6a8c13be..fedd111fd4 100755
> --- a/tests/qemu-iotests/235
> +++ b/tests/qemu-iotests/235
> @@ -25,7 +25,7 @@ from iotests import qemu_img_create, qemu_io, file_path, log
>   
>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>   
> -from qemu import QEMUMachine
> +from qemu.machine import QEMUMachine
>   
>   # Note:
>   # This test was added to check that mirror dead-lock was fixed (see previous
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index 0556bdcf9e..4b496f1551 100755
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -18,7 +18,8 @@ import logging
>   import time
>   import datetime
>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> -from qemu import QEMUMachine, kvm_available
> +from qemu import kvm_available

Perhaps kvm_available deserves another house as well.

In '[PATCH 2/3] tests/acceptance: Introduce the "accel" tag' I created 
the avocado_qemu.accel module to host code related with accelerators and 
use on acceptance tests. So maybe I should create that module actually 
in qemu and then put kvm_available there. What do you think?

- Wainer


> +from qemu.machine import QEMUMachine
>   import subprocess
>   import hashlib
>   import optparse

Re: [Qemu-devel] [RFC PATCH v2 1/3] python/qemu: split QEMUMachine out from underneath __init__.py

[John Snow]

On 6/28/19 12:05 PM, Wainer dos Santos Moschetta wrote:
> 
> On 06/27/2019 06:28 PM, John Snow wrote:
>> It's not obvious that something named __init__.py actually houses
>> important code that isn't relevant to python packaging glue. Move the
>> QEMUMachine and related error classes out into their own module.
>>
>> Adjust users to the new import location.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   python/qemu/__init__.py                   | 502 +--------------------
>>   python/qemu/machine.py                    | 520 ++++++++++++++++++++++
>>   python/qemu/qtest.py                      |   2 +-
>>   scripts/device-crash-test                 |   2 +-
>>   scripts/render_block_graph.py             |   2 +-
>>   tests/acceptance/avocado_qemu/__init__.py |   2 +-
>>   tests/acceptance/virtio_version.py        |   2 +-
>>   tests/migration/guestperf/engine.py       |  22 +-
>>   tests/qemu-iotests/235                    |   2 +-
>>   tests/vm/basevm.py                        |   3 +-
>>   10 files changed, 540 insertions(+), 519 deletions(-)
>>   create mode 100644 python/qemu/machine.py
>>
>> diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
>> index dbaf8a5311..6c919a3d56 100644
>> --- a/python/qemu/__init__.py
>> +++ b/python/qemu/__init__.py
>> @@ -12,17 +12,11 @@
>>   # Based on qmp.py.
>>   #
>>   -import errno
>>   import logging
>>   import os
>> -import subprocess
>> -import re
>> -import shutil
>> -import socket
>> -import tempfile
>>     from . import qmp
>> -
>> +from . import machine
>>     LOG = logging.getLogger(__name__)
>>   @@ -39,497 +33,3 @@ def kvm_available(target_arch=None):
>>           if target_arch != ADDITIONAL_ARCHES.get(host_arch):
>>               return False
>>       return os.access("/dev/kvm", os.R_OK | os.W_OK)
>> -
>> -
>> -class QEMUMachineError(Exception):
>> -    """
>> -    Exception called when an error in QEMUMachine happens.
>> -    """
>> -
>> -
>> -class QEMUMachineAddDeviceError(QEMUMachineError):
>> -    """
>> -    Exception raised when a request to add a device can not be fulfilled
>> -
>> -    The failures are caused by limitations, lack of information or
>> conflicting
>> -    requests on the QEMUMachine methods.  This exception does not
>> represent
>> -    failures reported by the QEMU binary itself.
>> -    """
>> -
>> -class MonitorResponseError(qmp.QMPError):
>> -    """
>> -    Represents erroneous QMP monitor reply
>> -    """
>> -    def __init__(self, reply):
>> -        try:
>> -            desc = reply["error"]["desc"]
>> -        except KeyError:
>> -            desc = reply
>> -        super(MonitorResponseError, self).__init__(desc)
>> -        self.reply = reply
>> -
>> -
>> -class QEMUMachine(object):
>> -    """
>> -    A QEMU VM
>> -
>> -    Use this object as a context manager to ensure the QEMU process
>> terminates::
>> -
>> -        with VM(binary) as vm:
>> -            ...
>> -        # vm is guaranteed to be shut down here
>> -    """
>> -
>> -    def __init__(self, binary, args=None, wrapper=None, name=None,
>> -                 test_dir="/var/tmp", monitor_address=None,
>> -                 socket_scm_helper=None):
>> -        '''
>> -        Initialize a QEMUMachine
>> -
>> -        @param binary: path to the qemu binary
>> -        @param args: list of extra arguments
>> -        @param wrapper: list of arguments used as prefix to qemu binary
>> -        @param name: prefix for socket and log file names (default:
>> qemu-PID)
>> -        @param test_dir: where to create socket and log file
>> -        @param monitor_address: address for QMP monitor
>> -        @param socket_scm_helper: helper program, required for
>> send_fd_scm()
>> -        @note: Qemu process is not started until launch() is used.
>> -        '''
>> -        if args is None:
>> -            args = []
>> -        if wrapper is None:
>> -            wrapper = []
>> -        if name is None:
>> -            name = "qemu-%d" % os.getpid()
>> -        self._name = name
>> -        self._monitor_address = monitor_address
>> -        self._vm_monitor = None
>> -        self._qemu_log_path = None
>> -        self._qemu_log_file = None
>> -        self._popen = None
>> -        self._binary = binary
>> -        self._args = list(args)     # Force copy args in case we
>> modify them
>> -        self._wrapper = wrapper
>> -        self._events = []
>> -        self._iolog = None
>> -        self._socket_scm_helper = socket_scm_helper
>> -        self._qmp = None
>> -        self._qemu_full_args = None
>> -        self._test_dir = test_dir
>> -        self._temp_dir = None
>> -        self._launched = False
>> -        self._machine = None
>> -        self._console_set = False
>> -        self._console_device_type = None
>> -        self._console_address = None
>> -        self._console_socket = None
>> -
>> -        # just in case logging wasn't configured by the main script:
>> -        logging.basicConfig()
>> -
>> -    def __enter__(self):
>> -        return self
>> -
>> -    def __exit__(self, exc_type, exc_val, exc_tb):
>> -        self.shutdown()
>> -        return False
>> -
>> -    # This can be used to add an unused monitor instance.
>> -    def add_monitor_null(self):
>> -        self._args.append('-monitor')
>> -        self._args.append('null')
>> -
>> -    def add_fd(self, fd, fdset, opaque, opts=''):
>> -        """
>> -        Pass a file descriptor to the VM
>> -        """
>> -        options = ['fd=%d' % fd,
>> -                   'set=%d' % fdset,
>> -                   'opaque=%s' % opaque]
>> -        if opts:
>> -            options.append(opts)
>> -
>> -        # This did not exist before 3.4, but since then it is
>> -        # mandatory for our purpose
>> -        if hasattr(os, 'set_inheritable'):
>> -            os.set_inheritable(fd, True)
>> -
>> -        self._args.append('-add-fd')
>> -        self._args.append(','.join(options))
>> -        return self
>> -
>> -    # Exactly one of fd and file_path must be given.
>> -    # (If it is file_path, the helper will open that file and pass its
>> -    # own fd)
>> -    def send_fd_scm(self, fd=None, file_path=None):
>> -        # In iotest.py, the qmp should always use unix socket.
>> -        assert self._qmp.is_scm_available()
>> -        if self._socket_scm_helper is None:
>> -            raise QEMUMachineError("No path to socket_scm_helper set")
>> -        if not os.path.exists(self._socket_scm_helper):
>> -            raise QEMUMachineError("%s does not exist" %
>> -                                   self._socket_scm_helper)
>> -
>> -        # This did not exist before 3.4, but since then it is
>> -        # mandatory for our purpose
>> -        if hasattr(os, 'set_inheritable'):
>> -            os.set_inheritable(self._qmp.get_sock_fd(), True)
>> -            if fd is not None:
>> -                os.set_inheritable(fd, True)
>> -
>> -        fd_param = ["%s" % self._socket_scm_helper,
>> -                    "%d" % self._qmp.get_sock_fd()]
>> -
>> -        if file_path is not None:
>> -            assert fd is None
>> -            fd_param.append(file_path)
>> -        else:
>> -            assert fd is not None
>> -            fd_param.append(str(fd))
>> -
>> -        devnull = open(os.path.devnull, 'rb')
>> -        proc = subprocess.Popen(fd_param, stdin=devnull,
>> stdout=subprocess.PIPE,
>> -                                stderr=subprocess.STDOUT,
>> close_fds=False)
>> -        output = proc.communicate()[0]
>> -        if output:
>> -            LOG.debug(output)
>> -
>> -        return proc.returncode
>> -
>> -    @staticmethod
>> -    def _remove_if_exists(path):
>> -        """
>> -        Remove file object at path if it exists
>> -        """
>> -        try:
>> -            os.remove(path)
>> -        except OSError as exception:
>> -            if exception.errno == errno.ENOENT:
>> -                return
>> -            raise
>> -
>> -    def is_running(self):
>> -        return self._popen is not None and self._popen.poll() is None
>> -
>> -    def exitcode(self):
>> -        if self._popen is None:
>> -            return None
>> -        return self._popen.poll()
>> -
>> -    def get_pid(self):
>> -        if not self.is_running():
>> -            return None
>> -        return self._popen.pid
>> -
>> -    def _load_io_log(self):
>> -        if self._qemu_log_path is not None:
>> -            with open(self._qemu_log_path, "r") as iolog:
>> -                self._iolog = iolog.read()
>> -
>> -    def _base_args(self):
>> -        if isinstance(self._monitor_address, tuple):
>> -            moncdev = "socket,id=mon,host=%s,port=%s" % (
>> -                self._monitor_address[0],
>> -                self._monitor_address[1])
>> -        else:
>> -            moncdev = 'socket,id=mon,path=%s' % self._vm_monitor
>> -        args = ['-chardev', moncdev,
>> -                '-mon', 'chardev=mon,mode=control',
>> -                '-display', 'none', '-vga', 'none']
>> -        if self._machine is not None:
>> -            args.extend(['-machine', self._machine])
>> -        if self._console_set:
>> -            self._console_address = os.path.join(self._temp_dir,
>> -                                                 self._name +
>> "-console.sock")
>> -            chardev = ('socket,id=console,path=%s,server,nowait' %
>> -                       self._console_address)
>> -            args.extend(['-chardev', chardev])
>> -            if self._console_device_type is None:
>> -                args.extend(['-serial', 'chardev:console'])
>> -            else:
>> -                device = '%s,chardev=console' %
>> self._console_device_type
>> -                args.extend(['-device', device])
>> -        return args
>> -
>> -    def _pre_launch(self):
>> -        self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
>> -        if self._monitor_address is not None:
>> -            self._vm_monitor = self._monitor_address
>> -        else:
>> -            self._vm_monitor = os.path.join(self._temp_dir,
>> -                                            self._name +
>> "-monitor.sock")
>> -        self._qemu_log_path = os.path.join(self._temp_dir, self._name
>> + ".log")
>> -        self._qemu_log_file = open(self._qemu_log_path, 'wb')
>> -
>> -        self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
>> -                                            server=True)
>> -
>> -    def _post_launch(self):
>> -        self._qmp.accept()
>> -
>> -    def _post_shutdown(self):
>> -        if self._qemu_log_file is not None:
>> -            self._qemu_log_file.close()
>> -            self._qemu_log_file = None
>> -
>> -        self._qemu_log_path = None
>> -
>> -        if self._console_socket is not None:
>> -            self._console_socket.close()
>> -            self._console_socket = None
>> -
>> -        if self._temp_dir is not None:
>> -            shutil.rmtree(self._temp_dir)
>> -            self._temp_dir = None
>> -
>> -    def launch(self):
>> -        """
>> -        Launch the VM and make sure we cleanup and expose the
>> -        command line/output in case of exception
>> -        """
>> -
>> -        if self._launched:
>> -            raise QEMUMachineError('VM already launched')
>> -
>> -        self._iolog = None
>> -        self._qemu_full_args = None
>> -        try:
>> -            self._launch()
>> -            self._launched = True
>> -        except:
>> -            self.shutdown()
>> -
>> -            LOG.debug('Error launching VM')
>> -            if self._qemu_full_args:
>> -                LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
>> -            if self._iolog:
>> -                LOG.debug('Output: %r', self._iolog)
>> -            raise
>> -
>> -    def _launch(self):
>> -        """
>> -        Launch the VM and establish a QMP connection
>> -        """
>> -        devnull = open(os.path.devnull, 'rb')
>> -        self._pre_launch()
>> -        self._qemu_full_args = (self._wrapper + [self._binary] +
>> -                                self._base_args() + self._args)
>> -        LOG.debug('VM launch command: %r', '
>> '.join(self._qemu_full_args))
>> -        self._popen = subprocess.Popen(self._qemu_full_args,
>> -                                       stdin=devnull,
>> -                                       stdout=self._qemu_log_file,
>> -                                       stderr=subprocess.STDOUT,
>> -                                       shell=False,
>> -                                       close_fds=False)
>> -        self._post_launch()
>> -
>> -    def wait(self):
>> -        """
>> -        Wait for the VM to power off
>> -        """
>> -        self._popen.wait()
>> -        self._qmp.close()
>> -        self._load_io_log()
>> -        self._post_shutdown()
>> -
>> -    def shutdown(self):
>> -        """
>> -        Terminate the VM and clean up
>> -        """
>> -        if self.is_running():
>> -            try:
>> -                self._qmp.cmd('quit')
>> -                self._qmp.close()
>> -            except:
>> -                self._popen.kill()
>> -            self._popen.wait()
>> -
>> -        self._load_io_log()
>> -        self._post_shutdown()
>> -
>> -        exitcode = self.exitcode()
>> -        if exitcode is not None and exitcode < 0:
>> -            msg = 'qemu received signal %i: %s'
>> -            if self._qemu_full_args:
>> -                command = ' '.join(self._qemu_full_args)
>> -            else:
>> -                command = ''
>> -            LOG.warn(msg, -exitcode, command)
>> -
>> -        self._launched = False
>> -
>> -    def qmp(self, cmd, conv_keys=True, **args):
>> -        """
>> -        Invoke a QMP command and return the response dict
>> -        """
>> -        qmp_args = dict()
>> -        for key, value in args.items():
>> -            if conv_keys:
>> -                qmp_args[key.replace('_', '-')] = value
>> -            else:
>> -                qmp_args[key] = value
>> -
>> -        return self._qmp.cmd(cmd, args=qmp_args)
>> -
>> -    def command(self, cmd, conv_keys=True, **args):
>> -        """
>> -        Invoke a QMP command.
>> -        On success return the response dict.
>> -        On failure raise an exception.
>> -        """
>> -        reply = self.qmp(cmd, conv_keys, **args)
>> -        if reply is None:
>> -            raise qmp.QMPError("Monitor is closed")
>> -        if "error" in reply:
>> -            raise MonitorResponseError(reply)
>> -        return reply["return"]
>> -
>> -    def get_qmp_event(self, wait=False):
>> -        """
>> -        Poll for one queued QMP events and return it
>> -        """
>> -        if len(self._events) > 0:
>> -            return self._events.pop(0)
>> -        return self._qmp.pull_event(wait=wait)
>> -
>> -    def get_qmp_events(self, wait=False):
>> -        """
>> -        Poll for queued QMP events and return a list of dicts
>> -        """
>> -        events = self._qmp.get_events(wait=wait)
>> -        events.extend(self._events)
>> -        del self._events[:]
>> -        self._qmp.clear_events()
>> -        return events
>> -
>> -    @staticmethod
>> -    def event_match(event, match=None):
>> -        """
>> -        Check if an event matches optional match criteria.
>> -
>> -        The match criteria takes the form of a matching subdict. The
>> event is
>> -        checked to be a superset of the subdict, recursively, with
>> matching
>> -        values whenever the subdict values are not None.
>> -
>> -        This has a limitation that you cannot explicitly check for
>> None values.
>> -
>> -        Examples, with the subdict queries on the left:
>> -         - None matches any object.
>> -         - {"foo": None} matches {"foo": {"bar": 1}}
>> -         - {"foo": None} matches {"foo": 5}
>> -         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
>> -         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
>> -        """
>> -        if match is None:
>> -            return True
>> -
>> -        try:
>> -            for key in match:
>> -                if key in event:
>> -                    if not QEMUMachine.event_match(event[key],
>> match[key]):
>> -                        return False
>> -                else:
>> -                    return False
>> -            return True
>> -        except TypeError:
>> -            # either match or event wasn't iterable (not a dict)
>> -            return match == event
>> -
>> -    def event_wait(self, name, timeout=60.0, match=None):
>> -        """
>> -        event_wait waits for and returns a named event from QMP with
>> a timeout.
>> -
>> -        name: The event to wait for.
>> -        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
>> -        match: Optional match criteria. See event_match for details.
>> -        """
>> -        return self.events_wait([(name, match)], timeout)
>> -
>> -    def events_wait(self, events, timeout=60.0):
>> -        """
>> -        events_wait waits for and returns a named event from QMP with
>> a timeout.
>> -
>> -        events: a sequence of (name, match_criteria) tuples.
>> -                The match criteria are optional and may be None.
>> -                See event_match for details.
>> -        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
>> -        """
>> -        def _match(event):
>> -            for name, match in events:
>> -                if (event['event'] == name and
>> -                    self.event_match(event, match)):
>> -                    return True
>> -            return False
>> -
>> -        # Search cached events
>> -        for event in self._events:
>> -            if _match(event):
>> -                self._events.remove(event)
>> -                return event
>> -
>> -        # Poll for new events
>> -        while True:
>> -            event = self._qmp.pull_event(wait=timeout)
>> -            if _match(event):
>> -                return event
>> -            self._events.append(event)
>> -
>> -        return None
>> -
>> -    def get_log(self):
>> -        """
>> -        After self.shutdown or failed qemu execution, this returns
>> the output
>> -        of the qemu process.
>> -        """
>> -        return self._iolog
>> -
>> -    def add_args(self, *args):
>> -        """
>> -        Adds to the list of extra arguments to be given to the QEMU
>> binary
>> -        """
>> -        self._args.extend(args)
>> -
>> -    def set_machine(self, machine_type):
>> -        """
>> -        Sets the machine type
>> -
>> -        If set, the machine type will be added to the base arguments
>> -        of the resulting QEMU command line.
>> -        """
>> -        self._machine = machine_type
>> -
>> -    def set_console(self, device_type=None):
>> -        """
>> -        Sets the device type for a console device
>> -
>> -        If set, the console device and a backing character device will
>> -        be added to the base arguments of the resulting QEMU command
>> -        line.
>> -
>> -        This is a convenience method that will either use the provided
>> -        device type, or default to a "-serial chardev:console" command
>> -        line argument.
>> -
>> -        The actual setting of command line arguments will be be done at
>> -        machine launch time, as it depends on the temporary directory
>> -        to be created.
>> -
>> -        @param device_type: the device type, such as "isa-serial".  If
>> -                            None is given (the default value) a "-serial
>> -                            chardev:console" command line argument will
>> -                            be used instead, resorting to the machine's
>> -                            default device type.
>> -        """
>> -        self._console_set = True
>> -        self._console_device_type = device_type
>> -
>> -    @property
>> -    def console_socket(self):
>> -        """
>> -        Returns a socket connected to the console
>> -        """
>> -        if self._console_socket is None:
>> -            self._console_socket = socket.socket(socket.AF_UNIX,
>> -                                                 socket.SOCK_STREAM)
>> -            self._console_socket.connect(self._console_address)
>> -        return self._console_socket
>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> new file mode 100644
>> index 0000000000..a8a311b035
>> --- /dev/null
>> +++ b/python/qemu/machine.py
>> @@ -0,0 +1,520 @@
>> +# QEMU library
>> +#
>> +# Copyright (C) 2015-2016 Red Hat Inc.
>> +# Copyright (C) 2012 IBM Corp.
>> +#
>> +# Authors:
>> +#  Fam Zheng <famz@redhat.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2.  See
>> +# the COPYING file in the top-level directory.
>> +#
>> +# Based on qmp.py.
>> +#
>> +
>> +import errno
>> +import logging
>> +import os
>> +import subprocess
>> +import re
>> +import shutil
>> +import socket
>> +import tempfile
>> +
>> +from . import qmp
>> +
>> +LOG = logging.getLogger(__name__)
>> +
>> +class QEMUMachineError(Exception):
>> +    """
>> +    Exception called when an error in QEMUMachine happens.
>> +    """
>> +
>> +
>> +class QEMUMachineAddDeviceError(QEMUMachineError):
>> +    """
>> +    Exception raised when a request to add a device can not be fulfilled
>> +
>> +    The failures are caused by limitations, lack of information or
>> conflicting
>> +    requests on the QEMUMachine methods.  This exception does not
>> represent
>> +    failures reported by the QEMU binary itself.
>> +    """
>> +
>> +
>> +class MonitorResponseError(qmp.QMPError):
>> +    """
>> +    Represents erroneous QMP monitor reply
>> +    """
>> +    def __init__(self, reply):
>> +        try:
>> +            desc = reply["error"]["desc"]
>> +        except KeyError:
>> +            desc = reply
>> +        super(MonitorResponseError, self).__init__(desc)
>> +        self.reply = reply
>> +
>> +
>> +class QEMUMachine(object):
>> +    """
>> +    A QEMU VM
>> +
>> +    Use this object as a context manager to ensure the QEMU process
>> terminates::
>> +
>> +        with VM(binary) as vm:
>> +            ...
>> +        # vm is guaranteed to be shut down here
>> +    """
>> +
>> +    def __init__(self, binary, args=None, wrapper=None, name=None,
>> +                 test_dir="/var/tmp", monitor_address=None,
>> +                 socket_scm_helper=None):
>> +        '''
>> +        Initialize a QEMUMachine
>> +
>> +        @param binary: path to the qemu binary
>> +        @param args: list of extra arguments
>> +        @param wrapper: list of arguments used as prefix to qemu binary
>> +        @param name: prefix for socket and log file names (default:
>> qemu-PID)
>> +        @param test_dir: where to create socket and log file
>> +        @param monitor_address: address for QMP monitor
>> +        @param socket_scm_helper: helper program, required for
>> send_fd_scm()
>> +        @note: Qemu process is not started until launch() is used.
>> +        '''
>> +        if args is None:
>> +            args = []
>> +        if wrapper is None:
>> +            wrapper = []
>> +        if name is None:
>> +            name = "qemu-%d" % os.getpid()
>> +        self._name = name
>> +        self._monitor_address = monitor_address
>> +        self._vm_monitor = None
>> +        self._qemu_log_path = None
>> +        self._qemu_log_file = None
>> +        self._popen = None
>> +        self._binary = binary
>> +        self._args = list(args)     # Force copy args in case we
>> modify them
>> +        self._wrapper = wrapper
>> +        self._events = []
>> +        self._iolog = None
>> +        self._socket_scm_helper = socket_scm_helper
>> +        self._qmp = None
>> +        self._qemu_full_args = None
>> +        self._test_dir = test_dir
>> +        self._temp_dir = None
>> +        self._launched = False
>> +        self._machine = None
>> +        self._console_set = False
>> +        self._console_device_type = None
>> +        self._console_address = None
>> +        self._console_socket = None
>> +
>> +        # just in case logging wasn't configured by the main script:
>> +        logging.basicConfig()
>> +
>> +    def __enter__(self):
>> +        return self
>> +
>> +    def __exit__(self, exc_type, exc_val, exc_tb):
>> +        self.shutdown()
>> +        return False
>> +
>> +    # This can be used to add an unused monitor instance.
>> +    def add_monitor_null(self):
>> +        self._args.append('-monitor')
>> +        self._args.append('null')
>> +
>> +    def add_fd(self, fd, fdset, opaque, opts=''):
>> +        """
>> +        Pass a file descriptor to the VM
>> +        """
>> +        options = ['fd=%d' % fd,
>> +                   'set=%d' % fdset,
>> +                   'opaque=%s' % opaque]
>> +        if opts:
>> +            options.append(opts)
>> +
>> +        # This did not exist before 3.4, but since then it is
>> +        # mandatory for our purpose
>> +        if hasattr(os, 'set_inheritable'):
>> +            os.set_inheritable(fd, True)
>> +
>> +        self._args.append('-add-fd')
>> +        self._args.append(','.join(options))
>> +        return self
>> +
>> +    # Exactly one of fd and file_path must be given.
>> +    # (If it is file_path, the helper will open that file and pass its
>> +    # own fd)
>> +    def send_fd_scm(self, fd=None, file_path=None):
>> +        # In iotest.py, the qmp should always use unix socket.
>> +        assert self._qmp.is_scm_available()
>> +        if self._socket_scm_helper is None:
>> +            raise QEMUMachineError("No path to socket_scm_helper set")
>> +        if not os.path.exists(self._socket_scm_helper):
>> +            raise QEMUMachineError("%s does not exist" %
>> +                                   self._socket_scm_helper)
>> +
>> +        # This did not exist before 3.4, but since then it is
>> +        # mandatory for our purpose
>> +        if hasattr(os, 'set_inheritable'):
>> +            os.set_inheritable(self._qmp.get_sock_fd(), True)
>> +            if fd is not None:
>> +                os.set_inheritable(fd, True)
>> +
>> +        fd_param = ["%s" % self._socket_scm_helper,
>> +                    "%d" % self._qmp.get_sock_fd()]
>> +
>> +        if file_path is not None:
>> +            assert fd is None
>> +            fd_param.append(file_path)
>> +        else:
>> +            assert fd is not None
>> +            fd_param.append(str(fd))
>> +
>> +        devnull = open(os.path.devnull, 'rb')
>> +        proc = subprocess.Popen(fd_param, stdin=devnull,
>> stdout=subprocess.PIPE,
>> +                                stderr=subprocess.STDOUT,
>> close_fds=False)
>> +        output = proc.communicate()[0]
>> +        if output:
>> +            LOG.debug(output)
>> +
>> +        return proc.returncode
>> +
>> +    @staticmethod
>> +    def _remove_if_exists(path):
>> +        """
>> +        Remove file object at path if it exists
>> +        """
>> +        try:
>> +            os.remove(path)
>> +        except OSError as exception:
>> +            if exception.errno == errno.ENOENT:
>> +                return
>> +            raise
>> +
>> +    def is_running(self):
>> +        return self._popen is not None and self._popen.poll() is None
>> +
>> +    def exitcode(self):
>> +        if self._popen is None:
>> +            return None
>> +        return self._popen.poll()
>> +
>> +    def get_pid(self):
>> +        if not self.is_running():
>> +            return None
>> +        return self._popen.pid
>> +
>> +    def _load_io_log(self):
>> +        if self._qemu_log_path is not None:
>> +            with open(self._qemu_log_path, "r") as iolog:
>> +                self._iolog = iolog.read()
>> +
>> +    def _base_args(self):
>> +        if isinstance(self._monitor_address, tuple):
>> +            moncdev = "socket,id=mon,host=%s,port=%s" % (
>> +                self._monitor_address[0],
>> +                self._monitor_address[1])
>> +        else:
>> +            moncdev = 'socket,id=mon,path=%s' % self._vm_monitor
>> +        args = ['-chardev', moncdev,
>> +                '-mon', 'chardev=mon,mode=control',
>> +                '-display', 'none', '-vga', 'none']
>> +        if self._machine is not None:
>> +            args.extend(['-machine', self._machine])
>> +        if self._console_set:
>> +            self._console_address = os.path.join(self._temp_dir,
>> +                                                 self._name +
>> "-console.sock")
>> +            chardev = ('socket,id=console,path=%s,server,nowait' %
>> +                       self._console_address)
>> +            args.extend(['-chardev', chardev])
>> +            if self._console_device_type is None:
>> +                args.extend(['-serial', 'chardev:console'])
>> +            else:
>> +                device = '%s,chardev=console' %
>> self._console_device_type
>> +                args.extend(['-device', device])
>> +        return args
>> +
>> +    def _pre_launch(self):
>> +        self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
>> +        if self._monitor_address is not None:
>> +            self._vm_monitor = self._monitor_address
>> +        else:
>> +            self._vm_monitor = os.path.join(self._temp_dir,
>> +                                            self._name +
>> "-monitor.sock")
>> +        self._qemu_log_path = os.path.join(self._temp_dir, self._name
>> + ".log")
>> +        self._qemu_log_file = open(self._qemu_log_path, 'wb')
>> +
>> +        self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
>> +                                            server=True)
>> +
>> +    def _post_launch(self):
>> +        self._qmp.accept()
>> +
>> +    def _post_shutdown(self):
>> +        if self._qemu_log_file is not None:
>> +            self._qemu_log_file.close()
>> +            self._qemu_log_file = None
>> +
>> +        self._qemu_log_path = None
>> +
>> +        if self._console_socket is not None:
>> +            self._console_socket.close()
>> +            self._console_socket = None
>> +
>> +        if self._temp_dir is not None:
>> +            shutil.rmtree(self._temp_dir)
>> +            self._temp_dir = None
>> +
>> +    def launch(self):
>> +        """
>> +        Launch the VM and make sure we cleanup and expose the
>> +        command line/output in case of exception
>> +        """
>> +
>> +        if self._launched:
>> +            raise QEMUMachineError('VM already launched')
>> +
>> +        self._iolog = None
>> +        self._qemu_full_args = None
>> +        try:
>> +            self._launch()
>> +            self._launched = True
>> +        except:
>> +            self.shutdown()
>> +
>> +            LOG.debug('Error launching VM')
>> +            if self._qemu_full_args:
>> +                LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
>> +            if self._iolog:
>> +                LOG.debug('Output: %r', self._iolog)
>> +            raise
>> +
>> +    def _launch(self):
>> +        """
>> +        Launch the VM and establish a QMP connection
>> +        """
>> +        devnull = open(os.path.devnull, 'rb')
>> +        self._pre_launch()
>> +        self._qemu_full_args = (self._wrapper + [self._binary] +
>> +                                self._base_args() + self._args)
>> +        LOG.debug('VM launch command: %r', '
>> '.join(self._qemu_full_args))
>> +        self._popen = subprocess.Popen(self._qemu_full_args,
>> +                                       stdin=devnull,
>> +                                       stdout=self._qemu_log_file,
>> +                                       stderr=subprocess.STDOUT,
>> +                                       shell=False,
>> +                                       close_fds=False)
>> +        self._post_launch()
>> +
>> +    def wait(self):
>> +        """
>> +        Wait for the VM to power off
>> +        """
>> +        self._popen.wait()
>> +        self._qmp.close()
>> +        self._load_io_log()
>> +        self._post_shutdown()
>> +
>> +    def shutdown(self):
>> +        """
>> +        Terminate the VM and clean up
>> +        """
>> +        if self.is_running():
>> +            try:
>> +                self._qmp.cmd('quit')
>> +                self._qmp.close()
>> +            except:
>> +                self._popen.kill()
>> +            self._popen.wait()
>> +
>> +        self._load_io_log()
>> +        self._post_shutdown()
>> +
>> +        exitcode = self.exitcode()
>> +        if exitcode is not None and exitcode < 0:
>> +            msg = 'qemu received signal %i: %s'
>> +            if self._qemu_full_args:
>> +                command = ' '.join(self._qemu_full_args)
>> +            else:
>> +                command = ''
>> +            LOG.warn(msg, -exitcode, command)
>> +
>> +        self._launched = False
>> +
>> +    def qmp(self, cmd, conv_keys=True, **args):
>> +        """
>> +        Invoke a QMP command and return the response dict
>> +        """
>> +        qmp_args = dict()
>> +        for key, value in args.items():
>> +            if conv_keys:
>> +                qmp_args[key.replace('_', '-')] = value
>> +            else:
>> +                qmp_args[key] = value
>> +
>> +        return self._qmp.cmd(cmd, args=qmp_args)
>> +
>> +    def command(self, cmd, conv_keys=True, **args):
>> +        """
>> +        Invoke a QMP command.
>> +        On success return the response dict.
>> +        On failure raise an exception.
>> +        """
>> +        reply = self.qmp(cmd, conv_keys, **args)
>> +        if reply is None:
>> +            raise qmp.QMPError("Monitor is closed")
>> +        if "error" in reply:
>> +            raise MonitorResponseError(reply)
>> +        return reply["return"]
>> +
>> +    def get_qmp_event(self, wait=False):
>> +        """
>> +        Poll for one queued QMP events and return it
>> +        """
>> +        if len(self._events) > 0:
>> +            return self._events.pop(0)
>> +        return self._qmp.pull_event(wait=wait)
>> +
>> +    def get_qmp_events(self, wait=False):
>> +        """
>> +        Poll for queued QMP events and return a list of dicts
>> +        """
>> +        events = self._qmp.get_events(wait=wait)
>> +        events.extend(self._events)
>> +        del self._events[:]
>> +        self._qmp.clear_events()
>> +        return events
>> +
>> +    @staticmethod
>> +    def event_match(event, match=None):
>> +        """
>> +        Check if an event matches optional match criteria.
>> +
>> +        The match criteria takes the form of a matching subdict. The
>> event is
>> +        checked to be a superset of the subdict, recursively, with
>> matching
>> +        values whenever the subdict values are not None.
>> +
>> +        This has a limitation that you cannot explicitly check for
>> None values.
>> +
>> +        Examples, with the subdict queries on the left:
>> +         - None matches any object.
>> +         - {"foo": None} matches {"foo": {"bar": 1}}
>> +         - {"foo": None} matches {"foo": 5}
>> +         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
>> +         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
>> +        """
>> +        if match is None:
>> +            return True
>> +
>> +        try:
>> +            for key in match:
>> +                if key in event:
>> +                    if not QEMUMachine.event_match(event[key],
>> match[key]):
>> +                        return False
>> +                else:
>> +                    return False
>> +            return True
>> +        except TypeError:
>> +            # either match or event wasn't iterable (not a dict)
>> +            return match == event
>> +
>> +    def event_wait(self, name, timeout=60.0, match=None):
>> +        """
>> +        event_wait waits for and returns a named event from QMP with
>> a timeout.
>> +
>> +        name: The event to wait for.
>> +        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
>> +        match: Optional match criteria. See event_match for details.
>> +        """
>> +        return self.events_wait([(name, match)], timeout)
>> +
>> +    def events_wait(self, events, timeout=60.0):
>> +        """
>> +        events_wait waits for and returns a named event from QMP with
>> a timeout.
>> +
>> +        events: a sequence of (name, match_criteria) tuples.
>> +                The match criteria are optional and may be None.
>> +                See event_match for details.
>> +        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
>> +        """
>> +        def _match(event):
>> +            for name, match in events:
>> +                if (event['event'] == name and
>> +                    self.event_match(event, match)):
>> +                    return True
>> +            return False
>> +
>> +        # Search cached events
>> +        for event in self._events:
>> +            if _match(event):
>> +                self._events.remove(event)
>> +                return event
>> +
>> +        # Poll for new events
>> +        while True:
>> +            event = self._qmp.pull_event(wait=timeout)
>> +            if _match(event):
>> +                return event
>> +            self._events.append(event)
>> +
>> +        return None
>> +
>> +    def get_log(self):
>> +        """
>> +        After self.shutdown or failed qemu execution, this returns
>> the output
>> +        of the qemu process.
>> +        """
>> +        return self._iolog
>> +
>> +    def add_args(self, *args):
>> +        """
>> +        Adds to the list of extra arguments to be given to the QEMU
>> binary
>> +        """
>> +        self._args.extend(args)
>> +
>> +    def set_machine(self, machine_type):
>> +        """
>> +        Sets the machine type
>> +
>> +        If set, the machine type will be added to the base arguments
>> +        of the resulting QEMU command line.
>> +        """
>> +        self._machine = machine_type
>> +
>> +    def set_console(self, device_type=None):
>> +        """
>> +        Sets the device type for a console device
>> +
>> +        If set, the console device and a backing character device will
>> +        be added to the base arguments of the resulting QEMU command
>> +        line.
>> +
>> +        This is a convenience method that will either use the provided
>> +        device type, or default to a "-serial chardev:console" command
>> +        line argument.
>> +
>> +        The actual setting of command line arguments will be be done at
>> +        machine launch time, as it depends on the temporary directory
>> +        to be created.
>> +
>> +        @param device_type: the device type, such as "isa-serial".  If
>> +                            None is given (the default value) a "-serial
>> +                            chardev:console" command line argument will
>> +                            be used instead, resorting to the machine's
>> +                            default device type.
>> +        """
>> +        self._console_set = True
>> +        self._console_device_type = device_type
>> +
>> +    @property
>> +    def console_socket(self):
>> +        """
>> +        Returns a socket connected to the console
>> +        """
>> +        if self._console_socket is None:
>> +            self._console_socket = socket.socket(socket.AF_UNIX,
>> +                                                 socket.SOCK_STREAM)
>> +            self._console_socket.connect(self._console_address)
>> +        return self._console_socket
>> diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
>> index eb45824dd0..eebcc233ed 100644
>> --- a/python/qemu/qtest.py
>> +++ b/python/qemu/qtest.py
>> @@ -14,7 +14,7 @@
>>   import socket
>>   import os
>>   -from . import QEMUMachine
>> +from .machine import QEMUMachine
>>       class QEMUQtestProtocol(object):
>> diff --git a/scripts/device-crash-test b/scripts/device-crash-test
>> index a6748910ad..15f213a6cd 100755
>> --- a/scripts/device-crash-test
>> +++ b/scripts/device-crash-test
>> @@ -36,7 +36,7 @@ import argparse
>>   from itertools import chain
>>     sys.path.append(os.path.join(os.path.dirname(__file__), '..',
>> 'python'))
>> -from qemu import QEMUMachine
>> +from qemu.machine import QEMUMachine
>>     logger = logging.getLogger('device-crash-test')
>>   dbg = logger.debug
>> diff --git a/scripts/render_block_graph.py
>> b/scripts/render_block_graph.py
>> index 3e9d282a49..656f0388ad 100755
>> --- a/scripts/render_block_graph.py
>> +++ b/scripts/render_block_graph.py
>> @@ -25,7 +25,7 @@ import json
>>   from graphviz import Digraph
>>     sys.path.append(os.path.join(os.path.dirname(__file__), '..',
>> 'python'))
>> -from qemu import MonitorResponseError
>> +from qemu.machine import MonitorResponseError
>>       def perm(arr):
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py
>> b/tests/acceptance/avocado_qemu/__init__.py
>> index 2b236a1cf0..aee5d820ed 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -17,7 +17,7 @@ import avocado
>>   SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..',
>> '..')
>>   sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
>>   -from qemu import QEMUMachine
>> +from qemu.machine import QEMUMachine
>>     def is_readable_executable_file(path):
>>       return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>> diff --git a/tests/acceptance/virtio_version.py
>> b/tests/acceptance/virtio_version.py
>> index 8b97453ff8..33593c29dd 100644
>> --- a/tests/acceptance/virtio_version.py
>> +++ b/tests/acceptance/virtio_version.py
>> @@ -12,7 +12,7 @@ import sys
>>   import os
>>     sys.path.append(os.path.join(os.path.dirname(__file__), '..',
>> '..', 'python'))
>> -from qemu import QEMUMachine
>> +from qemu.machine import QEMUMachine
>>   from avocado_qemu import Test
>>     # Virtio Device IDs:
>> diff --git a/tests/migration/guestperf/engine.py
>> b/tests/migration/guestperf/engine.py
>> index 0e304660b8..f13dbea800 100644
>> --- a/tests/migration/guestperf/engine.py
>> +++ b/tests/migration/guestperf/engine.py
>> @@ -30,7 +30,7 @@ from guestperf.timings import TimingRecord, Timings
>>     sys.path.append(os.path.join(os.path.dirname(__file__),
>>                                '..', '..', '..', 'python'))
>> -import qemu
>> +from qemu.machine import QEMUMachine
>>       class Engine(object):
>> @@ -386,17 +386,17 @@ class Engine(object):
>>               dstmonaddr = "/var/tmp/qemu-dst-%d-monitor.sock" %
>> os.getpid()
>>           srcmonaddr = "/var/tmp/qemu-src-%d-monitor.sock" % os.getpid()
>>   -        src = qemu.QEMUMachine(self._binary,
>> -                               args=self._get_src_args(hardware),
>> -                               wrapper=self._get_src_wrapper(hardware),
>> -                               name="qemu-src-%d" % os.getpid(),
>> -                               monitor_address=srcmonaddr)
>> +        src = QEMUMachine(self._binary,
>> +                          args=self._get_src_args(hardware),
>> +                          wrapper=self._get_src_wrapper(hardware),
>> +                          name="qemu-src-%d" % os.getpid(),
>> +                          monitor_address=srcmonaddr)
>>   -        dst = qemu.QEMUMachine(self._binary,
>> -                               args=self._get_dst_args(hardware, uri),
>> -                               wrapper=self._get_dst_wrapper(hardware),
>> -                               name="qemu-dst-%d" % os.getpid(),
>> -                               monitor_address=dstmonaddr)
>> +        dst = QEMUMachine(self._binary,
>> +                          args=self._get_dst_args(hardware, uri),
>> +                          wrapper=self._get_dst_wrapper(hardware),
>> +                          name="qemu-dst-%d" % os.getpid(),
>> +                          monitor_address=dstmonaddr)
>>             try:
>>               src.launch()
>> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
>> index 2b6a8c13be..fedd111fd4 100755
>> --- a/tests/qemu-iotests/235
>> +++ b/tests/qemu-iotests/235
>> @@ -25,7 +25,7 @@ from iotests import qemu_img_create, qemu_io,
>> file_path, log
>>     sys.path.append(os.path.join(os.path.dirname(__file__), '..',
>> '..', 'python'))
>>   -from qemu import QEMUMachine
>> +from qemu.machine import QEMUMachine
>>     # Note:
>>   # This test was added to check that mirror dead-lock was fixed (see
>> previous
>> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
>> index 0556bdcf9e..4b496f1551 100755
>> --- a/tests/vm/basevm.py
>> +++ b/tests/vm/basevm.py
>> @@ -18,7 +18,8 @@ import logging
>>   import time
>>   import datetime
>>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..',
>> 'python'))
>> -from qemu import QEMUMachine, kvm_available
>> +from qemu import kvm_available
> 
> Perhaps kvm_available deserves another house as well.
> 
> In '[PATCH 2/3] tests/acceptance: Introduce the "accel" tag' I created
> the avocado_qemu.accel module to host code related with accelerators and
> use on acceptance tests. So maybe I should create that module actually
> in qemu and then put kvm_available there. What do you think?
> 
> - Wainer
> 
> 

Sounds good to me; it does stick out as weirdly placed now.

[Qemu-devel] [RFC PATCH v2 2/3] machine.py: minor delinting

[John Snow]

Since we're out in a new module, do a quick cursory pass of some of the
more obvious style issues.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/machine.py | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index a8a311b035..49445e675b 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -1,5 +1,10 @@
-# QEMU library
-#
+"""
+QEMU machine module:
+
+The machine module primarily provides the QEMUMachine class,
+which provides facilities for managing the lifetime of a QEMU VM.
+"""
+
 # Copyright (C) 2015-2016 Red Hat Inc.
 # Copyright (C) 2012 IBM Corp.
 #
@@ -16,7 +21,6 @@ import errno
 import logging
 import os
 import subprocess
-import re
 import shutil
 import socket
 import tempfile
@@ -119,8 +123,10 @@ class QEMUMachine(object):
         self.shutdown()
         return False
 
-    # This can be used to add an unused monitor instance.
     def add_monitor_null(self):
+        """
+        This can be used to add an unused monitor instance.
+        """
         self._args.append('-monitor')
         self._args.append('null')
 
@@ -143,10 +149,13 @@ class QEMUMachine(object):
         self._args.append(','.join(options))
         return self
 
-    # Exactly one of fd and file_path must be given.
-    # (If it is file_path, the helper will open that file and pass its
-    # own fd)
     def send_fd_scm(self, fd=None, file_path=None):
+        """
+        Send an fd or file_path to socket_scm_helper.
+
+        Exactly one of fd and file_path must be given.
+        If it is file_path, the helper will open that file and pass its own fd.
+        """
         # In iotest.py, the qmp should always use unix socket.
         assert self._qmp.is_scm_available()
         if self._socket_scm_helper is None:
@@ -194,14 +203,17 @@ class QEMUMachine(object):
             raise
 
     def is_running(self):
+        """Returns true if the VM is running."""
         return self._popen is not None and self._popen.poll() is None
 
     def exitcode(self):
+        """Returns the exit code if possible, or None."""
         if self._popen is None:
             return None
         return self._popen.poll()
 
     def get_pid(self):
+        """Returns the PID of the running process, or None."""
         if not self.is_running():
             return None
         return self._popen.pid
@@ -339,7 +351,7 @@ class QEMUMachine(object):
                 command = ' '.join(self._qemu_full_args)
             else:
                 command = ''
-            LOG.warn(msg, -exitcode, command)
+            LOG.warning(msg, -exitcode, command)
 
         self._launched = False
 
@@ -373,7 +385,7 @@ class QEMUMachine(object):
         """
         Poll for one queued QMP events and return it
         """
-        if len(self._events) > 0:
+        if self._events:
             return self._events.pop(0)
         return self._qmp.pull_event(wait=wait)
 
@@ -441,8 +453,7 @@ class QEMUMachine(object):
         """
         def _match(event):
             for name, match in events:
-                if (event['event'] == name and
-                    self.event_match(event, match)):
+                if event['event'] == name and self.event_match(event, match):
                     return True
             return False
 
-- 
2.21.0

[Qemu-devel] [RFC PATCH v2 3/3] QEMUMachine: Don't suppress stack traces on close

[John Snow]

We do need to run cleanup, but we should re-raise the error.

Rename _post_shutdown to _shutdown_cleanup, then use the
_post_shutdown name for the routine that will always run
at the conclusion of the shutdown() method.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/machine.py | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 49445e675b..b36f0c3ef9 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -264,7 +264,8 @@ class QEMUMachine(object):
     def _post_launch(self):
         self._qmp.accept()
 
-    def _post_shutdown(self):
+    def _shutdown_cleanup(self):
+        self._load_io_log()
         if self._qemu_log_file is not None:
             self._qemu_log_file.close()
             self._qemu_log_file = None
@@ -326,8 +327,20 @@ class QEMUMachine(object):
         """
         self._popen.wait()
         self._qmp.close()
-        self._load_io_log()
-        self._post_shutdown()
+        self._shutdown_cleanup()
+
+    def _post_shutdown(self):
+        self._shutdown_cleanup()
+        exitcode = self.exitcode()
+        if exitcode is not None and exitcode < 0:
+            msg = 'qemu received signal %i: %s'
+            if self._qemu_full_args:
+                command = ' '.join(self._qemu_full_args)
+            else:
+                command = ''
+            LOG.warning(msg, -int(exitcode), command)
+
+        self._launched = False
 
     def shutdown(self):
         """
@@ -339,21 +352,13 @@ class QEMUMachine(object):
                 self._qmp.close()
             except:
                 self._popen.kill()
-            self._popen.wait()
-
-        self._load_io_log()
-        self._post_shutdown()
-
-        exitcode = self.exitcode()
-        if exitcode is not None and exitcode < 0:
-            msg = 'qemu received signal %i: %s'
-            if self._qemu_full_args:
-                command = ' '.join(self._qemu_full_args)
-            else:
-                command = ''
-            LOG.warning(msg, -exitcode, command)
-
-        self._launched = False
+                # Don't hide exceptions from crashes
+                raise
+            finally:
+                self._popen.wait()
+                self._post_shutdown()
+        else:
+            self._post_shutdown()
 
     def qmp(self, cmd, conv_keys=True, **args):
         """
-- 
2.21.0

Re: [Qemu-devel] [RFC PATCH v2 0/3] python: refactor qemu/__init__.py

[John Snow]

I didn't actually mean to retain the RFC tag, but oh well.

On 6/27/19 5:28 PM, John Snow wrote:
> There's a lot of code hiding in what is ostensibly a package
> configuration file. Let's break that out into something more visible.
> 
> V2:
>  - Retain python 2.7+ compatibility :(
>  - Refactor shutdown a little more aggressively
> 
> John Snow (3):
>   python/qemu: split QEMUMachine out from underneath __init__.py
>   machine.py: minor delinting
>   QEMUMachine: Don't suppress stack traces on close
> 
>  python/qemu/__init__.py                   | 502 +-------------------
>  python/qemu/machine.py                    | 536 ++++++++++++++++++++++
>  python/qemu/qtest.py                      |   2 +-
>  scripts/device-crash-test                 |   2 +-
>  scripts/render_block_graph.py             |   2 +-
>  tests/acceptance/avocado_qemu/__init__.py |   2 +-
>  tests/acceptance/virtio_version.py        |   2 +-
>  tests/migration/guestperf/engine.py       |  22 +-
>  tests/qemu-iotests/235                    |   2 +-
>  tests/vm/basevm.py                        |   3 +-
>  10 files changed, 556 insertions(+), 519 deletions(-)
>  create mode 100644 python/qemu/machine.py
> 

Re: [Qemu-devel] [RFC PATCH v2 0/3] python: refactor qemu/__init__.py

[Eduardo Habkost]

On Thu, Jun 27, 2019 at 05:32:17PM -0400, John Snow wrote:
> I didn't actually mean to retain the RFC tag, but oh well.

I'm queueing patch 1-2 for QEMU 4.1.

Patch 3 will require more careful review.  If you can demonstrate
it fixes a problem in some use cases, we can call it a bug fix
and include it after soft freeze.  ;)

> 
> On 6/27/19 5:28 PM, John Snow wrote:
> > There's a lot of code hiding in what is ostensibly a package
> > configuration file. Let's break that out into something more visible.
> > 
> > V2:
> >  - Retain python 2.7+ compatibility :(
> >  - Refactor shutdown a little more aggressively
> > 
> > John Snow (3):
> >   python/qemu: split QEMUMachine out from underneath __init__.py
> >   machine.py: minor delinting
> >   QEMUMachine: Don't suppress stack traces on close
> > 
> >  python/qemu/__init__.py                   | 502 +-------------------
> >  python/qemu/machine.py                    | 536 ++++++++++++++++++++++
> >  python/qemu/qtest.py                      |   2 +-
> >  scripts/device-crash-test                 |   2 +-
> >  scripts/render_block_graph.py             |   2 +-
> >  tests/acceptance/avocado_qemu/__init__.py |   2 +-
> >  tests/acceptance/virtio_version.py        |   2 +-
> >  tests/migration/guestperf/engine.py       |  22 +-
> >  tests/qemu-iotests/235                    |   2 +-
> >  tests/vm/basevm.py                        |   3 +-
> >  10 files changed, 556 insertions(+), 519 deletions(-)
> >  create mode 100644 python/qemu/machine.py
> > 
> 

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH v2 0/3] python: refactor qemu/__init__.py

[John Snow]

On 7/1/19 5:27 PM, Eduardo Habkost wrote:
> On Thu, Jun 27, 2019 at 05:32:17PM -0400, John Snow wrote:
>> I didn't actually mean to retain the RFC tag, but oh well.
> 
> I'm queueing patch 1-2 for QEMU 4.1.
> 
> Patch 3 will require more careful review.  If you can demonstrate
> it fixes a problem in some use cases, we can call it a bug fix
> and include it after soft freeze.  ;)
> 

Right.

Mostly, it's that it ignores things like ctrl^c -- it ignores ANY
failure. I don't think that it should.

In cases where QEMU crashes, the caller will now get an exception -- but
the cleanup will still faithfully execute. There are some exceptions we
may genuinely wish to suppress here, but it's hard to enumerate which
ones without going too broadly.

It's fine to exclude this for now, but I think it's still something we
want eventually.

Re: [Qemu-devel] [RFC PATCH v2 0/3] python: refactor qemu/__init__.py

[Eduardo Habkost]

On Mon, Jul 01, 2019 at 07:17:39PM -0400, John Snow wrote:
> 
> 
> On 7/1/19 5:27 PM, Eduardo Habkost wrote:
> > On Thu, Jun 27, 2019 at 05:32:17PM -0400, John Snow wrote:
> >> I didn't actually mean to retain the RFC tag, but oh well.
> > 
> > I'm queueing patch 1-2 for QEMU 4.1.
> > 
> > Patch 3 will require more careful review.  If you can demonstrate
> > it fixes a problem in some use cases, we can call it a bug fix
> > and include it after soft freeze.  ;)
> > 
> 
> Right.
> 
> Mostly, it's that it ignores things like ctrl^c -- it ignores ANY
> failure. I don't think that it should.

Agreed.

> 
> In cases where QEMU crashes, the caller will now get an exception -- but
> the cleanup will still faithfully execute. There are some exceptions we
> may genuinely wish to suppress here, but it's hard to enumerate which
> ones without going too broadly.
> 
> It's fine to exclude this for now, but I think it's still something we
> want eventually.

We just need to ensure all callers of shutdown() are ready for
this change.  I know that scripts/device-crash-test expects
shutdown() to not raise an exception if QEMU crashed, inside
checkOneTestCase().  Maybe that's the only caller that needs to
change.

-- 
Eduardo