[PATCH 00/14] python: misc cleanups for python code

[PATCH 00/14] python: misc cleanups for python code

[Daniel P. Berrangé]

This spun out of understanding why we didn't get prior warning
about our use of the deprecated 'asyncio.get_event_loop' feature.
The answer is that python doesn't print any deprecation messages
by default unless directly triggered by the __main__ module.

So here I wanted to get all python warning messages enabled for
the iotests and functional tests, so we get a heads up of any
other failures coming out way.

In doing this it uncovered numerous places where we failed to
cleanup with dangling file handles.

I further uncovered lots of old compat code for py3.6 which
we stopped needing a long while ago.

I'm in two minds over whether the very last commit is suitable
or not though.  The iotests do strict comparison of stdout /
stderr, so if we allow python warnings to appear that will
break the iotests. At the same time though we do really want
to see these warnings, especially when developers run the
iotests manually, so just turning them on in CI is not very
desirable. So wondering how to get the warnings for devs but
not for users running from released tarballs?

Also note that I ran 'check', 'check -qcow' and 'check -nbd'
but this doesn't cover all possible iotest usage. So there
might still be some resource leaks hiding in there which will
cause failures for devs running tests with unusual config
scenarios.


Daniel P. Berrangé (13):
  python: avoid deprecation warning with get_event_loop
  python: drop 'is_closing' back compat helper
  python: drop 'wait_closed' back compat helper
  python: drop 'asyncio_run' back compat helper
  python: drop 'create_task' back compat helper
  iotests: drop compat for old version context manager
  functional: ensure log handlers are closed
  functional: ensure sockets and files are closed
  functional: always enable all python warnings
  python: ensure QEMUQtestProtocol closes its socket
  iotests/147: ensure temporary sockets are closed before exiting
  iotests/151: ensure subprocesses are cleaned up
  iotests/check: always enable all python warnings

Richard W.M. Jones (1):
  python: Replace asyncio.get_event_loop for Python 3.14

 python/qemu/machine/qtest.py           |  2 +
 python/qemu/qmp/legacy.py              | 10 ++-
 python/qemu/qmp/protocol.py            | 13 ++--
 python/qemu/qmp/qmp_tui.py             | 10 +--
 python/qemu/qmp/util.py                | 97 +-------------------------
 python/tests/protocol.py               |  9 ++-
 tests/functional/qemu_test/testcase.py |  6 ++
 tests/functional/test_multiprocess.py  |  3 +
 tests/functional/test_virtio_gpu.py    |  2 +
 tests/qemu-iotests/147                 |  1 +
 tests/qemu-iotests/151                 |  5 ++
 tests/qemu-iotests/check               |  4 ++
 tests/qemu-iotests/testenv.py          |  7 +-
 tests/qemu-iotests/testrunner.py       |  9 +--
 14 files changed, 51 insertions(+), 127 deletions(-)

-- 
2.49.0

[PATCH 01/14] python: Replace asyncio.get_event_loop for Python 3.14

[Daniel P. Berrangé]

From: Richard W.M. Jones <rjones@redhat.com>

In Python 3.14, no asyncio event loop gets generated automatically.
Instead create one when we need it.  This should work with Python 3.13
as well.  This change was suggested here:

https://bugzilla.redhat.com/show_bug.cgi?id=2375004#c4

See-also: https://docs.python.org/3.14/whatsnew/3.14.html#id7
Thanks: Miro Hrončok, Daniel P. Berrangé
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
---
 python/qemu/qmp/legacy.py  | 5 ++++-
 python/qemu/qmp/qmp_tui.py | 2 +-
 python/tests/protocol.py   | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
index 22a2b5616e..e11d05afbd 100644
--- a/python/qemu/qmp/legacy.py
+++ b/python/qemu/qmp/legacy.py
@@ -86,7 +86,10 @@ def __init__(self,
                 "server argument should be False when passing a socket")
 
         self._qmp = QMPClient(nickname)
-        self._aloop = asyncio.get_event_loop()
+        try:
+            self._aloop = asyncio.get_event_loop()
+        except RuntimeError:
+            self._aloop = asyncio.new_event_loop()
         self._address = address
         self._timeout: Optional[float] = None
 
diff --git a/python/qemu/qmp/qmp_tui.py b/python/qemu/qmp/qmp_tui.py
index 2d9ebbd20b..7dfb03c9ad 100644
--- a/python/qemu/qmp/qmp_tui.py
+++ b/python/qemu/qmp/qmp_tui.py
@@ -377,7 +377,7 @@ def run(self, debug: bool = False) -> None:
         screen = urwid.raw_display.Screen()
         screen.set_terminal_properties(256)
 
-        self.aloop = asyncio.get_event_loop()
+        self.aloop = asyncio.new_event_loop()
         self.aloop.set_debug(debug)
 
         # Gracefully handle SIGTERM and SIGINT signals
diff --git a/python/tests/protocol.py b/python/tests/protocol.py
index 56c4d441f9..db5d54d83f 100644
--- a/python/tests/protocol.py
+++ b/python/tests/protocol.py
@@ -228,7 +228,7 @@ def async_test(async_test_method):
         Decorator; adds SetUp and TearDown to async tests.
         """
         async def _wrapper(self, *args, **kwargs):
-            loop = asyncio.get_event_loop()
+            loop = asyncio.new_event_loop()
             loop.set_debug(True)
 
             await self._asyncSetUp()
-- 
2.49.0

[PATCH 02/14] python: avoid deprecation warning with get_event_loop

[Daniel P. Berrangé]

We need to call get_event_loop but have no way of knowing ahead
of time whether the current thread has an event loop of not. We
can handle a missing event loop, but we need to hide the warning
python will emit to avoid tripping up iotests expected output.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 python/qemu/qmp/legacy.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
index e11d05afbd..c6ab3edc86 100644
--- a/python/qemu/qmp/legacy.py
+++ b/python/qemu/qmp/legacy.py
@@ -34,6 +34,7 @@
     TypeVar,
     Union,
 )
+import warnings
 
 from .error import QMPError
 from .protocol import Runstate, SocketAddrT
@@ -87,7 +88,11 @@ def __init__(self,
 
         self._qmp = QMPClient(nickname)
         try:
-            self._aloop = asyncio.get_event_loop()
+            with warnings.catch_warnings():
+                # Python <= 3.13 will trigger deprecation warnings
+                # if no event loop is set
+                warnings.simplefilter("ignore")
+                self._aloop = asyncio.get_event_loop()
         except RuntimeError:
             self._aloop = asyncio.new_event_loop()
         self._address = address
-- 
2.49.0

[PATCH 03/14] python: drop 'is_closing' back compat helper

[Daniel P. Berrangé]

Our minimum python is now 3.9, so back compat with python
3.6 is no longer required.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 python/qemu/qmp/protocol.py |  3 +--
 python/qemu/qmp/util.py     | 16 ----------------
 2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py
index a4ffdfad51..9a7ada4a1e 100644
--- a/python/qemu/qmp/protocol.py
+++ b/python/qemu/qmp/protocol.py
@@ -39,7 +39,6 @@
     create_task,
     exception_summary,
     flush,
-    is_closing,
     pretty_traceback,
     upper_half,
     wait_closed,
@@ -825,7 +824,7 @@ async def _bh_close_stream(self, error_pathway: bool = False) -> None:
         if not self._writer:
             return
 
-        if not is_closing(self._writer):
+        if not self._writer.is_closing():
             self.logger.debug("Closing StreamWriter.")
             self._writer.close()
 
diff --git a/python/qemu/qmp/util.py b/python/qemu/qmp/util.py
index ca6225e9cd..39fc341f2f 100644
--- a/python/qemu/qmp/util.py
+++ b/python/qemu/qmp/util.py
@@ -104,22 +104,6 @@ def create_task(coro: Coroutine[Any, Any, T],
     return asyncio.ensure_future(coro, loop=loop)
 
 
-def is_closing(writer: asyncio.StreamWriter) -> bool:
-    """
-    Python 3.6-compatible `asyncio.StreamWriter.is_closing` wrapper.
-
-    :param writer: The `asyncio.StreamWriter` object.
-    :return: `True` if the writer is closing, or closed.
-    """
-    if sys.version_info >= (3, 7):
-        return writer.is_closing()
-
-    # Python 3.6:
-    transport = writer.transport
-    assert isinstance(transport, asyncio.WriteTransport)
-    return transport.is_closing()
-
-
 async def wait_closed(writer: asyncio.StreamWriter) -> None:
     """
     Python 3.6-compatible `asyncio.StreamWriter.wait_closed` wrapper.
