Re: [libvirt] RFC: Introduce API to return configuration/state paths of the network driver

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(a)redhat.com&gt; 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

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.