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

[Osier Yang]

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 at redhat.com> 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