[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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

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.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]