[libvirt] [PATCHv2 2/4] net-dhcp-leases: Implement the remote protocol
Osier Yang
jyang at redhat.com
Fri Sep 13 09:26:57 UTC 2013
On 13/09/13 05:53, Nehal J Wani wrote:
> Implement RPC calls for virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC
>
> daemon/remote.c
> * Define remoteSerializeNetworkDHCPLeases,
> remoteDispatchNetworkGetDHCPLeases
> * Define remoteDispatchNetworkGetDHCPLeasesForMAC
>
> src/remote/remote_driver.c
> * Define remoteNetworkGetDHCPLeases
> * Define remoteNetworkGetDHCPLeasesForMAC
>
> src/remote/remote_protocol.x
> * New RPC procedure: REMOTE_PROC_NETWORK_GET_DHCP_LEASES
> * Define structs remote_network_dhcp_leases, remote_network_get_dhcp_leases_args,
> remote_network_get_dhcp_leases_ret
> * New RPC procedure: REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC
> * Define structs remote_network_dhcp_leases_for_mac, remote_network_get_dhcp_leases_for_mac_args,
> remote_network_get_dhcp_leases_for_mac_ret
>
> src/remote_protocol-structs
> * New structs added
>
> src/rpc/gendispatch.pl
> * Add exception (s/Dhcp/DHCP) for auto-generating names of the remote functions
> in ./daemon/remote_dispatch.h
>
> ---
> daemon/remote.c | 133 ++++++++++++++++++++++++++++++++++++++
> src/remote/remote_driver.c | 150 +++++++++++++++++++++++++++++++++++++++++++
> src/remote/remote_protocol.x | 50 ++++++++++++++-
> src/remote_protocol-structs | 32 +++++++++
> src/rpc/gendispatch.pl | 1 +
> 5 files changed, 365 insertions(+), 1 deletion(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 2aff7c1..de03739 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -5137,7 +5137,140 @@ cleanup:
> return rv;
> }
>
> +static int
> +remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,
> + int nleases,
> + remote_network_get_dhcp_leases_ret *ret)
> +{
> + size_t i;
> +
> + if (nleases > REMOTE_NETWORK_DHCPLEASES_MAX) {
I think REMOTE_NETWORK_DHCP_LEASES_MAX is better.
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Number of leases is %d, which exceeds max limit: %d"),
> + nleases, REMOTE_NETWORK_DHCPLEASES_MAX);
> + return -1;
> + }
> +
> + if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0)
> + goto error;
If you "return -1;" here....
> +
> + ret->leases.leases_len = nleases;
> +
> + for (i = 0; i < nleases; i++) {
> + virNetworkDHCPLeasesPtr lease = leases[i];
> + remote_network_dhcp_lease *lease_ret = &(ret->leases.leases_val[i]);
> + lease_ret->expirytime = lease->expirytime;
> + lease_ret->type = lease->type;
> + lease_ret->prefix = lease->prefix;
> +
> + if ((VIR_STRDUP(lease_ret->mac, lease->mac) < 0) ||
> + (VIR_STRDUP(lease_ret->ipaddr, lease->ipaddr) < 0) ||
> + (VIR_STRDUP(lease_ret->hostname, lease->hostname) < 0) ||
> + (VIR_STRDUP(lease_ret->clientid, lease->clientid) < 0))
> + goto error;
> + }
> +
> + return 0;
> +
> +error:
> + if (ret->leases.leases_val) {
then no need for this checking.
> + for (i = 0; i < nleases; i++) {
> + remote_network_dhcp_lease *lease_ret = &(ret->leases.leases_val[i]);
> + virNetworkDHCPLeaseFree((virNetworkDHCPLeasesPtr)lease_ret);
> + }
> + }
> + return -1;
> +}
> +
> +static int
> +remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,
> + virNetServerClientPtr client,
> + virNetMessagePtr msg ATTRIBUTE_UNUSED,
> + virNetMessageErrorPtr rerr,
> + remote_network_get_dhcp_leases_args *args,
> + remote_network_get_dhcp_leases_ret *ret)
> +{
> + int rv = -1;
> + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
> + virNetworkDHCPLeasesPtr *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 = virNetworkGetDHCPLeases(net, &leases, args->flags)) < 0)
> + goto cleanup;
> +
> + if (remoteSerializeNetworkDHCPLeases(leases, nleases, ret) < 0)
> + goto cleanup;
>
> + rv = nleases;
> +
> +cleanup:
> + if (rv < 0)
> + virNetMessageSaveError(rerr);
> + if (leases) {
> + size_t i;
> + for (i = 0; i < nleases; i++) {
> + virNetworkDHCPLeaseFree(leases[i]);
> + }
> + }
Network object "net" is leaked. [1]
> + VIR_FREE(leases);
> + return rv;
> +}
> +
> +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;
> + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
> + virNetworkDHCPLeasesPtr *leases = NULL;
> + virNetworkPtr net = NULL;
> + int nleases = 0;
> + const char *mac = NULL;
> +
> + 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;
> +
> + mac = args->mac ? *args->mac : NULL;
> +
> + if ((nleases = virNetworkGetDHCPLeasesForMAC(net, mac, &leases, args->flags)) < 0)
> + goto cleanup;
> +
> + if (remoteSerializeNetworkDHCPLeases(leases, nleases,
> + (remote_network_get_dhcp_leases_ret *)ret) < 0)
> + goto cleanup;
> +
> + rv = nleases;
> +
> +cleanup:
> + if (rv < 0)
> + virNetMessageSaveError(rerr);
> + if (leases) {
> + size_t i;
> + for (i = 0; i < nleases; i++) {
> + virNetworkDHCPLeaseFree(leases[i]);
> + }
> + }
> + VIR_FREE(leases);
Same as [1]
> + return rv;
> +}
>
> /*----- Helpers. -----*/
>
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 62e77a5..693de42 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -6599,6 +6599,154 @@ done:
> return rv;
> }
>
> +static int
> +remoteNetworkGetDHCPLeasesForMAC(virNetworkPtr net,
> + const char *mac,
> + virNetworkDHCPLeasesPtr **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;
> +
> + virNetworkDHCPLeasesPtr *leases_ret;
> + remoteDriverLock(priv);
> +
> + make_nonnull_network(&args.net, net);
> + args.mac = mac ? (char **)&mac : NULL;
> + args.flags = flags;
> +
> + 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_DHCPLEASES_MAX) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Number of leases is %d, which exceeds max limit: %d"),
> + ret.leases.leases_len, REMOTE_NETWORK_DHCPLEASES_MAX);
Likewise, s/DHCPLEASES/DHCP_LEASES/,
> + return -1;
> + goto cleanup;
> + }
> +
> + if (ret.leases.leases_len &&
> + VIR_ALLOC_N(leases_ret, ret.leases.leases_len) < 0) {
[2] The API declares the returned array is NULL terminated, but here it
doesn't,
you should allocate one more element for it:
VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1)
> + goto cleanup;
> + }
> +
> + for (i = 0; i < ret.leases.leases_len; i++) {
> + if (VIR_ALLOC(leases_ret[i]) < 0)
> + goto cleanup;
> +
> + virNetworkDHCPLeasesPtr lease = leases_ret[i];
> + remote_network_dhcp_lease *lease_ret = &(ret.leases.leases_val[i]);
> + lease->expirytime = lease_ret->expirytime;
> + lease->type = lease_ret->type;
> + lease->prefix = lease_ret->prefix;
> +
> + if ((VIR_STRDUP(lease->mac, lease_ret->mac) < 0) ||
> + (VIR_STRDUP(lease->ipaddr, lease_ret->ipaddr) < 0) ||
> + (VIR_STRDUP(lease->hostname, lease_ret->hostname) < 0) ||
> + (VIR_STRDUP(lease->clientid, lease_ret->clientid) < 0))
> + goto cleanup;
> + }
> +
> + *leases = leases_ret;
> + leases_ret = NULL;
> + rv = ret.leases.leases_len;
> +
> +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;
> +}
> +
> +static int
> +remoteNetworkGetDHCPLeases(virNetworkPtr net,
> + virNetworkDHCPLeasesPtr **leases,
> + unsigned int flags)
> +{
> + int rv = -1;
> + size_t i;
> + struct private_data *priv = net->conn->networkPrivateData;
> + remote_network_get_dhcp_leases_args args;
> + remote_network_get_dhcp_leases_ret ret;
> +
> + virNetworkDHCPLeasesPtr *leases_ret;
> + remoteDriverLock(priv);
> +
> + make_nonnull_network(&args.net, net);
> + args.flags = flags;
> +
> + memset(&ret, 0, sizeof(ret));
> +
> + if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES,
> + (xdrproc_t)xdr_remote_network_get_dhcp_leases_args, (char *)&args,
> + (xdrproc_t)xdr_remote_network_get_dhcp_leases_ret, (char *)&ret) == -1) {
> + goto done;
> + }
> +
> + if (ret.leases.leases_len > REMOTE_NETWORK_DHCPLEASES_MAX) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Number of leases is %d, which exceeds max limit: %d"),
> + ret.leases.leases_len, REMOTE_NETWORK_DHCPLEASES_MAX);
s/DHCPLEASES/DHCP_LEASES/,
> + return -1;
> + goto cleanup;
> + }
> +
> + if (ret.leases.leases_len &&
> + VIR_ALLOC_N(leases_ret, ret.leases.leases_len) < 0) {
Same problem as [2]
> + goto cleanup;
> + }
> +
> + for (i = 0; i < ret.leases.leases_len; i++) {
> + if (VIR_ALLOC(leases_ret[i]) < 0)
> + goto cleanup;
> +
> + virNetworkDHCPLeasesPtr lease = leases_ret[i];
> + remote_network_dhcp_lease *lease_ret = &(ret.leases.leases_val[i]);
> + lease->expirytime = lease_ret->expirytime;
> + lease->type = lease_ret->type;
> + lease->prefix = lease_ret->prefix;
> +
> + if ((VIR_STRDUP(lease->mac, lease_ret->mac) < 0) ||
> + (VIR_STRDUP(lease->ipaddr, lease_ret->ipaddr) < 0) ||
> + (VIR_STRDUP(lease->hostname, lease_ret->hostname) < 0) ||
> + (VIR_STRDUP(lease->clientid, lease_ret->clientid) < 0))
> + goto cleanup;
> + }
> +
> + *leases = leases_ret;
> + leases_ret = NULL;
> + rv = ret.leases.leases_len;
> +
> +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_ret,
> + (char *) &ret);
> +
> +done:
> + remoteDriverUnlock(priv);
> + return rv;
> +}
> +
> static void
> remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event)
> {
> @@ -6958,6 +7106,8 @@ static virNetworkDriver network_driver = {
> .networkSetAutostart = remoteNetworkSetAutostart, /* 0.3.0 */
> .networkIsActive = remoteNetworkIsActive, /* 0.7.3 */
> .networkIsPersistent = remoteNetworkIsPersistent, /* 0.7.3 */
> + .networkGetDHCPLeases = remoteNetworkGetDHCPLeases, /* 1.1.3 */
> + .networkGetDHCPLeasesForMAC = remoteNetworkGetDHCPLeasesForMAC, /* 1.1.3 */
> };
>
> static virInterfaceDriver interface_driver = {
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index 85ad9ba..c455038 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -232,6 +232,11 @@ const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64;
> /* Upper limit on number of job stats */
> const REMOTE_DOMAIN_JOB_STATS_MAX = 16;
>
> +/*
> + * Upper limit on the maximum number of leases in one lease file
> + */
> +const REMOTE_NETWORK_DHCPLEASES_MAX = 32768;
> +
> /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */
> typedef opaque remote_uuid[VIR_UUID_BUFLEN];
>
> @@ -2835,6 +2840,35 @@ struct remote_domain_event_device_removed_msg {
> remote_nonnull_string devAlias;
> };
>
> +struct remote_network_dhcp_lease {
> + hyper expirytime;
> + remote_nonnull_string mac;
> + remote_nonnull_string ipaddr;
> + remote_nonnull_string hostname;
> + remote_nonnull_string clientid;
> + int type;
> + unsigned int prefix;
> +};
> +
> +struct remote_network_get_dhcp_leases_args {
> + remote_nonnull_network net;
> + unsigned int flags;
> +};
> +
> +struct remote_network_get_dhcp_leases_ret {
> + remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCPLEASES_MAX>;
Likewise.
> +};
> +
> +struct remote_network_get_dhcp_leases_for_mac_args {
> + remote_nonnull_network net;
> + remote_string mac;
> + unsigned int flags;
> +};
> +
> +struct remote_network_get_dhcp_leases_for_mac_ret {
> + remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCPLEASES_MAX>;
Likewise.
Osier
More information about the libvir-list
mailing list