[Qemu-devel] [PATCH v2 0/3] scripts: Remove 'debug' parameter from QEMUMachine & QEMUMonitorProtocol

[Qemu-devel] [PATCH v2 0/3] scripts: Remove 'debug' parameter from QEMUMachine & QEMUMonitorProtocol

[Eduardo Habkost]

Changes v1 -> v2:
* Rebased to python-next (some patches from v1 are already queued
  there)S
* guestperf: Inline init_logging() method on all callers because
  not all classes derive from BaseShell (reported by Lukáš
  Doktor)

This series removes the 'debug' parameter from QEMUMachine and
QEMUMonitorProtocol and lets scripts use the logging module to
enable debugging messages.

Eduardo Habkost (3):
  guestperf: Configure logging on all shell frontends
  scripts: Remove debug parameter from QEMUMonitorProtocol
  scripts: Remove debug parameter from QEMUMachine

 scripts/qemu.py                     |  9 +++------
 scripts/qmp/qmp.py                  | 16 +++++++---------
 tests/migration/guestperf/engine.py |  6 ++----
 tests/migration/guestperf/shell.py  | 13 +++++++++++++
 tests/qemu-iotests/iotests.py       |  2 --
 5 files changed, 25 insertions(+), 21 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH v2 1/3] guestperf: Configure logging on all shell frontends

[Eduardo Habkost]

The logging module will eventually replace the 'debug' parameter
in QEMUMachine and QEMUMonitorProtocol.

Cc: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Inline init_logging() method on all callers because not all
  classes derive from BaseShell (reported by Lukáš Doktor)
---
 tests/migration/guestperf/shell.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/migration/guestperf/shell.py b/tests/migration/guestperf/shell.py
index 7992459a97..b272978f47 100644
--- a/tests/migration/guestperf/shell.py
+++ b/tests/migration/guestperf/shell.py
@@ -26,6 +26,7 @@ sys.path.append(os.path.join(os.path.dirname(__file__),
 import argparse
 import fnmatch
 import platform
+import logging
 
 from guestperf.hardware import Hardware
 from guestperf.engine import Engine
@@ -147,6 +148,10 @@ class Shell(BaseShell):
 
     def run(self, argv):
         args = self._parser.parse_args(argv)
+        logging.basicConfig(level=(logging.DEBUG if args.debug else
+                                   logging.INFO if args.verbose else
+                                   logging.WARN))
+
 
         engine = self.get_engine(args)
         hardware = self.get_hardware(args)
@@ -179,6 +184,10 @@ class BatchShell(BaseShell):
 
     def run(self, argv):
         args = self._parser.parse_args(argv)
+        logging.basicConfig(level=(logging.DEBUG if args.debug else
+                                   logging.INFO if args.verbose else
+                                   logging.WARN))
+
 
         engine = self.get_engine(args)
         hardware = self.get_hardware(args)
@@ -231,6 +240,10 @@ class PlotShell(object):
 
     def run(self, argv):
         args = self._parser.parse_args(argv)
+        logging.basicConfig(level=(logging.DEBUG if args.debug else
+                                   logging.INFO if args.verbose else
+                                   logging.WARN))
+
 
         if len(args.reports) == 0:
             print >>sys.stderr, "At least one report required"
-- 
2.13.6

[Qemu-devel] [PATCH v2 2/3] scripts: Remove debug parameter from QEMUMonitorProtocol

[Eduardo Habkost]

Use logging module for the QMP debug messages.  The only scripts
that set debug=True are iotests.py and guestperf/engine.py, and
they already call logging.basicConfig() to set up logging.

Scripts that don't configure logging are safe as long as they
don't need debugging output, because debug messages don't trigger
the "No handlers could be found for logger" message from the
Python logging module.

Scripts that already configure logging but don't use debug=True
(e.g. scripts/vm/basevm.py) will get QMP debugging enabled for
free.

Cc: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Fam Zheng <famz@redhat.com>
Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Actually remove debug parameter from method definition
  (Fam Zheng)
* Fix "<<<" vs ">>>" confusion
  (Fam Zheng)
* Remove "import sys" line
  (Lukáš Doktor)
---
 scripts/qemu.py    |  3 +--
 scripts/qmp/qmp.py | 16 +++++++---------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index c9a106fbce..f6d2e68627 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -177,8 +177,7 @@ class QEMUMachine(object):
 
     def _pre_launch(self):
         self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
-                                                server=True,
-                                                debug=self._debug)
+                                                server=True)
 
     def _post_launch(self):
         self._qmp.accept()
diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index ef12e8a1a0..07c9632e9e 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -11,7 +11,7 @@
 import json
 import errno
 import socket
-import sys
+import logging
 
 
 class QMPError(Exception):
@@ -32,12 +32,14 @@ class QMPTimeoutError(QMPError):
 
 class QEMUMonitorProtocol(object):
 
+    #: Logger object for debugging messages
+    logger = logging.getLogger('QMP')
     #: Socket's error class
     error = socket.error
     #: Socket's timeout
     timeout = socket.timeout
 
-    def __init__(self, address, server=False, debug=False):
+    def __init__(self, address, server=False):
         """
         Create a QEMUMonitorProtocol class.
 
@@ -51,7 +53,6 @@ class QEMUMonitorProtocol(object):
         """
         self.__events = []
         self.__address = address
