From 45a28c29aeafa7aceb2a7e6784066647e7fd6e18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Tue, 14 Jul 2020 15:55:04 +0200
Subject: [PATCH] Fix VM validity check for cached VM objects

Qubes().domains.refresh_cache() tries to preserve cached VM objects if
the class matches - this way if an application keeps reference to any,
it will still be the same as freshly obtained from the collection, and
also it will receive cache updates/invalidates based on events.

The check for class change was invalid - on core-admin-client side we
have just one QubesVM class with 'klass' attribute. This leads to VM
objects being disconnected from VMCollection and stale properties cache
there (because they no longer receive events).

Fix the check.

And also add a test if indeed the same object is returned.
---
 qubesadmin/app.py       |  2 +-
 qubesadmin/tests/app.py | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/qubesadmin/app.py b/qubesadmin/app.py
index 041cd1c..1537d9e 100644
--- a/qubesadmin/app.py
+++ b/qubesadmin/app.py
@@ -82,7 +82,7 @@ class VMCollection(object):
             if vm.name not in self._vm_list:
                 # VM no longer exists
                 del self._vm_objects[name]
-            elif vm.__class__.__name__ != self._vm_list[vm.name]['class']:
+            elif vm.klass != self._vm_list[vm.name]['class']:
                 # VM class have changed
                 del self._vm_objects[name]
             # TODO: some generation ID, to detect VM re-creation
diff --git a/qubesadmin/tests/app.py b/qubesadmin/tests/app.py
index 4ab6e21..5a57015 100644
--- a/qubesadmin/tests/app.py
+++ b/qubesadmin/tests/app.py
@@ -158,6 +158,30 @@ class TC_00_VMCollection(qubesadmin.tests.QubesTestCase):
             self.fail('VM not found in collection')
         self.assertAllCalled()
 
+    def test_012_getitem_cached_object(self):
+        self.app.expected_calls[('dom0', 'admin.vm.List', None, None)] = \
+            b'0\x00test-vm class=AppVM state=Running\n'
+        try:
+            vm1 = self.app.domains['test-vm']
+            vm2 = self.app.domains['test-vm']
+            self.assertIs(vm1, vm2)
+        except KeyError:
+            self.fail('VM not found in collection')
+        self.app.domains.clear_cache()
+        # even after clearing the cache, the same instance should be returned
+        # if the class haven't changed
+        vm3 = self.app.domains['test-vm']
+        self.assertIs(vm1, vm3)
+
+        # change the class and expected different object
+        self.app.expected_calls[('dom0', 'admin.vm.List', None, None)] = \
+            b'0\x00test-vm class=StandaloneVM state=Running\n'
+        self.app.domains.clear_cache()
+        vm4 = self.app.domains['test-vm']
+        self.assertIsNot(vm1, vm4)
+        self.assertAllCalled()
+
+
 
 class TC_10_QubesBase(qubesadmin.tests.QubesTestCase):
     def setUp(self):