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/daemon/remote.c b/daemon/remote.c index 9ffc1cb..ea16789 100644 --- a/daemon/remote.c +++ b/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 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 594521e..032d6e6 100644 --- a/include/libvirt/libvirt.h.in +++ b/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 diff --git a/src/driver.h b/src/driver.h index 6e72e92..5018068 100644 --- a/src/driver.h +++ b/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; }; diff --git a/src/libvirt.c b/src/libvirt.c index 566f984..49c9d16 100644 --- a/src/libvirt.c +++ b/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,6 +21085,7 @@ virNodeGetFreePages(virConnectPtr conn, */ int virNetworkGetDHCPLeases(virNetworkPtr network, + const char *mac, virNetworkDHCPLeasePtr **leases, unsigned int flags) { @@ -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: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f64462e..65a5b43 100644 --- a/src/libvirt_public.syms +++ b/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 .... diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1c20b66..0ece432 100644 --- a/src/network/bridge_driver.c +++ b/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 = { diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 76ce4a9..04039ac 100644 --- a/src/remote/remote_driver.c +++ b/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 == NULL ? NULL : (char **) &mac; 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 = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 4b75bdb..bff2c47 100644 --- a/src/remote/remote_protocol.x +++ b/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 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 222f125..a14e1fd 100644 --- a/src/remote_protocol-structs +++ b/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, }; 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)) { vshError(ctl, _("Failed to get leases info for %s"), name); goto cleanup; } -- 1.9.3