Re: [PATCH] net: merge virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC

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

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]