All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: fam@euphon.net, berrange@redhat.com,
	"Robert Foley" <robert.foley@linaro.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	richard.henderson@linaro.org, f4bug@amsat.org, cota@braap.org,
	"Cleber Rosa" <crosa@redhat.com>,
	aurelien@aurel32.net, "Eduardo Habkost" <ehabkost@redhat.com>
Subject: [PATCH v3 15/16] python/qemu: Change ConsoleSocket to optionally drain socket.
Date: Fri, 24 Jul 2020 07:45:08 +0100	[thread overview]
Message-ID: <20200724064509.331-16-alex.bennee@linaro.org> (raw)
In-Reply-To: <20200724064509.331-1-alex.bennee@linaro.org>

From: Robert Foley <robert.foley@linaro.org>

The primary purpose of this change is to clean up
machine.py's console_socket property to return a single type,
a ConsoleSocket.

ConsoleSocket now derives from a socket, which means that
in the default case (of not draining), machine.py
will see the same behavior as it did prior to ConsoleSocket.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200717203041.9867-3-robert.foley@linaro.org>
---
 python/qemu/console_socket.py | 92 +++++++++++++++++++++--------------
 python/qemu/machine.py        | 13 ++---
 2 files changed, 59 insertions(+), 46 deletions(-)

diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index 09986bc2152..70869fbbdc4 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -13,68 +13,75 @@ which can drain a socket and optionally dump the bytes to file.
 # the COPYING file in the top-level directory.
 #
 
-import asyncore
 import socket
 import threading
 from collections import deque
 import time
 
 
-class ConsoleSocket(asyncore.dispatcher):
+class ConsoleSocket(socket.socket):
     """
     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 (if drain==True), 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):
+    def __init__(self, address, file=None, drain=False):
         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)
-        self._sock.connect(address)
+        socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM)
+        self.connect(address)
         self._logfile = None
         if file:
             self._logfile = open(file, "w")
-        asyncore.dispatcher.__init__(self, sock=self._sock)
         self._open = True
-        self._thread_start()
+        if drain:
+            self._drain_thread = self._thread_start()
+        else:
+            self._drain_thread = None
 
-    def _thread_start(self):
-        """Kick off a thread to wait on the asyncore.loop"""
-        if self._asyncore_thread is not None:
-            return
-        self._asyncore_thread = threading.Thread(target=asyncore.loop,
-                                                 kwargs={'timeout':1})
-        self._asyncore_thread.daemon = True
-        self._asyncore_thread.start()
+    def _drain_fn(self):
+        """Drains the socket and runs while the socket is open."""
+        while self._open:
+            try:
+                self._drain_socket()
+            except socket.timeout:
+                # The socket is expected to timeout since we set a
+                # short timeout to allow the thread to exit when
+                # self._open is set to False.
+                time.sleep(self._sleep_time)
 
-    def handle_close(self):
-        """redirect close to base class"""
-        # Call the base class close, but not self.close() since
-        # handle_close() occurs in the context of the thread which
-        # self.close() attempts to join.
-        asyncore.dispatcher.close(self)
+    def _thread_start(self):
+        """Kick off a thread to drain the socket."""
+        # Configure socket to not block and timeout.
+        # This allows our drain thread to not block
+        # on recieve and exit smoothly.
+        socket.socket.setblocking(self, False)
+        socket.socket.settimeout(self, 1)
+        drain_thread = threading.Thread(target=self._drain_fn)
+        drain_thread.daemon = True
+        drain_thread.start()
+        return drain_thread
 
     def close(self):
         """Close the base object and wait for the thread to terminate"""
         if self._open:
             self._open = False
-            asyncore.dispatcher.close(self)
-            if self._asyncore_thread is not None:
-                thread, self._asyncore_thread = self._asyncore_thread, None
+            if self._drain_thread is not None:
+                thread, self._drain_thread = self._drain_thread, None
                 thread.join()
+            socket.socket.close(self)
             if self._logfile:
                 self._logfile.close()
                 self._logfile = None
 
-    def handle_read(self):
+    def _drain_socket(self):
         """process arriving characters into in memory _buffer"""
-        data = asyncore.dispatcher.recv(self, 1)
+        data = socket.socket.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.
@@ -85,27 +92,38 @@ class ConsoleSocket(asyncore.dispatcher):
         for c in string:
             self._buffer.extend(c)
 
-    def recv(self, buffer_size=1):
+    def recv(self, bufsize=1):
         """Return chars from in memory buffer.
            Maintains the same API as socket.socket.recv.
         """
+        if self._drain_thread is None:
+            # Not buffering the socket, pass thru to socket.
+            return socket.socket.recv(self, bufsize)
         start_time = time.time()
-        while len(self._buffer) < buffer_size:
+        while len(self._buffer) < bufsize:
             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(buffer_size)])
+        chars = ''.join([self._buffer.popleft() for i in range(bufsize)])
         # 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
         # socket w/o our intervention.
         return chars.encode("latin1")
 
-    def set_blocking(self):
-        """Maintain compatibility with socket API"""
-        pass
+    def setblocking(self, value):
+        """When not draining we pass thru to the socket,
+           since when draining we control socket blocking.
+        """
+        if self._drain_thread is None:
+            socket.socket.setblocking(self, value)
 
     def settimeout(self, seconds):
-        """Set current timeout on recv"""
-        self._recv_timeout_sec = seconds
+        """When not draining we pass thru to the socket,
+           since when draining we control the timeout.
+        """
+        if seconds is not None:
+            self._recv_timeout_sec = seconds
+        if self._drain_thread is None:
+            socket.socket.settimeout(self, seconds)
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 9956360a792..c5f777b9e72 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -23,7 +23,6 @@ import os
 import subprocess
 import shutil
 import signal
-import socket
 import tempfile
 from typing import Optional, Type
 from types import TracebackType
@@ -673,12 +672,8 @@ class QEMUMachine:
         Returns a socket connected to the console
         """
         if self._console_socket is None:
