Re: [PATCH v3 14/16] python/qemu: Cleanup changes to ConsoleSocket

[Philippe Mathieu-Daudé]

+John who requested the changes.

On 7/24/20 8:45 AM, Alex Bennée wrote:
> From: Robert Foley <robert.foley@linaro.org>
> 
> The changes to console_socket.py and machine.py are to
> cleanup for pylint and flake8.
> 
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20200717203041.9867-2-robert.foley@linaro.org>
> ---
>  python/qemu/console_socket.py | 57 ++++++++++++++++++-----------------
>  python/qemu/machine.py        |  7 +++--
>  python/qemu/pylintrc          |  2 +-
>  3 files changed, 34 insertions(+), 32 deletions(-)
> 
> diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
> index 830cb7c6282..09986bc2152 100644
> --- a/python/qemu/console_socket.py
> +++ b/python/qemu/console_socket.py
> @@ -1,12 +1,9 @@
> -#!/usr/bin/env python3
> -#
> -# This python module implements a ConsoleSocket object which is
> -# designed always drain the socket itself, and place
> -# the bytes into a in memory buffer for later processing.
> -#
> -# Optionally a file path can be passed in and we will also
> -# dump the characters to this file for debug.
> -#
> +"""
> +QEMU Console Socket Module:
> +
> +This python module implements a ConsoleSocket object,
> +which can drain a socket and optionally dump the bytes to file.
> +"""
>  # Copyright 2020 Linaro
>  #
>  # Authors:
> @@ -15,20 +12,27 @@
>  # This code is licensed under the GPL version 2 or later.  See
>  # the COPYING file in the top-level directory.
>  #
> +
>  import asyncore
>  import socket
>  import threading
> -import io
> -import os
> -import sys
>  from collections import deque
>  import time
> -import traceback
> +
>  
>  class ConsoleSocket(asyncore.dispatcher):
> +    """
> +    ConsoleSocket represents a socket attached to a char device.
>  
> +    Drains the socket and places the bytes into an in memory buffer
> +    for later processing.
> +
> +    Optionally a file path can be passed in and we will also
> +    dump the characters to this file for debugging purposes.
> +    """
>      def __init__(self, address, file=None):
>          self._recv_timeout_sec = 300
> +        self._sleep_time = 0.5
>          self._buffer = deque()
>          self._asyncore_thread = None
>          self._sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
> @@ -70,31 +74,28 @@ class ConsoleSocket(asyncore.dispatcher):
>  
>      def handle_read(self):
>          """process arriving characters into in memory _buffer"""
> -        try:
> -            data = asyncore.dispatcher.recv(self, 1)
> -            # latin1 is needed since there are some chars
> -            # we are receiving that cannot be encoded to utf-8
> -            # such as 0xe2, 0x80, 0xA6.
> -            string = data.decode("latin1")
> -        except:
> -            print("Exception seen.")
> -            traceback.print_exc()
> -            return
> +        data = asyncore.dispatcher.recv(self, 1)

A bit more than the commit description, but I don't mind.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +        # latin1 is needed since there are some chars
> +        # we are receiving that cannot be encoded to utf-8
> +        # such as 0xe2, 0x80, 0xA6.
> +        string = data.decode("latin1")
>          if self._logfile:
>              self._logfile.write("{}".format(string))
>              self._logfile.flush()
>          for c in string:
>              self._buffer.extend(c)
>  
> -    def recv(self, n=1, sleep_delay_s=0.1):
> -        """Return chars from in memory buffer"""
> +    def recv(self, buffer_size=1):
> +        """Return chars from in memory buffer.
> +           Maintains the same API as socket.socket.recv.
> +        """
>          start_time = time.time()
> -        while len(self._buffer) < n:
> -            time.sleep(sleep_delay_s)
> +        while len(self._buffer) < buffer_size:
> +            time.sleep(self._sleep_time)
>              elapsed_sec = time.time() - start_time
>              if elapsed_sec > self._recv_timeout_sec:
>                  raise socket.timeout
> -        chars = ''.join([self._buffer.popleft() for i in range(n)])
> +        chars = ''.join([self._buffer.popleft() for i in range(buffer_size)])
>          # We choose to use latin1 to remain consistent with
>          # handle_read() and give back the same data as the user would
>          # receive if they were reading directly from the
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index 80c4d4a8b6e..9956360a792 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -27,7 +27,7 @@ import socket
>  import tempfile
>  from typing import Optional, Type
>  from types import TracebackType
> -from qemu.console_socket import ConsoleSocket
> +from . import console_socket
>  
>  from . import qmp
>  
> @@ -674,8 +674,9 @@ class QEMUMachine:
>          """
>          if self._console_socket is None:
>              if self._drain_console:
> -                self._console_socket = ConsoleSocket(self._console_address,
> -                                                    
> file=self._console_log_path)
> +                self._console_socket = console_socket.ConsoleSocket(
> +                    self._console_address,
> +                    file=self._console_log_path)
>              else:
>                  self._console_socket = socket.socket(socket.AF_UNIX,
>                                                       socket.SOCK_STREAM)
> diff --git a/python/qemu/pylintrc b/python/qemu/pylintrc
> index 5d6ae7367d8..3f69205000d 100644
> --- a/python/qemu/pylintrc
> +++ b/python/qemu/pylintrc
> @@ -33,7 +33,7 @@ good-names=i,
>             Run,
>             _,
>             fd,
> -
> +           c,
>  [VARIABLES]
>  
>  [STRING]
>