[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 16/22] python/machine: Handle QMP errors on close more meticulousl
From: |
John Snow |
Subject: |
[PULL 16/22] python/machine: Handle QMP errors on close more meticulously |
Date: |
Mon, 1 Nov 2021 13:30:00 -0400 |
To use the AQMP backend, Machine just needs to be a little more diligent
about what happens when closing a QMP connection. The operation is no
longer a freebie in the async world; it may return errors encountered in
the async bottom half on incoming message receipt, etc.
(AQMP's disconnect, ultimately, serves as the quiescence point where all
async contexts are gathered together, and any final errors reported at
that point.)
Because async QMP continues to check for messages asynchronously, it's
almost certainly likely that the loop will have exited due to EOF after
issuing the last 'quit' command. That error will ultimately be bubbled
up when attempting to close the QMP connection. The manager class here
then is free to discard it -- if it was expected.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 20211026175612.4127598-3-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
python/qemu/machine/machine.py | 48 +++++++++++++++++++++++++++++-----
1 file changed, 42 insertions(+), 6 deletions(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 0bd40bc2f76..a0cf69786b4 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -342,9 +342,15 @@ def _post_shutdown(self) -> None:
# Comprehensive reset for the failed launch case:
self._early_cleanup()
- if self._qmp_connection:
- self._qmp.close()
- self._qmp_connection = None
+ try:
+ self._close_qmp_connection()
+ except Exception as err: # pylint: disable=broad-except
+ LOG.warning(
+ "Exception closing QMP connection: %s",
+ str(err) if str(err) else type(err).__name__
+ )
+ finally:
+ assert self._qmp_connection is None
self._close_qemu_log_file()
@@ -420,6 +426,31 @@ def _launch(self) -> None:
close_fds=False)
self._post_launch()
+ def _close_qmp_connection(self) -> None:
+ """
+ Close the underlying QMP connection, if any.
+
+ Dutifully report errors that occurred while closing, but assume
+ that any error encountered indicates an abnormal termination
+ process and not a failure to close.
+ """
+ if self._qmp_connection is None:
+ return
+
+ try:
+ self._qmp.close()
+ except EOFError:
+ # EOF can occur as an Exception here when using the Async
+ # QMP backend. It indicates that the server closed the
+ # stream. If we successfully issued 'quit' at any point,
+ # then this was expected. If the remote went away without
+ # our permission, it's worth reporting that as an abnormal
+ # shutdown case.
+ if not (self._user_killed or self._quit_issued):
+ raise
+ finally:
+ self._qmp_connection = None
+
def _early_cleanup(self) -> None:
"""
Perform any cleanup that needs to happen before the VM exits.
@@ -460,9 +491,14 @@ def _soft_shutdown(self, timeout: Optional[int]) -> None:
self._early_cleanup()
if self._qmp_connection:
- if not self._quit_issued:
- # Might raise ConnectionReset
- self.qmp('quit')
+ try:
+ if not self._quit_issued:
+ # May raise ExecInterruptedError or StateError if the
+ # connection dies or has *already* died.
+ self.qmp('quit')
+ finally:
+ # Regardless, we want to quiesce the connection.
+ self._close_qmp_connection()
# May raise subprocess.TimeoutExpired
self._subp.wait(timeout=timeout)
--
2.31.1
- [PULL 06/22] iotests/297: Split run_linters apart into run_pylint and run_mypy, (continued)
- [PULL 06/22] iotests/297: Split run_linters apart into run_pylint and run_mypy, John Snow, 2021/11/01
- [PULL 07/22] iotests/297: refactor run_[mypy|pylint] as generic execution shim, John Snow, 2021/11/01
- [PULL 08/22] iotests/297: Change run_linter() to raise an exception on failure, John Snow, 2021/11/01
- [PULL 09/22] iotests/297: update tool availability checks, John Snow, 2021/11/01
- [PULL 10/22] iotests/297: split test into sub-cases, John Snow, 2021/11/01
- [PULL 11/22] iotests: split linters.py out from 297, John Snow, 2021/11/01
- [PULL 12/22] iotests/linters: Add entry point for linting via Python CI, John Snow, 2021/11/01
- [PULL 13/22] iotests/linters: Add workaround for mypy bug #9852, John Snow, 2021/11/01
- [PULL 14/22] python: Add iotest linters to test suite, John Snow, 2021/11/01
- [PULL 15/22] python/machine: remove has_quit argument, John Snow, 2021/11/01
- [PULL 16/22] python/machine: Handle QMP errors on close more meticulously,
John Snow <=
- [PULL 17/22] python/aqmp: Remove scary message, John Snow, 2021/11/01
- [PULL 18/22] iotests: Accommodate async QMP Exception classes, John Snow, 2021/11/01
- [PULL 19/22] iotests: Conditionally silence certain AQMP errors, John Snow, 2021/11/01
- [PULL 20/22] iotests/300: avoid abnormal shutdown race condition, John Snow, 2021/11/01
- [PULL 22/22] python, iotests: replace qmp with aqmp, John Snow, 2021/11/01
- [PULL 21/22] python/aqmp: Create sync QMP wrapper for iotests, John Snow, 2021/11/01
- Re: [PULL 00/22] Python patches, Richard Henderson, 2021/11/02