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

[Amador Pahim]

On Mon, Jul 31, 2017 at 4:50 PM, Eduardo Habkost <address@hidden> wrote:
> 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 <address@hidden>
>> ---
>>  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().

Sure, this is a leftover from my tests.

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

Ok, agreed.

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