On Wed, Sep 11, 2013 at 10:35:04PM +0530, Nehal J Wani wrote:
On Wed, Sep 11, 2013 at 9:25 PM, Daniel P. Berrange
<berrange(a)redhat.com> wrote:
>
> On Wed, Sep 11, 2013 at 09:00:09PM +0530, Nehal J Wani wrote:
> > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> > index a47e33c..3885baf 100644
> > --- a/include/libvirt/libvirt.h.in
> > +++ b/include/libvirt/libvirt.h.in
> > @@ -2800,6 +2800,28 @@ int virConnectNumOfDefinedInterfaces
(virConnectPtr conn);
> > int virConnectListDefinedInterfaces (virConnectPtr conn,
> > char **const names,
> > int maxnames);
> > +
> > +typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
> > +typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
> > +struct _virNetworkDHCPLeases {
> > + long long expirytime;
> > + char *mac;
> > + char *ipaddr;
> > + char *hostname;
> > + char *clientid;
> > +};
>
> This is missing
>
> int type; /* virIPAddrType */
>
> since it can be IPv4 or IPv6 in general.
>
> Also we have no 'prefix' field here. Although the 'prefix' is
present in
> the network XML config, I think it would be useful to include it here
> too, for parity with the virDomainInterfaceIPAddress struct
>
The virNetworkDHCPLeases struct was made according to the lease
parameters returned by dnsmasq. Are you suggesting that libvirt might
shift to some other DHCP server in future? Also, in case the DHCP server
doesn't return the IP address type and/or prefix, in that case, should these
values be NULL, when returned to the user?
When doing API design we must *not* make assumptions about the current
impl that libvirt happens to have. We must always think of possible
future changes. The DHCP server doesn't need to return prefix - we have
that available in virNetworkDefPtr. As for IP address type, we can currently
hardcode that as IPV4, since we know dnsmasq only uses that. If we later
make use of DHCPv6 we can worry about distinguishing them at that point
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 :|