[libvirt] [PATCH v8 1/4] domifaddr: Implement the public APIs
John Ferlan
jferlan at redhat.com
Fri Feb 13 14:39:45 UTC 2015
On 01/25/2015 01:38 PM, Nehal J Wani wrote:
> Define helper function virDomainInterfaceFree, which allows
> the upper layer application to free the domain interface object
> conveniently.
>
> The API is going to provide multiple methods by flags, e.g.
> * Query guest agent
> * Parse DHCP lease file
>
> include/libvirt/libvirt-domain.h
> * Define virDomainInterfaceAddresses, virDomainInterfaceFree
> * Define structs virDomainInterface, virDomainIPAddress
>
> src/driver-hypervisor.h:
> * Define domainInterfaceAddresses
>
> src/libvirt-domain.c:
> * Implement virDomainInterfaceAddresses
> * Implement virDomainInterfaceFree
>
> src/libvirt_public.syms:
> * Export the new symbols
>
> Signed-off-by: Nehal J Wani <nehaljw.kkd1 at gmail.com>
> ---
> include/libvirt/libvirt-domain.h | 27 ++++++++
> src/driver-hypervisor.h | 5 ++
> src/libvirt-domain.c | 129 +++++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 6 ++
> 4 files changed, 167 insertions(+)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 4dbd7f5..1f832d0 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -3682,5 +3682,32 @@ typedef struct _virTypedParameter virMemoryParameter;
> */
> typedef virMemoryParameter *virMemoryParameterPtr;
>
> +typedef enum {
> + VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE = (1 << 0), /* Parse DHCP lease file */
> + VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT = (1 << 1), /* Query qemu guest agent */
> +} virDomainInterfaceAddressesFlags;
> +
> +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress;
> +typedef virDomainIPAddress *virDomainIPAddressPtr;
> +struct _virDomainInterfaceIPAddress {
> + int type; /* virIPAddrType */
> + char *addr; /* IP address */
> + unsigned int prefix; /* IP address prefix */
> +};
> +
> +typedef struct _virDomainInterface virDomainInterface;
> +typedef virDomainInterface *virDomainInterfacePtr;
> +struct _virDomainInterface {
> + char *name; /* interface name */
> + char *hwaddr; /* hardware address */
> + unsigned int naddrs; /* number of items in @addrs */
> + virDomainIPAddressPtr addrs; /* array of IP addresses */
> +};
> +
> +int virDomainInterfaceAddresses(virDomainPtr dom,
> + virDomainInterfacePtr **ifaces,
> + unsigned int flags);
> +
> +void virDomainInterfaceFree(virDomainInterfacePtr iface);
While I haven't followed this from the first RFC or taken the time to
look at all 8 patches, I'll assume this set of data has been "agreed
upon" as the relatively important set of IP Address and Network
Interface data. Looking at the .0 comments it seems the desire was for
some more flexibility to handle future possible issues - I guess my
comment there is - IPv4 isn't changing much and getting IPv6 adoption as
the norm seems to have been an uphill religious battle for quite a few
years - so as they say - change isn't very frequent thus perhaps this is
a "safe set" of data to collect/display
>
> #endif /* __VIR_LIBVIRT_DOMAIN_H__ */
<...snip...>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 492e90a..4149332 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11249,3 +11249,132 @@ virDomainFSInfoFree(virDomainFSInfoPtr info)
> VIR_FREE(info->devAlias[i]);
> VIR_FREE(info->devAlias);
> }
> +
> +/**
> + * virDomainInterfaceAddresses:
> + * @dom: domain object
> + * @ifaces: pointer to an array of pointers pointing to interface objects
> + * @flags: bitwise-OR of virDomainInterfaceAddressesFlags
> + *
> + * Return a pointer to the allocated array of pointers pointing to interfaces
> + * present in given domain along with their IP and MAC addresses. Note that
> + * single interface can have multiple or even 0 IP address.
> + *
> + * This API dynamically allocates the virDomainInterfacePtr struct based on
> + * how many interfaces domain @dom has, usually there's 1:1 correlation. The
> + * count of the interfaces is returned as the return value.
> + *
> + * In case @flags includes VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT, a configured
> + * guest agent is needed for successful return from this API. Moreover, if
> + * guest agent is used then the interface name is the one seen by guest OS.
> + * To match such interface with the one from @dom XML use MAC address or IP
> + * range.
> + *
> + * The lease-file parsing method returns the interface name of the form "vnetN",
> + * which is different from what guest agent returns (like ethN or emN), and
> + * since the MAC address from guest agent might be different with what @dom XML
> + * specifies, we have no way to convert it into the names present in @dom
> + * config. Hence, it is not recommended to mix the flag ..._AGENT with
> + * ..._LEASE as it may lead to ambiguous results because we cannot be sure if
> + * the name came from the agent or from the other method.
Reads to me like perhaps this should be an EITHER ... OR situation and
not an AND type allowance...
> + *
> + * If 0 is passed as @flags, libvirt will choose the best way, and won't
> + * include agent in it.
> + *
> + * @ifaces->name and @ifaces->hwaddr are never NULL.
> + *
> + * The caller *must* free @ifaces when no longer needed. Usual use case
> + * looks like this:
> + *
> + * virDomainInterfacePtr *ifaces = NULL;
> + * int ifaces_count = 0;
> + * size_t i, j;
> + * virDomainPtr dom = ... obtain a domain here ...;
> + *
> + * if ((ifaces_count = virDomainInterfaceAddresses(dom, &ifaces, 0)) < 0)
> + * goto cleanup;
> + *
> + * ... do something with returned values, for example:
> + * for (i = 0; i < ifaces_count; i++) {
> + * printf("name: %s", ifaces[i]->name);
> + * if (ifaces[i]->hwaddr)
> + * printf(" hwaddr: %s", ifaces[i]->hwaddr);
> + *
> + * for (j = 0; j < ifaces[i]->naddrs; j++) {
> + * virDomainIPAddressPtr ip_addr = ifaces[i]->addrs + j;
> + * printf("[addr: %s prefix: %d type: %d]",
> + * ip_addr->addr, ip_addr->prefix, ip_addr->type);
> + * }
> + * printf("\n");
> + * }
> + *
> + * cleanup:
> + * if (ifaces)
> + * for (i = 0; i < ifaces_count; i++)
> + * virDomainInterfaceFree(ifaces[i]);
> + * free(ifaces);
> + *
> + * Returns the number of interfaces on success, -1 in case of error.
> + */
> +int
> +virDomainInterfaceAddresses(virDomainPtr dom,
> + virDomainInterfacePtr **ifaces,
> + unsigned int flags)
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(dom, "ifaces=%p, flags=%x", ifaces, flags);
> +
> + virResetLastError();
> +
> + conn = dom->conn;
Need a virCheckDomainReturn() prior to deref
> +
> + virCheckNonNullArgGoto(ifaces, error);
Add a "*ifaces = NULL;" here.
John
> +
> + if ((flags & VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT)
> + && (conn->flags & VIR_CONNECT_RO)) {
> + virReportError(VIR_ERR_OPERATION_DENIED, "%s",
> + _("Cannot query guest agent in readonly mode"));
> + goto error;
> + }
> +
> + if (conn->driver->domainInterfaceAddresses) {
> + int ret;
> + ret = conn->driver->domainInterfaceAddresses(dom, ifaces, flags);
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> + error:
> + virDispatchError(dom->conn);
> + return -1;
> +}
> +
> +
> +/**
> + * virDomainInterfaceFree:
> + * @iface: an interface object
> + *
> + * Free the interface object. The data structure is
> + * freed and should not be used thereafter.
> + */
> +void
> +virDomainInterfaceFree(virDomainInterfacePtr iface)
> +{
> + size_t i;
> +
> + if (!iface)
> + return;
> +
> + VIR_FREE(iface->name);
> + VIR_FREE(iface->hwaddr);
> +
> + for (i = 0; i < iface->naddrs; i++)
> + VIR_FREE(iface->addrs[i].addr);
> + VIR_FREE(iface->addrs);
> +
> + VIR_FREE(iface);
> +}
More information about the libvir-list
mailing list