[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/4] net-dhcp-leases: Implement the public APIs



On Wed, Sep 11, 2013 at 9:25 PM, Daniel P. Berrange <berrange 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?


> > +
> > +void virNetworkDHCPLeaseFree(virNetworkDHCPLeasesPtr lease);
> > +
> > +int virNetworkGetDHCPLeases(virNetworkPtr network,
> > +                            virNetworkDHCPLeasesPtr **leases,
> > +                            unsigned int flags);
> > +
> > +int virNetworkGetDHCPLeaseForMAC(virNetworkPtr network,
> > +                                 const char *mac,
> > +                                 virNetworkDHCPLeasesPtr leases,
>
> This should be '**leases' as with the other method. While our dnsmasq
> setup only supports a single lease, for IPv4, that is not suitably
> generic. We may well need to support DHCP v6 in the future which would
> mean we had multiple leases to return per MAC address.
>
> > +                                 unsigned int flags);
> > +
> >  /*
> >   * virConnectListAllInterfaces:
> >   *
>
> 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 :|




-- 
Nehal J Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]