From b2b2386db9c7707b6580a63aec8fdd710ff86fbd Mon Sep 17 00:00:00 2001 From: catborise Date: Tue, 3 Nov 2026 00:31:13 +0300 Subject: [PATCH] refactor(vrtManager): a VM's addresses are read once a page, not once an object wvmInstance kept the guest agent's and the ARP table's addresses from the first NIC it was asked about, and never read them again on that object; it carried a lease lookup that was never filled. The addresses are now one {MAC: (IPv4, IPv6)} read, kept like the domain XML only inside a cached_reads block: the VM page, which reads its NICs twice, asks the agent once (it may take its whole timeout), and a read outside a page's block is a new one. The agent's addresses still come before the ARP table's. --- instances/tests/test_live_guest.py | 5 +- vrtManager/instance.py | 80 +++++++------------- vrtManager/tests/test_instance_getters.py | 2 +- vrtManager/tests/test_interface_addresses.py | 14 +++- 4 files changed, 44 insertions(+), 57 deletions(-) diff --git a/instances/tests/test_live_guest.py b/instances/tests/test_live_guest.py index 449d93de..7bf97c1d 100644 --- a/instances/tests/test_live_guest.py +++ b/instances/tests/test_live_guest.py @@ -16,6 +16,7 @@ import unittest import libvirt import libvirt_qemu +from vrtManager.instance import agent_interfaces from computes.models import Compute from django.contrib.auth import get_user_model from django.test import TestCase @@ -267,9 +268,7 @@ class LiveGuestTestCase(TestCase): # The NIC is matched by MAC in the agent's report. Whether the guest has # an address depends on its network configuration, not on WebVirtCloud. mac = self.live(dom).xpath("devices/interface/mac/@address")[0] - proxy = inst.proxy - proxy.refresh_interface_addresses() - self.assertIn(mac, [nic["hwaddr"] for nic in proxy._ip_cache["qemuga"].values()]) + self.assertIn(mac, [nic["hwaddr"] for nic in agent_interfaces(inst.proxy.instance).values()]) def test_poweroff_shuts_the_guest_down(self): dom, inst = self.boot("goff") diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 327f6822..8d4fb39b 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -613,7 +613,6 @@ class wvmInstance(wvmConnect): def __init__(self, host, login, passwd, conn, vname, uuid=None): wvmConnect.__init__(self, host, login, passwd, conn) - self._ip_cache = None # A known VM is found by UUID: a name can be renamed or swapped # outside WebVirtCloud. No fallback to the name. self.instance = self.wvm.lookupByUUIDString(uuid) if uuid else self.get_instance(vname) @@ -787,61 +786,38 @@ class wvmInstance(wvmConnect): return range(1, int(pcpus + 1)) def get_interface_addresses(self, iface_mac): - if self._ip_cache is None: - self.refresh_interface_addresses() + """The running VM's addresses of the NIC with iface_mac: (IPv4, IPv6 + with prefix), from the guest agent when it has some, else from the + host's ARP table; (None, None) when neither has one. Read once in a + cached_reads block: the agent may take its whole timeout.""" + return self._cached("addresses", self._read_addresses).get(iface_mac, (None, None)) - qemuga = self._ip_cache["qemuga"] - arp = self._ip_cache["arp"] - leases = [] - - def extract_dom(info): - ipv4 = [] - ipv6 = [] - for addrs in info.values(): - if addrs["hwaddr"] != iface_mac: - continue - if not addrs["addrs"]: - continue - for addr in addrs["addrs"]: + def _read_addresses(self): + """{MAC: (IPv4 list, IPv6 list)}: each MAC's addresses from the first source that has some.""" + if self.get_status() != 1: + return {} + sources = [] + if self.is_agent_ready(): + with contextlib.suppress(libvirtError): + sources.append(agent_interfaces(self.instance)) + with contextlib.suppress(libvirtError): + sources.append(self.instance.interfaceAddresses(VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP)) + found = {} + for interfaces in sources: + here = {} + for interface in interfaces.values(): + if not interface["hwaddr"]: + continue # the loopback, a tunnel + ipv4, ipv6 = here.setdefault(interface["hwaddr"], ([], [])) + for addr in interface["addrs"] or []: if addr["type"] == 0: ipv4.append(addr["addr"]) elif addr["type"] == 1 and not str(addr["addr"]).startswith("fe80"): - ipv6.append(addr["addr"] + "/" + str(addr["prefix"])) - return ipv4, ipv6 - - def extract_lease(info): - ipv4 = [] - ipv6 = [] - if info["mac"] == iface_mac: - if info["type"] == 0: - ipv4.append(info["ipaddr"]) - elif info["type"] == 1: - ipv6.append(info["ipaddr"]) - return ipv4, ipv6 - - for ips in [qemuga] + leases + [arp]: - ipv4, ipv6 = extract_lease(ips) if "expirytime" in ips else extract_dom(ips) - if ipv4 or ipv6: - return ipv4, ipv6 - return None, None - - def _get_interface_addresses(self, source): - # ("Calling interfaceAddresses source=%s", source) - with contextlib.suppress(libvirtError): - return self.instance.interfaceAddresses(source) - return {} - - def refresh_interface_addresses(self): - self._ip_cache = {"qemuga": {}, "arp": {}} - - if self.get_status() != 1: - return - - if self.is_agent_ready(): - with contextlib.suppress(libvirtError): - self._ip_cache["qemuga"] = agent_interfaces(self.instance) - - self._ip_cache["arp"] = self._get_interface_addresses(VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP) + ipv6.append(f"{addr['addr']}/{addr['prefix']}") + for mac, (ipv4, ipv6) in here.items(): + if (ipv4 or ipv6) and mac not in found: + found[mac] = (ipv4, ipv6) + return found def get_net_devices(self, config=False): def networks(ctx): diff --git a/vrtManager/tests/test_instance_getters.py b/vrtManager/tests/test_instance_getters.py index 4b4d5b45..7488dc5a 100644 --- a/vrtManager/tests/test_instance_getters.py +++ b/vrtManager/tests/test_instance_getters.py @@ -31,7 +31,7 @@ XML = """ def proxy(): p = wvmInstance.__new__(wvmInstance) # no connection: only these calls - p._ip_cache = {"qemuga": {}, "arp": {}} + p._read_addresses = dict # no addresses p._XMLDesc = lambda flags: XML volumes = {"/iso/x.iso": ("x.iso", "iso")} diff --git a/vrtManager/tests/test_interface_addresses.py b/vrtManager/tests/test_interface_addresses.py index cb107650..48db89c3 100644 --- a/vrtManager/tests/test_interface_addresses.py +++ b/vrtManager/tests/test_interface_addresses.py @@ -27,7 +27,6 @@ AGENT = [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4", "ip-address def vm(agent): inst = wvmInstance.__new__(wvmInstance) - inst._ip_cache = None inst.instance = MagicMock() inst.instance.interfaceAddresses.side_effect = lambda source: {VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP: ARP}[source] patch.object(inst, "get_status", return_value=1).start() @@ -54,6 +53,19 @@ class InterfaceAddressesTestCase(unittest.TestCase): self.assertEqual([c.args for c in inst.instance.interfaceAddresses.call_args_list], [(VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP,)]) + def test_the_addresses_are_read_once_in_a_page_and_again_after_it(self): + inst = vm(agent=True) + with inst.cached_reads(): + inst.get_interface_addresses(MAC) + inst.get_interface_addresses("52:54:00:00:00:02") + self.assertEqual(self.agent.call_count, 1) + # outside a page's block, a new read: a NIC's address can change + inst.get_interface_addresses(MAC) + self.assertEqual(self.agent.call_count, 2) + + def test_a_mac_without_addresses_has_none(self): + self.assertEqual(vm(agent=False).get_interface_addresses("52:54:00:00:00:02"), (None, None)) + def test_an_agent_that_does_not_answer_leaves_the_arp_address(self): self.agent.side_effect = libvirtError("guest agent didn't respond to command within '10' seconds") self.assertEqual(vm(agent=True).get_interface_addresses(MAC), (["192.0.2.10"], []))