From 70b81255a15200d35c4eadcbaa9876a75dbc3232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 4 Nov 2025 00:28:32 +0100 Subject: [PATCH 1/2] Cache device assignments/attachments Those are enumerated several times when listing devices, but they change rarely (most qubes don't get any devices attached usually). Cache both assignments and attachments to speed things up. And similarly to properties and power state cache - invalidate the cache based on events. Fixes QubesOS/qubes-issues#10380 --- qubesadmin/app.py | 2 ++ qubesadmin/devices.py | 43 ++++++++++++++++++++++++++++++++++- qubesadmin/events/__init__.py | 42 +++++++++++++++++++++++----------- qubesadmin/tests/devices.py | 43 +++++++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 14 deletions(-) diff --git a/qubesadmin/app.py b/qubesadmin/app.py index 57a688da..2080ee43 100644 --- a/qubesadmin/app.py +++ b/qubesadmin/app.py @@ -797,8 +797,10 @@ def _invalidate_cache_all(self): # pylint: disable=protected-access self.domains.clear_cache() for vm in self.domains._vm_objects.values(): + assert isinstance(vm, qubesadmin.vm.QubesVM) vm._power_state_cache = None vm._properties_cache = {} + vm.devices.clear_cache() self._properties_cache = {} diff --git a/qubesadmin/devices.py b/qubesadmin/devices.py index 9214d4f8..25414a2b 100644 --- a/qubesadmin/devices.py +++ b/qubesadmin/devices.py @@ -62,6 +62,12 @@ def __init__(self, vm, class_): self._vm = vm self._class = class_ self._dev_cache = {} + #: attachments cache, `None` means "not cached (yet)", + #: in contrast to empty list which means "cached empty list" + self._attachment_cache = None + #: assignments cache, `None` means "not cached (yet)", + #: in contrast to empty list which means "cached empty list" + self._assignment_cache = None def attach(self, assignment: DeviceAssignment) -> None: """ @@ -75,6 +81,9 @@ def attach(self, assignment: DeviceAssignment) -> None: "did you mean `qvm-pci assign --required ...`" ) self._add(assignment, "attach") + # clear the whole cache instead of saving provided assignment, it might + # get modified before actually attaching + self._attachment_cache = None def detach(self, assignment: DeviceAssignment) -> None: """ @@ -84,6 +93,7 @@ def detach(self, assignment: DeviceAssignment) -> None: (obtained from :py:meth:`assignments`) """ self._remove(assignment, "detach") + self._assignment_cache = None def assign(self, assignment: DeviceAssignment) -> None: """ @@ -113,6 +123,9 @@ def assign(self, assignment: DeviceAssignment) -> None: ) self._add(assignment, "assign") + # clear the whole cache instead of saving provided assignment, it might + # get modified before actually assigning + self._assignment_cache = None def unassign(self, assignment: DeviceAssignment) -> None: """ @@ -122,6 +135,7 @@ def unassign(self, assignment: DeviceAssignment) -> None: (obtained from :py:meth:`assignments`) """ self._remove(assignment, "unassign") + self._assignment_cache = None def _add(self, assignment: DeviceAssignment, action: str) -> None: """ @@ -186,6 +200,10 @@ def get_attached_devices(self) -> Iterable[DeviceAssignment]: """ List devices which are attached to this vm. """ + if self._attachment_cache is not None: + yield from self._attachment_cache + return + new_cache = [] assignments_str = self._vm.qubesd_call( None, "admin.vm.device.{}.Attached".format(self._class) ).decode() @@ -195,9 +213,14 @@ def get_attached_devices(self) -> Iterable[DeviceAssignment]: head, self._class, self._vm.app.domains, blind=True ) - yield DeviceAssignment.deserialize( + assignment = DeviceAssignment.deserialize( untrusted_rest.encode("ascii"), expected_device=device ) + new_cache.append(assignment) + yield assignment + + if self._vm.app.cache_enabled: + self._attachment_cache = new_cache def get_assigned_devices( self, required_only: bool = False @@ -207,6 +230,12 @@ def get_assigned_devices( Safe to access before libvirt bootstrap. """ + if self._assignment_cache is not None: + for assignment in self._assignment_cache: + if not required_only or assignment.required: + yield assignment + return + new_cache = [] assignments_str = self._vm.qubesd_call( None, "admin.vm.device.{}.Assigned".format(self._class) ).decode() @@ -219,9 +248,13 @@ def get_assigned_devices( assignment = DeviceAssignment.deserialize( untrusted_rest.encode("ascii"), expected_device=device ) + new_cache.append(assignment) if not required_only or assignment.required: yield assignment + if self._vm.app.cache_enabled: + self._assignment_cache = new_cache + def get_exposed_devices(self) -> Iterable[DeviceInfo]: """ List devices exposed by this vm. @@ -252,6 +285,7 @@ def update_assignment(self, device: Port, required: AssignmentMode): repr(device), required.value.encode("utf-8"), ) + self._assignment_cache = None __iter__ = get_exposed_devices @@ -260,6 +294,8 @@ def clear_cache(self): Clear cache of available devices. """ self._dev_cache.clear() + self._assignment_cache = None + self._attachment_cache = None def __getitem__(self, item): """Get device object with given port_id. @@ -325,3 +361,8 @@ def allow(self, *interfaces: Iterable[DeviceInterface]): None, "".join(repr(ifc) for ifc in interfaces).encode('ascii'), ) + + def clear_cache(self): + """Clear cache of all available device classes""" + for devclass in self.values(): + devclass.clear_cache() diff --git a/qubesadmin/events/__init__.py b/qubesadmin/events/__init__.py index 353cc180..a6a5f1e3 100644 --- a/qubesadmin/events/__init__.py +++ b/qubesadmin/events/__init__.py @@ -229,6 +229,35 @@ def handle(self, subject, event, **kwargs): vm = kwargs['vm'] self.app.domains.clear_cache(invalidate_name=str(vm)) subject = None + # invalidate cache if needed; call it before other handlers + # as those may want to use cached value + if event.startswith('property-set:') or \ + event.startswith('property-reset:'): + self.app._invalidate_cache(subject, event, **kwargs) + elif event in ('domain-pre-start', 'domain-start', 'domain-shutdown', + 'domain-paused', 'domain-unpaused', + 'domain-start-failed'): + self.app._update_power_state_cache(subject, event, **kwargs) + subject.devices.clear_cache() + elif event == 'connection-established': + # on (re)connection, clear cache completely - we don't have + # guarantee about not missing any events before this point + self.app._invalidate_cache_all() + elif event.split(":")[0] in ( + "device-assign", + "device-unassign", + "device-assignment-changed" + ): + devclass = event.split(":")[1] + subject.devices[devclass]._assignment_cache = None + elif event.split(":")[0] in ( + "device-attach", + "device-detach", + "device-removed" + ): + devclass = event.split(":")[1] + subject.devices[devclass]._attachment_cache = None + # deserialize known attributes if event.startswith('device-'): try: @@ -256,19 +285,6 @@ def handle(self, subject, event, **kwargs): kwargs['port'], devclass, self.app.domains, blind=True) except (KeyError, ValueError): pass - # invalidate cache if needed; call it before other handlers - # as those may want to use cached value - if event.startswith('property-set:') or \ - event.startswith('property-reset:'): - self.app._invalidate_cache(subject, event, **kwargs) - elif event in ('domain-pre-start', 'domain-start', 'domain-shutdown', - 'domain-paused', 'domain-unpaused', - 'domain-start-failed'): - self.app._update_power_state_cache(subject, event, **kwargs) - elif event == 'connection-established': - # on (re)connection, clear cache completely - we don't have - # guarantee about not missing any events before this point - self.app._invalidate_cache_all() handlers = [h_func for h_name, h_func_set in self.handlers.items() for h_func in h_func_set diff --git a/qubesadmin/tests/devices.py b/qubesadmin/tests/devices.py index 6c5beae4..bb3dc691 100644 --- a/qubesadmin/tests/devices.py +++ b/qubesadmin/tests/devices.py @@ -255,6 +255,49 @@ def test_041_assignments_options(self): self.assertAllCalled() + def test_042_assignments_cache(self): + self.app.cache_enabled = True + self.app.expected_calls[ + ('test-vm', 'admin.vm.device.test.Attached', None, None)] = \ + (b"0\0test-vm2+dev1 backend_domain='test-vm2' port_id='dev1' " + b"mode='manual' devclass='test' " + b"frontend_domain='test-vm' _ro='True'\n") + self.app.expected_calls[ + ('test-vm', 'admin.vm.device.test.Assigned', None, None)] = \ + (b"0\0test-vm3+dev2 backend_domain='test-vm3' devclass='test' " + b"port_id='dev2' mode='required' " + b"frontend_domain='test-vm' _ro='False'\n") + # populate cache + list(self.vm.devices['test'].get_dedicated_devices()) + + self.assertAllCalled() + self.app.expected_calls.clear() + + # get again, should be cached now + assigns = sorted(list( + self.vm.devices['test'].get_dedicated_devices())) + + self.assertEqual(len(assigns), 2) + self.assertIsInstance(assigns[0], DeviceAssignment) + self.assertEqual(assigns[0].backend_domain, + self.app.domains['test-vm2']) + self.assertEqual(assigns[0].port_id, 'dev1') + self.assertEqual(assigns[0].frontend_domain, + self.app.domains['test-vm']) + self.assertEqual(assigns[0].options, {'ro': 'True'}) + self.assertEqual(assigns[0].required, False) + self.assertEqual(assigns[0].devclass, 'test') + + self.assertIsInstance(assigns[1], DeviceAssignment) + self.assertEqual(assigns[1].backend_domain, + self.app.domains['test-vm3']) + self.assertEqual(assigns[1].port_id, 'dev2') + self.assertEqual(assigns[1].frontend_domain, + self.app.domains['test-vm']) + self.assertEqual(assigns[1].options, {'ro': 'False'}) + self.assertEqual(assigns[1].required, True) + self.assertEqual(assigns[1].devclass, 'test') + def test_050_required(self): self.app.expected_calls[ ('test-vm', 'admin.vm.device.test.Assigned', None, None)] = \ From 537080349eba19688db17292d3860d9123e89759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 4 Nov 2025 02:49:50 +0100 Subject: [PATCH 2/2] Enable cache for qvm-device While it doesn't enable events to have proper cache invalidation, it's also short lived program that can speed up by avoiding repeated enumeration. --- qubesadmin/tools/qvm_device.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qubesadmin/tools/qvm_device.py b/qubesadmin/tools/qvm_device.py index eedb7624..c57b96db 100644 --- a/qubesadmin/tools/qvm_device.py +++ b/qubesadmin/tools/qvm_device.py @@ -757,6 +757,7 @@ def get_parser(device_class=None): def main(args=None, app=None): """Main routine of :program:`qvm-block`.""" app = app or qubesadmin.Qubes() + app.cache_enabled = True basename = os.path.basename(sys.argv[0]) devclass = None