-        self._debug = debug
         self.__sock = self.__get_sock()
         self.__sockfile = None
         if server:
@@ -83,8 +84,7 @@ class QEMUMonitorProtocol(object):
                 return
             resp = json.loads(data)
             if 'event' in resp:
-                if self._debug:
-                    print >>sys.stderr, "QMP:<<< %s" % resp
+                self.logger.debug("<<< %s", resp)
                 self.__events.append(resp)
                 if not only_event:
                     continue
@@ -164,8 +164,7 @@ class QEMUMonitorProtocol(object):
         @return QMP response as a Python dict or None if the connection has
                 been closed
         """
-        if self._debug:
-            print >>sys.stderr, "QMP:>>> %s" % qmp_cmd
+        self.logger.debug(">>> %s", qmp_cmd)
         try:
             self.__sock.sendall(json.dumps(qmp_cmd))
         except socket.error as err:
@@ -173,8 +172,7 @@ class QEMUMonitorProtocol(object):
                 return
             raise socket.error(err)
         resp = self.__json_read()
-        if self._debug:
-            print >>sys.stderr, "QMP:<<< %s" % resp
+        self.logger.debug("<<< %s", resp)
         return resp
 
     def cmd(self, name, args=None, cmd_id=None):
-- 
2.13.6

[Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine

[Eduardo Habkost]

All scripts that use the QEMUMachine and QEMUQtestMachine classes
(device-crash-test, tests/migration/*, iotests.py, basevm.py)
already configure logging.

The basicConfig() call inside QEMUMachine.__init__() is being
kept just to make sure a script would still work if it didn't
configure logging.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/qemu.py                     | 6 ++----
 tests/migration/guestperf/engine.py | 6 ++----
 tests/qemu-iotests/iotests.py       | 2 --
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f6d2e68627..9bfdf6d37d 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -54,7 +54,7 @@ class QEMUMachine(object):
 
     def __init__(self, binary, args=None, wrapper=None, name=None,
                  test_dir="/var/tmp", monitor_address=None,
-                 socket_scm_helper=None, debug=False):
+                 socket_scm_helper=None):
         '''
         Initialize a QEMUMachine
 
@@ -65,7 +65,6 @@ class QEMUMachine(object):
         @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()"
-        @param debug: enable debug mode
         @note: Qemu process is not started until launch() is used.
         '''
         if args is None:
@@ -85,12 +84,11 @@ class QEMUMachine(object):
         self._events = []
         self._iolog = None
         self._socket_scm_helper = socket_scm_helper
-        self._debug = debug
         self._qmp = None
         self._qemu_full_args = None
 
         # just in case logging wasn't configured by the main script:
-        logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
+        logging.basicConfig()
 
     def __enter__(self):
         return self
diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
index 0a13050bc6..e14d4320b2 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -388,15 +388,13 @@ class Engine(object):
                                args=self._get_src_args(hardware),
                                wrapper=self._get_src_wrapper(hardware),
                                name="qemu-src-%d" % os.getpid(),
-                               monitor_address=srcmonaddr,
-                               debug=self._debug)
+                               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,
-                               debug=self._debug)
+                               monitor_address=dstmonaddr)
 
         try:
             src.launch()
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 36a7757aaf..6f057904a9 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -195,8 +195,6 @@ class VM(qtest.QEMUQtestMachine):
         super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
                                  test_dir=test_dir,
                                  socket_scm_helper=socket_scm_helper)
-        if debug:
-            self._debug = True
         self._num_drives = 0
 
     def add_device(self, opts):
-- 
2.13.6

Re: [Qemu-devel] [PATCH v2 1/3] guestperf: Configure logging on all shell frontends

[Lukáš Doktor]

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

Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
> The logging module will eventually replace the 'debug' parameter
> in QEMUMachine and QEMUMonitorProtocol.
> 
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Inline init_logging() method on all callers because not all
>   classes derive from BaseShell (reported by Lukáš Doktor)
> ---
>  tests/migration/guestperf/shell.py | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tests/migration/guestperf/shell.py b/tests/migration/guestperf/shell.py
> index 7992459a97..b272978f47 100644
> --- a/tests/migration/guestperf/shell.py
> +++ b/tests/migration/guestperf/shell.py
> @@ -26,6 +26,7 @@ sys.path.append(os.path.join(os.path.dirname(__file__),
>  import argparse
>  import fnmatch
>  import platform
> +import logging
>  
>  from guestperf.hardware import Hardware
>  from guestperf.engine import Engine
> @@ -147,6 +148,10 @@ class Shell(BaseShell):
>  
>      def run(self, argv):
>          args = self._parser.parse_args(argv)
> +        logging.basicConfig(level=(logging.DEBUG if args.debug else
> +                                   logging.INFO if args.verbose else
> +                                   logging.WARN))
> +
>  
>          engine = self.get_engine(args)
>          hardware = self.get_hardware(args)
> @@ -179,6 +184,10 @@ class BatchShell(BaseShell):
>  
>      def run(self, argv):
>          args = self._parser.parse_args(argv)
> +        logging.basicConfig(level=(logging.DEBUG if args.debug else
> +                                   logging.INFO if args.verbose else
> +                                   logging.WARN))
> +
>  
>          engine = self.get_engine(args)
>          hardware = self.get_hardware(args)
> @@ -231,6 +240,10 @@ class PlotShell(object):
>  
>      def run(self, argv):
>          args = self._parser.parse_args(argv)
> +        logging.basicConfig(level=(logging.DEBUG if args.debug else
> +                                   logging.INFO if args.verbose else
> +                                   logging.WARN))
> +
>  
>          if len(args.reports) == 0:
>              print >>sys.stderr, "At least one report required"
> 


Yep, this does the trick, also using a shared function would be IMO better (especially if we need to tweak the setup), something like

```diff
diff --git a/tests/migration/guestperf/shell.py b/tests/migration/guestperf/shell.py
index b272978..c1108ae 100644
--- a/tests/migration/guestperf/shell.py
+++ b/tests/migration/guestperf/shell.py
@@ -36,6 +36,12 @@ from guestperf.plot import Plot
 from guestperf.report import Report
 
 
+def _init_logging(args):
+    logging.basicConfig(level=(logging.DEBUG if args.debug else
+                               logging.INFO if args.verbose else
+                               logging.WARN))
+
+
 class BaseShell(object):
 
     def __init__(self):
@@ -148,10 +154,7 @@ class Shell(BaseShell):
 
     def run(self, argv):
         args = self._parser.parse_args(argv)
-        logging.basicConfig(level=(logging.DEBUG if args.debug else
-                                   logging.INFO if args.verbose else
-                                   logging.WARN))
-
+        _init_logging(args)
 
         engine = self.get_engine(args)
         hardware = self.get_hardware(args)
@@ -184,10 +187,7 @@ class BatchShell(BaseShell):
 
     def run(self, argv):
         args = self._parser.parse_args(argv)
-        logging.basicConfig(level=(logging.DEBUG if args.debug else
-                                   logging.INFO if args.verbose else
-                                   logging.WARN))
-
+        _init_logging(args)
 
         engine = self.get_engine(args)
         hardware = self.get_hardware(args)
@@ -240,10 +240,7 @@ class PlotShell(object):
 
     def run(self, argv):
         args = self._parser.parse_args(argv)
-        logging.basicConfig(level=(logging.DEBUG if args.debug else
-                                   logging.INFO if args.verbose else
-                                   logging.WARN))
-
+        _init_logging(args)
 
         if len(args.reports) == 0:
             print >>sys.stderr, "At least one report required"
```

Anyway both versions are fine by me.

Reviewed-by: Lukáš Doktor <ldoktor@redhat.com> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 502 bytes --]

Re: [Qemu-devel] [PATCH v2 2/3] scripts: Remove debug parameter from QEMUMonitorProtocol

[Lukáš Doktor]

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

Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
> Use logging module for the QMP debug messages.  The only scripts
> that set debug=True are iotests.py and guestperf/engine.py, and
> they already call logging.basicConfig() to set up logging.
> 
> Scripts that don't configure logging are safe as long as they
> don't need debugging output, because debug messages don't trigger
> the "No handlers could be found for logger" message from the
> Python logging module.
> 
> Scripts that already configure logging but don't use debug=True
> (e.g. scripts/vm/basevm.py) will get QMP debugging enabled for
> free.
> 
> Cc: "Alex Bennée" <alex.bennee@linaro.org>
> Cc: Fam Zheng <famz@redhat.com>
> Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Actually remove debug parameter from method definition
>   (Fam Zheng)
> * Fix "<<<" vs ">>>" confusion
>   (Fam Zheng)
> * Remove "import sys" line
>   (Lukáš Doktor)
> ---
>  scripts/qemu.py    |  3 +--
>  scripts/qmp/qmp.py | 16 +++++++---------
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index c9a106fbce..f6d2e68627 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -177,8 +177,7 @@ class QEMUMachine(object):
>  
>      def _pre_launch(self):
>          self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
> -                                                server=True,
> -                                                debug=self._debug)
> +                                                server=True)
>  
>      def _post_launch(self):
>          self._qmp.accept()
> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
> index ef12e8a1a0..07c9632e9e 100644
> --- a/scripts/qmp/qmp.py
> +++ b/scripts/qmp/qmp.py
> @@ -11,7 +11,7 @@
>  import json
>  import errno
>  import socket
> -import sys
> +import logging
>  
>  
>  class QMPError(Exception):
> @@ -32,12 +32,14 @@ class QMPTimeoutError(QMPError):
>  
>  class QEMUMonitorProtocol(object):
>  
> +    #: Logger object for debugging messages
> +    logger = logging.getLogger('QMP')
>      #: Socket's error class
>      error = socket.error
>      #: Socket's timeout
>      timeout = socket.timeout
>  
> -    def __init__(self, address, server=False, debug=False):
> +    def __init__(self, address, server=False):
>          """
>          Create a QEMUMonitorProtocol class.
>  
> @@ -51,7 +53,6 @@ class QEMUMonitorProtocol(object):
>          """
>          self.__events = []
>          self.__address = address
> -        self._debug = debug
>          self.__sock = self.__get_sock()
>          self.__sockfile = None
>          if server:
> @@ -83,8 +84,7 @@ class QEMUMonitorProtocol(object):
>                  return
>              resp = json.loads(data)
>              if 'event' in resp:
> -                if self._debug:
> -                    print >>sys.stderr, "QMP:<<< %s" % resp
> +                self.logger.debug("<<< %s", resp)
>                  self.__events.append(resp)
>                  if not only_event:
>                      continue
> @@ -164,8 +164,7 @@ class QEMUMonitorProtocol(object):
>          @return QMP response as a Python dict or None if the connection has
>                  been closed
>          """
> -        if self._debug:
> -            print >>sys.stderr, "QMP:>>> %s" % qmp_cmd
> +        self.logger.debug(">>> %s", qmp_cmd)
>          try:
>              self.__sock.sendall(json.dumps(qmp_cmd))
>          except socket.error as err:
> @@ -173,8 +172,7 @@ class QEMUMonitorProtocol(object):
>                  return
>              raise socket.error(err)
>          resp = self.__json_read()
> -        if self._debug:
> -            print >>sys.stderr, "QMP:<<< %s" % resp
> +        self.logger.debug("<<< %s", resp)
>          return resp
>  
>      def cmd(self, name, args=None, cmd_id=None):
> 

