diff --git a/instances/test_detail_reads.py b/instances/test_detail_reads.py new file mode 100644 index 00000000..d8fa95b5 --- /dev/null +++ b/instances/test_detail_reads.py @@ -0,0 +1,66 @@ +"""The VM page reads each kind of domain XML and the host capabilities once, +through the real getters, on libvirt's test driver.""" + +from unittest.mock import patch + +import libvirt +from computes.models import Compute +from django.contrib.auth import get_user_model +from django.test import TestCase +from django.urls import reverse +from instances.models import Instance + +# a VM with a VNC console, on libvirt's test driver +DOMAIN = """read-cache-vm1281 +hvm + +""" + + +class DetailReadsTests(TestCase): + def setUp(self): + self.conn = libvirt.open("test:///default") # each open is a fresh, private test host + self.addCleanup(self.conn.close) + domain = self.conn.defineXML(DOMAIN) + self.compute = Compute.objects.create(name="testdrv", hostname="localhost", login="", password="", type=4) + self.vm = Instance.objects.create(compute=self.compute, name=domain.name(), uuid=domain.UUIDString()) + # what the test driver lacks: machine types, network filters + for target, value in (("get_machine_types", ["pc"]), ("get_nwfilters", [])): + patcher = patch(f"vrtManager.connection.wvmConnect.{target}", return_value=value) + patcher.start() + self.addCleanup(patcher.stop) + self.admin = get_user_model().objects.create_superuser("dr-admin", "dr@example.com", "pw") + + def render(self): + xml_reads, cap_reads = [], [] + real_xml, real_caps = libvirt.virDomain.XMLDesc, libvirt.virConnect.getCapabilities + + def xml(dom, flags=0): + xml_reads.append(flags) + return real_xml(dom, flags) + + def caps(conn): + cap_reads.append(1) + return real_caps(conn) + + self.client.force_login(self.admin) + with patch("vrtManager.connection.connection_manager.get_connection", return_value=self.conn), patch.object( + libvirt.virDomain, "XMLDesc", xml + ), patch.object(libvirt.virConnect, "getCapabilities", caps): + response = self.client.get(reverse("instances:instance", args=[self.vm.id])) + self.assertEqual(response.status_code, 200) + return response, xml_reads, cap_reads + + def test_each_xml_and_the_capabilities_are_read_once(self): + _, xml_reads, cap_reads = self.render() + self.assertEqual(sorted(xml_reads), sorted(set(xml_reads)), "an XML read twice") + self.assertEqual(len(cap_reads), 1) + + def test_a_change_after_the_page_reads_fresh_xml(self): + # outside the page the getters read libvirt each time, as changes need + with patch("vrtManager.connection.connection_manager.get_connection", return_value=self.conn): + proxy = self.vm.proxy + proxy.set_console_passwd("pass1") + proxy.set_console_keymap("de") + self.assertEqual(proxy.get_console_passwd(), "pass1") + self.assertEqual(proxy.get_console_keymap(), "de") diff --git a/instances/views.py b/instances/views.py index 947b69b9..118a94be 100755 --- a/instances/views.py +++ b/instances/views.py @@ -95,6 +95,13 @@ def instance(request, pk): msg = _("The host of '%(name)s' cannot be reached right now.") % {"name": instance.name} messages.error(request, msg) return redirect(reverse("instances:index")) + # The page only reads the VM, and its getters read the same XML some 30 + # times: libvirt is asked once per kind of XML. + with instance.proxy.cached_reads(): + return _instance_page(request, pk, instance, compute) + + +def _instance_page(request, pk, instance, compute): computes = Compute.objects.all().order_by("name") computes_count = computes.count() users = User.objects.all().order_by("username") diff --git a/vrtManager/instance.py b/vrtManager/instance.py index b0cf5084..1cb9c11d 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -186,6 +186,9 @@ class wvmInstances(wvmConnect): class wvmInstance(wvmConnect): + # set by cached_reads(): libvirt answers reused within the block + _read_cache = None + def __init__(self, host, login, passwd, conn, vname, uuid=None): wvmConnect.__init__(self, host, login, passwd, conn) self._ip_cache = None @@ -249,12 +252,45 @@ class wvmInstance(wvmConnect): def delete(self, flags=0): self.instance.undefineFlags(flags) + @contextlib.contextmanager + def cached_reads(self): + """ + Within the block the domain XML (per flags value) and the host + capabilities are read from libvirt once, for pages that only read: a + VM page reads the XML in some 30 getters. Code that changes the VM + must not run inside: most changes (devices, QoS, memory) do not go + through _defineXML, the only call that drops the cache. + """ + if self._read_cache is not None: + raise RuntimeError("cached_reads blocks do not nest") + self._read_cache = {} + try: + yield self + finally: + self._read_cache = None + def _XMLDesc(self, flag): - return self.instance.XMLDesc(flag) + cache = self._read_cache + if cache is None: + return self.instance.XMLDesc(flag) + key = ("xml", flag) + if key not in cache: + cache[key] = self.instance.XMLDesc(flag) + return cache[key] def _defineXML(self, xml): + if self._read_cache is not None: + self._read_cache.clear() return self.wvm.defineXML(xml) + def get_cap_xml(self): + cache = self._read_cache + if cache is None: + return super().get_cap_xml() + if "capabilities" not in cache: + cache["capabilities"] = super().get_cap_xml() + return cache["capabilities"] + def get_status(self): """ VIR_DOMAIN_NOSTATE = 0 diff --git a/vrtManager/test_read_cache.py b/vrtManager/test_read_cache.py new file mode 100644 index 00000000..5facf5e5 --- /dev/null +++ b/vrtManager/test_read_cache.py @@ -0,0 +1,113 @@ +"""wvmInstance.cached_reads(): inside the block the domain XML (per flags) and +the host capabilities are read from libvirt once; outside it every read goes +to libvirt, as code that changes the VM needs.""" + +import unittest + +from django.conf import settings + +if not settings.configured: + settings.configure(MAC_OUI="52:54:10") + +from vrtManager.instance import wvmInstance + + +class FakeDomain: + def __init__(self): + self.reads = [] + self.fail = False + + def XMLDesc(self, flags): + if self.fail: + raise RuntimeError("libvirt error") + self.reads.append(flags) + return f"" + + +class FakeConn: + def __init__(self): + self.caps = 0 + self.defined = [] + + def getCapabilities(self): + self.caps += 1 + return "" + + def defineXML(self, xml): + self.defined.append(xml) + + +def proxy(): + p = wvmInstance.__new__(wvmInstance) # no connection: only these calls + p.instance = FakeDomain() + p.wvm = FakeConn() + return p + + +class CachedReadsTestCase(unittest.TestCase): + def test_outside_the_block_every_read_goes_to_libvirt(self): + p = proxy() + p._XMLDesc(0) + p._XMLDesc(0) + p.get_cap_xml() + p.get_cap_xml() + self.assertEqual((p.instance.reads, p.wvm.caps), ([0, 0], 2)) + + def test_inside_the_block_each_flags_value_is_read_once(self): + p = proxy() + with p.cached_reads(): + first = [p._XMLDesc(0), p._XMLDesc(1), p._XMLDesc(3)] + again = [p._XMLDesc(0), p._XMLDesc(1), p._XMLDesc(3)] + p.get_cap_xml() + p.get_cap_xml() + self.assertEqual(first, again) + self.assertEqual(p.instance.reads, [0, 1, 3]) + self.assertEqual(p.wvm.caps, 1) + + def test_the_block_ends_with_its_cache(self): + p = proxy() + with p.cached_reads(): + p._XMLDesc(0) + with p.cached_reads(): + p._XMLDesc(0) + p._XMLDesc(0) + self.assertEqual(p.instance.reads, [0, 0, 0]) + + def test_an_error_inside_the_block_still_ends_it(self): + p = proxy() + with self.assertRaises(ValueError), p.cached_reads(): + p._XMLDesc(0) + raise ValueError + p._XMLDesc(0) + self.assertEqual(p.instance.reads, [0, 0]) + + def test_a_failed_read_is_not_cached(self): + p = proxy() + with p.cached_reads(): + p.instance.fail = True + with self.assertRaises(RuntimeError): + p._XMLDesc(0) + p.instance.fail = False + p._XMLDesc(0) + self.assertEqual(p.instance.reads, [0]) + + def test_a_definition_change_drops_the_cache(self): + p = proxy() + with p.cached_reads(): + p._XMLDesc(3) + p._defineXML("") + p._XMLDesc(3) + self.assertEqual(p.instance.reads, [3, 3]) + + def test_blocks_do_not_nest(self): + p = proxy() + with p.cached_reads(), self.assertRaises(RuntimeError), p.cached_reads(): + pass + + def test_two_proxies_do_not_share_a_cache(self): + a, b = proxy(), proxy() + with a.cached_reads(): + a._XMLDesc(0) + b._XMLDesc(0) + b._XMLDesc(0) + self.assertEqual((a.instance.reads, b.instance.reads), ([0], [0, 0]))