On Wed, Aug 14, 2013 at 4:29 AM, Eric Blake <eblake@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);



Structs to be used:

/*In include/libvirt/libvirt.h.in */

struct _virNetworkDHCPLeases {
    long long expirytime;
    char *macaddr;
    char *ipaddr;
    char *hostname;
    char *clientid;
};

/*In src/remote/remote_protocol.x*/

struct remote_network_dhcp_leases {
    hyper expirytime;
    remote_nonnull_string macaddr;
    remote_nonnull_string ipaddr;
    remote_nonnull_string hostname;
    remote_nonnull_string clientid;
};

struct remote_network_get_dhcp_leases_args {
    remote_nonnull_network net;
    remote_string macaddr;
    unsigned int flags;
};

struct remote_network_get_dhcp_leases_ret {
    remote_network_dhcp_leases leases<>;
};


The following two blocks are required more than one, so should
we be introducing helper functions for them?

if ((VIR_STRDUP((*leases)[0].macaddr, leaseparams[1]) < 0) ||
    (VIR_STRDUP((*leases)[0].ipaddr, leaseparams[2]) < 0) ||
    (VIR_STRDUP((*leases)[0].hostname, leaseparams[3]) < 0) ||
    (VIR_STRDUP((*leases)[0].clientid, leaseparams[4]) < 0)) 
    goto cleanup;

for (i = 0; i < nleases; i++) {
     VIR_FREE(leases[i].macaddr);
     VIR_FREE(leases[i].ipaddr);
     VIR_FREE(leases[i].hostname);
     VIR_FREE(leases[i].clientid);
}


--
Nehal J. Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com