[libvirt] [PATCHv6 1/5] domifaddr: Implement the public APIs

[Daniel P. Berrange]

On Sun, Sep 08, 2013 at 02:43:09PM +0800, Osier Yang wrote:
> On 06/09/13 23:18, Nehal J Wani wrote:
> 
> 
> The documentation for virDomainInterfaceAddresses is removed in this
> version.
> 
> >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 lease file of dnsmasq
> >   * DHCP snooping
> >
> >At this stage, it will only work with guest agent. Passing other
> >flags will result in error: "Method hasn't been implemented yet"
> 
> Hm, not sure if I like this.
> 
> >
> >include/libvirt/libvirt.h.in:
> >   * Define virDomainInterfaceAddresses, virDomainInterfaceFree
> >   * Define structs virDomainInterface, virDomainIPAddress
> >
> >python/generator.py:
> >   * Skip the auto-generation for virDomainInterfaceAddresses
> >     and virDomainInterfaceFree
> >
> >src/driver.h:
> >   * Define domainInterfaceAddresses
> >
> >src/libvirt.c:
> >   * Implement virDomainInterfaceAddresses
> >   * Implement virDomainInterfaceFree
> >
> >src/libvirt_public.syms:
> >   * Export the new symbols
> >
> >---
> >  include/libvirt/libvirt.h.in |  38 ++++++++++++
> >  python/generator.py          |   3 +
> >  src/driver.h                 |   6 ++
> >  src/libvirt.c                | 135 +++++++++++++++++++++++++++++++++++++++++++
> >  src/libvirt_public.syms      |   6 ++
> >  5 files changed, 188 insertions(+)
> >
> >diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> >index a47e33c..0995080 100644
> >--- a/include/libvirt/libvirt.h.in
> >+++ b/include/libvirt/libvirt.h.in
> >@@ -2044,6 +2044,44 @@ int                     virDomainGetInterfaceParameters (virDomainPtr dom,
> >                                                           virTypedParameterPtr params,
> >                                                           int *nparams, unsigned int flags);
> >+typedef enum {
> >+    VIR_DOMAIN_INTERFACE_ADDRESSES_SNOOP = (1 << 0), /* Snoop traffic */
> >+    VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE = (1 << 1), /* Parse DHCP lease file */
> >+    VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT = (1 << 2), /* Query qemu guest agent */
> 
> I don't see a good reason for expose the flags which are not
> supported yet. It will
> keep biting us until the implementations are done. Especially when you have
> documentations for them in virsh, API comments. (I see you exposed them as
> options in 4/5).

I tend to agree that we should only define flags once they are
supported by at least one driver.

Also if we put the virNetwork DHCP lease series in first, then we 
can be sure that this API will work when no flags are set too.

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 :|