Re: [Qemu-devel] [RFC PATCH 2/2] machine.py: minor delinting

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 5/29/19 2:54 PM, John Snow wrote:
> On 5/29/19 1:18 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>>
>>> Since we're out in a new module, do a quick cursory pass of some of the
>>> more obvious style issues.
>>>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>>  python/qemu/machine.py | 23 +++++++++++++++--------
>>>  1 file changed, 15 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>>> index a8a311b035..925046fc20 100644
>>> --- a/python/qemu/machine.py
>>> +++ b/python/qemu/machine.py
>>> @@ -16,7 +16,6 @@ import errno
>>>  import logging
>>>  import os
>>>  import subprocess
>>> -import re
>>>  import shutil
>>>  import socket
>>>  import tempfile
>>> @@ -54,7 +53,7 @@ class MonitorResponseError(qmp.QMPError):
>>>          self.reply = reply
>>>  
>>>  
>>> -class QEMUMachine(object):
>>> +class QEMUMachine:
>>
>> Premature.  Makes it a "classic class" in Python 2.  See
>>
>> https://docs.python.org/2.7/reference/datamodel.html#new-style-and-classic-classes
>> https://pythonclock.org/
>>
> 
> I thought we had dropped support for 2? No? I need to double-check this
> patch then. I have not been testing with 2.

Deprecated doesn't mean dropped :(

> 
>>>      """
>>>      A QEMU VM
>>>  
>>> @@ -119,8 +118,10 @@ class QEMUMachine(object):
>>>          self.shutdown()
>>>          return False
>>>  
>>> -    # This can be used to add an unused monitor instance.
>>>      def add_monitor_null(self):
>>> +        """
>>> +        This can be used to add an unused monitor instance.
>>> +        """
>>>          self._args.append('-monitor')
>>>          self._args.append('null')
>>>  
>>> @@ -143,10 +144,13 @@ class QEMUMachine(object):
>>>          self._args.append(','.join(options))
>>>          return self
>>>  
>>> -    # Exactly one of fd and file_path must be given.
>>> -    # (If it is file_path, the helper will open that file and pass its
>>> -    # own fd)
>>>      def send_fd_scm(self, fd=None, file_path=None):
>>> +        """
>>> +        Send an fd or file_path to socket_scm_helper.
>>> +
>>> +        Exactly one of fd and file_path must be given.
>>> +        If it is file_path, the helper will open that file and pass its own fd.
>>> +        """
>>>          # In iotest.py, the qmp should always use unix socket.
>>>          assert self._qmp.is_scm_available()
>>>          if self._socket_scm_helper is None:
>>> @@ -194,14 +198,17 @@ class QEMUMachine(object):
>>>              raise
>>>  
>>>      def is_running(self):
>>> +        """Returns true if the VM is running."""
>>>          return self._popen is not None and self._popen.poll() is None
>>>  
>>>      def exitcode(self):
>>> +        """Returns the exit code if possible, or None."""
>>>          if self._popen is None:
>>>              return None
>>>          return self._popen.poll()
>>>  
>>>      def get_pid(self):
>>> +        """Returns the PID of the running process, or None."""
>>>          if not self.is_running():
>>>              return None
>>>          return self._popen.pid
>>> @@ -339,7 +346,7 @@ class QEMUMachine(object):
>>>                  command = ' '.join(self._qemu_full_args)
>>>              else:
>>>                  command = ''
>>> -            LOG.warn(msg, -exitcode, command)
>>> +            LOG.warning(msg, -exitcode, command)
>>
>> Is this a bug fix?
>>
> 
> No, "warn" is just deprecated.
> 
>>>  
>>>          self._launched = False
>>>  
>>> @@ -373,7 +380,7 @@ class QEMUMachine(object):
>>>          """
>>>          Poll for one queued QMP events and return it
>>>          """
>>> -        if len(self._events) > 0:
>>> +        if self._events:
>>>              return self._events.pop(0)
>>>          return self._qmp.pull_event(wait=wait)
> 
>