-- 
2.49.0

[PATCH 04/14] python: drop 'wait_closed' back compat helper

[Daniel P. Berrangé]

Our minimum python is now 3.9, so back compat with python
3.6 is no longer required.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 python/qemu/qmp/protocol.py |  3 +--
 python/qemu/qmp/util.py     | 29 -----------------------------
 2 files changed, 1 insertion(+), 31 deletions(-)

diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py
index 9a7ada4a1e..deb6b20d29 100644
--- a/python/qemu/qmp/protocol.py
+++ b/python/qemu/qmp/protocol.py
@@ -41,7 +41,6 @@
     flush,
     pretty_traceback,
     upper_half,
-    wait_closed,
 )
 
 
@@ -830,7 +829,7 @@ async def _bh_close_stream(self, error_pathway: bool = False) -> None:
 
         self.logger.debug("Waiting for StreamWriter to close ...")
         try:
-            await wait_closed(self._writer)
+            await self._writer.wait_closed()
         except Exception:  # pylint: disable=broad-except
             # It's hard to tell if the Stream is already closed or
             # not. Even if one of the tasks has failed, it may have
diff --git a/python/qemu/qmp/util.py b/python/qemu/qmp/util.py
index 39fc341f2f..b5e9750576 100644
--- a/python/qemu/qmp/util.py
+++ b/python/qemu/qmp/util.py
@@ -104,35 +104,6 @@ def create_task(coro: Coroutine[Any, Any, T],
     return asyncio.ensure_future(coro, loop=loop)
 
 
-async def wait_closed(writer: asyncio.StreamWriter) -> None:
-    """
-    Python 3.6-compatible `asyncio.StreamWriter.wait_closed` wrapper.
-
-    :param writer: The `asyncio.StreamWriter` to wait on.
-    """
-    if sys.version_info >= (3, 7):
-        await writer.wait_closed()
-        return
-
-    # Python 3.6
-    transport = writer.transport
-    assert isinstance(transport, asyncio.WriteTransport)
-
-    while not transport.is_closing():
-        await asyncio.sleep(0)
-
-    # This is an ugly workaround, but it's the best I can come up with.
-    sock = transport.get_extra_info('socket')
-
-    if sock is None:
-        # Our transport doesn't have a socket? ...
-        # Nothing we can reasonably do.
-        return
-
-    while sock.fileno() != -1:
-        await asyncio.sleep(0)
-
-
 def asyncio_run(coro: Coroutine[Any, Any, T], *, debug: bool = False) -> T:
     """
     Python 3.6-compatible `asyncio.run` wrapper.
-- 
2.49.0

[PATCH 05/14] python: drop 'asyncio_run' back compat helper

[Daniel P. Berrangé]

Our minimum python is now 3.9, so back compat with python
3.6 is no longer required.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 python/qemu/qmp/util.py  | 19 -------------------
 python/tests/protocol.py |  2 +-
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/python/qemu/qmp/util.py b/python/qemu/qmp/util.py
index b5e9750576..7f9e718154 100644
--- a/python/qemu/qmp/util.py
+++ b/python/qemu/qmp/util.py
@@ -104,25 +104,6 @@ def create_task(coro: Coroutine[Any, Any, T],
     return asyncio.ensure_future(coro, loop=loop)
 
 
-def asyncio_run(coro: Coroutine[Any, Any, T], *, debug: bool = False) -> T:
-    """
-    Python 3.6-compatible `asyncio.run` wrapper.
-
-    :param coro: A coroutine to execute now.
-    :return: The return value from the coroutine.
-    """
-    if sys.version_info >= (3, 7):
-        return asyncio.run(coro, debug=debug)
-
-    # Python 3.6
-    loop = asyncio.get_event_loop()
-    loop.set_debug(debug)
-    ret = loop.run_until_complete(coro)
-    loop.close()
-
-    return ret
-
-
 # ----------------------------
 # Section: Logging & Debugging
 # ----------------------------
diff --git a/python/tests/protocol.py b/python/tests/protocol.py
index db5d54d83f..4a0ee94727 100644
--- a/python/tests/protocol.py
+++ b/python/tests/protocol.py
@@ -8,7 +8,7 @@
 
 from qemu.qmp import ConnectError, Runstate
 from qemu.qmp.protocol import AsyncProtocol, StateError
-from qemu.qmp.util import asyncio_run, create_task
+from qemu.qmp.util import create_task
 
 
 class NullProtocol(AsyncProtocol[None]):
-- 
2.49.0

[PATCH 06/14] python: drop 'create_task' back compat helper

[Daniel P. Berrangé]

Our minimum python is now 3.9, so back compat with python
3.6 is no longer required.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 python/qemu/qmp/protocol.py |  7 +++----
 python/qemu/qmp/qmp_tui.py  |  8 ++++----
 python/qemu/qmp/util.py     | 33 +--------------------------------
 python/tests/protocol.py    |  7 +++----
 4 files changed, 11 insertions(+), 44 deletions(-)

diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py
index deb6b20d29..4aff0ea423 100644
--- a/python/qemu/qmp/protocol.py
+++ b/python/qemu/qmp/protocol.py
@@ -36,7 +36,6 @@
 from .error import QMPError
 from .util import (
     bottom_half,
-    create_task,
     exception_summary,
     flush,
     pretty_traceback,
@@ -661,8 +660,8 @@ async def _establish_session(self) -> None:
         reader_coro = self._bh_loop_forever(self._bh_recv_message, 'Reader')
         writer_coro = self._bh_loop_forever(self._bh_send_message, 'Writer')
 
-        self._reader_task = create_task(reader_coro)
-        self._writer_task = create_task(writer_coro)
+        self._reader_task = asyncio.create_task(reader_coro)
+        self._writer_task = asyncio.create_task(writer_coro)
 
         self._bh_tasks = asyncio.gather(
             self._reader_task,
@@ -687,7 +686,7 @@ def _schedule_disconnect(self) -> None:
         if not self._dc_task:
             self._set_state(Runstate.DISCONNECTING)
             self.logger.debug("Scheduling disconnect.")
-            self._dc_task = create_task(self._bh_disconnect())
+            self._dc_task = asyncio.create_task(self._bh_disconnect())
 
     @upper_half
     async def _wait_disconnect(self) -> None:
diff --git a/python/qemu/qmp/qmp_tui.py b/python/qemu/qmp/qmp_tui.py
index 7dfb03c9ad..61e8b3773c 100644
--- a/python/qemu/qmp/qmp_tui.py
+++ b/python/qemu/qmp/qmp_tui.py
@@ -40,7 +40,7 @@
 from .message import DeserializationError, Message, UnexpectedTypeError
 from .protocol import ConnectError, Runstate
 from .qmp_client import ExecInterruptedError, QMPClient
-from .util import create_task, pretty_traceback
+from .util import pretty_traceback
 
 
 # The name of the signal that is used to update the history list
@@ -225,7 +225,7 @@ def cb_send_to_server(self, raw_msg: str) -> None:
         """
         try:
             msg = Message(bytes(raw_msg, encoding='utf-8'))
-            create_task(self._send_to_server(msg))
+            asyncio.create_task(self._send_to_server(msg))
         except (DeserializationError, UnexpectedTypeError) as err:
             raw_msg = format_json(raw_msg)
             logging.info('Invalid message: %s', err.error_message)
@@ -246,7 +246,7 @@ def kill_app(self) -> None:
         Initiates killing of app. A bridge between asynchronous and synchronous
         code.
         """
-        create_task(self._kill_app())
+        asyncio.create_task(self._kill_app())
 
     async def _kill_app(self) -> None:
         """
@@ -393,7 +393,7 @@ def run(self, debug: bool = False) -> None:
                                    handle_mouse=True,
                                    event_loop=event_loop)
 
-        create_task(self.manage_connection(), self.aloop)
+        self.aloop.create_task(self.manage_connection())
         try:
             main_loop.run()
         except Exception as err:
diff --git a/python/qemu/qmp/util.py b/python/qemu/qmp/util.py
index 7f9e718154..c44a5aacbc 100644
--- a/python/qemu/qmp/util.py
+++ b/python/qemu/qmp/util.py
@@ -13,13 +13,7 @@
 import asyncio
 import sys
 import traceback
