Re: [Qemu-devel] [PATCH v6 3/7] qemu.py: use python logging system

[Eduardo Habkost <ehabkost@redhat.com>]

On Mon, Jul 31, 2017 at 10:51:06AM +0200, Amador Pahim wrote:
> Let's provide extra control and flexibility by using python logging
> system instead of print and/or sys.std*.write().
> 
> Signed-off-by: Amador Pahim <apahim@redhat.com>
> ---
>  scripts/qemu.py | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 77565eb092..e3ea534ec4 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -13,6 +13,7 @@
>  #
>  
>  import errno
> +import logging
>  import string
>  import os
>  import sys
> @@ -20,11 +21,19 @@ import subprocess
>  import qmp.qmp
>  
>  
> +logging.basicConfig()

Please don't call logging.basicConfig() from a module, as this
will break scripts that already use logging.basicConfig().

This breaks the following:

  $ ./scripts/device-crash-test --debug /usr/bin/qemu-system-x86_64


> +LOG = logging.getLogger(__name__)
> +
> +
>  class QEMUMachine(object):
>      '''A QEMU VM'''
>  
>      def __init__(self, binary, args=[], wrapper=[], name=None, test_dir="/var/tmp",
>                   monitor_address=None, socket_scm_helper=None, debug=False):
> +        if debug:
> +            LOG.setLevel(logging.DEBUG)
> +        else:
> +            LOG.setLevel(logging.INFO)

This makes the debug argument for one QEMUMachine object affect
other QEMUMachine objects.

What about:
    self.logger = LOG.getChild(name)
    self.logger.setLevel(...)

But do we really need to support per-QEMUMachine verbosity
levels?  We could simply eliminate the 'debug' argument and let
scripts configure logging.

>          if name is None:
>              name = "qemu-%d" % os.getpid()
>          if monitor_address is None:
> @@ -62,10 +71,10 @@ class QEMUMachine(object):
>          # In iotest.py, the qmp should always use unix socket.
>          assert self._qmp.is_scm_available()
>          if self._socket_scm_helper is None:
> -            print >>sys.stderr, "No path to socket_scm_helper set"
> +            LOG.error("No path to socket_scm_helper set")
>              return -1
>          if os.path.exists(self._socket_scm_helper) == False:
> -            print >>sys.stderr, "%s does not exist" % self._socket_scm_helper
> +            LOG.error("%s does not exist", self._socket_scm_helper)
>              return -1
>          fd_param = ["%s" % self._socket_scm_helper,
>                      "%d" % self._qmp.get_sock_fd(),
> @@ -154,7 +163,8 @@ class QEMUMachine(object):
>  
>              exitcode = self._popen.wait()
>              if exitcode < 0:
> -                sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode, ' '.join(self._args)))
> +                LOG.error('qemu received signal %i: %s', -exitcode,
> +                          ' '.join(self._args))
>              self._load_io_log()
>              self._post_shutdown()
>  
> -- 
> 2.13.3
> 

-- 
Eduardo