Skip to content

Commit

Permalink
Fix VM validity check for cached VM objects
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
marmarek committed Jul 14, 2020
1 parent 1a4cdba commit 45a28c2
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
2 changes: 1 addition & 1 deletion qubesadmin/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def refresh_cache(self, force=False):
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
Expand Down
24 changes: 24 additions & 0 deletions qubesadmin/tests/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,30 @@ def test_011_getitem_non_cache_power_state(self):
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):
Expand Down

0 comments on commit 45a28c2

Please sign in to comment.