-from typing import (
-    Any,
-    Coroutine,
-    Optional,
-    TypeVar,
-    cast,
-)
+from typing import TypeVar, cast
 
 
 T = TypeVar('T')
@@ -79,31 +73,6 @@ def bottom_half(func: T) -> T:
     return func
 
 
-# -------------------------------
-# Section: Compatibility Wrappers
-# -------------------------------
-
-
-def create_task(coro: Coroutine[Any, Any, T],
-                loop: Optional[asyncio.AbstractEventLoop] = None
-                ) -> 'asyncio.Future[T]':
-    """
-    Python 3.6-compatible `asyncio.create_task` wrapper.
-
-    :param coro: The coroutine to execute in a task.
-    :param loop: Optionally, the loop to create the task in.
-
-    :return: An `asyncio.Future` object.
-    """
-    if sys.version_info >= (3, 7):
-        if loop is not None:
-            return loop.create_task(coro)
-        return asyncio.create_task(coro)  # pylint: disable=no-member
-
-    # Python 3.6:
-    return asyncio.ensure_future(coro, loop=loop)
-
-
 # ----------------------------
 # Section: Logging & Debugging
 # ----------------------------
diff --git a/python/tests/protocol.py b/python/tests/protocol.py
index 4a0ee94727..9bb23b6a7b 100644
--- a/python/tests/protocol.py
+++ b/python/tests/protocol.py
@@ -8,7 +8,6 @@
 
 from qemu.qmp import ConnectError, Runstate
 from qemu.qmp.protocol import AsyncProtocol, StateError
-from qemu.qmp.util import create_task
 
 
 class NullProtocol(AsyncProtocol[None]):
@@ -124,7 +123,7 @@ async def _runner():
             if allow_cancellation:
                 return
             raise
-    return create_task(_runner())
+    return asyncio.create_task(_runner())
 
 
 @contextmanager
@@ -271,7 +270,7 @@ async def _watcher():
                     msg=f"Expected state '{state.name}'",
                 )
 
-        self.runstate_watcher = create_task(_watcher())
+        self.runstate_watcher = asyncio.create_task(_watcher())
         # Kick the loop and force the task to block on the event.
         await asyncio.sleep(0)
 
@@ -589,7 +588,7 @@ async def _asyncTearDown(self):
     async def testSmoke(self):
         with TemporaryDirectory(suffix='.qmp') as tmpdir:
             sock = os.path.join(tmpdir, type(self.proto).__name__ + ".sock")
-            server_task = create_task(self.server.start_server_and_accept(sock))
+            server_task = asyncio.create_task(self.server.start_server_and_accept(sock))
 
             # give the server a chance to start listening [...]
             await asyncio.sleep(0)
-- 
2.49.0

[PATCH 07/14] iotests: drop compat for old version context manager

[Daniel P. Berrangé]

Our minimum python is now 3.9, so back compat with prior
python versions is no longer required.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/testenv.py    | 7 ++-----
 tests/qemu-iotests/testrunner.py | 9 ++-------
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 6326e46b7b..29caaa8a34 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -22,15 +22,12 @@
 from pathlib import Path
 import shutil
 import collections
+import contextlib
 import random
 import subprocess
 import glob
 from typing import List, Dict, Any, Optional
 
-if sys.version_info >= (3, 9):
-    from contextlib import AbstractContextManager as ContextManager
-else:
-    from typing import ContextManager
 
 DEF_GDB_OPTIONS = 'localhost:12345'
 
@@ -58,7 +55,7 @@ def get_default_machine(qemu_prog: str) -> str:
     return default_machine
 
 
-class TestEnv(ContextManager['TestEnv']):
+class TestEnv(contextlib.AbstractContextManager['TestEnv']):
     """
     Manage system environment for running tests
 
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 2e236c8fa3..14cc8492f9 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -30,11 +30,6 @@
 from typing import List, Optional, Any, Sequence, Dict
 from testenv import TestEnv
 
-if sys.version_info >= (3, 9):
-    from contextlib import AbstractContextManager as ContextManager
-else:
-    from typing import ContextManager
-
 
 def silent_unlink(path: Path) -> None:
     try:
@@ -57,7 +52,7 @@ def file_diff(file1: str, file2: str) -> List[str]:
         return res
 
 
-class LastElapsedTime(ContextManager['LastElapsedTime']):
+class LastElapsedTime(contextlib.AbstractContextManager['LastElapsedTime']):
     """ Cache for elapsed time for tests, to show it during new test run
 
     It is safe to use get() at any time.  To use update(), you must either
@@ -112,7 +107,7 @@ def __init__(self, status: str, description: str = '',
         self.interrupted = interrupted
 
 
-class TestRunner(ContextManager['TestRunner']):
+class TestRunner(contextlib.AbstractContextManager['TestRunner']):
     shared_self = None
 
     @staticmethod
-- 
2.49.0

[PATCH 08/14] functional: ensure log handlers are closed

[Daniel P. Berrangé]

This avoids a resource leak warning from python when the
log handler is garbage collected.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/testcase.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index 2082c6fce4..71c7160adc 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -232,6 +232,7 @@ def tearDown(self):
             self.socketdir = None
         self.machinelog.removeHandler(self._log_fh)
         self.log.removeHandler(self._log_fh)
+        self._log_fh.close()
 
     def main():
         path = os.path.basename(sys.argv[0])[:-3]
@@ -399,4 +400,5 @@ def tearDown(self):
         for vm in self._vms.values():
             vm.shutdown()
         logging.getLogger('console').removeHandler(self._console_log_fh)
+        self._console_log_fh.close()
         super().tearDown()
-- 
2.49.0

[PATCH 09/14] functional: ensure sockets and files are closed

[Daniel P. Berrangé]

The multiprocess and virtio_gpu tests open sockets but then forget
to close them, which triggers resource leak warnings

The virtio_gpu test also fails to close a log file it opens.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/test_multiprocess.py | 3 +++
 tests/functional/test_virtio_gpu.py   | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/tests/functional/test_multiprocess.py b/tests/functional/test_multiprocess.py
index 751cf10e63..92d5207b0e 100755
--- a/tests/functional/test_multiprocess.py
+++ b/tests/functional/test_multiprocess.py
@@ -83,6 +83,9 @@ def do_test(self, kernel_asset, initrd_asset,
                                           'cat /sys/bus/pci/devices/*/uevent',
                                           'PCI_ID=1000:0012')
 
+        proxy_sock.close()
+        remote_sock.close()
+
     def test_multiprocess(self):
         kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
         if self.arch == 'x86_64':
diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
index 81c9156d63..be96de24da 100755
--- a/tests/functional/test_virtio_gpu.py
+++ b/tests/functional/test_virtio_gpu.py
@@ -108,6 +108,7 @@ def test_vhost_user_vga_virgl(self):
             shell=False,
             close_fds=False,
         )
+        self._vug_log_file.close()
 
         self.vm.set_console()
         self.vm.add_args("-cpu", "host")
@@ -135,6 +136,7 @@ def test_vhost_user_vga_virgl(self):
                                           "features: +virgl +edid")
         self.vm.shutdown()
         qemu_sock.close()
+        vug_sock.close()
         vugp.terminate()
         vugp.wait()
 
-- 
2.49.0

[PATCH 10/14] functional: always enable all python warnings

[Daniel P. Berrangé]

Of most importance is that this gives us a heads-up if anything
we rely on has been deprecated. The default python behaviour
only emits a warning if triggered from __main__ which is very
limited.

Setting the env variable further ensures that any python child
processes will also display warnings.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/testcase.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index 71c7160adc..2a78e735f1 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -19,6 +19,7 @@
 from subprocess import run
 import sys
 import tempfile
+import warnings
 import unittest
 import uuid
 
@@ -235,6 +236,9 @@ def tearDown(self):
         self._log_fh.close()
 
     def main():
+        warnings.simplefilter("default")
+        os.environ["PYTHONWARNINGS"] = "default"
+
         path = os.path.basename(sys.argv[0])[:-3]
 
         cache = os.environ.get("QEMU_TEST_PRECACHE", None)
-- 
2.49.0

[PATCH 11/14] python: ensure QEMUQtestProtocol closes its socket

[Daniel P. Berrangé]