This one looks good, but in order to no break qemu-iotests verbose mode it requires fix to qtest/iotests:

```diff
diff --git a/scripts/qtest.py b/scripts/qtest.py
index df0daf2..0e955a8 100644
--- a/scripts/qtest.py
+++ b/scripts/qtest.py
@@ -77,12 +77,12 @@ class QEMUQtestMachine(qemu.QEMUMachine):
     '''A QEMU VM'''
 
     def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
-                 socket_scm_helper=None):
+                 socket_scm_helper=None, debug=False):
         if name is None:
             name = "qemu-%d" % os.getpid()
         super(QEMUQtestMachine,
               self).__init__(binary, args, name=name, test_dir=test_dir,
-                             socket_scm_helper=socket_scm_helper)
+                             socket_scm_helper=socket_scm_helper, debug=debug)
         self._qtest = None
         self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1af117e..989ebd3 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -193,9 +193,8 @@ class VM(qtest.QEMUQtestMachine):
         name = "qemu%s-%d" % (path_suffix, os.getpid())
         super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
                                  test_dir=test_dir,
-                                 socket_scm_helper=socket_scm_helper)
-        if debug:
-            self._debug = True
+                                 socket_scm_helper=socket_scm_helper,
+                                 debug=debug)
         self._num_drives = 0
 
     def add_device(self, opts):
```

