On 15/08/13 19:32, Daniel P. Berrange wrote:
On Wed, Aug 14, 2013 at 04:57:51AM +0530, Nehal J. Wani wrote:
On Wed, Aug 14, 2013 at 4:29 AM, Eric Blake <eblake@xxxxxxxxxx> wrote:
On 08/13/2013 04:48 PM, Eric Blake wrote:
virNetworkGetDHCPLeaseForMAC(virNetworkPtr network,
unsigned char *macaddr,
I personally think the public API should stick to stringized
representations. Yes, it's less friendly to machine code, but
internally, our src/util/virmacaddr.h helps transfer between stringized
and binary. Furthermore, we already have other API that operates on
stringized versions, but none that operates on raw (see
virDomainSetInterfaceParameters). Knowing what representation we are
targetting impacts how this API will look.
For comparison, look at the API for virDomainLookupByUUID (takes a raw
uuid - fixed number of bytes, unsigned char* argument) vs.
virDomainLookupByUUIDString (takes a stringized uuid, char* argument).
If efficiency were a concern, then I'd propose that we have two API:
virNetworkGetDHCPLeaseForMAC(virNetworkPtr network,
unsigned char *macaddr, ...)
virNetworkGetDHCPLeaseForMACString(virNetworkPtr network,
char *macaddr, ...)
But I don't think efficiency is a concern, and rather than add a new API
that has to have dual forms, I'd rather stick with the shorter name but
using the stringized form of MAC.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
I would like to see the API as:
/**
* virNetworkGetDHCPLeases:
* @network: pointer to network object
* @macaddr: MAC Address of an interface
* @leases: pointer to an array of structs which will hold the leases
* @nleases: number of leases in output
* @flags: extra flags, not used yet, so callers should always pass 0
*
* The API returns the leases of all interfaces by default, and if
* @macaddr is specified,.only the lease of the interface which
* matches the @macaddr is returned.
*
* Returns number of leases on success, -1 otherwise
*/
int virNetworkGetDHCPLeases(virNetworkPtr network,
const char *macaddr,
virNetworkDHCPLeasesPtr *leases,
int *nleases,
unsigned int flags);
In common with my feedback on the other IP addr API, I'd
suggest that we should use virNetworkDHCPLeasesPtr **leases,
e an array of pointers to objects, not an array of objects.
And remove the nleases parameter - just use the return value
Agreed.
Also I'd like to see two separate APIs here - one to get a list
of all leases, and one to get the single lease for a specific
MAC address.
I'm wondering what's the principle of introducing a new API now,
previously in cases similar with this, we would like have a single
API instead of two, I.E. make the API more general enough if
the intended purposes could be aggregated into one. Did
we see the simple enough APIs are more popular for upper
apps, and change to have more simple API instead?
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list