While QEMUQtestMachine closes the socket that was passed to
QEMUQtestProtocol, the python resource leak manager still
believes that the copy QEMUQtestProtocol holds is open. We
must explicitly call close to avoid this leak warnnig.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 python/qemu/machine/qtest.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
index 4f5ede85b2..781f674ffa 100644
--- a/python/qemu/machine/qtest.py
+++ b/python/qemu/machine/qtest.py
@@ -177,6 +177,8 @@ def _post_shutdown(self) -> None:
             self._qtest_sock_pair[0].close()
             self._qtest_sock_pair[1].close()
             self._qtest_sock_pair = None
+        if self._qtest is not None:
+            self._qtest.close()
         super()._post_shutdown()
 
     def qtest(self, cmd: str) -> str:
-- 
2.49.0

[PATCH 12/14] iotests/147: ensure temporary sockets are closed before exiting

[Daniel P. Berrangé]

This avoids the python resource leak detector from issuing warnings
in the iotests.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/147 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index 6d6f077a14..3e14bd389a 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -277,6 +277,7 @@ class BuiltinNBD(NBDBlockdevAddBase):
                      } }
         self.client_test(filename, flatten_sock_addr(address), 'nbd-export')
 
+        sockfd.close()
         self._server_down()
 
 
-- 
2.49.0

[PATCH 13/14] iotests/151: ensure subprocesses are cleaned up

[Daniel P. Berrangé]

The iotest 151 creates a bunch of subprocesses, with their stdout
connected to a pipe but never reads any data from them and does
not gurantee the processes are killed on cleanup.

This triggers resource leak warnings from python when the
subprocess.Popen object is garbage collected.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/151 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/qemu-iotests/151 b/tests/qemu-iotests/151
index f2ff9c5dac..06ee3585db 100755
--- a/tests/qemu-iotests/151
+++ b/tests/qemu-iotests/151
@@ -263,6 +263,11 @@ class TestThrottledWithNbdExportBase(iotests.QMPTestCase):
                         break
                     except subprocess.TimeoutExpired:
                         self.vm.qtest(f'clock_step {1 * 1000 * 1000 * 1000}')
+                try:
+                    p.kill()
+                    p.stdout.close()
+                except:
+                    pass
         except IndexError:
             pass
 
-- 
2.49.0

[PATCH 14/14] iotests/check: always enable all python warnings

[Daniel P. Berrangé]

Of most importance is that this gives us a heads-up if anything
we rely on has been deprecated. The default python behaviour
only emits a warning if triggered from __main__ which is very
limited.

Setting the env variable further ensures that any python child
processes will also display warnings.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/check | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 545f9ec7bd..d9b7c1d598 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -21,6 +21,7 @@ import sys
 import argparse
 import shutil
 from pathlib import Path
+import warnings
 
 from findtests import TestFinder
 from testenv import TestEnv
@@ -137,6 +138,9 @@ def make_argparser() -> argparse.ArgumentParser:
 
 
 if __name__ == '__main__':
+    warnings.simplefilter("default")
+    os.environ["PYTHONWARNINGS"] = "default"
+
     args = make_argparser().parse_args()
 
     env = TestEnv(source_dir=args.source_dir,
-- 
2.49.0

Re: [PATCH 08/14] functional: ensure log handlers are closed

[Thomas Huth]

On 15/07/2025 16.30, Daniel P. Berrangé wrote:
> This avoids a resource leak warning from python when the
> log handler is garbage collected.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/testcase.py | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> index 2082c6fce4..71c7160adc 100644
> --- a/tests/functional/qemu_test/testcase.py
> +++ b/tests/functional/qemu_test/testcase.py
> @@ -232,6 +232,7 @@ def tearDown(self):
>               self.socketdir = None
>           self.machinelog.removeHandler(self._log_fh)
>           self.log.removeHandler(self._log_fh)
> +        self._log_fh.close()
>   
>       def main():
>           path = os.path.basename(sys.argv[0])[:-3]
> @@ -399,4 +400,5 @@ def tearDown(self):
>           for vm in self._vms.values():
>               vm.shutdown()
>           logging.getLogger('console').removeHandler(self._console_log_fh)
> +        self._console_log_fh.close()
>           super().tearDown()

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 09/14] functional: ensure sockets and files are closed

[Thomas Huth]

On 15/07/2025 16.30, Daniel P. Berrangé wrote:
> The multiprocess and virtio_gpu tests open sockets but then forget
> to close them, which triggers resource leak warnings
> 
> The virtio_gpu test also fails to close a log file it opens.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/test_multiprocess.py | 3 +++
>   tests/functional/test_virtio_gpu.py   | 2 ++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/tests/functional/test_multiprocess.py b/tests/functional/test_multiprocess.py
> index 751cf10e63..92d5207b0e 100755
> --- a/tests/functional/test_multiprocess.py
> +++ b/tests/functional/test_multiprocess.py
> @@ -83,6 +83,9 @@ def do_test(self, kernel_asset, initrd_asset,
>                                             'cat /sys/bus/pci/devices/*/uevent',
>                                             'PCI_ID=1000:0012')
>   
> +        proxy_sock.close()
> +        remote_sock.close()
> +
>       def test_multiprocess(self):
>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
>           if self.arch == 'x86_64':
> diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
> index 81c9156d63..be96de24da 100755
> --- a/tests/functional/test_virtio_gpu.py
> +++ b/tests/functional/test_virtio_gpu.py
> @@ -108,6 +108,7 @@ def test_vhost_user_vga_virgl(self):
>               shell=False,
>               close_fds=False,
>           )
> +        self._vug_log_file.close()

Maybe cleaner to close it at the end of the function?

  Thomas


>           self.vm.set_console()
>           self.vm.add_args("-cpu", "host")
> @@ -135,6 +136,7 @@ def test_vhost_user_vga_virgl(self):
>                                             "features: +virgl +edid")
>           self.vm.shutdown()
>           qemu_sock.close()
> +        vug_sock.close()
>           vugp.terminate()
>           vugp.wait()
>   

Re: [PATCH 09/14] functional: ensure sockets and files are closed

[Daniel P. Berrangé]

On Tue, Jul 15, 2025 at 05:03:07PM +0200, Thomas Huth wrote:
> On 15/07/2025 16.30, Daniel P. Berrangé wrote:
> > The multiprocess and virtio_gpu tests open sockets but then forget
> > to close them, which triggers resource leak warnings
> > 
> > The virtio_gpu test also fails to close a log file it opens.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   tests/functional/test_multiprocess.py | 3 +++
> >   tests/functional/test_virtio_gpu.py   | 2 ++
> >   2 files changed, 5 insertions(+)
> > 
> > diff --git a/tests/functional/test_multiprocess.py b/tests/functional/test_multiprocess.py
> > index 751cf10e63..92d5207b0e 100755
> > --- a/tests/functional/test_multiprocess.py
> > +++ b/tests/functional/test_multiprocess.py
> > @@ -83,6 +83,9 @@ def do_test(self, kernel_asset, initrd_asset,
> >                                             'cat /sys/bus/pci/devices/*/uevent',
> >                                             'PCI_ID=1000:0012')
> > +        proxy_sock.close()
> > +        remote_sock.close()
> > +
> >       def test_multiprocess(self):
> >           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
> >           if self.arch == 'x86_64':
> > diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
> > index 81c9156d63..be96de24da 100755
> > --- a/tests/functional/test_virtio_gpu.py
> > +++ b/tests/functional/test_virtio_gpu.py
> > @@ -108,6 +108,7 @@ def test_vhost_user_vga_virgl(self):
> >               shell=False,
> >               close_fds=False,
> >           )
> > +        self._vug_log_file.close()
> 
> Maybe cleaner to close it at the end of the function?

This was the last place it was needed - just out of sight in the context
of the diff the file is passed to Popen() which connects it to stderr.
So nothing in this test may access the file after this point to avoid
concurent overlapping writes to the same file.

> 
>  Thomas
> 
> 
> >           self.vm.set_console()
> >           self.vm.add_args("-cpu", "host")
> > @@ -135,6 +136,7 @@ def test_vhost_user_vga_virgl(self):
> >                                             "features: +virgl +edid")
> >           self.vm.shutdown()
> >           qemu_sock.close()
> > +        vug_sock.close()
> >           vugp.terminate()
> >           vugp.wait()
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 09/14] functional: ensure sockets and files are closed

[Thomas Huth]

