On Thu, 16 Jul 2020 at 09:42, Alex Bennée <alex.ben...@linaro.org> wrote: > <snip> > > + self._drain_thread = None > > + socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM) > > + self.connect(address) > > + self._drain = drain > > We end up with two variables that represent the fact we have draining > happening. Could we rationalise it into: > > if drain: > self._drain_thread = self._thread_start() > else > self._drain_thread = None # if this is needed > > And then tests for: > > if not self._drain: > > become > > if self._drain_thread is None:
Good point, this is simpler. Will update. <snip> > > + if self._drain and self._drain_thread is not None: > > + thread, self._drain_thread = self._drain_thread, None > Would self._drain ever not have self._drain_thread set? No, I believe that if drain is set, it results in _drain_thread also being set. This will be cleaned up once we drop the _drain. > > > thread.join() > > + socket.socket.close(self) > <snip> > > diff --git a/python/qemu/machine.py b/python/qemu/machine.py > > index 6769359766..62709d86e4 100644 > > --- a/python/qemu/machine.py > > +++ b/python/qemu/machine.py > > @@ -22,7 +22,6 @@ import logging > > import os > > import subprocess > > import shutil > > -import socket > > FYI minor patch conflict here with master OK, will rebase and fix this conflict. Thanks & Regards, -Rob > > > import tempfile > > from typing import Optional, Type > > from types import TracebackType > > @@ -591,12 +590,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 > > > -- > Alex Bennée