On 06/26/14 17:29, Ján Tomko wrote:
On 06/26/2014 04:51 PM, Peter Krempa wrote:
Instead of maintaining two very similar APIs, add the "@mac" parameter to virNetworkGetDHCPLeases and kill virNetworkGetDHCPLeasesForMAC. Both of those functions would return data the same way, so making @mac an optional filter simplifies a lot of stuff. --- daemon/remote.c | 69 +----------------------------------------- include/libvirt/libvirt.h.in | 6 +--- src/driver.h | 8 +---- src/libvirt.c | 70 ++++++------------------------------------- src/libvirt_public.syms | 1 - src/network/bridge_driver.c | 69 +++++++++++------------------------------- src/remote/remote_driver.c | 71 ++------------------------------------------ src/remote/remote_protocol.x | 20 ++----------- src/remote_protocol-structs | 15 +--------- tools/virsh-network.c | 5 +--- 10 files changed, 35 insertions(+), 299 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 566f984..49c9d16 100644 --- a/src/libvirt.c +++ b/src/libvirt.c
@@ -21110,65 +21117,6 @@ virNetworkGetDHCPLeases(virNetworkPtr network, return -1; }
-/** - * virNetworkGetDHCPLeasesForMAC: - * @network: Pointer to network object - * @mac: ASCII formatted MAC address of an interface - * @leases: Pointer to a variable to store the array containing details on - * obtained leases, or NULL if the list is not required (just returns - * number of leases). - * @flags: extra flags, not used yet, so callers should always pass 0 - * - * The API fetches leases info of the interface which matches with the - * given @mac. There can be multiple leases for a single @mac because this - * API supports DHCPv6 too. - * - * Returns the number of leases found or -1 and sets @leases to NULL in case of - * error. On success, the array stored into @leases is guaranteed to have an - * extra allocated element set to NULL but not included in the return count, - * to make iteration easier. The caller is responsible for calling - * virNetworkDHCPLeaseFree() on each array element, then calling free() on @leases. - * - * See virNetworkGetDHCPLeases() for more details on list contents. - */ -int -virNetworkGetDHCPLeasesForMAC(virNetworkPtr network, - const char *mac, - virNetworkDHCPLeasePtr **leases, - unsigned int flags) -{ - virConnectPtr conn; - - VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x", - network, mac, leases, flags);
You should add mac to the debug message at the start of the other API.
- - virResetLastError(); - - if (leases) - *leases = NULL; - - virCheckNonNullArgGoto(mac, error); - - virCheckNetworkReturn(network, -1);
--- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7614,6 +7615,7 @@ remoteNetworkGetDHCPLeases(virNetworkPtr net, remoteDriverLock(priv);
make_nonnull_network(&args.net, net); + args.mac = mac == NULL ? NULL : (char **) &mac;
Nit: mac ? (char **) &mac : NULL would be IMO nicer.
args.flags = flags; args.need_results = !!leases;
diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 2d5b9be..e7499fa 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1348,10 +1348,7 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) if (!(network = vshCommandOptNetwork(ctl, cmd, &name))) return false;
- nleases = mac ? virNetworkGetDHCPLeasesForMAC(network, mac, &leases, flags) - : virNetworkGetDHCPLeases(network, &leases, flags); - - if (nleases < 0) { + if ((nleases = virNetworkGetDHCPLeases(network, mac, &leases, flags) < 0)) {
Wrong parenthesising.
ACK with the debug message added and virsh functionality fixed.
Jan
I've resolved all your comments and pushed this patch. Thanks Peter