Re: [libvirt] [PATCHv4 1/5] domifaddr: Implement the public API

On Tue, Aug 27, 2013 at 6:06 PM, Daniel P. Berrange <berrange(a)redhat.com&gt; wrote: > > On Sun, Aug 25, 2013 at 04:45:41AM +0530, Nehal J Wani wrote: > > 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 */ > > +}; > > s/int/unsigned int/ since there's no reason for prefix to ever > be negative. > > > + > > +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); > > Don't put a space between the function name & opening bracket. > > The double pluralization reads oddly to me. I'd suggest we > just name this 'virDomainInterfaceAddresses' > > ie remove the 's' from "Interfaces" here, and in all similarly > named functions in this series. > The double pluralization was chosen because there are multiple IP Addresses per interface and there are multiple interfaces. If we choose to use 'virDomainInterfaceAddresses', then it would look as if this function will report all the IP Addresses related to a single Interface only.