net: merge virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC

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

@@ -6292,6 +6292,7 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,
         goto cleanup;
 
     if ((nleases = virNetworkGetDHCPLeases(net,
+                                           args->mac ? *args->mac : NULL,
                                            args->need_results ? &leases : NULL,
                                            args->flags)) < 0)
         goto cleanup;
@@ -6336,74 +6337,6 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,
 }
 
 
-static int
-remoteDispatchNetworkGetDHCPLeasesForMAC(virNetServerPtr server ATTRIBUTE_UNUSED,
-                                         virNetServerClientPtr client,
-                                         virNetMessagePtr msg ATTRIBUTE_UNUSED,
-                                         virNetMessageErrorPtr rerr,
-                                         remote_network_get_dhcp_leases_for_mac_args *args,
-                                         remote_network_get_dhcp_leases_for_mac_ret *ret)
-{
-    int rv = -1;
-    size_t i;
-    struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
-    virNetworkDHCPLeasePtr *leases = NULL;
-    virNetworkPtr net = NULL;
-    int nleases = 0;
-
-    if (!priv->conn) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
-        goto cleanup;
-    }
-
-    if (!(net = get_nonnull_network(priv->conn, args->net)))
-        goto cleanup;
-
-    if ((nleases = virNetworkGetDHCPLeasesForMAC(net, args->mac,
-                                                 args->need_results ? &leases : NULL,
-                                                 args->flags)) < 0)
-        goto cleanup;
-
-    if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Number of leases is %d, which exceeds max limit: %d"),
-                       nleases, REMOTE_NETWORK_DHCP_LEASES_MAX);
-        return -1;
-    }
-
-    if (leases && nleases) {
-        if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0)
-            goto cleanup;
-
-        ret->leases.leases_len = nleases;
-
-        for (i = 0; i < nleases; i++) {
-            if (remoteSerializeDHCPLease(ret->leases.leases_val + i, leases[i]) < 0)
-                goto cleanup;
-        }
-
-    } else {
-        ret->leases.leases_len = 0;
-        ret->leases.leases_val = NULL;
-    }
-
-    ret->ret = nleases;
-
-    rv = 0;
-
- cleanup:
-    if (rv < 0)
-        virNetMessageSaveError(rerr);
-    if (leases) {
-        for (i = 0; i < nleases; i++)
-            virNetworkDHCPLeaseFree(leases[i]);
-        VIR_FREE(leases);
-    }
-    virNetworkFree(net);
-    return rv;
-}
-
-
 /*----- Helpers. -----*/
 
 /* get_nonnull_domain and get_nonnull_network turn an on-wire

include/libvirt/libvirt.h.in

@@ -5169,14 +5169,10 @@ struct _virNetworkDHCPLease {
 void virNetworkDHCPLeaseFree(virNetworkDHCPLeasePtr lease);
 
 int virNetworkGetDHCPLeases(virNetworkPtr network,
+                            const char *mac,
                             virNetworkDHCPLeasePtr **leases,
                             unsigned int flags);
 
-int virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
-                                  const char *mac,
-                                  virNetworkDHCPLeasePtr **leases,
-                                  unsigned int flags);
-
 /**
  * virConnectNetworkEventGenericCallback:
  * @conn: the connection pointer

src/driver.h

@@ -1184,15 +1184,10 @@ typedef int
 
 typedef int
 (*virDrvNetworkGetDHCPLeases)(virNetworkPtr network,
+                              const char *mac,
                               virNetworkDHCPLeasePtr **leases,
                               unsigned int flags);
 
-typedef int
-(*virDrvNetworkGetDHCPLeasesForMAC)(virNetworkPtr network,
-                                    const char *mac,
-                                    virNetworkDHCPLeasePtr **leases,
-                                    unsigned int flags);
-
 typedef struct _virDriver virDriver;
 typedef virDriver *virDriverPtr;
 
@@ -1546,7 +1541,6 @@ struct _virNetworkDriver {
     virDrvNetworkIsActive networkIsActive;
     virDrvNetworkIsPersistent networkIsPersistent;
     virDrvNetworkGetDHCPLeases networkGetDHCPLeases;
-    virDrvNetworkGetDHCPLeasesForMAC networkGetDHCPLeasesForMAC;
 };
 
 

src/libvirt.c

@@ -21014,6 +21014,7 @@ virNodeGetFreePages(virConnectPtr conn,
 /**
  * virNetworkGetDHCPLeases:
  * @network: Pointer to network object
+ * @mac: Optional 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).
@@ -21040,6 +21041,11 @@ virNodeGetFreePages(virConnectPtr conn,
  * Note: @mac, @iaid, @ipaddr, @clientid are in ASCII form, not raw bytes.
  * Note: @expirytime can 0, in case the lease is for infinite time.
  *
+ * The API fetches leases info of guests in the specified network. If the
+ * optional parameter @mac is specified, the returned list will contain only
+ * lease info about a specific guest interface with @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
@@ -21057,7 +21063,7 @@ virNodeGetFreePages(virConnectPtr conn,
  * int nleases;
  * unsigned int flags = 0;
  *
- * nleases = virNetworkGetDHCPLeases(network, &leases, flags);
+ * nleases = virNetworkGetDHCPLeases(network, NULL, &leases, flags);
  * if (nleases < 0)
  *     error();
  *
@@ -21079,12 +21085,13 @@ virNodeGetFreePages(virConnectPtr conn,
  */
 int
 virNetworkGetDHCPLeases(virNetworkPtr network,
+                        const char *mac,
                         virNetworkDHCPLeasePtr **leases,
                         unsigned int flags)
 {
     virConnectPtr conn;
-    VIR_DEBUG("network=%p, leases=%p, flags=%x",
-               network, leases, flags);
+    VIR_DEBUG("network=%p, mac='%s' leases=%p, flags=%x",
+               network, NULLSTR(mac), leases, flags);
 
     virResetLastError();
 
@@ -21097,7 +21104,7 @@ virNetworkGetDHCPLeases(virNetworkPtr network,
 
     if (conn->networkDriver && conn->networkDriver->networkGetDHCPLeases) {
         int ret;
-        ret = conn->networkDriver->networkGetDHCPLeases(network, leases, flags);
+        ret = conn->networkDriver->networkGetDHCPLeases(network, mac, leases, flags);
         if (ret < 0)
             goto error;
         return ret;
@@ -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);
-
-    virResetLastError();
-
-    if (leases)
-        *leases = NULL;
-
-    virCheckNonNullArgGoto(mac, error);
-
-    virCheckNetworkReturn(network, -1);
-
-    conn = network->conn;
-
-    if (conn->networkDriver &&
-        conn->networkDriver->networkGetDHCPLeasesForMAC) {
-        int ret;
-        ret = conn->networkDriver->networkGetDHCPLeasesForMAC(network, mac,
-                                                              leases, flags);
-        if (ret < 0)
-            goto error;
-        return ret;
-    }
-
-    virReportUnsupportedError();
-
- error:
-    virDispatchError(network->conn);
-    return -1;
-}
 
 /**
  * virNetworkDHCPLeaseFree:

src/libvirt_public.syms

@@ -663,7 +663,6 @@ LIBVIRT_1.2.6 {
         virNodeGetFreePages;
         virNetworkDHCPLeaseFree;
         virNetworkGetDHCPLeases;
-        virNetworkGetDHCPLeasesForMAC;
 } LIBVIRT_1.2.5;
 
 # .... define new API here using predicted next version number ....

src/network/bridge_driver.c

@@ -3369,9 +3369,10 @@ static int networkSetAutostart(virNetworkPtr net,
 }
 
 static int
-networkGetDHCPLeasesHelper(virNetworkObjPtr obj,
-                           const char *mac,
-                           virNetworkDHCPLeasePtr **leases)
+networkGetDHCPLeases(virNetworkPtr network,
+                     const char *mac,
+                     virNetworkDHCPLeasePtr **leases,
+                     unsigned int flags)
 {
     size_t i, j;
     size_t nleases = 0;
@@ -3391,6 +3392,15 @@ networkGetDHCPLeasesHelper(virNetworkObjPtr obj,
     virNetworkIpDefPtr ipdef_tmp = NULL;
     virNetworkDHCPLeasePtr lease = NULL;
     virNetworkDHCPLeasePtr *leases_ret = NULL;
+    virNetworkObjPtr obj;
+
+    virCheckFlags(0, -1);
+
+    if (!(obj = networkObjFromNetwork(network)))
+        return -1;
+
+    if (virNetworkGetDHCPLeasesEnsureACL(network->conn, obj->def) < 0)
+        goto cleanup;
 
     /* Retrieve custom leases file location */
     custom_lease_file = networkDnsmasqLeaseFileNameCustom(obj->def->bridge);
@@ -3530,6 +3540,10 @@ networkGetDHCPLeasesHelper(virNetworkObjPtr obj,
     VIR_FREE(lease_entries);
     VIR_FREE(custom_lease_file);
     virJSONValueFree(leases_array);
+
+    if (obj)
+        virNetworkObjUnlock(obj);
+
     return rv;
 
  error:
@@ -3541,54 +3555,6 @@ networkGetDHCPLeasesHelper(virNetworkObjPtr obj,
     goto cleanup;
 }
 
-static int
-networkGetDHCPLeases(virNetworkPtr network,
-                     virNetworkDHCPLeasePtr **leases,
-                     unsigned int flags)
-{
-    int rv = -1;
-    virNetworkObjPtr obj;
-
-    virCheckFlags(0, -1);
-
-    if (!(obj = networkObjFromNetwork(network)))
-        return rv;
-
-    if (virNetworkGetDHCPLeasesEnsureACL(network->conn, obj->def) < 0)
-        goto cleanup;
-
-    rv = networkGetDHCPLeasesHelper(obj, NULL, leases);
-
- cleanup:
-    if (obj)
-        virNetworkObjUnlock(obj);
-    return rv;
-}
-
-static int
-networkGetDHCPLeasesForMAC(virNetworkPtr network,
-                           const char *mac,
-                           virNetworkDHCPLeasePtr **leases,
-                           unsigned int flags)
-{
-    int rv = -1;
-    virNetworkObjPtr obj;
-
-    virCheckFlags(0, -1);
-
-    if (!(obj = networkObjFromNetwork(network)))
-        return rv;
-
-    if (virNetworkGetDHCPLeasesForMACEnsureACL(network->conn, obj->def) < 0)
-        goto cleanup;
-
-    rv = networkGetDHCPLeasesHelper(obj, mac, leases);
-
- cleanup:
-    if (obj)
-        virNetworkObjUnlock(obj);
-    return rv;
-}
 
 static virNetworkDriver networkDriver = {
     "Network",
@@ -3616,7 +3582,6 @@ static virNetworkDriver networkDriver = {
     .networkIsActive = networkIsActive, /* 0.7.3 */
     .networkIsPersistent = networkIsPersistent, /* 0.7.3 */
     .networkGetDHCPLeases = networkGetDHCPLeases, /* 1.2.6 */
-    .networkGetDHCPLeasesForMAC = networkGetDHCPLeasesForMAC, /* 1.2.6 */
 };
 
 static virStateDriver networkStateDriver = {

src/remote/remote_driver.c

@@ -7601,6 +7601,7 @@ remoteSerializeDHCPLease(virNetworkDHCPLeasePtr lease_dst, remote_network_dhcp_l
 
 static int
 remoteNetworkGetDHCPLeases(virNetworkPtr net,
+                           const char *mac,
                            virNetworkDHCPLeasePtr **leases,
                            unsigned int flags)
 {
@@ -7614,6 +7615,7 @@ remoteNetworkGetDHCPLeases(virNetworkPtr net,
     remoteDriverLock(priv);
 
     make_nonnull_network(&args.net, net);
+    args.mac = mac ? (char **) &mac : NULL;
     args.flags = flags;
     args.need_results = !!leases;
 
@@ -7665,74 +7667,6 @@ remoteNetworkGetDHCPLeases(virNetworkPtr net,
 }
 
 
-static int
-remoteNetworkGetDHCPLeasesForMAC(virNetworkPtr net,
-                                 const char *mac,
-                                 virNetworkDHCPLeasePtr **leases,
-                                 unsigned int flags)
-{
-    int rv = -1;
-    size_t i;
-    struct private_data *priv = net->conn->networkPrivateData;
-    remote_network_get_dhcp_leases_for_mac_args args;
-    remote_network_get_dhcp_leases_for_mac_ret ret;
-
-    virNetworkDHCPLeasePtr *leases_ret = NULL;
-    remoteDriverLock(priv);
-
-    make_nonnull_network(&args.net, net);
-    args.mac = (char *) mac;
-    args.flags = flags;
-    args.need_results = !!leases;
-
-    memset(&ret, 0, sizeof(ret));
-
-    if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC,
-             (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_args, (char *)&args,
-             (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret, (char *)&ret) == -1)
-        goto done;
-
-    if (ret.leases.leases_len > REMOTE_NETWORK_DHCP_LEASES_MAX) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Number of leases is %d, which exceeds max limit: %d"),
-                       ret.leases.leases_len, REMOTE_NETWORK_DHCP_LEASES_MAX);
-        goto cleanup;
-    }
-
-    if (leases) {
-        if (ret.leases.leases_len &&
-            VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1) < 0)
-            goto cleanup;
-
-        for (i = 0; i < ret.leases.leases_len; i++) {
-            if (VIR_ALLOC(leases_ret[i]) < 0)
-                goto cleanup;
-
-            if (remoteSerializeDHCPLease(leases_ret[i], &ret.leases.leases_val[i]) < 0)
-                goto cleanup;
-        }
-
-        *leases = leases_ret;
-        leases_ret = NULL;
-    }
-
-    rv = ret.ret;
-
- cleanup:
-    if (leases_ret) {
-        for (i = 0; i < ret.leases.leases_len; i++)
-            virNetworkDHCPLeaseFree(leases_ret[i]);
-        VIR_FREE(leases_ret);
-    }
-    xdr_free((xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret,
-             (char *) &ret);
-
- done:
-    remoteDriverUnlock(priv);
-    return rv;
-}
-
-
 /* get_nonnull_domain and get_nonnull_network turn an on-wire
  * (name, uuid) pair into virDomainPtr or virNetworkPtr object.
  * These can return NULL if underlying memory allocations fail,
@@ -8098,7 +8032,6 @@ static virNetworkDriver network_driver = {
     .networkIsActive = remoteNetworkIsActive, /* 0.7.3 */
     .networkIsPersistent = remoteNetworkIsPersistent, /* 0.7.3 */
     .networkGetDHCPLeases = remoteNetworkGetDHCPLeases, /* 1.2.6 */
-    .networkGetDHCPLeasesForMAC = remoteNetworkGetDHCPLeasesForMAC, /* 1.2.6 */
 };
 
 static virInterfaceDriver interface_driver = {

src/remote/remote_protocol.x

@@ -3035,6 +3035,7 @@ struct remote_network_dhcp_lease {
 
 struct remote_network_get_dhcp_leases_args {
     remote_nonnull_network net;
+    remote_string mac;
     int need_results;
     unsigned int flags;
 };
@@ -3044,18 +3045,6 @@ struct remote_network_get_dhcp_leases_ret {
     unsigned int ret;
 };
 
-struct remote_network_get_dhcp_leases_for_mac_args {
-    remote_nonnull_network net;
-    remote_nonnull_string mac;
-    int need_results;
-    unsigned int flags;
-};
-
-struct remote_network_get_dhcp_leases_for_mac_ret {
-    remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>;
-    unsigned int ret;
-};
-
 /*----- Protocol. -----*/
 
 /* Define the program number, protocol version and procedure numbers here. */
@@ -5413,11 +5402,6 @@ enum remote_procedure {
      * @generate: none
      * @acl: network:read
      */
-    REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 341,
+    REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 341
 
-    /**
-     * @generate: none
-     * @acl: network:read
-     */
-    REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC = 342
 };

src/remote_protocol-structs

@@ -2498,6 +2498,7 @@ struct remote_network_dhcp_lease {
 };
 struct remote_network_get_dhcp_leases_args {
         remote_nonnull_network     net;
+        remote_string              mac;
         int                        need_results;
         u_int                      flags;
 };
@@ -2508,19 +2509,6 @@ struct remote_network_get_dhcp_leases_ret {
         } leases;
         u_int                      ret;
 };
-struct remote_network_get_dhcp_leases_for_mac_args {
-        remote_nonnull_network     net;
-        remote_nonnull_string      mac;
-        int                        need_results;
-        u_int                      flags;
-};
-struct remote_network_get_dhcp_leases_for_mac_ret {
-        struct {
-                u_int              leases_len;
-                remote_network_dhcp_lease * leases_val;
-        } leases;
-        u_int                      ret;
-};
 enum remote_procedure {
         REMOTE_PROC_CONNECT_OPEN = 1,
         REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -2863,5 +2851,4 @@ enum remote_procedure {
         REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2 = 339,
         REMOTE_PROC_NODE_GET_FREE_PAGES = 340,
         REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 341,
-        REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC = 342,
 };

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) {
         vshError(ctl, _("Failed to get leases info for %s"), name);
         goto cleanup;
     }