diff --git a/vrtManager/connection.py b/vrtManager/connection.py index c52c024b..1c98c3dd 100644 --- a/vrtManager/connection.py +++ b/vrtManager/connection.py @@ -9,6 +9,7 @@ from urllib.parse import quote import libvirt from django.conf import settings +from django.utils.functional import cached_property from libvirt import libvirtError from vrtManager import util from vrtManager.rwlock import ReadWriteLock @@ -397,9 +398,19 @@ class wvmConnect(object): def is_qemu(self): return self.wvm.getURI().startswith("qemu") + @cached_property + def capabilities_xml(self): + """The host's capabilities, read once per object (a request): most getters below parse them""" + return self.wvm.getCapabilities() + + @cached_property + def dom_cap_xmls(self): + """The domain capabilities read so far, by emulator, arch, machine and domain type""" + return {} + def get_cap_xml(self): """Return xml capabilities""" - return self.wvm.getCapabilities() + return self.capabilities_xml def get_dom_cap_xml(self, arch, machine): """ Return domain capabilities xml""" @@ -409,7 +420,10 @@ class wvmConnect(object): machine_types = self.get_machine_types(arch) if not machine or machine not in machine_types: machine = "pc" if "pc" in machine_types else machine_types[0] - return self.wvm.getDomainCapabilities(emulatorbin, arch, machine, virttype) + key = (emulatorbin, arch, machine, virttype) + if key not in self.dom_cap_xmls: + self.dom_cap_xmls[key] = self.wvm.getDomainCapabilities(*key) + return self.dom_cap_xmls[key] def get_capabilities(self, arch): """ Host Capabilities for specified architecture """ diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 67ff8cb0..0f4e44a7 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -491,11 +491,12 @@ class wvmInstance(wvmConnect): def cached_reads(self): """ Within the block the domain XML (per flags value), the VM's state, - the host's capabilities and info, and each disk's volume are read - from libvirt once, for pages that only read: a VM page reads the XML - in some 30 getters and each disk in two lists. 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. + the host's info and each disk's volume are read from libvirt once + (the host's capabilities are read once per object anyway), for pages + that only read: a VM page reads the XML in some 30 getters and each + disk in two lists. 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") @@ -521,13 +522,6 @@ class wvmInstance(wvmConnect): self._read_cache.clear() return self.wvm.defineXML(xml) - def get_cap_xml(self): - return self._cached("capabilities", super().get_cap_xml) - - def get_dom_cap_xml(self, arch, machine): - read = super().get_dom_cap_xml - return self._cached(("domain capabilities", arch, machine), lambda: read(arch, machine)) - def _host_info(self): return self._cached("host info", self.wvm.getInfo) diff --git a/vrtManager/test_capabilities_once.py b/vrtManager/test_capabilities_once.py new file mode 100644 index 00000000..97b9fa99 --- /dev/null +++ b/vrtManager/test_capabilities_once.py @@ -0,0 +1,73 @@ +"""A wvmConnect reads the host's capabilities once, and its domain +capabilities once per emulator, arch, machine and domain type: the create +page's getters parse them some 60 times. An object lives for one request.""" + +import unittest + +from django.conf import settings + +if not settings.configured: + settings.configure(MAC_OUI="52:54:10") + +from vrtManager.connection import wvmConnect + +CAPABILITIES = """x86_64 +hvm/usr/bin/qemu-kvm +pc-q35-9.0q35pc +""" + + +class FakeConn: + def __init__(self): + self.reads = [] + + def getCapabilities(self): + self.reads.append("capabilities") + return CAPABILITIES + + def getDomainCapabilities(self, emulator, arch, machine, virttype): + self.reads.append((emulator, arch, machine, virttype)) + return "" + + +def connection(): + conn = wvmConnect.__new__(wvmConnect) # no libvirt connection: only these calls + conn.wvm = FakeConn() + return conn + + +class CapabilitiesOnceTestCase(unittest.TestCase): + def test_the_capabilities_are_read_once_for_all_getters(self): + conn = connection() + for _ in range(2): + conn.get_emulator("x86_64") + conn.get_machine_types("x86_64") + conn.get_hypervisors_domain_types() + conn.get_cap_xml() + self.assertEqual(conn.wvm.reads, ["capabilities"]) + + def test_domain_capabilities_are_read_once_per_machine(self): + conn = connection() + for _ in range(2): + conn.get_dom_cap_xml("x86_64", "q35") + conn.get_dom_cap_xml("x86_64", "pc") + conn.get_dom_cap_xml("x86_64", "unknown") # falls back to pc + self.assertEqual( + conn.wvm.reads, + ["capabilities", ("/usr/bin/qemu-kvm", "x86_64", "q35", "kvm"), ("/usr/bin/qemu-kvm", "x86_64", "pc", "kvm")], + ) + + def test_two_objects_do_not_share_their_reads(self): + a, b = connection(), connection() + a.get_cap_xml() + b.get_cap_xml() + self.assertEqual((a.wvm.reads, b.wvm.reads), (["capabilities"], ["capabilities"])) + + def test_a_failed_read_is_not_kept(self): + conn = connection() + conn.wvm.getCapabilities = lambda: (_ for _ in ()).throw(RuntimeError("libvirt error")) + with self.assertRaises(RuntimeError): + conn.get_cap_xml() + conn.wvm = FakeConn() + conn.get_cap_xml() + self.assertEqual(conn.wvm.reads, ["capabilities"]) diff --git a/vrtManager/test_read_cache.py b/vrtManager/test_read_cache.py index ea5e1aab..b6b96760 100644 --- a/vrtManager/test_read_cache.py +++ b/vrtManager/test_read_cache.py @@ -1,10 +1,10 @@ """wvmInstance.cached_reads(): inside the block the domain XML (per flags), -the VM's state, the host capabilities and info, and each disk's volume are -read from libvirt once; outside it every read goes to libvirt, as code that -changes the VM needs.""" +the VM's state, the host info and each disk's volume are read from libvirt +once; outside it every read goes to libvirt, as code that changes the VM +needs. The host capabilities are read once per object.""" import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock from django.conf import settings @@ -12,7 +12,6 @@ if not settings.configured: settings.configure(MAC_OUI="52:54:10") from libvirt import libvirtError -from vrtManager.connection import wvmConnect from vrtManager.instance import wvmInstance @@ -61,8 +60,8 @@ class CachedReadsTestCase(unittest.TestCase): p = proxy() p._XMLDesc(0) p._XMLDesc(0) - p.get_cap_xml() - p.get_cap_xml() + p._host_info() + p._host_info() self.assertEqual((p.instance.reads, p.wvm.caps), ([0, 0], 2)) def test_inside_the_block_each_flags_value_is_read_once(self): @@ -70,8 +69,8 @@ class CachedReadsTestCase(unittest.TestCase): 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() + p._host_info() + p._host_info() self.assertEqual(first, again) self.assertEqual(p.instance.reads, [0, 1, 3]) self.assertEqual(p.wvm.caps, 1) @@ -135,17 +134,6 @@ class CachedReadsTestCase(unittest.TestCase): p.get_status() self.assertEqual(p.instance.reads, ["info", "info"]) - def test_domain_capabilities_are_read_once_per_arch_and_machine(self): - p = proxy() - with patch.object(wvmConnect, "get_dom_cap_xml", return_value="") as read: - with p.cached_reads(): - for _ in range(2): - p.get_dom_cap_xml("x86_64", "q35") - p.get_dom_cap_xml("x86_64", "pc") - p.get_dom_cap_xml("x86_64", "q35") - self.assertEqual([c.args for c in read.call_args_list], [("x86_64", "q35"), ("x86_64", "pc"), ("x86_64", "q35")]) - - def volume_proxy(paths): """A proxy whose host has volumes at paths (pool "pool", size 10, used 4).""" p = proxy()