From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
"Cleber Rosa" <crosa@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Joel Stanley" <joel@jms.id.au>,
"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
"Beraldo Leal" <bleal@redhat.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Pavel Dovgalyuk" <pavel.dovgaluk@ispras.ru>,
qemu-arm@nongnu.org, "Thomas Huth" <thuth@redhat.com>,
"Jiaxun Yang" <jiaxun.yang@flygoat.com>,
"John Snow" <jsnow@redhat.com>,
"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
"Peter Maydell" <peter.maydell@linaro.org>,
"Laurent Vivier" <lvivier@redhat.com>,
"Aurelien Jarno" <aurelien@aurel32.net>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Daniel P. Berrangé" <berrange@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>,
"Nicholas Piggin" <npiggin@gmail.com>
Subject: [PATCH 8/9] tests/avocado: Fix console data loss
Date: Thu, 14 Sep 2023 16:54:21 +0100 [thread overview]
Message-ID: <20230914155422.426639-9-alex.bennee@linaro.org> (raw)
In-Reply-To: <20230914155422.426639-1-alex.bennee@linaro.org>
From: Nicholas Piggin <npiggin@gmail.com>
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>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
python/qemu/machine/machine.py | 19 +++++++++++++++++++
tests/avocado/avocado_qemu/__init__.py | 2 +-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index c16a0b6fed..35d5a672db 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -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
@@ -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()
@@ -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:
"""
diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
index 33090903f1..0172a359b7 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -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:
--
2.39.2
next prev parent reply other threads:[~2023-09-14 15:55 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-14 15:54 [PATCH 0/9] testing/next: avocado, gitlab, docker, cirrus Alex Bennée
2023-09-14 15:54 ` [PATCH 1/9] tests: update Debian images to Bookworm Alex Bennée
2023-09-14 16:10 ` Thomas Huth
2023-09-15 9:14 ` Philippe Mathieu-Daudé
2023-09-15 9:24 ` Daniel P. Berrangé
2023-09-15 10:53 ` Philippe Mathieu-Daudé
2023-09-15 11:24 ` Thomas Huth
2023-09-15 12:00 ` Daniel P. Berrangé
2023-09-14 15:54 ` [PATCH 2/9] gitlab: fix typo/spelling in comments Alex Bennée
2023-09-14 16:07 ` Thomas Huth
2023-09-14 17:21 ` Philippe Mathieu-Daudé
2023-09-14 15:54 ` [PATCH 3/9] tests/docker: Update docker-loongarch-cross toolchain Alex Bennée
2023-09-14 15:54 ` [PATCH 4/9] microbit: add missing qtest_quit() call Alex Bennée
2023-09-14 15:54 ` [PATCH 5/9] qtest: kill orphaned qtest QEMU processes on FreeBSD Alex Bennée
2023-09-14 15:54 ` [PATCH 6/9] gitlab: make Cirrus CI timeout explicit Alex Bennée
2023-09-14 15:54 ` [PATCH 7/9] gitlab: make Cirrus CI jobs gating Alex Bennée
2023-09-15 9:15 ` Philippe Mathieu-Daudé
2023-09-14 15:54 ` Alex Bennée [this message]
2023-09-15 9:18 ` [PATCH 8/9] tests/avocado: Fix console data loss Philippe Mathieu-Daudé
2023-09-14 15:54 ` [PATCH 9/9] tests/avocado: Disable MIPS Malta tests due to GitLab issue #1884 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=20230914155422.426639-9-alex.bennee@linaro.org \
--to=alex.bennee@linaro.org \
--cc=armbru@redhat.com \
--cc=aurelien@aurel32.net \
--cc=berrange@redhat.com \
--cc=bleal@redhat.com \
--cc=crosa@redhat.com \
--cc=jiaxun.yang@flygoat.com \
--cc=joel@jms.id.au \
--cc=jsnow@redhat.com \
--cc=lvivier@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=mark.cave-ayland@ilande.co.uk \
--cc=npiggin@gmail.com \
--cc=pavel.dovgaluk@ispras.ru \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=thuth@redhat.com \
--cc=wainersm@redhat.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).