Re: [Qemu-devel] [PATCH v4 01/10] qemu.py: Pylint/style fixes

[Lukáš Doktor]

Dne 8.8.2017 v 14:38 Stefan Hajnoczi napsal(a):
> On Wed, Jul 26, 2017 at 04:42:17PM +0200, Lukáš Doktor wrote:
>> No actual code changes, just several pylint/style fixes and docstring
>> clarifications.
>>
>> Signed-off-by: Lukáš Doktor <address@hidden>
>> ---
>>  scripts/qemu.py | 76 
>> ++++++++++++++++++++++++++++++++++++++++-----------------
>>  1 file changed, 53 insertions(+), 23 deletions(-)
>>
>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>> index 880e3e8..191c916 100644
>> --- a/scripts/qemu.py
>> +++ b/scripts/qemu.py
>> @@ -23,8 +23,22 @@ import qmp.qmp
>>  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):
>> +    def __init__(self, binary, args=[], wrapper=[], name=None,
>> +                 test_dir="/var/tmp", monitor_address=None,
>> +                 socket_scm_helper=None, debug=False):
>> +        '''
>> +        Create a QEMUMachine object
>> +
>> +        @param binary: path to the qemu binary (str)
>> +        @param args: initial list of extra arguments
>> +        @param wrapper: list of arguments used as prefix to qemu binary
>> +        @param name: name of this object (used for log/monitor/... file 
>> names)
>> +        @param test_dir: base location to put log/monitor/... files in
>> +        @param monitor_address: custom address for QMP monitor
>> +        @param socket_scm_helper: path to scm_helper binary (to forward fds)
>> +        @param debug: enable debug mode (forwarded to QMP helper and such)
>> +        @note: Qemu process is not started until launch() is used.
>> +        '''
>>          if name is None:
>>              name = "qemu-%d" % os.getpid()
>>          if monitor_address is None:
>> @@ -33,12 +47,13 @@ class QEMUMachine(object):
>>          self._qemu_log_path = os.path.join(test_dir, name + ".log")
>>          self._popen = None
>>          self._binary = binary
>> -        self._args = list(args) # Force copy args in case we modify them
>> +        self._args = list(args)     # Force copy args in case we modify them
>>          self._wrapper = wrapper
>>          self._events = []
>>          self._iolog = None
>>          self._socket_scm_helper = socket_scm_helper
>>          self._debug = debug
>> +        self._qmp = None
>>  
>>      # This can be used to add an unused monitor instance.
>>      def add_monitor_telnet(self, ip, port):
>> @@ -64,16 +79,16 @@ class QEMUMachine(object):
>>          if self._socket_scm_helper is None:
>>              print >>sys.stderr, "No path to socket_scm_helper set"
>>              return -1
>> -        if os.path.exists(self._socket_scm_helper) == False:
>> +        if os.path.exists(self._socket_scm_helper) is False:
> 
> PEP8 says:
> 
>   Don't compare boolean values to True or False using ==.
> 
> https://www.python.org/dev/peps/pep-0008/#id51
> 
> This should be:
> 
>   if not os.path.exists(self._socket_scm_helper):
> 
Hello Stefan,

yep, you are right. I'll fix it in v5

>>      def command(self, cmd, conv_keys=True, **args):
>> +        '''
>> +        Invoke a QMP command and on success report result dict or on failure
> 
> s/report/return/ ?
> 
Don't see much difference, but I'll use the "return" in the next version.

>> +        raise exception with details.
>> +        '''
>>          reply = self.qmp(cmd, conv_keys, **args)
>>          if reply is None:
>>              raise Exception("Monitor is closed")
>> @@ -193,15 +216,18 @@ class QEMUMachine(object):
>>          return events
>>  
>>      def event_wait(self, name, timeout=60.0, match=None):
>> -        # Test if 'match' is a recursive subset of 'event'
>> -        def event_match(event, match=None):
>> +        ''' Wait for event in QMP, optionally filter results by match. '''
> 
> Why are spaces around this docstring?
> 

Sorry, this is a custom from other project's guidelines. I'll fix it.

Lukáš

signature.asc
Description: OpenPGP digital signature