[libvirt] [PATCH v8 1/4] domifaddr: Implement the public APIs

[Daniel P. Berrange]

On Mon, Jan 26, 2015 at 12:08:46AM +0530, 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);
>  
>  #endif /* __VIR_LIBVIRT_DOMAIN_H__ */
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index a1d2a0a..174c7bd 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -1174,6 +1174,10 @@ typedef int
>                          unsigned int cellCount,
>                          unsigned int flags);
>  
> +typedef int
> +(*virDrvDomainInterfaceAddresses)(virDomainPtr dom,
> +                                  virDomainInterfacePtr **ifaces,
> +                                  unsigned int flags);
>  
>  typedef struct _virHypervisorDriver virHypervisorDriver;
>  typedef virHypervisorDriver *virHypervisorDriverPtr;
> @@ -1401,6 +1405,7 @@ struct _virHypervisorDriver {
>      virDrvConnectGetAllDomainStats connectGetAllDomainStats;
>      virDrvNodeAllocPages nodeAllocPages;
>      virDrvDomainGetFSInfo domainGetFSInfo;
> +    virDrvDomainInterfaceAddresses domainInterfaceAddresses;
>  };
>  
>  
> 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.

I can't help but have a nagging feeling this ambiguos return data means our
API design is wrong. Instead of having a bitwise OR of flags to select the
data source, should we perhaps use a straight enum, so the application always
specifies exactly which data source to use ?

eg an API that is

  enum {
    VIR_DOMAIN_INTERFACE_ADDRESSES_SOURCE_LEASES,
    VIR_DOMAIN_INTERFACE_ADDRESSES_SOURCE_AGENT,
    VIR_DOMAIN_INTERFACE_ADDRESSES_SOURCE_SNOOP,
  };

  virDomainInterfaceAddresses(virDomainPtr dom,
                              int source,
                              virDomainInterfacePtr **ifaces,
			      unsigned int flags);

so the caller always knows how to interpret the returned data for
mac addr andname. Leave the flags field unused, for future help
if needed.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|