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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list