Merge branch 'device-cache'

* device-cache:
  Enable cache for qvm-device
  Cache device assignments/attachments

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,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

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

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