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