[PATCH 1/2] bhyve: implement domainInterfaceAddresses and domainGetHostname
Implement the domainInterfaceAddresses and domainGetHostname APIs. These APIs could use multiple sources of information, though for bhyve only the 'lease' source is supported. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_driver.c | 149 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index a7bd16704e..33a772ccdb 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1846,6 +1846,153 @@ bhyveDomainGetVcpuPinInfo(virDomainPtr domain, return ret; } + +static int +bhyveDomainInterfaceAddresses(virDomainPtr domain, + virDomainInterfacePtr **ifaces, + unsigned int source, + unsigned int flags) +{ + virDomainObj *vm = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(vm = bhyveDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainInterfaceAddressesEnsureACL(domain->conn, vm->def, source) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0) + goto cleanup; + + switch (source) { + case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE: + ret = virDomainNetDHCPInterfaces(vm->def, ifaces); + break; + + case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT: + case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP: + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("Unsupported IP address data source %1$d"), + source); + break; + + default: + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("Unknown IP address data source %1$d"), + source); + break; + } + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + +static int +bhyveDomainGetHostnameLease(virDomainObj *vm, + char **hostname) +{ + char macaddr[VIR_MAC_STRING_BUFLEN]; + g_autoptr(virConnect) conn = NULL; + virNetworkDHCPLeasePtr *leases = NULL; + int n_leases; + size_t i, j; + int ret = -1; + + if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0) + return -1; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + if (!(conn = virGetConnectNetwork())) + goto endjob; + + for (i = 0; i < vm->def->nnets; i++) { + g_autoptr(virNetwork) network = NULL; + virDomainNetDef *net = vm->def->nets[i]; + + if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK) + continue; + + virMacAddrFormat(&net->mac, macaddr); + network = virNetworkLookupByName(conn, net->data.network.name); + + if (!network) + goto endjob; + + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto endjob; + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + if (lease->hostname && !*hostname) + *hostname = g_strdup(lease->hostname); + + virNetworkDHCPLeaseFree(lease); + } + + VIR_FREE(leases); + + if (*hostname) + break; + } + + ret = 0; + endjob: + virDomainObjEndJob(vm); + return ret; +} + +static char * +bhyveDomainGetHostname(virDomainPtr domain, + unsigned int flags) +{ + virDomainObj *vm = NULL; + char *hostname = NULL; + + virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE | + VIR_DOMAIN_GET_HOSTNAME_AGENT, NULL); + + VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_GET_HOSTNAME_LEASE, + VIR_DOMAIN_GET_HOSTNAME_AGENT, + NULL); + + if (!(flags & VIR_DOMAIN_GET_HOSTNAME_AGENT)) + flags |= VIR_DOMAIN_GET_HOSTNAME_LEASE; + + if (!(vm = bhyveDomObjFromDomain(domain))) + return NULL; + + if (virDomainGetHostnameEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_GET_HOSTNAME_AGENT) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("getting hostname from agent is not supported")); + goto cleanup; + } else if (flags & VIR_DOMAIN_GET_HOSTNAME_LEASE) { + if (bhyveDomainGetHostnameLease(vm, &hostname) < 0) + goto cleanup; + } + + if (!hostname) { + virReportError(VIR_ERR_NO_HOSTNAME, + _("no hostname found for domain %1$s"), + vm->def->name); + goto cleanup; + } + + cleanup: + virDomainObjEndAPI(&vm); + return hostname; +} + static virHypervisorDriver bhyveHypervisorDriver = { .name = "bhyve", .connectURIProbe = bhyveConnectURIProbe, @@ -1912,6 +2059,8 @@ static virHypervisorDriver bhyveHypervisorDriver = { .domainGetVcpusFlags = bhyveDomainGetVcpusFlags, /* 12.1.0 */ .domainGetMaxVcpus = bhyveDomainGetMaxVcpus, /* 12.1.0 */ .domainGetVcpuPinInfo = bhyveDomainGetVcpuPinInfo, /* 12.1.0 */ + .domainInterfaceAddresses = bhyveDomainInterfaceAddresses, /* 12.3.0 */ + .domainGetHostname = bhyveDomainGetHostname, /* 12.3.0 */ }; -- 2.52.0
The current qemuDomainGetHostnameLease() implementation jumps to the "endjob" label when it finds hostname. As the label is defined after "ret = 0", qemuDomainGetHostnameLease() returns -1 in this case. That works because in qemuDomainGetHostname() it is used like that: ... if (qemuDomainGetHostnameLease(vm, &hostname) < 0) goto cleanup; ... cleanup: virDomainObjEndAPI(&vm); return hostname; } So it works, but it looks confusing. To make more consistent, use 'break' in qemuDomainGetHostnameLease() when the hostname is found, so it returns 0 in this case. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2d31d4aa31..b51f644241 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16471,7 +16471,7 @@ qemuDomainGetHostnameLease(virDomainObj *vm, VIR_FREE(leases); if (*hostname) - goto endjob; + break; } ret = 0; -- 2.52.0
In summary I'd say 'Fix' or 'Fix success return from'. The function isn't untidy, it's a real bug. Just that the only caller doesn't care. On Sat, Apr 11, 2026 at 15:35:13 +0200, Roman Bogorodskiy wrote:
The current qemuDomainGetHostnameLease() implementation jumps to the "endjob" label when it finds hostname. As the label is defined after "ret = 0", qemuDomainGetHostnameLease() returns -1 in this case.
That works because in qemuDomainGetHostname() it is used like that:
... if (qemuDomainGetHostnameLease(vm, &hostname) < 0) goto cleanup;
...
cleanup: virDomainObjEndAPI(&vm); return hostname; }
So it works, but it looks confusing. To make more consistent, use 'break' in qemuDomainGetHostnameLease() when the hostname is found, so it returns 0 in this case.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Fixes: a4a5827c9fc396f2b1848c1d393385535b106d1a
--- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2d31d4aa31..b51f644241 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16471,7 +16471,7 @@ qemuDomainGetHostnameLease(virDomainObj *vm, VIR_FREE(leases);
if (*hostname) - goto endjob; + break; }
The function also returns 0 if no hostname was found. I'd say that's semantically okay, but in such case the function ought to initialize the 'hostname' pointer to NULL before doing anything as in such case it leaves the value un-touched.
ret = 0;
With the commit message fixed and 'hostname' initialized to NULL: Reviewed-by: Peter Krempa <pkrempa@redhat.com> I suppose that you also want to do the same change in 1/2.
On Sat, Apr 11, 2026 at 15:35:12 +0200, Roman Bogorodskiy wrote:
Implement the domainInterfaceAddresses and domainGetHostname APIs. These APIs could use multiple sources of information, though for bhyve only the 'lease' source is supported.
These would go better if split to 2 patches.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_driver.c | 149 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index a7bd16704e..33a772ccdb 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c
[...]
+static int +bhyveDomainGetHostnameLease(virDomainObj *vm, + char **hostname) +{ + char macaddr[VIR_MAC_STRING_BUFLEN]; + g_autoptr(virConnect) conn = NULL; + virNetworkDHCPLeasePtr *leases = NULL; + int n_leases; + size_t i, j; + int ret = -1; + + if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0) + return -1; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + if (!(conn = virGetConnectNetwork())) + goto endjob; + + for (i = 0; i < vm->def->nnets; i++) { + g_autoptr(virNetwork) network = NULL; + virDomainNetDef *net = vm->def->nets[i]; + + if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK) + continue; + + virMacAddrFormat(&net->mac, macaddr); + network = virNetworkLookupByName(conn, net->data.network.name); + + if (!network) + goto endjob; + + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto endjob; + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + if (lease->hostname && !*hostname) + *hostname = g_strdup(lease->hostname); + + virNetworkDHCPLeaseFree(lease); + } + + VIR_FREE(leases); + + if (*hostname) + break; + } + + ret = 0; + endjob: + virDomainObjEndJob(vm); + return ret; +} + +static char * +bhyveDomainGetHostname(virDomainPtr domain, + unsigned int flags) +{ + virDomainObj *vm = NULL; + char *hostname = NULL; + + virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE | + VIR_DOMAIN_GET_HOSTNAME_AGENT, NULL);
IMO you shouldn't allow the _AGENT flag here ...
+ + VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_GET_HOSTNAME_LEASE, + VIR_DOMAIN_GET_HOSTNAME_AGENT, + NULL); + + if (!(flags & VIR_DOMAIN_GET_HOSTNAME_AGENT)) + flags |= VIR_DOMAIN_GET_HOSTNAME_LEASE; + + if (!(vm = bhyveDomObjFromDomain(domain))) + return NULL; + + if (virDomainGetHostnameEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_GET_HOSTNAME_AGENT) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("getting hostname from agent is not supported")); + goto cleanup;
... if you intend to not support it. At this point it doesn't matter that much (just one extra error message for the translators). I'm working on an API to probe supported flags though at which point it would matter as this would signal that the _AGENT option is supported.
+ } else if (flags & VIR_DOMAIN_GET_HOSTNAME_LEASE) { + if (bhyveDomainGetHostnameLease(vm, &hostname) < 0) + goto cleanup; + } + + if (!hostname) { + virReportError(VIR_ERR_NO_HOSTNAME, + _("no hostname found for domain %1$s"), + vm->def->name); + goto cleanup; + }
With the handling of _AGENT fixed: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Peter Krempa wrote:
On Sat, Apr 11, 2026 at 15:35:12 +0200, Roman Bogorodskiy wrote:
Implement the domainInterfaceAddresses and domainGetHostname APIs. These APIs could use multiple sources of information, though for bhyve only the 'lease' source is supported.
These would go better if split to 2 patches.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_driver.c | 149 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index a7bd16704e..33a772ccdb 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c
[...]
+static int +bhyveDomainGetHostnameLease(virDomainObj *vm, + char **hostname) +{ + char macaddr[VIR_MAC_STRING_BUFLEN]; + g_autoptr(virConnect) conn = NULL; + virNetworkDHCPLeasePtr *leases = NULL; + int n_leases; + size_t i, j; + int ret = -1; + + if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0) + return -1; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + if (!(conn = virGetConnectNetwork())) + goto endjob; + + for (i = 0; i < vm->def->nnets; i++) { + g_autoptr(virNetwork) network = NULL; + virDomainNetDef *net = vm->def->nets[i]; + + if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK) + continue; + + virMacAddrFormat(&net->mac, macaddr); + network = virNetworkLookupByName(conn, net->data.network.name); + + if (!network) + goto endjob; + + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto endjob; + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + if (lease->hostname && !*hostname) + *hostname = g_strdup(lease->hostname); + + virNetworkDHCPLeaseFree(lease); + } + + VIR_FREE(leases); + + if (*hostname) + break; + } + + ret = 0; + endjob: + virDomainObjEndJob(vm); + return ret; +} + +static char * +bhyveDomainGetHostname(virDomainPtr domain, + unsigned int flags) +{ + virDomainObj *vm = NULL; + char *hostname = NULL; + + virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE | + VIR_DOMAIN_GET_HOSTNAME_AGENT, NULL);
IMO you shouldn't allow the _AGENT flag here ...
+ + VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_GET_HOSTNAME_LEASE, + VIR_DOMAIN_GET_HOSTNAME_AGENT, + NULL); + + if (!(flags & VIR_DOMAIN_GET_HOSTNAME_AGENT)) + flags |= VIR_DOMAIN_GET_HOSTNAME_LEASE; + + if (!(vm = bhyveDomObjFromDomain(domain))) + return NULL; + + if (virDomainGetHostnameEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_GET_HOSTNAME_AGENT) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("getting hostname from agent is not supported")); + goto cleanup;
... if you intend to not support it.
At this point it doesn't matter that much (just one extra error message for the translators). I'm working on an API to probe supported flags though at which point it would matter as this would signal that the _AGENT option is supported.
I intend to support the agent too. Actually, I'm currently working on implementing a bunch of APIs using the Qemu guest agent in my private branch, and I decided to submit these two changes separately to reduce size of the agent series when it's ready.
+ } else if (flags & VIR_DOMAIN_GET_HOSTNAME_LEASE) { + if (bhyveDomainGetHostnameLease(vm, &hostname) < 0) + goto cleanup; + } + + if (!hostname) { + virReportError(VIR_ERR_NO_HOSTNAME, + _("no hostname found for domain %1$s"), + vm->def->name); + goto cleanup; + }
With the handling of _AGENT fixed:
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Tue, Apr 14, 2026 at 11:49:44 +0200, Roman Bogorodskiy wrote:
Peter Krempa wrote:
On Sat, Apr 11, 2026 at 15:35:12 +0200, Roman Bogorodskiy wrote:
Implement the domainInterfaceAddresses and domainGetHostname APIs. These APIs could use multiple sources of information, though for bhyve only the 'lease' source is supported.
These would go better if split to 2 patches.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_driver.c | 149 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index a7bd16704e..33a772ccdb 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c
[...]
+static int +bhyveDomainGetHostnameLease(virDomainObj *vm, + char **hostname) +{ + char macaddr[VIR_MAC_STRING_BUFLEN]; + g_autoptr(virConnect) conn = NULL; + virNetworkDHCPLeasePtr *leases = NULL; + int n_leases; + size_t i, j; + int ret = -1; + + if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0) + return -1; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + if (!(conn = virGetConnectNetwork())) + goto endjob; + + for (i = 0; i < vm->def->nnets; i++) { + g_autoptr(virNetwork) network = NULL; + virDomainNetDef *net = vm->def->nets[i]; + + if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK) + continue; + + virMacAddrFormat(&net->mac, macaddr); + network = virNetworkLookupByName(conn, net->data.network.name); + + if (!network) + goto endjob; + + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto endjob; + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + if (lease->hostname && !*hostname) + *hostname = g_strdup(lease->hostname); + + virNetworkDHCPLeaseFree(lease); + } + + VIR_FREE(leases); + + if (*hostname) + break; + } + + ret = 0; + endjob: + virDomainObjEndJob(vm); + return ret; +} + +static char * +bhyveDomainGetHostname(virDomainPtr domain, + unsigned int flags) +{ + virDomainObj *vm = NULL; + char *hostname = NULL; + + virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE | + VIR_DOMAIN_GET_HOSTNAME_AGENT, NULL);
IMO you shouldn't allow the _AGENT flag here ...
+ + VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_GET_HOSTNAME_LEASE, + VIR_DOMAIN_GET_HOSTNAME_AGENT, + NULL); + + if (!(flags & VIR_DOMAIN_GET_HOSTNAME_AGENT)) + flags |= VIR_DOMAIN_GET_HOSTNAME_LEASE; + + if (!(vm = bhyveDomObjFromDomain(domain))) + return NULL; + + if (virDomainGetHostnameEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_GET_HOSTNAME_AGENT) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("getting hostname from agent is not supported")); + goto cleanup;
... if you intend to not support it.
At this point it doesn't matter that much (just one extra error message for the translators). I'm working on an API to probe supported flags though at which point it would matter as this would signal that the _AGENT option is supported.
I intend to support the agent too.
Actually, I'm currently working on implementing a bunch of APIs using the Qemu guest agent in my private branch, and I decided to submit these two changes separately to reduce size of the agent series when it's ready.
Sure, then add the _AGENT flag to virCheckFlags once you'll be adding that support.
participants (2)
-
Peter Krempa -
Roman Bogorodskiy