Skip to content

Commit

Permalink
tests/avocado: Fix console data loss
Browse files Browse the repository at this point in the history
Occasionally some avocado tests will fail waiting for console line
despite the machine running correctly. Console data goes missing, as can
be seen in the console log. This is due to _console_interaction calling
makefile() on the console socket each time it is invoked, which must be
losing old buffer contents when going out of scope.

It is not enough to makefile() with buffered=0. That helps significantly
but data loss is still possible. My guess is that readline() has a line
buffer even when the file is in unbuffered mode, that can eat data.

Fix this by providing a console file that persists for the life of the
console.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
Message-Id: <20230912131340.405619-1-npiggin@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: John Snow <jsnow@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230914155422.426639-9-alex.bennee@linaro.org>
  • Loading branch information
npiggin authored and stsquad committed Sep 20, 2023
1 parent eca74af commit f0ec14c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
19 changes: 19 additions & 0 deletions python/qemu/machine/machine.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ def __init__(self,
self.sock_dir, f"{self._name}.con"
)
self._console_socket: Optional[socket.socket] = None
self._console_file: Optional[socket.SocketIO] = None
self._remove_files: List[str] = []
self._user_killed = False
self._quit_issued = False
Expand Down Expand Up @@ -509,6 +510,11 @@ def _early_cleanup(self) -> None:
# If we keep the console socket open, we may deadlock waiting
# for QEMU to exit, while QEMU is waiting for the socket to
# become writable.
if self._console_file is not None:
LOG.debug("Closing console file")
self._console_file.close()
self._console_file = None

if self._console_socket is not None:
LOG.debug("Closing console socket")
self._console_socket.close()
Expand Down Expand Up @@ -874,12 +880,25 @@ def console_socket(self) -> socket.socket:
Returns a socket connected to the console
"""
if self._console_socket is None:
LOG.debug("Opening console socket")
self._console_socket = console_socket.ConsoleSocket(
self._console_address,
file=self._console_log_path,
drain=self._drain_console)
return self._console_socket

@property
def console_file(self) -> socket.SocketIO:
"""
Returns a file associated with the console socket
"""
if self._console_file is None:
LOG.debug("Opening console file")
self._console_file = self.console_socket.makefile(mode='rb',
buffering=0,
encoding='utf-8')
return self._console_file

@property
def temp_dir(self) -> str:
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/avocado/avocado_qemu/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def _console_interaction(test, success_message, failure_message,
assert not keep_sending or send_string
if vm is None:
vm = test.vm
console = vm.console_socket.makefile(mode='rb', encoding='utf-8')
console = vm.console_file
console_logger = logging.getLogger('console')
while True:
if send_string:
Expand Down

0 comments on commit f0ec14c

Please sign in to comment.