<div dir="ltr"><br><br><br>On Wed, Aug 14, 2013 at 4:29 AM, Eric Blake <<a href="mailto:eblake@redhat.com" target="_blank">eblake@redhat.com</a>> wrote:<br>><br>> On 08/13/2013 04:48 PM, Eric Blake wrote:<br>><br>

> >>    virNetworkGetDHCPLeaseForMAC(virNetworkPtr network,<br>
> >>                                 unsigned char *macaddr,<br>> ><br>> > I personally think the public API should stick to stringized<br>> > representations.  Yes, it's less friendly to machine code, but<br>


> > internally, our src/util/virmacaddr.h helps transfer between stringized<br>> > and binary.  Furthermore, we already have other API that operates on<br>> > stringized versions, but none that operates on raw (see<br>


> > virDomainSetInterfaceParameters).  Knowing what representation we are<br>> > targetting impacts how this API will look.<br>><br>> For comparison, look at the API for virDomainLookupByUUID (takes a raw<br>


> uuid - fixed number of bytes, unsigned char* argument) vs.<br>> virDomainLookupByUUIDString (takes a stringized uuid, char* argument).<br>> If efficiency were a concern, then I'd propose that we have two API:<br>


><br>> virNetworkGetDHCPLeaseForMAC(virNetworkPtr network,<br>>                              unsigned char *macaddr, ...)<br>><br>> virNetworkGetDHCPLeaseForMACString(virNetworkPtr network,<br>>                                    char *macaddr, ...)<br>


><br>> But I don't think efficiency is a concern, and rather than add a new API<br>> that has to have dual forms, I'd rather stick with the shorter name but<br>> using the stringized form of MAC.<br>><br>


> --<br>> Eric Blake   eblake redhat com    +1-919-301-3266<br>> Libvirt virtualization library <a href="http://libvirt.org" target="_blank">http://libvirt.org</a><br>><br><br><div>I would like to see the API as:</div>

<div><div><br></div><div><div>/**</div><div> * virNetworkGetDHCPLeases:</div><div> * @network: pointer to network object</div><div> * @macaddr: MAC Address of an interface</div>
<div> * @leases: pointer to an array of structs which will hold the leases</div><div> * @nleases: number of leases in output</div><div> * @flags: extra flags, not used yet, so callers should always pass 0</div><div> *</div>

<div> * The API <span style="font-family:arial,sans-serif;font-size:13px">returns the leases of all interfaces </span><span style="font-family:arial,sans-serif;font-size:13px">by default, and </span><span style="font-family:arial,sans-serif;font-size:13px">if</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"> * @macaddr </span><span style="font-size:13px;font-family:arial,sans-serif">is specified,.</span><span style="font-size:13px;font-family:arial,sans-serif">only the </span><span style="font-size:13px;font-family:arial,sans-serif">lease of the interface which</span></div>
<div><span style="font-size:13px;font-family:arial,sans-serif"> * matches the </span><span style="font-size:13px;font-family:arial,sans-serif">@macaddr is returned.</span></div><div> *</div><div> * Returns number of leases on success, -1 otherwise</div>
<div>
 */</div><div><div>int virNetworkGetDHCPLeases(virNetworkPtr network,</div><div>                            const char *macaddr,</div><div>                            virNetworkDHCPLeasesPtr *leases,</div><div>                            int *nleases,</div>

<div>                            unsigned int flags);</div></div><div><br></div><div><br></div><div><br></div><div>Structs to be used:</div><div><br></div><div>/*In include/libvirt/<a href="http://libvirt.h.in">libvirt.h.in</a> */</div>
<div><br></div><div><div>struct _virNetworkDHCPLeases {<br></div><div>
<div>    long long expirytime;</div><div>    char *macaddr;</div><div>    char *ipaddr;</div><div>    char *hostname;</div><div>    char *clientid;</div><div>};</div></div><div><br></div><div>/*In src/remote/remote_protocol.x*/</div>
<div><br></div><div><div>struct remote_network_dhcp_leases {</div><div>    hyper expirytime;</div><div>    remote_nonnull_string macaddr;</div><div>    remote_nonnull_string ipaddr;</div><div>    remote_nonnull_string hostname;</div>
<div>    remote_nonnull_string clientid;</div><div>};</div><div><br></div><div>struct remote_network_get_dhcp_leases_args {</div><div>    remote_nonnull_network net;</div><div>    remote_string macaddr;</div><div>    unsigned int flags;</div>
<div>};</div><div><br></div><div>struct remote_network_get_dhcp_leases_ret {</div><div>    remote_network_dhcp_leases leases<>;</div><div>};</div></div><div><br></div><div><br></div><div>The following two blocks are required more than one, so should</div>
<div>we be introducing helper functions for them?</div><div><br></div><div><div>if ((VIR_STRDUP((*leases)[0].macaddr, leaseparams[1]) < 0) ||</div><div>    (VIR_STRDUP((*leases)[0].ipaddr, leaseparams[2]) < 0) ||</div>
<div>    (VIR_STRDUP((*leases)[0].hostname, leaseparams[3]) < 0) ||</div><div>    (VIR_STRDUP((*leases)[0].clientid, leaseparams[4]) < 0)) </div><div>    goto cleanup;</div></div><div><br></div><div>for (i = 0; i < nleases; i++) {</div>
<div>     VIR_FREE(leases[i].macaddr);</div><div>     VIR_FREE(leases[i].ipaddr);</div><div>     VIR_FREE(leases[i].hostname);</div><div>     VIR_FREE(leases[i].clientid);</div><div>}</div><div><br></div><div><br></div>--<br>
Nehal J. Wani<br>UG3, BTech CS+MS(CL)<br>

IIIT-Hyderabad<br><a href="http://commandlinewani.blogspot.com" target="_blank">http://commandlinewani.blogspot.com</a></div></div></div></div>