On 15/07/2025 17.12, Daniel P. Berrangé wrote:
> On Tue, Jul 15, 2025 at 05:03:07PM +0200, Thomas Huth wrote:
>> On 15/07/2025 16.30, Daniel P. Berrangé wrote:
>>> The multiprocess and virtio_gpu tests open sockets but then forget
>>> to close them, which triggers resource leak warnings
>>>
>>> The virtio_gpu test also fails to close a log file it opens.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> ---
>>>    tests/functional/test_multiprocess.py | 3 +++
>>>    tests/functional/test_virtio_gpu.py   | 2 ++
>>>    2 files changed, 5 insertions(+)
>>>
>>> diff --git a/tests/functional/test_multiprocess.py b/tests/functional/test_multiprocess.py
>>> index 751cf10e63..92d5207b0e 100755
>>> --- a/tests/functional/test_multiprocess.py
>>> +++ b/tests/functional/test_multiprocess.py
>>> @@ -83,6 +83,9 @@ def do_test(self, kernel_asset, initrd_asset,
>>>                                              'cat /sys/bus/pci/devices/*/uevent',
>>>                                              'PCI_ID=1000:0012')
>>> +        proxy_sock.close()
>>> +        remote_sock.close()
>>> +
>>>        def test_multiprocess(self):
>>>            kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
>>>            if self.arch == 'x86_64':
>>> diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
>>> index 81c9156d63..be96de24da 100755
>>> --- a/tests/functional/test_virtio_gpu.py
>>> +++ b/tests/functional/test_virtio_gpu.py
>>> @@ -108,6 +108,7 @@ def test_vhost_user_vga_virgl(self):
>>>                shell=False,
>>>                close_fds=False,
>>>            )
>>> +        self._vug_log_file.close()
>>
>> Maybe cleaner to close it at the end of the function?
> 
> This was the last place it was needed - just out of sight in the context
> of the diff the file is passed to Popen() which connects it to stderr.
> So nothing in this test may access the file after this point to avoid
> concurent overlapping writes to the same file.

Ok, makes sense now.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 10/14] functional: always enable all python warnings

[Thomas Huth]

On 15/07/2025 16.30, Daniel P. Berrangé wrote:
> Of most importance is that this gives us a heads-up if anything
> we rely on has been deprecated. The default python behaviour
> only emits a warning if triggered from __main__ which is very
> limited.
> 
> Setting the env variable further ensures that any python child
> processes will also display warnings.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/testcase.py | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> index 71c7160adc..2a78e735f1 100644
> --- a/tests/functional/qemu_test/testcase.py
> +++ b/tests/functional/qemu_test/testcase.py
> @@ -19,6 +19,7 @@
>   from subprocess import run
>   import sys
>   import tempfile
> +import warnings
>   import unittest
>   import uuid
>   
> @@ -235,6 +236,9 @@ def tearDown(self):
>           self._log_fh.close()
>   
>       def main():
> +        warnings.simplefilter("default")
> +        os.environ["PYTHONWARNINGS"] = "default"
> +
>           path = os.path.basename(sys.argv[0])[:-3]
>   
>           cache = os.environ.get("QEMU_TEST_PRECACHE", None)

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 02/14] python: avoid deprecation warning with get_event_loop

[Eric Blake]

On Tue, Jul 15, 2025 at 03:30:11PM +0100, Daniel P. Berrangé wrote:
> We need to call get_event_loop but have no way of knowing ahead
> of time whether the current thread has an event loop of not. We

s/of not /or not /

> can handle a missing event loop, but we need to hide the warning
> python will emit to avoid tripping up iotests expected output.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  python/qemu/qmp/legacy.py | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 03/14] python: drop 'is_closing' back compat helper

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> Our minimum python is now 3.9, so back compat with python
> 3.6 is no longer required.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 01/14] python: Replace asyncio.get_event_loop for Python 3.14

[John Snow]

On Tue, Jul 15, 2025 at 10:30 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> From: Richard W.M. Jones <rjones@redhat.com>
>
> In Python 3.14, no asyncio event loop gets generated automatically.
> Instead create one when we need it.  This should work with Python 3.13
> as well.  This change was suggested here:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=2375004#c4
>
> See-also: https://docs.python.org/3.14/whatsnew/3.14.html#id7
> Thanks: Miro Hrončok, Daniel P. Berrangé
> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

We fixed this in the standalone tree, but I hadn't synchronized the
changes over yet. Shall we take your patches first, and then I'll
worry about synchronizing later?

(I desperately need to drop this code from the qemu.git tree, urgh.)

> ---
>  python/qemu/qmp/legacy.py  | 5 ++++-
>  python/qemu/qmp/qmp_tui.py | 2 +-
>  python/tests/protocol.py   | 2 +-
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
> index 22a2b5616e..e11d05afbd 100644
> --- a/python/qemu/qmp/legacy.py
> +++ b/python/qemu/qmp/legacy.py
> @@ -86,7 +86,10 @@ def __init__(self,
>                  "server argument should be False when passing a socket")
>
>          self._qmp = QMPClient(nickname)
> -        self._aloop = asyncio.get_event_loop()
> +        try:
> +            self._aloop = asyncio.get_event_loop()
> +        except RuntimeError:
> +            self._aloop = asyncio.new_event_loop()
>          self._address = address
>          self._timeout: Optional[float] = None
>
> diff --git a/python/qemu/qmp/qmp_tui.py b/python/qemu/qmp/qmp_tui.py
> index 2d9ebbd20b..7dfb03c9ad 100644
> --- a/python/qemu/qmp/qmp_tui.py
> +++ b/python/qemu/qmp/qmp_tui.py
> @@ -377,7 +377,7 @@ def run(self, debug: bool = False) -> None:
>          screen = urwid.raw_display.Screen()
>          screen.set_terminal_properties(256)
>
> -        self.aloop = asyncio.get_event_loop()
> +        self.aloop = asyncio.new_event_loop()
>          self.aloop.set_debug(debug)
>
>          # Gracefully handle SIGTERM and SIGINT signals
> diff --git a/python/tests/protocol.py b/python/tests/protocol.py
> index 56c4d441f9..db5d54d83f 100644
> --- a/python/tests/protocol.py
> +++ b/python/tests/protocol.py
> @@ -228,7 +228,7 @@ def async_test(async_test_method):
>          Decorator; adds SetUp and TearDown to async tests.
>          """
>          async def _wrapper(self, *args, **kwargs):
> -            loop = asyncio.get_event_loop()
> +            loop = asyncio.new_event_loop()
>              loop.set_debug(True)
>
>              await self._asyncSetUp()
> --
> 2.49.0
>

Re: [PATCH 02/14] python: avoid deprecation warning with get_event_loop

[John Snow]

On Wed, Jul 23, 2025 at 5:25 PM Eric Blake <eblake@redhat.com> wrote:
>
> On Tue, Jul 15, 2025 at 03:30:11PM +0100, Daniel P. Berrangé wrote:
> > We need to call get_event_loop but have no way of knowing ahead
> > of time whether the current thread has an event loop of not. We
>
> s/of not /or not /
>
> > can handle a missing event loop, but we need to hide the warning
> > python will emit to avoid tripping up iotests expected output.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

Do you mind if I collect this patch and apply it against the standalone tree?

--js

> > ---
> >  python/qemu/qmp/legacy.py | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.
> Virtualization:  qemu.org | libguestfs.org
>

Re: [PATCH 01/14] python: Replace asyncio.get_event_loop for Python 3.14

[Daniel P. Berrangé]

On Tue, Aug 19, 2025 at 03:47:50PM -0400, John Snow wrote:
> On Tue, Jul 15, 2025 at 10:30 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > From: Richard W.M. Jones <rjones@redhat.com>
> >
> > In Python 3.14, no asyncio event loop gets generated automatically.
> > Instead create one when we need it.  This should work with Python 3.13
> > as well.  This change was suggested here:
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=2375004#c4
> >
> > See-also: https://docs.python.org/3.14/whatsnew/3.14.html#id7
> > Thanks: Miro Hrončok, Daniel P. Berrangé
> > Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> We fixed this in the standalone tree, but I hadn't synchronized the
> changes over yet. Shall we take your patches first, and then I'll
> worry about synchronizing later?

This really needs to be in 10.1.0, because tests crash and
burn under python 3.14 without this - we're applying this
in Fedora builds locally to deal with the brokeness.

> (I desperately need to drop this code from the qemu.git tree, urgh.)

IMHO qemu.git should be considered authoritative right now because
that's what we're actually shipping & thus take priority for merge.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 02/14] python: avoid deprecation warning with get_event_loop

[Daniel P. Berrangé]

On Tue, Aug 19, 2025 at 03:49:30PM -0400, John Snow wrote:
> On Wed, Jul 23, 2025 at 5:25 PM Eric Blake <eblake@redhat.com> wrote:
> >
> > On Tue, Jul 15, 2025 at 03:30:11PM +0100, Daniel P. Berrangé wrote:
> > > We need to call get_event_loop but have no way of knowing ahead
> > > of time whether the current thread has an event loop of not. We
> >
> > s/of not /or not /
> >
> > > can handle a missing event loop, but we need to hide the warning
> > > python will emit to avoid tripping up iotests expected output.
> > >
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> Do you mind if I collect this patch and apply it against the standalone tree?

If it applies cleanly, then go ahead, and likewise for anything
else following in this series.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 03/14] python: drop 'is_closing' back compat helper

[John Snow]

On Tue, Jul 15, 2025 at 10:31 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Our minimum python is now 3.9, so back compat with python
> 3.6 is no longer required.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

Another thing that happened in the standalone tree, but not in
qemu.git. Ditto patches 4, 5, 6.

> ---
>  python/qemu/qmp/protocol.py |  3 +--
>  python/qemu/qmp/util.py     | 16 ----------------
>  2 files changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py
> index a4ffdfad51..9a7ada4a1e 100644
> --- a/python/qemu/qmp/protocol.py
> +++ b/python/qemu/qmp/protocol.py
> @@ -39,7 +39,6 @@
>      create_task,
>      exception_summary,
>      flush,
> -    is_closing,
>      pretty_traceback,
>      upper_half,
>      wait_closed,
> @@ -825,7 +824,7 @@ async def _bh_close_stream(self, error_pathway: bool = False) -> None:
>          if not self._writer:
>              return
>
> -        if not is_closing(self._writer):
> +        if not self._writer.is_closing():
>              self.logger.debug("Closing StreamWriter.")
>              self._writer.close()
>
> diff --git a/python/qemu/qmp/util.py b/python/qemu/qmp/util.py
> index ca6225e9cd..39fc341f2f 100644
> --- a/python/qemu/qmp/util.py
> +++ b/python/qemu/qmp/util.py
> @@ -104,22 +104,6 @@ def create_task(coro: Coroutine[Any, Any, T],
>      return asyncio.ensure_future(coro, loop=loop)
>
>
> -def is_closing(writer: asyncio.StreamWriter) -> bool:
> -    """
> -    Python 3.6-compatible `asyncio.StreamWriter.is_closing` wrapper.
> -
> -    :param writer: The `asyncio.StreamWriter` object.
> -    :return: `True` if the writer is closing, or closed.
> -    """
> -    if sys.version_info >= (3, 7):
> -        return writer.is_closing()
> -
> -    # Python 3.6:
> -    transport = writer.transport
> -    assert isinstance(transport, asyncio.WriteTransport)
> -    return transport.is_closing()
> -
> -
>  async def wait_closed(writer: asyncio.StreamWriter) -> None:
>      """
>      Python 3.6-compatible `asyncio.StreamWriter.wait_closed` wrapper.
> --
> 2.49.0
>

