[libvirt] [PATCHv3 1/5] domifaddr: Implement the public API
Osier Yang
jyang at redhat.com
Fri Aug 23 09:52:28 UTC 2013
On 23/08/13 06:18, nehaljwani wrote:
> Define a new API virDomainInterfacesAddresses, which returns
> the address information of a running domain's interfaces(s).
> If no interface name is specified, it returns the information
> of all interfaces, otherwise it only returns the information
> of the specificed interface. The address information includes
> the MAC and IP addresses.
>
> The API is going to provide multiple methods by flags, e.g.
>
> * Query guest agent
> * Parse lease file of dnsmasq
> * DHCP snooping
>
> But at this stage, it will only work with guest agent, and flags
> won't be supported.
>
> include/libvirt/libvirt.h.in:
> * Define virDomainInterfacesAddresses
> * Define structs virDomainInterface, virDomainIPAddress
>
> python/generator.py:
> * Skip the auto-generation for virDomainInterfacesAddresses
>
> src/driver.h:
> * Define domainInterfacesAddresses
>
> src/libvirt.c:
> * Implement virDomainInterfacesAddresses
>
> src/libvirt_public.syms:
> * Export the new symbol
You introduced virDomainInterfaceFree in the new series,
the commit log should be changed to be consistent.
>
> ---
> include/libvirt/libvirt.h.in | 33 +++++++++++++
> python/generator.py | 3 ++
> src/driver.h | 6 +++
> src/libvirt.c | 110 +++++++++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 6 +++
> 5 files changed, 158 insertions(+)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index a47e33c..deb1e1f 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2044,6 +2044,39 @@ int virDomainGetInterfaceParameters (virDomainPtr dom,
> virTypedParameterPtr params,
> int *nparams, unsigned int flags);
>
> +typedef enum {
> + VIR_IP_ADDR_TYPE_IPV4,
> + VIR_IP_ADDR_TYPE_IPV6,
> +
> +#ifdef VIR_ENUM_SENTINELS
> + VIR_IP_ADDR_TYPE_LAST
> +#endif
> +} virIPAddrType;
> +
> +
> +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress;
> +typedef virDomainIPAddress *virDomainIPAddressPtr;
> +struct _virDomainInterfaceIPAddress {
> + int type; /* virIPAddrType */
> + char *addr; /* IP address */
> + 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 virDomainInterfacesAddresses (virDomainPtr dom,
> + virDomainInterfacePtr **ifaces,
> + unsigned int flags);
> +
> +void virDomainInterfaceFree (virDomainInterfacePtr iface);
> +
> /* Management of domain block devices */
>
> int virDomainBlockPeek (virDomainPtr dom,
> diff --git a/python/generator.py b/python/generator.py
> index 427cebc..4f3c8ee 100755
> --- a/python/generator.py
> +++ b/python/generator.py
> @@ -458,6 +458,7 @@ skip_impl = (
> 'virNodeGetMemoryParameters',
> 'virNodeSetMemoryParameters',
> 'virNodeGetCPUMap',
> + 'virDomainInterfacesAddresses',
> 'virDomainMigrate3',
> 'virDomainMigrateToURI3',
> )
> @@ -560,6 +561,8 @@ skip_function = (
> "virTypedParamsGetString",
> "virTypedParamsGetUInt",
> "virTypedParamsGetULLong",
> +
> + "virDomainInterfaceFree", #Only for convenience in C
Consider changing the comment into "# Only useful in C".
> )
>
> lxc_skip_function = (
> diff --git a/src/driver.h b/src/driver.h
> index be64333..8b6182e 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -518,6 +518,11 @@ typedef int
> unsigned int flags);
>
> typedef int
> +(*virDrvDomainInterfacesAddresses)(virDomainPtr dom,
> + virDomainInterfacePtr **ifaces,
> + unsigned int flags);
> +
> +typedef int
> (*virDrvDomainMemoryStats)(virDomainPtr domain,
> struct _virDomainMemoryStat *stats,
> unsigned int nr_stats,
> @@ -1238,6 +1243,7 @@ struct _virDriver {
> virDrvDomainInterfaceStats domainInterfaceStats;
> virDrvDomainSetInterfaceParameters domainSetInterfaceParameters;
> virDrvDomainGetInterfaceParameters domainGetInterfaceParameters;
> + virDrvDomainInterfacesAddresses domainInterfacesAddresses;
> virDrvDomainMemoryStats domainMemoryStats;
> virDrvDomainBlockPeek domainBlockPeek;
> virDrvDomainMemoryPeek domainMemoryPeek;
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 07a3fd5..2d4b0bf 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -8643,6 +8643,97 @@ error:
> return -1;
> }
>
> + /**
> + * virDomainInterfacesAddresses:
> + * @dom: domain object
> + * @ifaces: array of @dom interfaces
Not correct. Note that you changed the API interface in this series.
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * Return an array of interfaces present in given domain along with
Like above, this should be changed too.
> + * 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 elements allocated is stored in
> + * @ifaces_count.
In this new implementation, there is now @ifaces_count, the count
of the interfaces is returned as the return value.
> + *
> + * Note that for some hypervisors, 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.
> + *
> + * @ifaces->name is never NULL, @ifaces->hwaddr might be 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 = virDomainInterfacesAddresses(dom, &ifaces, 0)) < 0)
The argument type is "virDomainInterfacePtr **" now. So you
have an error here.
> + * goto cleanup;
> + *
> + * ... do something with returned values, for example:
> + * for (i = 0; i < ifaces_count; i++) {
> + * printf("name: %s", ifaces[i].name);
And you will want ifaces[i]->name instead. Similiar for other memebers
accessing.
> + * 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:
> + * for (i = 0; i < ifaces_count; i++) {
> + * if(ifaces[i])
> + * virDomainInterfaceFree(ifaces[i]);
Indention problem.
> + * }
> + * free(ifaces);
> + *
> + * Returns 0 on success, -1 in case of error.
> + */
> +int
> +virDomainInterfacesAddresses(virDomainPtr dom,
> + virDomainInterfacePtr **ifaces,
> + unsigned int flags)
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(dom, "ifaces=%p, flags=%x", ifaces, flags);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECTED_DOMAIN(dom)) {
> + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + goto error;
> + }
> +
> + conn = dom->conn;
> +
> + virCheckNonNullArgGoto(ifaces, error);
> +
> + if (conn->driver->domainInterfacesAddresses) {
> + int ret;
> + ret = conn->driver->domainInterfacesAddresses(dom, ifaces, flags);
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> + virDispatchError(dom ? dom->conn : NULL);
> + return -1;
> +}
> +
> /**
> * virDomainMemoryStats:
> * @dom: pointer to the domain object
> @@ -21961,3 +22052,22 @@ 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;
> + VIR_FREE((*iface).name);
Why not "iface->name"? also you need to check if "iface" is NULL
or not, otherwise it could cause the crash, unless the user checking
it himself before calling virDomainInterfaceFree, but that says you
introduce a bad api then.
> + VIR_FREE((*iface).hwaddr);
> + for (i = 0; i < (*iface).naddrs; i++)
> + VIR_FREE((*iface).addrs[i].addr);
> + VIR_FREE((*iface).addrs);
> +}
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index bbdf78a..df1c166 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -634,4 +634,10 @@ LIBVIRT_1.1.1 {
> virDomainSetMemoryStatsPeriod;
> } LIBVIRT_1.1.0;
>
> +LIBVIRT_1.1.2 {
> + global:
> + virDomainInterfacesAddresses;
> + virDomainInterfaceFree;
> +} LIBVIRT_1.1.1;
> +
> # .... define new API here using predicted next version number ....
More information about the libvir-list
mailing list