| Message ID | 20170927130339.21444-6-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | scripts: Remove 'debug' parameter from QEMUMachine & QEMUMonitorProtocol | expand |
On Wed, Sep 27, 2017 at 10:03:39AM -0300, Eduardo Habkost wrote: > 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. I don't find that compelling. IIUC, if we remove this basicConfig they'll see a message that logging is not configured, which is a suitable hint to fix the script to configure logging. > > # just in case logging wasn't configured by the main script: > - logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN)) > + logging.basicConfig() So I'd just remove this line entirely Regards, Daniel
On Wed, Sep 27, 2017 at 02:22:24PM +0100, Daniel P. Berrange wrote: > On Wed, Sep 27, 2017 at 10:03:39AM -0300, Eduardo Habkost wrote: > > 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. > > I don't find that compelling. IIUC, if we remove this basicConfig > they'll see a message that logging is not configured, which is a > suitable hint to fix the script to configure logging. I don't see the benefit of requiring the caller to configure logging even if they just want the default behavior (WARN loglevel, logged to stderr). > > > > # just in case logging wasn't configured by the main script: > > - logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN)) > > + logging.basicConfig() > > So I'd just remove this line entirely I think it does no harm, and can save people from wasting time googling for "No handlers could be found for logger" just to find out they need to add a logging.basicConfig() call to their script.
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):
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(-)