(because the `debug` used to be set after `__init__`, but the logging is initialized during `__init__`.)

Therefor conditional ACK when the qtest/iotest fix precedes this commit.

Lukáš


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 502 bytes --]

Re: [Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine

[Lukáš Doktor]

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

Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
> All scripts that use the QEMUMachine and QEMUQtestMachine classes
> (device-crash-test, tests/migration/*, iotests.py, basevm.py)
> already configure logging.
> 
> The basicConfig() call inside QEMUMachine.__init__() is being
> kept just to make sure a script would still work if it didn't
> configure logging.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  scripts/qemu.py                     | 6 ++----
>  tests/migration/guestperf/engine.py | 6 ++----
>  tests/qemu-iotests/iotests.py       | 2 --
>  3 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index f6d2e68627..9bfdf6d37d 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -54,7 +54,7 @@ class QEMUMachine(object):
>  
>      def __init__(self, binary, args=None, wrapper=None, name=None,
>                   test_dir="/var/tmp", monitor_address=None,
> -                 socket_scm_helper=None, debug=False):
> +                 socket_scm_helper=None):
>          '''
>          Initialize a QEMUMachine
>  
> @@ -65,7 +65,6 @@ class QEMUMachine(object):
>          @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()"
> -        @param debug: enable debug mode
>          @note: Qemu process is not started until launch() is used.
>          '''
>          if args is None:
> @@ -85,12 +84,11 @@ class QEMUMachine(object):
>          self._events = []
>          self._iolog = None
>          self._socket_scm_helper = socket_scm_helper
> -        self._debug = debug
>          self._qmp = None
>          self._qemu_full_args = None
>  
>          # just in case logging wasn't configured by the main script:
> -        logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
> +        logging.basicConfig()
Yes, this behaves the same as `debug=False`

>  
>      def __enter__(self):
>          return self
> diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
> index 0a13050bc6..e14d4320b2 100644
> --- a/tests/migration/guestperf/engine.py
> +++ b/tests/migration/guestperf/engine.py
> @@ -388,15 +388,13 @@ class Engine(object):
>                                 args=self._get_src_args(hardware),
>                                 wrapper=self._get_src_wrapper(hardware),
>                                 name="qemu-src-%d" % os.getpid(),
> -                               monitor_address=srcmonaddr,
> -                               debug=self._debug)
> +                               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,
> -                               debug=self._debug)
> +                               monitor_address=dstmonaddr)
>  
>          try:
>              src.launch()
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 36a7757aaf..6f057904a9 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -195,8 +195,6 @@ class VM(qtest.QEMUQtestMachine):
>          super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
>                                   test_dir=test_dir,
>                                   socket_scm_helper=socket_scm_helper)
> -        if debug:
> -            self._debug = True

And this is the main issue. So instead of the fix I proposed in previous commit major changes to "tests/qemu-iotests/iotests.py" are necessary.

>          self._num_drives = 0
>  
>      def add_device(self, opts):
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 502 bytes --]

Re: [Qemu-devel] [PATCH v2 2/3] scripts: Remove debug parameter from QEMUMonitorProtocol

[Eduardo Habkost]

On Sat, Oct 07, 2017 at 10:26:14AM +0200, Lukáš Doktor wrote:
> Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
> > Use logging module for the QMP debug messages.  The only scripts
> > that set debug=True are iotests.py and guestperf/engine.py, and
> > they already call logging.basicConfig() to set up logging.
> > 
> > Scripts that don't configure logging are safe as long as they
> > don't need debugging output, because debug messages don't trigger
> > the "No handlers could be found for logger" message from the
> > Python logging module.
> > 
> > Scripts that already configure logging but don't use debug=True
> > (e.g. scripts/vm/basevm.py) will get QMP debugging enabled for
> > free.
> > 
> > Cc: "Alex Bennée" <alex.bennee@linaro.org>
> > Cc: Fam Zheng <famz@redhat.com>
> > Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v1 -> v2:
> > * Actually remove debug parameter from method definition
> >   (Fam Zheng)
> > * Fix "<<<" vs ">>>" confusion
> >   (Fam Zheng)
> > * Remove "import sys" line
> >   (Lukáš Doktor)
> > ---
> >  scripts/qemu.py    |  3 +--
> >  scripts/qmp/qmp.py | 16 +++++++---------
> >  2 files changed, 8 insertions(+), 11 deletions(-)
> > 
> > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > index c9a106fbce..f6d2e68627 100644
> > --- a/scripts/qemu.py
> > +++ b/scripts/qemu.py
> > @@ -177,8 +177,7 @@ class QEMUMachine(object):
> >  
> >      def _pre_launch(self):
> >          self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
> > -                                                server=True,
> > -                                                debug=self._debug)
> > +                                                server=True)
> >  
> >      def _post_launch(self):
> >          self._qmp.accept()
> > diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
> > index ef12e8a1a0..07c9632e9e 100644
> > --- a/scripts/qmp/qmp.py
> > +++ b/scripts/qmp/qmp.py
> > @@ -11,7 +11,7 @@
> >  import json
> >  import errno
> >  import socket
> > -import sys
> > +import logging
> >  
> >  
> >  class QMPError(Exception):
> > @@ -32,12 +32,14 @@ class QMPTimeoutError(QMPError):
> >  
> >  class QEMUMonitorProtocol(object):
> >  
> > +    #: Logger object for debugging messages
> > +    logger = logging.getLogger('QMP')
> >      #: Socket's error class
> >      error = socket.error
> >      #: Socket's timeout
> >      timeout = socket.timeout
> >  
> > -    def __init__(self, address, server=False, debug=False):
> > +    def __init__(self, address, server=False):
> >          """
> >          Create a QEMUMonitorProtocol class.
> >  
> > @@ -51,7 +53,6 @@ class QEMUMonitorProtocol(object):
> >          """
> >          self.__events = []
> >          self.__address = address
> > -        self._debug = debug
> >          self.__sock = self.__get_sock()
> >          self.__sockfile = None
> >          if server:
> > @@ -83,8 +84,7 @@ class QEMUMonitorProtocol(object):
> >                  return
> >              resp = json.loads(data)
> >              if 'event' in resp:
> > -                if self._debug:
> > -                    print >>sys.stderr, "QMP:<<< %s" % resp
> > +                self.logger.debug("<<< %s", resp)
> >                  self.__events.append(resp)
> >                  if not only_event:
> >                      continue
> > @@ -164,8 +164,7 @@ class QEMUMonitorProtocol(object):
> >          @return QMP response as a Python dict or None if the connection has
> >                  been closed
> >          """
> > -        if self._debug:
> > -            print >>sys.stderr, "QMP:>>> %s" % qmp_cmd
> > +        self.logger.debug(">>> %s", qmp_cmd)
> >          try:
> >              self.__sock.sendall(json.dumps(qmp_cmd))
> >          except socket.error as err:
> > @@ -173,8 +172,7 @@ class QEMUMonitorProtocol(object):
> >                  return
> >              raise socket.error(err)
> >          resp = self.__json_read()
> > -        if self._debug:
> > -            print >>sys.stderr, "QMP:<<< %s" % resp
> > +        self.logger.debug("<<< %s", resp)
> >          return resp
> >  
> >      def cmd(self, name, args=None, cmd_id=None):
> > 
> 
> This one looks good, but in order to no break qemu-iotests verbose mode it requires fix to qtest/iotests:
> 
> ```diff
> diff --git a/scripts/qtest.py b/scripts/qtest.py
> index df0daf2..0e955a8 100644
> --- a/scripts/qtest.py
> +++ b/scripts/qtest.py
> @@ -77,12 +77,12 @@ class QEMUQtestMachine(qemu.QEMUMachine):
>      '''A QEMU VM'''
>  
>      def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
> -                 socket_scm_helper=None):
> +                 socket_scm_helper=None, debug=False):
>          if name is None:
>              name = "qemu-%d" % os.getpid()
>          super(QEMUQtestMachine,
>                self).__init__(binary, args, name=name, test_dir=test_dir,
> -                             socket_scm_helper=socket_scm_helper)
> +                             socket_scm_helper=socket_scm_helper, debug=debug)
>          self._qtest = None
>          self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
>  
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 1af117e..989ebd3 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -193,9 +193,8 @@ class VM(qtest.QEMUQtestMachine):
>          name = "qemu%s-%d" % (path_suffix, os.getpid())
>          super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
>                                   test_dir=test_dir,
> -                                 socket_scm_helper=socket_scm_helper)
> -        if debug:
> -            self._debug = True
> +                                 socket_scm_helper=socket_scm_helper,
> +                                 debug=debug)
>          self._num_drives = 0
>  
>      def add_device(self, opts):
> ```

I'm confused by why the above patch is necessary.  We are in the
process of removing the 'debug' parameter from QEMUMachine and
QEMUMonitorProtocol, why would we add a debug parameter to
QEMUQtestMachine and iotests.py?

> 
> (because the `debug` used to be set after `__init__`, but the logging is initialized during `__init__`.)
> 
> Therefor conditional ACK when the qtest/iotest fix precedes this commit.

Do you mean the following?

  Message-Id: <20170927130339.21444-3-ehabkost@redhat.com>
  Subject: [Qemu-devel] [PATCH 2/5] iotests: Set up Python logging
  https://www.mail-archive.com/qemu-devel@nongnu.org/msg485036.html
  https://github.com/ehabkost/qemu/commit/afa79b55676dcd1859aa9d1f983c9dfbbcc13197

It is already queued on python-next.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine

[Eduardo Habkost]

On Sat, Oct 07, 2017 at 10:34:57AM +0200, Lukáš Doktor wrote:
> Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
> > All scripts that use the QEMUMachine and QEMUQtestMachine classes
> > (device-crash-test, tests/migration/*, iotests.py, basevm.py)
> > already configure logging.
> > 
> > The basicConfig() call inside QEMUMachine.__init__() is being
> > kept just to make sure a script would still work if it didn't
> > configure logging.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  scripts/qemu.py                     | 6 ++----
> >  tests/migration/guestperf/engine.py | 6 ++----
> >  tests/qemu-iotests/iotests.py       | 2 --
> >  3 files changed, 4 insertions(+), 10 deletions(-)
> > 
> > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > index f6d2e68627..9bfdf6d37d 100644
> > --- a/scripts/qemu.py
> > +++ b/scripts/qemu.py
> > @@ -54,7 +54,7 @@ class QEMUMachine(object):
> >  
> >      def __init__(self, binary, args=None, wrapper=None, name=None,
> >                   test_dir="/var/tmp", monitor_address=None,
> > -                 socket_scm_helper=None, debug=False):
> > +                 socket_scm_helper=None):
> >          '''
> >          Initialize a QEMUMachine
> >  
> > @@ -65,7 +65,6 @@ class QEMUMachine(object):
> >          @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()"
> > -        @param debug: enable debug mode
> >          @note: Qemu process is not started until launch() is used.
> >          '''
> >          if args is None:
> > @@ -85,12 +84,11 @@ class QEMUMachine(object):
> >          self._events = []
> >          self._iolog = None
> >          self._socket_scm_helper = socket_scm_helper
> > -        self._debug = debug
> >          self._qmp = None
> >          self._qemu_full_args = None
> >  
> >          # just in case logging wasn't configured by the main script:
> > -        logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
> > +        logging.basicConfig()
> Yes, this behaves the same as `debug=False`
> 
> >  
> >      def __enter__(self):
> >          return self
> > diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
> > index 0a13050bc6..e14d4320b2 100644
> > --- a/tests/migration/guestperf/engine.py
> > +++ b/tests/migration/guestperf/engine.py
> > @@ -388,15 +388,13 @@ class Engine(object):
> >                                 args=self._get_src_args(hardware),
> >                                 wrapper=self._get_src_wrapper(hardware),
> >                                 name="qemu-src-%d" % os.getpid(),
> > -                               monitor_address=srcmonaddr,
> > -                               debug=self._debug)
> > +                               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,
> > -                               debug=self._debug)
> > +                               monitor_address=dstmonaddr)
> >  
> >          try:
> >              src.launch()
> > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> > index 36a7757aaf..6f057904a9 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -195,8 +195,6 @@ class VM(qtest.QEMUQtestMachine):
> >          super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
> >                                   test_dir=test_dir,
> >                                   socket_scm_helper=socket_scm_helper)
> > -        if debug:
> > -            self._debug = True
> 
> And this is the main issue. So instead of the fix I proposed in
> previous commit major changes to
> "tests/qemu-iotests/iotests.py" are necessary.

Could you clarify what those major changes are?

iotests.py was already changed to call basicConfig() according to
args.debug in main().


> 
> >          self._num_drives = 0
> >  
> >      def add_device(self, opts):
> > 
> 




-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine

[Lukáš Doktor]

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

Dne 10.10.2017 v 04:50 Eduardo Habkost napsal(a):
> On Sat, Oct 07, 2017 at 10:34:57AM +0200, Lukáš Doktor wrote:
>> Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
>>> All scripts that use the QEMUMachine and QEMUQtestMachine classes
>>> (device-crash-test, tests/migration/*, iotests.py, basevm.py)
>>> already configure logging.
>>>
>>> The basicConfig() call inside QEMUMachine.__init__() is being
>>> kept just to make sure a script would still work if it didn't
>>> configure logging.
>>>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>> ---
>>>  scripts/qemu.py                     | 6 ++----
>>>  tests/migration/guestperf/engine.py | 6 ++----
>>>  tests/qemu-iotests/iotests.py       | 2 --
>>>  3 files changed, 4 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>> index f6d2e68627..9bfdf6d37d 100644
>>> --- a/scripts/qemu.py
>>> +++ b/scripts/qemu.py
>>> @@ -54,7 +54,7 @@ class QEMUMachine(object):
>>>  
>>>      def __init__(self, binary, args=None, wrapper=None, name=None,
>>>                   test_dir="/var/tmp", monitor_address=None,
>>> -                 socket_scm_helper=None, debug=False):
>>> +                 socket_scm_helper=None):
>>>          '''
>>>          Initialize a QEMUMachine
>>>  
>>> @@ -65,7 +65,6 @@ class QEMUMachine(object):
>>>          @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()"
>>> -        @param debug: enable debug mode
>>>          @note: Qemu process is not started until launch() is used.
>>>          '''
>>>          if args is None:
>>> @@ -85,12 +84,11 @@ class QEMUMachine(object):
>>>          self._events = []
>>>          self._iolog = None
>>>          self._socket_scm_helper = socket_scm_helper
>>> -        self._debug = debug
>>>          self._qmp = None
>>>          self._qemu_full_args = None
>>>  
>>>          # just in case logging wasn't configured by the main script:
>>> -        logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
>>> +        logging.basicConfig()
>> Yes, this behaves the same as `debug=False`
>>
>>>  
>>>      def __enter__(self):
>>>          return self
>>> diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
>>> index 0a13050bc6..e14d4320b2 100644
>>> --- a/tests/migration/guestperf/engine.py
>>> +++ b/tests/migration/guestperf/engine.py
>>> @@ -388,15 +388,13 @@ class Engine(object):
>>>                                 args=self._get_src_args(hardware),
>>>                                 wrapper=self._get_src_wrapper(hardware),
>>>                                 name="qemu-src-%d" % os.getpid(),
>>> -                               monitor_address=srcmonaddr,
>>> -                               debug=self._debug)
>>> +                               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,
>>> -                               debug=self._debug)
>>> +                               monitor_address=dstmonaddr)
>>>  
>>>          try:
>>>              src.launch()
>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>> index 36a7757aaf..6f057904a9 100644
>>> --- a/tests/qemu-iotests/iotests.py
>>> +++ b/tests/qemu-iotests/iotests.py
>>> @@ -195,8 +195,6 @@ class VM(qtest.QEMUQtestMachine):
>>>          super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
>>>                                   test_dir=test_dir,
>>>                                   socket_scm_helper=socket_scm_helper)
>>> -        if debug:
>>> -            self._debug = True
>>
>> And this is the main issue. So instead of the fix I proposed in
>> previous commit major changes to
>> "tests/qemu-iotests/iotests.py" are necessary.
> 
> Could you clarify what those major changes are?
> 
> iotests.py was already changed to call basicConfig() according to
> args.debug in main().
> 

Hello Eduardo,

I'm sorry, I was not aware of that patch. I think I used git://github.com/ehabkost/qemu.git branch python-next as HEAD, but either I forgot to update it, or that patch was not yet pushed, or was I suppose to use different HEAD? I'm sorry, I'm not used to this kind of workflow.

Btw the afa79b55676dcd1859aa9d1f983c9dfbbcc13197 is exactly what I had in mind. The major changes I mentioned were related to `TestRunner` stream and it's verbosity, which was not part of that commit and is not really necessary.

Anyway with that commit this works well.

Reviewed-by: Lukáš Doktor <ldoktor@redhat.com> 

> 
>>
>>>          self._num_drives = 0
>>>  
>>>      def add_device(self, opts):
>>>
>>
> 
> 
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 502 bytes --]

Re: [Qemu-devel] [PATCH v2 2/3] scripts: Remove debug parameter from QEMUMonitorProtocol

[Lukáš Doktor]

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

Dne 10.10.2017 v 04:49 Eduardo Habkost napsal(a):
> On Sat, Oct 07, 2017 at 10:26:14AM +0200, Lukáš Doktor wrote:
>> Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
>>> Use logging module for the QMP debug messages.  The only scripts
>>> that set debug=True are iotests.py and guestperf/engine.py, and
>>> they already call logging.basicConfig() to set up logging.
>>>
>>> Scripts that don't configure logging are safe as long as they
>>> don't need debugging output, because debug messages don't trigger
>>> the "No handlers could be found for logger" message from the
>>> Python logging module.
>>>
>>> Scripts that already configure logging but don't use debug=True
>>> (e.g. scripts/vm/basevm.py) will get QMP debugging enabled for
>>> free.
>>>
>>> Cc: "Alex Bennée" <alex.bennee@linaro.org>
>>> Cc: Fam Zheng <famz@redhat.com>
>>> Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>> ---
>>> Changes v1 -> v2:
>>> * Actually remove debug parameter from method definition
>>>   (Fam Zheng)
>>> * Fix "<<<" vs ">>>" confusion
>>>   (Fam Zheng)
>>> * Remove "import sys" line
>>>   (Lukáš Doktor)
>>> ---
>>>  scripts/qemu.py    |  3 +--
>>>  scripts/qmp/qmp.py | 16 +++++++---------
>>>  2 files changed, 8 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>> index c9a106fbce..f6d2e68627 100644
>>> --- a/scripts/qemu.py
>>> +++ b/scripts/qemu.py
>>> @@ -177,8 +177,7 @@ class QEMUMachine(object):
>>>  
>>>      def _pre_launch(self):
>>>          self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
>>> -                                                server=True,
>>> -                                                debug=self._debug)
>>> +                                                server=True)
>>>  
>>>      def _post_launch(self):
>>>          self._qmp.accept()
>>> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
>>> index ef12e8a1a0..07c9632e9e 100644
>>> --- a/scripts/qmp/qmp.py
>>> +++ b/scripts/qmp/qmp.py
>>> @@ -11,7 +11,7 @@
>>>  import json
>>>  import errno
>>>  import socket
>>> -import sys
>>> +import logging
>>>  
>>>  
>>>  class QMPError(Exception):
>>> @@ -32,12 +32,14 @@ class QMPTimeoutError(QMPError):
>>>  
>>>  class QEMUMonitorProtocol(object):
>>>  
>>> +    #: Logger object for debugging messages
>>> +    logger = logging.getLogger('QMP')
>>>      #: Socket's error class
>>>      error = socket.error
>>>      #: Socket's timeout
>>>      timeout = socket.timeout
>>>  
>>> -    def __init__(self, address, server=False, debug=False):
>>> +    def __init__(self, address, server=False):
>>>          """
>>>          Create a QEMUMonitorProtocol class.
>>>  
>>> @@ -51,7 +53,6 @@ class QEMUMonitorProtocol(object):
>>>          """
>>>          self.__events = []
>>>          self.__address = address
>>> -        self._debug = debug
>>>          self.__sock = self.__get_sock()
>>>          self.__sockfile = None
>>>          if server:
>>> @@ -83,8 +84,7 @@ class QEMUMonitorProtocol(object):
>>>                  return
>>>              resp = json.loads(data)
>>>              if 'event' in resp:
>>> -                if self._debug:
>>> -                    print >>sys.stderr, "QMP:<<< %s" % resp
>>> +                self.logger.debug("<<< %s", resp)
>>>                  self.__events.append(resp)
>>>                  if not only_event:
>>>                      continue
>>> @@ -164,8 +164,7 @@ class QEMUMonitorProtocol(object):
>>>          @return QMP response as a Python dict or None if the connection has
>>>                  been closed
>>>          """
>>> -        if self._debug:
>>> -            print >>sys.stderr, "QMP:>>> %s" % qmp_cmd
>>> +        self.logger.debug(">>> %s", qmp_cmd)
>>>          try:
>>>              self.__sock.sendall(json.dumps(qmp_cmd))
>>>          except socket.error as err:
>>> @@ -173,8 +172,7 @@ class QEMUMonitorProtocol(object):
>>>                  return
>>>              raise socket.error(err)
>>>          resp = self.__json_read()
>>> -        if self._debug:
>>> -            print >>sys.stderr, "QMP:<<< %s" % resp
>>> +        self.logger.debug("<<< %s", resp)
>>>          return resp
>>>  
>>>      def cmd(self, name, args=None, cmd_id=None):
>>>
>>
>> This one looks good, but in order to no break qemu-iotests verbose mode it requires fix to qtest/iotests:
>>
>> ```diff
>> diff --git a/scripts/qtest.py b/scripts/qtest.py
>> index df0daf2..0e955a8 100644
>> --- a/scripts/qtest.py
>> +++ b/scripts/qtest.py
>> @@ -77,12 +77,12 @@ class QEMUQtestMachine(qemu.QEMUMachine):
>>      '''A QEMU VM'''
>>  
>>      def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
>> -                 socket_scm_helper=None):
>> +                 socket_scm_helper=None, debug=False):
>>          if name is None:
>>              name = "qemu-%d" % os.getpid()
>>          super(QEMUQtestMachine,
>>                self).__init__(binary, args, name=name, test_dir=test_dir,
>> -                             socket_scm_helper=socket_scm_helper)
>> +                             socket_scm_helper=socket_scm_helper, debug=debug)
>>          self._qtest = None
>>          self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
>>  
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 1af117e..989ebd3 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -193,9 +193,8 @@ class VM(qtest.QEMUQtestMachine):
>>          name = "qemu%s-%d" % (path_suffix, os.getpid())
>>          super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
>>                                   test_dir=test_dir,
>> -                                 socket_scm_helper=socket_scm_helper)
>> -        if debug:
>> -            self._debug = True
>> +                                 socket_scm_helper=socket_scm_helper,
>> +                                 debug=debug)
>>          self._num_drives = 0
>>  
>>      def add_device(self, opts):
>> ```
> 
> I'm confused by why the above patch is necessary.  We are in the
> process of removing the 'debug' parameter from QEMUMachine and
> QEMUMonitorProtocol, why would we add a debug parameter to
> QEMUQtestMachine and iotests.py?
> 
>>
>> (because the `debug` used to be set after `__init__`, but the logging is initialized during `__init__`.)
>>
>> Therefor conditional ACK when the qtest/iotest fix precedes this commit.
> 
> Do you mean the following?
> 
>   Message-Id: <20170927130339.21444-3-ehabkost@redhat.com>
>   Subject: [Qemu-devel] [PATCH 2/5] iotests: Set up Python logging
>   https://www.mail-archive.com/qemu-devel@nongnu.org/msg485036.html
>   https://github.com/ehabkost/qemu/commit/afa79b55676dcd1859aa9d1f983c9dfbbcc13197
> 

I'm sorry, my fault. With the afa79b55676dcd1859aa9d1f983c9dfbbcc13197 commit it works well and I haven't found any other issues.

Reviewed-by: Lukáš Doktor <ldoktor@redhat.com> 

> It is already queued on python-next.
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 502 bytes --]