[libvirt] [for 1.2.6] Redundancy of virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC

[Nehal J Wani]

On Thu, Jun 26, 2014 at 4:58 PM, Peter Krempa <pkrempa at redhat.com> wrote:
> Hi,
>
> when reviewing the patch to add python bindings for the said APIs it
> occurred to me that the two APIs are so close in their prototypes and
> way of functioning that we could actually merge them into one.
>
> Both of those return a list of lease structures and the only difference
> is the presence of the @mac argument.
>
> We could unify those two APIs into one with the following signature:
>
> int
> virNetworkGetDHCPLeases(virNetworkPtr network,
>                         const char *mac,
>                         virNetworkDHCPLeasePtr **leases,
>                         unsigned int flags)
>
> And tweak the semantics of @mac where when the user passes NULL we'd
> return the complete unfiltered list.
>
> This would simplify our API and also the python bindings.
>
> If we decide this is a good idea (in time for the release) I'll post
> patches to flesh out the redundant parts.
>
> Peter
>


A long long while ago, there was already a discussion on this

References:
(i) http://www.redhat.com/archives/libvir-list/2013-July/msg01609.html
(ii) http://www.redhat.com/archives/libvir-list/2013-July/msg01623.html
(iii) http://www.redhat.com/archives/libvir-list/2013-July/msg01624.html

For TL;DR:

Message 1:

At a conceptual level, what you're after here is a list of all the IP,
mac address mappings of the virtual network. This information is useful
even outside the context of the hypervisor driver method you're working
on. So we should create formal APIs for exposing this, something like:

   virNetworkGetDHCPLeases(virNetworkPtr network,
                           virNetworkDHCPLeasePtr *leases,
                           unsigned int nleases);

And/or this

   virNetworkGetDHCPLeaseForMAC(virNetworkPtr network,
                                unsigned char *macaddr,
                                virNetworkDHCPLeasePtr lease);

and a corresponding  'virsh net-dhcp-leases <netname>' command

Daniel

Message 2:
for the api interface:

int
virNetworkGetDHCPLeases(virNetworkPtr network,
unsigned char *macaddr,
virNetworkDHCPLeasePtr *leases,
unsigned int nleases);

i think this is better. which returns all of the leases if no mac is specified.
otherwise just returns the lease of the network matches the mac.

osier

Message 3:

I rather prefer to see separate APIs for this job as I described. Sure
you could have an optional macaddr parameter, but I think it is nicer
to just have clear APIs for the "list many" vs "get one" tasks.

Regards,
Daniel




Regards,
Nehal J Wani