On Fri, Feb 06, 2026 at 14:52:37 +0100, Michal Privoznik via Devel wrote:
> From: Michal Privoznik <[email protected]>
>
> The virDomainInterfaceAddresses() API accepts @source argument,
> but since this is hyperv, we can't really use _SRC_LEASE (we
> didn't spawn any dnsmasq there), not _SRC_ARP. The only source
> that's more or less usable is _SRC_AGENT. Okay, there's no QEMU
> guest agent running, but hyperv has its own guest agent. In my
> testing (with Linux guest) I had to install 'hyperv' package and
> then enable 'hv_kvp_daemon.service'. After that,
> Msvm_GuestNetworkAdapterConfiguration struct [1] contained guest
> IP addresses.
>
> There's one caveat though: the interface name
> (virDomainInterface::name). We don't fetch that one even for
> hypervDomainGetXMLDesc() case. And there's no <target dev=''/>
> either nor device alias (v12.0.0-43-g4009126f17). So just put
> InstanceID there for now, which is this long path, with some
> UUIDs, e.g.:
>
>
> Microsoft:5C58E5F2-946E-490F-B81D-6E2A7328640D\C85554E0-2B3B-487C-A557-D230BFF5F9E6\
>
> But hey, at least it's unique.
>
> 1:
> https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/msvm-guestnetworkadapterconfiguration
> Resolves: https://issues.redhat.com/browse/RHEL-145306
> Signed-off-by: Michal Privoznik <[email protected]>
> ---
>
> Admittedly, the 'virsh domifaddr' output is misaligned after this, but
> I'll post a separate patch for that.
Disclaimer: I have absolutely no idea about the hyperv specific bits.
>
> src/hyperv/hyperv_driver.c | 169 ++++++++++++++++++++++++++
> src/hyperv/hyperv_wmi_generator.input | 12 ++
> 2 files changed, 181 insertions(+)
>
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index fbc76544df..c25cb91c13 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -3654,6 +3654,174 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int
> codeset,
> }
>
>
> +static int
> +hypervDomainInterfaceAddressesParseOne(hypervPrivate *priv,
> +
> Msvm_EthernetPortAllocationSettingData *net,
> + virDomainInterfacePtr *oneIfaceRet)
> +{
> + g_autoptr(virDomainInterface) iface = NULL;
> + g_autoptr(Msvm_SyntheticEthernetPortSettingData) sepsd = NULL;
> + g_autoptr(Msvm_GuestNetworkAdapterConfiguration) aConfig = NULL;
> + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER;
> + virMacAddr macAddr = { 0 };
> + char macAddrStr[VIR_MAC_STRING_BUFLEN] = { 0 };
> +
> + VIR_DEBUG("Parsing ethernet adapter '%s'", net->data->InstanceID);
> +
> + iface = g_new0(virDomainInterface, 1);
> + iface->name = g_strdup(net->data->InstanceID);
> +
> + if (hypervDomainDefParseEthernetAdapterMAC(priv, net, &macAddr) < 0)
> + return -1;
> +
> + iface->hwaddr = g_strdup(virMacAddrFormat(&macAddr, macAddrStr));
> +
> + virBufferAsprintf(&query,
> + "ASSOCIATORS OF {%s} "
> + "WHERE AssocClass=Msvm_SettingDataComponent "
> + "ResultClass=Msvm_GuestNetworkAdapterConfiguration",
> + net->data->Parent);
'query' is unused after this.
> +
> + if (hypervGetWmiClass(Msvm_GuestNetworkAdapterConfiguration, &aConfig) <
> 0)
> + return -1;
> +
> + if (aConfig) {
> + size_t nAddr = aConfig->data->IPAddresses.count;
> + size_t i;
> +
> + if (aConfig->data->Subnets.count != nAddr) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("the number of IP addresses (%1$zu) does not
> match the number of subnets (%2$d)"),
> + nAddr, aConfig->data->Subnets.count);
> + return -1;
> + }
> +
> + iface->addrs = g_new0(virDomainIPAddress, nAddr);
> + iface->naddrs = nAddr;
> + for (i = 0; i < nAddr; i++) {
> + const char *ipAddrStr = ((const char **)
> aConfig->data->IPAddresses.data)[i];
> + const char *subnetAddrStr = ((const char **)
> aConfig->data->Subnets.data)[i];
> + virDomainIPAddressPtr ip = &iface->addrs[i];
> + int family;
> + int prefix;
> +
> + VIR_DEBUG("ipAddrStr='%s' subnetAddrStr='%s'",
> + ipAddrStr, subnetAddrStr);
> +
> + ip->addr = g_strdup(ipAddrStr);
> + family = virSocketAddrNumericFamily(ipAddrStr);
> + if (family == AF_INET6) {
> + ip->type = VIR_IP_ADDR_TYPE_IPV6;
> + } else if (family == AF_INET) {
> + ip->type = VIR_IP_ADDR_TYPE_IPV4;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unknown IP address family of '%1$s'"),
> + ipAddrStr);
> + return -1;
> + }
> +
> + prefix = virSocketAddrSubnetToPrefix(subnetAddrStr);
> + if (prefix < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected subnet mask '%1$s'"),
> + subnetAddrStr);
> + return -1;
> + }
> + ip->prefix = prefix;
> + }
> + }
> +
> + *oneIfaceRet = g_steal_pointer(&iface);
> + return 0;
> +}
> +
> +
> +static ssize_t
> +hypervDomainInterfaceAddressesParseList(hypervPrivate *priv,
> +
> Msvm_EthernetPortAllocationSettingData *nets,
> + virDomainInterfacePtr **ifaces)
> +{
> + Msvm_EthernetPortAllocationSettingData *entry = nets;
> + size_t nifaces = 0;
> +
> + while (entry) {
> + virDomainInterfacePtr oneIface = NULL;
> +
> + if (hypervDomainInterfaceAddressesParseOne(priv, entry, &oneIface) <
> 0)
> + return -1;
If this fails ....
> +
> + if (oneIface)
> + VIR_APPEND_ELEMENT(*ifaces, nifaces, oneIface);
... after this was partially filled, the caller has no way of figuring
out how many entries to free. This function should cleanup after itself
internally.
> +
> + entry = entry->next;
> + }
> +
> + return nifaces;
> +}
> +
> +
> +static int
> +hypervDomainInterfaceAddresses(virDomainPtr dom,
> + virDomainInterfacePtr **ifaces,
> + unsigned int source,
> + unsigned int flags)
> +{
> + hypervPrivate *priv = NULL;
> + char uuid_string[VIR_UUID_STRING_BUFLEN];
> + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL;
> + g_autoptr(Msvm_VirtualSystemSettingData) virtualSystemSettingData = NULL;
> + g_autoptr(Msvm_EthernetPortAllocationSettingData) nets = NULL;
> + virDomainInterfacePtr *ifacesRet = NULL;
> + ssize_t ifacesRetCount = 0;
> + int ret = -1;
> +
> + virCheckFlags(0, -1);
> +
> + if (source != VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT) {
> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> + _("Unknown IP address data source %1$d"),
> + source);
> + return -1;
> + }
> +
> + if (hypervMsvmComputerSystemFromDomain(dom, &computerSystem) < 0)
> + return -1;
> +
> + priv = dom->conn->privateData;
> + virUUIDFormat(dom->uuid, uuid_string);
> +
> + if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv,
> + uuid_string,
> +
> &virtualSystemSettingData) < 0) {
> + return -1;
> + }
> +
> + if (hypervGetEthernetPortAllocationSD(priv,
> +
> virtualSystemSettingData->data->InstanceID,
> + &nets) < 0) {
> + return -1;
> + }
> +
> + ifacesRetCount = hypervDomainInterfaceAddressesParseList(priv, nets,
> &ifacesRet);
> + if (ifacesRetCount < 0)
> + goto cleanup;
> +
> + *ifaces = g_steal_pointer(&ifacesRet);
> + ret = ifacesRetCount;
> +
> + cleanup:
> + if (ret < 0) {
The only case when 'ret' is <0 here is ...
> + while (ifacesRetCount > 0) {
... when this is also -1
> + virDomainInterfaceFree(ifacesRet[--ifacesRetCount]);
> + }
So all of this is dead code, and also actually due to flawed error
reporting in 'hypervDomainInterfaceAddressesParseList' leaks the array
if it was partially filled.
> + VIR_FREE(ifacesRet);
> + }
> +
> + return ret;
> +}
> +
> +
> static virHypervisorDriver hypervHypervisorDriver = {
> .name = "Hyper-V",
> .connectOpen = hypervConnectOpen, /* 0.9.5 */
I'm wondering what's the point of 'query' above, if it's a leftover from
debugging then I'm okay with that.
With the cleanup of partially filled data fixed:
Reviewed-by: Peter Krempa <[email protected]>
