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.5

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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.

-- 
Eduardo