qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v4 01/10] qemu.py: Pylint/style fixes
Date: Tue, 8 Aug 2017 13:38:29 +0100
User-agent: Mutt/1.8.3 (2017-05-23)

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

>      def command(self, cmd, conv_keys=True, **args):
> +        '''
> +        Invoke a QMP command and on success report result dict or on failure

s/report/return/ ?

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

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]