[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/3] scripts: Remove debug parameter from QEM
From: |
Lukáš Doktor |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/3] scripts: Remove debug parameter from QEMUMonitorProtocol |
Date: |
Sat, 7 Oct 2017 10:26:14 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
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" <address@hidden>
> Cc: Fam Zheng <address@hidden>
> Cc: "Philippe Mathieu-Daudé" <address@hidden>
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
> 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áš
signature.asc
Description: OpenPGP digital signature