Re: [PATCH 04/14] python: drop 'wait_closed' back compat helper

[John Snow]

On Tue, Jul 15, 2025 at 10:31 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Our minimum python is now 3.9, so back compat with python
> 3.6 is no longer required.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  python/qemu/qmp/protocol.py |  3 +--
>  python/qemu/qmp/util.py     | 29 -----------------------------
>  2 files changed, 1 insertion(+), 31 deletions(-)
>
> diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py
> index 9a7ada4a1e..deb6b20d29 100644
> --- a/python/qemu/qmp/protocol.py
> +++ b/python/qemu/qmp/protocol.py
> @@ -41,7 +41,6 @@
>      flush,
>      pretty_traceback,
>      upper_half,
> -    wait_closed,
>  )
>
>
> @@ -830,7 +829,7 @@ async def _bh_close_stream(self, error_pathway: bool = False) -> None:
>
>          self.logger.debug("Waiting for StreamWriter to close ...")
>          try:
> -            await wait_closed(self._writer)
> +            await self._writer.wait_closed()
>          except Exception:  # pylint: disable=broad-except
>              # It's hard to tell if the Stream is already closed or
>              # not. Even if one of the tasks has failed, it may have
> diff --git a/python/qemu/qmp/util.py b/python/qemu/qmp/util.py
> index 39fc341f2f..b5e9750576 100644
> --- a/python/qemu/qmp/util.py
> +++ b/python/qemu/qmp/util.py
> @@ -104,35 +104,6 @@ def create_task(coro: Coroutine[Any, Any, T],
>      return asyncio.ensure_future(coro, loop=loop)
>
>
> -async def wait_closed(writer: asyncio.StreamWriter) -> None:
> -    """
> -    Python 3.6-compatible `asyncio.StreamWriter.wait_closed` wrapper.
> -
> -    :param writer: The `asyncio.StreamWriter` to wait on.
> -    """
> -    if sys.version_info >= (3, 7):
> -        await writer.wait_closed()
> -        return
> -
> -    # Python 3.6
> -    transport = writer.transport
> -    assert isinstance(transport, asyncio.WriteTransport)
> -
> -    while not transport.is_closing():
> -        await asyncio.sleep(0)
> -
> -    # This is an ugly workaround, but it's the best I can come up with.
> -    sock = transport.get_extra_info('socket')
> -
> -    if sock is None:
> -        # Our transport doesn't have a socket? ...
> -        # Nothing we can reasonably do.
> -        return
> -
> -    while sock.fileno() != -1:
> -        await asyncio.sleep(0)
> -
> -
>  def asyncio_run(coro: Coroutine[Any, Any, T], *, debug: bool = False) -> T:
>      """
>      Python 3.6-compatible `asyncio.run` wrapper.
> --
> 2.49.0
>

Re: [PATCH 05/14] python: drop 'asyncio_run' back compat helper

[John Snow]

On Tue, Jul 15, 2025 at 10:31 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Our minimum python is now 3.9, so back compat with python
> 3.6 is no longer required.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  python/qemu/qmp/util.py  | 19 -------------------
>  python/tests/protocol.py |  2 +-
>  2 files changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/python/qemu/qmp/util.py b/python/qemu/qmp/util.py
> index b5e9750576..7f9e718154 100644
> --- a/python/qemu/qmp/util.py
> +++ b/python/qemu/qmp/util.py
> @@ -104,25 +104,6 @@ def create_task(coro: Coroutine[Any, Any, T],
>      return asyncio.ensure_future(coro, loop=loop)
>
>
> -def asyncio_run(coro: Coroutine[Any, Any, T], *, debug: bool = False) -> T:
> -    """
> -    Python 3.6-compatible `asyncio.run` wrapper.
> -
> -    :param coro: A coroutine to execute now.
> -    :return: The return value from the coroutine.
> -    """
> -    if sys.version_info >= (3, 7):
> -        return asyncio.run(coro, debug=debug)
> -
> -    # Python 3.6
> -    loop = asyncio.get_event_loop()
> -    loop.set_debug(debug)
> -    ret = loop.run_until_complete(coro)
> -    loop.close()
> -
> -    return ret
> -
> -
>  # ----------------------------
>  # Section: Logging & Debugging
>  # ----------------------------
> diff --git a/python/tests/protocol.py b/python/tests/protocol.py
> index db5d54d83f..4a0ee94727 100644
> --- a/python/tests/protocol.py
> +++ b/python/tests/protocol.py
> @@ -8,7 +8,7 @@
>
>  from qemu.qmp import ConnectError, Runstate
>  from qemu.qmp.protocol import AsyncProtocol, StateError
> -from qemu.qmp.util import asyncio_run, create_task
> +from qemu.qmp.util import create_task
>
>
>  class NullProtocol(AsyncProtocol[None]):
> --
> 2.49.0
>

Re: [PATCH 06/14] python: drop 'create_task' back compat helper

[John Snow]

On Tue, Jul 15, 2025 at 10:31 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Our minimum python is now 3.9, so back compat with python
> 3.6 is no longer required.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  python/qemu/qmp/protocol.py |  7 +++----
>  python/qemu/qmp/qmp_tui.py  |  8 ++++----
>  python/qemu/qmp/util.py     | 33 +--------------------------------
>  python/tests/protocol.py    |  7 +++----
>  4 files changed, 11 insertions(+), 44 deletions(-)
>
> diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py
> index deb6b20d29..4aff0ea423 100644
> --- a/python/qemu/qmp/protocol.py
> +++ b/python/qemu/qmp/protocol.py
> @@ -36,7 +36,6 @@
>  from .error import QMPError
>  from .util import (
>      bottom_half,
> -    create_task,
>      exception_summary,
>      flush,
>      pretty_traceback,
> @@ -661,8 +660,8 @@ async def _establish_session(self) -> None:
>          reader_coro = self._bh_loop_forever(self._bh_recv_message, 'Reader')
>          writer_coro = self._bh_loop_forever(self._bh_send_message, 'Writer')
>
> -        self._reader_task = create_task(reader_coro)
> -        self._writer_task = create_task(writer_coro)
> +        self._reader_task = asyncio.create_task(reader_coro)
> +        self._writer_task = asyncio.create_task(writer_coro)
>
>          self._bh_tasks = asyncio.gather(
>              self._reader_task,
> @@ -687,7 +686,7 @@ def _schedule_disconnect(self) -> None:
>          if not self._dc_task:
>              self._set_state(Runstate.DISCONNECTING)
>              self.logger.debug("Scheduling disconnect.")
> -            self._dc_task = create_task(self._bh_disconnect())
> +            self._dc_task = asyncio.create_task(self._bh_disconnect())
>
>      @upper_half
>      async def _wait_disconnect(self) -> None:
> diff --git a/python/qemu/qmp/qmp_tui.py b/python/qemu/qmp/qmp_tui.py
> index 7dfb03c9ad..61e8b3773c 100644
> --- a/python/qemu/qmp/qmp_tui.py
> +++ b/python/qemu/qmp/qmp_tui.py
> @@ -40,7 +40,7 @@
>  from .message import DeserializationError, Message, UnexpectedTypeError
>  from .protocol import ConnectError, Runstate
>  from .qmp_client import ExecInterruptedError, QMPClient
> -from .util import create_task, pretty_traceback
> +from .util import pretty_traceback
>
>
>  # The name of the signal that is used to update the history list
> @@ -225,7 +225,7 @@ def cb_send_to_server(self, raw_msg: str) -> None:
>          """
>          try:
>              msg = Message(bytes(raw_msg, encoding='utf-8'))
> -            create_task(self._send_to_server(msg))
> +            asyncio.create_task(self._send_to_server(msg))
>          except (DeserializationError, UnexpectedTypeError) as err:
>              raw_msg = format_json(raw_msg)
>              logging.info('Invalid message: %s', err.error_message)
> @@ -246,7 +246,7 @@ def kill_app(self) -> None:
>          Initiates killing of app. A bridge between asynchronous and synchronous
>          code.
>          """
> -        create_task(self._kill_app())
> +        asyncio.create_task(self._kill_app())
>
>      async def _kill_app(self) -> None:
>          """
> @@ -393,7 +393,7 @@ def run(self, debug: bool = False) -> None:
>                                     handle_mouse=True,
>                                     event_loop=event_loop)
>
> -        create_task(self.manage_connection(), self.aloop)
> +        self.aloop.create_task(self.manage_connection())
>          try:
>              main_loop.run()
>          except Exception as err:
> diff --git a/python/qemu/qmp/util.py b/python/qemu/qmp/util.py
> index 7f9e718154..c44a5aacbc 100644
> --- a/python/qemu/qmp/util.py
> +++ b/python/qemu/qmp/util.py
> @@ -13,13 +13,7 @@
>  import asyncio
>  import sys
>  import traceback
> -from typing import (
> -    Any,
> -    Coroutine,
> -    Optional,
> -    TypeVar,
> -    cast,
> -)
> +from typing import TypeVar, cast
>
>
>  T = TypeVar('T')
> @@ -79,31 +73,6 @@ def bottom_half(func: T) -> T:
>      return func
>
>
> -# -------------------------------
> -# Section: Compatibility Wrappers
> -# -------------------------------
> -
> -
> -def create_task(coro: Coroutine[Any, Any, T],
> -                loop: Optional[asyncio.AbstractEventLoop] = None
> -                ) -> 'asyncio.Future[T]':
> -    """
> -    Python 3.6-compatible `asyncio.create_task` wrapper.
> -
> -    :param coro: The coroutine to execute in a task.
> -    :param loop: Optionally, the loop to create the task in.
> -
> -    :return: An `asyncio.Future` object.
> -    """
> -    if sys.version_info >= (3, 7):
> -        if loop is not None:
> -            return loop.create_task(coro)
> -        return asyncio.create_task(coro)  # pylint: disable=no-member
> -
> -    # Python 3.6:
> -    return asyncio.ensure_future(coro, loop=loop)
> -
> -
>  # ----------------------------
>  # Section: Logging & Debugging
>  # ----------------------------
> diff --git a/python/tests/protocol.py b/python/tests/protocol.py
> index 4a0ee94727..9bb23b6a7b 100644
> --- a/python/tests/protocol.py
> +++ b/python/tests/protocol.py
> @@ -8,7 +8,6 @@
>
>  from qemu.qmp import ConnectError, Runstate
>  from qemu.qmp.protocol import AsyncProtocol, StateError
> -from qemu.qmp.util import create_task
>
>
>  class NullProtocol(AsyncProtocol[None]):
> @@ -124,7 +123,7 @@ async def _runner():
>              if allow_cancellation:
>                  return
>              raise
> -    return create_task(_runner())
> +    return asyncio.create_task(_runner())
>
>
>  @contextmanager
> @@ -271,7 +270,7 @@ async def _watcher():
>                      msg=f"Expected state '{state.name}'",
>                  )
>
> -        self.runstate_watcher = create_task(_watcher())
> +        self.runstate_watcher = asyncio.create_task(_watcher())
>          # Kick the loop and force the task to block on the event.
>          await asyncio.sleep(0)
>
> @@ -589,7 +588,7 @@ async def _asyncTearDown(self):
>      async def testSmoke(self):
>          with TemporaryDirectory(suffix='.qmp') as tmpdir:
>              sock = os.path.join(tmpdir, type(self.proto).__name__ + ".sock")
> -            server_task = create_task(self.server.start_server_and_accept(sock))
> +            server_task = asyncio.create_task(self.server.start_server_and_accept(sock))
>
>              # give the server a chance to start listening [...]
>              await asyncio.sleep(0)
> --
> 2.49.0
>

