From 258254c256dc88999b6a3227447e73fee2dd3837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Fri, 4 Oct 2024 15:38:12 +0200 Subject: [PATCH 1/6] tests: update for modular libvirt daemon It's now virxend service, not libvirtd. QubesOS/qubes-issues#9402 --- qubes/tests/integ/basic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qubes/tests/integ/basic.py b/qubes/tests/integ/basic.py index c659f4d74..1f574dd3e 100644 --- a/qubes/tests/integ/basic.py +++ b/qubes/tests/integ/basic.py @@ -285,7 +285,7 @@ def test_140_libvirt_events_reconnect(self): self.loop.run_until_complete(self.vm.create_on_disk()) self.loop.run_until_complete(self.vm.start()) p = self.loop.run_until_complete(asyncio.create_subprocess_exec( - 'systemctl', 'restart', 'libvirtd')) + 'systemctl', 'restart', 'virtxend')) self.loop.run_until_complete(p.communicate()) # check if events still works self.domain_paused_received = False @@ -302,7 +302,7 @@ def test_141_libvirt_objects_reconnect(self): # make sure libvirt object is cached self.app.domains[0].libvirt_domain.isActive() p = self.loop.run_until_complete(asyncio.create_subprocess_exec( - 'systemctl', 'restart', 'libvirtd')) + 'systemctl', 'restart', 'virtxend')) self.loop.run_until_complete(p.communicate()) # trigger reconnect with self.assertNotRaises(libvirt.libvirtError): From 3691338ec6a24efabdc4f1df906ae12ec1860602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Fri, 4 Oct 2024 15:40:18 +0200 Subject: [PATCH 2/6] Update service dependencies for modular libvirt daemon libvirtd.service is the monolithic one, virtxend is modular one. Add After=virtxend.socket, as it uses socket activation now. QubesOS/qubes-issues#9402 --- linux/systemd/qubes-core.service | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/systemd/qubes-core.service b/linux/systemd/qubes-core.service index 34ce40fde..3786250e4 100644 --- a/linux/systemd/qubes-core.service +++ b/linux/systemd/qubes-core.service @@ -1,6 +1,6 @@ [Unit] Description=Qubes Dom0 startup setup -After=qubes-db-dom0.service libvirtd.service xenconsoled.service qubesd.service qubes-qmemman.service +After=qubes-db-dom0.service libvirtd.service virtxend.socket xenconsoled.service qubesd.service qubes-qmemman.service # Cover legacy init.d script [Service] From 4643a8d1bb069ddbab9c03c57aec23dc43ffb072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 7 Oct 2024 02:26:24 +0200 Subject: [PATCH 3/6] Move undefining libvirt object to QubesVM class It's more logical to do that in the QubesVM class. Moving this also avoids protected-access pylint complain (which is very much valid complain). --- qubes/app.py | 10 +--------- qubes/tests/init.py | 3 +++ qubes/vm/qubesvm.py | 12 ++++++++++++ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/qubes/app.py b/qubes/app.py index fd65eb9fb..66cb6a3cb 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -531,15 +531,7 @@ def __delitem__(self, key): if not vm.is_halted(): raise qubes.exc.QubesVMNotHaltedError(vm) self.app.fire_event('domain-pre-delete', pre_event=True, vm=vm) - try: - if vm.libvirt_domain: - vm.libvirt_domain.undefine() - # pylint: disable=protected-access - vm._libvirt_domain = None - except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: - # already undefined - pass + vm.libvirt_undefine() del self._dict[vm.qid] self.app.fire_event('domain-delete', vm=vm) if getattr(vm, 'dispid', None): diff --git a/qubes/tests/init.py b/qubes/tests/init.py index fa4cc66c6..e52605032 100644 --- a/qubes/tests/init.py +++ b/qubes/tests/init.py @@ -351,6 +351,9 @@ def is_halted(self): def get_power_state(self): return "Halted" + def libvirt_undefine(self): + pass + class TestApp(qubes.tests.TestEmitter): pass diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py index f4571ffc2..a7d868bbe 100644 --- a/qubes/vm/qubesvm.py +++ b/qubes/vm/qubesvm.py @@ -1930,6 +1930,18 @@ async def clone_disk_files(self, src, pool=None, pools=None, ): # fire hooks await self.fire_event_async('domain-clone-files', src=src) + def libvirt_undefine(self): + """Undefine domain object in libvirt""" + try: + if self.libvirt_domain: + self.libvirt_domain.undefine() + except libvirt.libvirtError as e: + if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: + # already undefined + pass + if self._libvirt_domain is not None: + self._libvirt_domain = None + # # methods for querying domain state # From abbcf6e67c32ceb8055821a8c318659f45664cb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 7 Oct 2024 02:33:55 +0200 Subject: [PATCH 4/6] Drop references to libvirt objects when undefining the domain Normally it wouldn't matter because dropping reference to VirDomainWrapper would do. But if there is an exception that happened during libvirt call (like createWithFlags), the traceback attached to the exception would keep reference to VirDomainWrapper object, preventing closing libvirt connection. During normal runtime it isn't huge problem, as those exceptions are discarded soon after anyway. But during tests, they are all collected, and that results in leaking a lot of libvirt connection objects, and as a consequence open file descriptors. QubesOS/qubes-issues# --- qubes/app.py | 4 ++++ qubes/vm/qubesvm.py | 1 + 2 files changed, 5 insertions(+) diff --git a/qubes/app.py b/qubes/app.py index 66cb6a3cb..02bc84d61 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -92,6 +92,10 @@ def _reconnect_if_dead(self): self._vm = self._connection._conn.lookupByUUID(self._vm.UUID()) return is_dead + def close(self): + self._vm = None + self._connection = None + def __getattr__(self, attrname): attr = getattr(self._vm, attrname) if not isinstance(attr, collections.abc.Callable): diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py index a7d868bbe..299908e76 100644 --- a/qubes/vm/qubesvm.py +++ b/qubes/vm/qubesvm.py @@ -1940,6 +1940,7 @@ def libvirt_undefine(self): # already undefined pass if self._libvirt_domain is not None: + self._libvirt_domain.close() self._libvirt_domain = None # From f322f8d0a2942be5d62ec45b27bd390d0f5f696d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 7 Oct 2024 03:12:21 +0200 Subject: [PATCH 5/6] VirDomainWrapper: avoid keeping libvirt object ref in locals Complementary to the previous commit, avoid keeping keeping libvirt object (function) reference in locals, which become closure for the wrapper. Do getattr (again) when the actual call is made. The primary reason for this change is to avoid leaking objects during tests, but also this fixes a rare failure more - if external caller would cache returned function, it wouldn't work after reconnection. For example: # libvirt_domain is VirDomainWrapper() object >>> func = libvirt_domain.pause >>> func() >>> func() If reconnection happens during the first call (or between calls), the second call would use stale function reference. --- qubes/app.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qubes/app.py b/qubes/app.py index 02bc84d61..4031f8386 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -104,12 +104,14 @@ def __getattr__(self, attrname): @functools.wraps(attr) def wrapper(*args, **kwargs): try: - return attr(*args, **kwargs) + return getattr(self._vm, attrname)(*args, **kwargs) except libvirt.libvirtError: if self._reconnect_if_dead(): return getattr(self._vm, attrname)(*args, **kwargs) raise + del wrapper.__wrapped__ + del attr return wrapper From 506cd2a6db167646b92cb9977a4f0c4626677bc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 7 Oct 2024 17:08:59 +0200 Subject: [PATCH 6/6] tests: fix Whonix notifications interfering with test_300_bug_1028_gui_memory_pinning Whonix Gateway shows some notifications / windows on startup (Tor connection progress info, updates notification, news etc). When it happens at the wrong time, it will switch focus between two screenshots, affecting cursor image and thus causing the test to fail. Avoid this by disabling starting Tor for this test. --- qubes/tests/integ/vm_qrexec_gui.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qubes/tests/integ/vm_qrexec_gui.py b/qubes/tests/integ/vm_qrexec_gui.py index c90ce0b6f..a91b258ce 100644 --- a/qubes/tests/integ/vm_qrexec_gui.py +++ b/qubes/tests/integ/vm_qrexec_gui.py @@ -536,6 +536,8 @@ async def _test_300_bug_1028_gui_memory_pinning(self): # exclude from memory balancing self.testvm1.features['service.meminfo-writer'] = False + # prevent Whonix startup notifications from interfering + self.testvm1.features['service.whonix-tor-disable'] = True await self.testvm1.start() await self.wait_for_session(self.testvm1)