-            if self._drain_console:
-                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)
-                self._console_socket.connect(self._console_address)
+            self._console_socket = console_socket.ConsoleSocket(
+                self._console_address,
+                file=self._console_log_path,
+                drain=self._drain_console)
         return self._console_socket
-- 
2.20.1



  parent reply	other threads:[~2020-07-24  6:53 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-24  6:44 [PATCH v3 00/16] candidate fixes for 5.1-rc1 (testing, semihosting, OOM tcg, x86 fpu) Alex Bennée
2020-07-24  6:44 ` [PATCH v3 01/16] shippable: add one more qemu to registry url Alex Bennée
2020-07-24  6:44 ` [PATCH v3 02/16] semihosting: defer connect_chardevs a little more to use serialx Alex Bennée
2020-07-24  6:44 ` [PATCH v3 03/16] semihosting: don't send the trailing '\0' Alex Bennée
2020-07-24  6:44 ` [PATCH v3 04/16] util: add qemu_get_host_physmem utility function Alex Bennée
2020-07-24  6:44 ` [PATCH v3 05/16] util/oslib-win32: add qemu_get_host_physmem implementation Alex Bennée
2020-07-24  6:51   ` Stefan Weil
2020-07-24  6:44 ` [PATCH v3 06/16] accel/tcg: better handle memory constrained systems Alex Bennée
2020-07-24 13:41   ` Robert Foley
2020-07-24 18:22   ` Richard Henderson
2020-07-24  6:45 ` [PATCH v3 07/16] target/i386: floatx80: avoid compound literals in static initializers Alex Bennée
2020-07-24 18:23   ` Richard Henderson
2020-07-24  6:45 ` [PATCH v3 08/16] linux-user: don't use MAP_FIXED in pgd_find_hole_fallback Alex Bennée
2020-07-24  6:45 ` [PATCH v3 09/16] tests/docker: fix update command due to python3 str/bytes distinction Alex Bennée
2020-07-24  6:45 ` [PATCH v3 10/16] tests/docker: fix binfmt_misc image building Alex Bennée
2020-07-24  6:45 ` [PATCH v3 11/16] tests/docker: add support for DEB_KEYRING Alex Bennée
2020-07-24  6:45 ` [PATCH v3 12/16] linux-user: fix clock_nanosleep() Alex Bennée
2020-07-24 18:24   ` Richard Henderson
2020-07-24  6:45 ` [PATCH v3 13/16] linux-user, ppc: fix clock_nanosleep() for linux-user-ppc Alex Bennée
2020-07-24 18:24   ` [PATCH v3 13/16] linux-user,ppc: " Richard Henderson
2020-07-24  6:45 ` [PATCH v3 14/16] python/qemu: Cleanup changes to ConsoleSocket Alex Bennée
2020-07-24  7:25   ` Philippe Mathieu-Daudé
2020-07-24  6:45 ` Alex Bennée [this message]
2020-07-24  6:45 ` [PATCH v3 16/16] tests/vm: add shutdown timeout in basevm.py Alex Bennée

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200724064509.331-16-alex.bennee@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=aurelien@aurel32.net \
    --cc=berrange@redhat.com \
    --cc=cota@braap.org \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=fam@euphon.net \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=robert.foley@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.