Re: [PATCH 01/14] python: Replace asyncio.get_event_loop for Python 3.14

[John Snow]

On Tue, Aug 19, 2025 at 3:51 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Aug 19, 2025 at 03:47:50PM -0400, John Snow wrote:
> > On Tue, Jul 15, 2025 at 10:30 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > From: Richard W.M. Jones <rjones@redhat.com>
> > >
> > > In Python 3.14, no asyncio event loop gets generated automatically.
> > > Instead create one when we need it.  This should work with Python 3.13
> > > as well.  This change was suggested here:
> > >
> > > https://bugzilla.redhat.com/show_bug.cgi?id=2375004#c4
> > >
> > > See-also: https://docs.python.org/3.14/whatsnew/3.14.html#id7
> > > Thanks: Miro Hrončok, Daniel P. Berrangé
> > > Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
> >
> > Reviewed-by: John Snow <jsnow@redhat.com>
> >
> > We fixed this in the standalone tree, but I hadn't synchronized the
> > changes over yet. Shall we take your patches first, and then I'll
> > worry about synchronizing later?
>
> This really needs to be in 10.1.0, because tests crash and
> burn under python 3.14 without this - we're applying this
> in Fedora builds locally to deal with the brokeness.

Urk. I didn't realize we were waiting on this; please include it with
my blessings and apologies.

>
> > (I desperately need to drop this code from the qemu.git tree, urgh.)
>
> IMHO qemu.git should be considered authoritative right now because
> that's what we're actually shipping & thus take priority for merge.

Yup, you're right. I just mean I need to stop the duplication, because
stuff like this happens.

>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>

Re: [PATCH 07/14] iotests: drop compat for old version context manager

[John Snow]

On Tue, Jul 15, 2025 at 10:31 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Our minimum python is now 3.9, so back compat with prior
> python versions is no longer required.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [PATCH 10/14] functional: always enable all python warnings

[John Snow]

