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

Author: marmarek

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 = {}
 
 

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()

qubesadmin/events/__init__.py

@@ -229,6 +229,27 @@ 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 +277,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

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)] = \