On Wed, Oct 02, 2013 at 09:13:59AM +0100, Daniel P. Berrange wrote: > On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote: > > On 09/26/2013 02:08 AM, Nehal J Wani wrote: > > > Introduce 3 new APIs, virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC > > > and virNetworkDHCPLeaseFree. > > > > > > * virNetworkGetDHCPLeases: returns the dhcp leases information for a given > > > virtual network. > > > > > > For DHCPv4, the information includes: > > > - Expirytime > > > - MAC Address > > > - IPv4 address (with type and prefix) > > > - Hostname (can be NULL) > > > - Client ID (can be NULL) > > > > > > For DHCPv6, the information includes > > > - Expirytime > > > - IAID > > > - IPv6 address (with type and prefix) > > > - Hostname (can be NULL) > > > - Client DUID > > > > > > * virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for a > > > given virtual network and specified MAC Address. > > > > > > * virNetworkDHCPLeaseFree: allows the upper layer application to free the > > > network interface object conveniently. > > > > > > There is no support for flags, so user is expected to pass 0 for > > > both the APIs. > > > > > > > +typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases; > > > +typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr; > > > +struct _virNetworkDHCPLeases { > > > + long long expirytime; /* Seconds since epoch */ > > > + union { > > > + char *mac; /* MAC address */ > > > + unsigned long iaid; /* Identity association identifier (IAID) */ > > > + } id; > > > > I'm not sure I like iaid - the whole point of this interface was to > > return IP addresses associated with a MAC. Either the iaid is important > > and deserves a separate field, or all we care about is the MAC address. > > Not to mention that you didn't document which leg of the id union is > > valid based on the type discriminator. > > Agreed, we want the MAC address to be unconditionally available > here. IMHO the IAID is not something we care about exposing. That > is a impl detail of the DHCP comms protocol that is not useful > to people outside. Actually, I see that we already expose the IAID concept to the user in the XML for networks <ip family="ipv6" address="2001:db8:ca2:2::1" prefix="64" > <dhcp> <host name="paul" ip="2001:db8:ca2:2:3::1" /> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2" /> <host id="0:3:0:1:0:16:3e:11:22:33" name="ralph" ip="2001:db8:ca2:2:3::3" /> <host id="0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63" name="badbob" ip="2001:db8:ca2:2:3::4" /> </dhcp> </ip> The 'id' value is mapping to the IAID. So it is sensible to expose this field in the virNetworkDHCPLease struct, but as a separate field, not as a union. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list