On Tue, Jul 15, 2025 at 11:15 AM Thomas Huth <thuth@redhat.com> wrote:
>
> On 15/07/2025 16.30, Daniel P. Berrangé wrote:
> > Of most importance is that this gives us a heads-up if anything
> > we rely on has been deprecated. The default python behaviour
> > only emits a warning if triggered from __main__ which is very
> > limited.
> >
> > Setting the env variable further ensures that any python child
> > processes will also display warnings.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Looks like 8, 9, 10 already got merged; skipping.

> > ---
> >   tests/functional/qemu_test/testcase.py | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> > index 71c7160adc..2a78e735f1 100644
> > --- a/tests/functional/qemu_test/testcase.py
> > +++ b/tests/functional/qemu_test/testcase.py
> > @@ -19,6 +19,7 @@
> >   from subprocess import run
> >   import sys
> >   import tempfile
> > +import warnings
> >   import unittest
> >   import uuid
> >
> > @@ -235,6 +236,9 @@ def tearDown(self):
> >           self._log_fh.close()
> >
> >       def main():
> > +        warnings.simplefilter("default")
> > +        os.environ["PYTHONWARNINGS"] = "default"
> > +
> >           path = os.path.basename(sys.argv[0])[:-3]
> >
> >           cache = os.environ.get("QEMU_TEST_PRECACHE", None)
>
> Acked-by: Thomas Huth <thuth@redhat.com>
>

Re: [PATCH 11/14] python: ensure QEMUQtestProtocol closes its socket

[John Snow]

On Tue, Jul 15, 2025 at 10:32 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> While QEMUQtestMachine closes the socket that was passed to
> QEMUQtestProtocol, the python resource leak manager still
> believes that the copy QEMUQtestProtocol holds is open. We
> must explicitly call close to avoid this leak warnnig.

Huh. I wonder if that's a false positive, or if it's actually true?

>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Well, either way...

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  python/qemu/machine/qtest.py | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
> index 4f5ede85b2..781f674ffa 100644
> --- a/python/qemu/machine/qtest.py
> +++ b/python/qemu/machine/qtest.py
> @@ -177,6 +177,8 @@ def _post_shutdown(self) -> None:
>              self._qtest_sock_pair[0].close()
>              self._qtest_sock_pair[1].close()
>              self._qtest_sock_pair = None
> +        if self._qtest is not None:
> +            self._qtest.close()
>          super()._post_shutdown()
>
>      def qtest(self, cmd: str) -> str:
> --
> 2.49.0
>

Re: [PATCH 11/14] python: ensure QEMUQtestProtocol closes its socket

[Daniel P. Berrangé]

On Tue, Aug 19, 2025 at 04:12:12PM -0400, John Snow wrote:
> On Tue, Jul 15, 2025 at 10:32 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > While QEMUQtestMachine closes the socket that was passed to
> > QEMUQtestProtocol, the python resource leak manager still
> > believes that the copy QEMUQtestProtocol holds is open. We
> > must explicitly call close to avoid this leak warnnig.
> 
> Huh. I wonder if that's a false positive, or if it's actually true?

I'm  not sure to be honest, but the extra close doesn't seem
to cause any ill effects.

> 
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Well, either way...
> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> > ---
> >  python/qemu/machine/qtest.py | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
> > index 4f5ede85b2..781f674ffa 100644
> > --- a/python/qemu/machine/qtest.py
> > +++ b/python/qemu/machine/qtest.py
> > @@ -177,6 +177,8 @@ def _post_shutdown(self) -> None:
> >              self._qtest_sock_pair[0].close()
> >              self._qtest_sock_pair[1].close()
> >              self._qtest_sock_pair = None
> > +        if self._qtest is not None:
> > +            self._qtest.close()
> >          super()._post_shutdown()
> >
> >      def qtest(self, cmd: str) -> str:
> > --
> > 2.49.0
> >
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 13/14] iotests/151: ensure subprocesses are cleaned up

[John Snow]

On Tue, Jul 15, 2025 at 10:32 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The iotest 151 creates a bunch of subprocesses, with their stdout
> connected to a pipe but never reads any data from them and does
> not gurantee the processes are killed on cleanup.
>
> This triggers resource leak warnings from python when the
> subprocess.Popen object is garbage collected.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/151 | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/tests/qemu-iotests/151 b/tests/qemu-iotests/151
> index f2ff9c5dac..06ee3585db 100755
> --- a/tests/qemu-iotests/151
> +++ b/tests/qemu-iotests/151
> @@ -263,6 +263,11 @@ class TestThrottledWithNbdExportBase(iotests.QMPTestCase):
>                          break
>                      except subprocess.TimeoutExpired:
>                          self.vm.qtest(f'clock_step {1 * 1000 * 1000 * 1000}')
> +                try:
> +                    p.kill()
> +                    p.stdout.close()
> +                except:
> +                    pass

You can decide how much you care here, but using "except Exception:"
will allow critical exceptions to still crash the program, like
KeyboardInterrupt and SystemExit et al. It's good hygiene for things
that may or may not hang so you can still use Ctrl+C.

>          except IndexError:
>              pass
>
> --
> 2.49.0
>

Re: [PATCH 12/14] iotests/147: ensure temporary sockets are closed before exiting

[John Snow]

On Tue, Jul 15, 2025 at 10:32 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> This avoids the python resource leak detector from issuing warnings
> in the iotests.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  tests/qemu-iotests/147 | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
> index 6d6f077a14..3e14bd389a 100755
> --- a/tests/qemu-iotests/147
> +++ b/tests/qemu-iotests/147
> @@ -277,6 +277,7 @@ class BuiltinNBD(NBDBlockdevAddBase):
>                       } }
>          self.client_test(filename, flatten_sock_addr(address), 'nbd-export')
>
> +        sockfd.close()
>          self._server_down()
>
>
> --
> 2.49.0
>

Re: [PATCH 14/14] iotests/check: always enable all python warnings

[John Snow]

On Tue, Jul 15, 2025 at 10:32 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Of most importance is that this gives us a heads-up if anything
> we rely on has been deprecated. The default python behaviour
> only emits a warning if triggered from __main__ which is very
> limited.
>
> Setting the env variable further ensures that any python child
> processes will also display warnings.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

Thanks for this one. I sent a PR for the first two patches, I will
stage everything else.

> ---
>  tests/qemu-iotests/check | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 545f9ec7bd..d9b7c1d598 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -21,6 +21,7 @@ import sys
>  import argparse
>  import shutil
>  from pathlib import Path
> +import warnings
>
>  from findtests import TestFinder
>  from testenv import TestEnv
> @@ -137,6 +138,9 @@ def make_argparser() -> argparse.ArgumentParser:
>
>
>  if __name__ == '__main__':
> +    warnings.simplefilter("default")
> +    os.environ["PYTHONWARNINGS"] = "default"
> +
>      args = make_argparser().parse_args()
>
>      env = TestEnv(source_dir=args.source_dir,
> --
> 2.49.0
>

Re: [PATCH 13/14] iotests/151: ensure subprocesses are cleaned up

[Daniel P. Berrangé]

On Tue, Aug 19, 2025 at 05:37:04PM -0400, John Snow wrote:
> On Tue, Jul 15, 2025 at 10:32 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > The iotest 151 creates a bunch of subprocesses, with their stdout
> > connected to a pipe but never reads any data from them and does
> > not gurantee the processes are killed on cleanup.
> >
> > This triggers resource leak warnings from python when the
> > subprocess.Popen object is garbage collected.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  tests/qemu-iotests/151 | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/tests/qemu-iotests/151 b/tests/qemu-iotests/151
> > index f2ff9c5dac..06ee3585db 100755
> > --- a/tests/qemu-iotests/151
> > +++ b/tests/qemu-iotests/151
> > @@ -263,6 +263,11 @@ class TestThrottledWithNbdExportBase(iotests.QMPTestCase):
> >                          break
> >                      except subprocess.TimeoutExpired:
> >                          self.vm.qtest(f'clock_step {1 * 1000 * 1000 * 1000}')
> > +                try:
> > +                    p.kill()
> > +                    p.stdout.close()
> > +                except:
> > +                    pass
> 
> You can decide how much you care here, but using "except Exception:"
> will allow critical exceptions to still crash the program, like
> KeyboardInterrupt and SystemExit et al. It's good hygiene for things
> that may or may not hang so you can still use Ctrl+C.

Yes, that should be changed as you describe.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|