Re: [libvirt] [PATCHv5 2/4] net-dhcp-leases: Implement the remote protocol

On Tue, Nov 26, 2013 at 02:35:59AM +0530, Nehal J Wani wrote: What about lease_dist->interface and lease_dst->ipaddr - shouldn't we free them too ? Leaking the contents of each of these ie you need VIR_FREE(*mac_tmp) before here too 'goto cleanup' otherwise you're leaking memory > + } > + > + 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; > +} > + > + > +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); > + 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 = 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. -----*/ > diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c > index df7558b..83da5b2 100644 > --- a/src/remote/remote_driver.c > +++ b/src/remote/remote_driver.c > @@ -1687,6 +1687,51 @@ error: > return -1; > } > > +/* Copy contents of remote_network_dhcp_lease to virNetworkDHCPLeasesPtr */ > +static int > +remoteSerializeDHCPLease(virNetworkDHCPLeasesPtr lease_dst, remote_network_dhcp_lease *lease_src) > +{ > + lease_dst->expirytime = lease_src->expirytime; > + lease_dst->type = lease_src->type; > + lease_dst->prefix = lease_src->prefix; > + > + if (VIR_STRDUP(lease_dst->interface, lease_src->interface) < 0) > + goto error; > + > + if (VIR_STRDUP(lease_dst->ipaddr, lease_src->ipaddr) < 0) > + goto error; > + > + if (lease_src->mac) { > + if (VIR_STRDUP(lease_dst->mac, *lease_src->mac) < 0) > + goto error; > + } else > + lease_src->mac = NULL; > + > + if (lease_src->iaid) { > + if (VIR_STRDUP(lease_dst->iaid, *lease_src->iaid) < 0) > + goto error; > + } else > + lease_src->iaid = NULL; > + > + if (lease_src->hostname) { > + if (VIR_STRDUP(lease_dst->hostname, *lease_src->hostname) < 0) > + goto error; > + } else > + lease_src->hostname = NULL; > + > + if (lease_src->clientid) { > + if (VIR_STRDUP(lease_dst->clientid, *lease_src->clientid) < 0) > + goto error; > + } else > + lease_src->clientid = NULL; > + > + return 0; > + > +error: > + virNetworkDHCPLeaseFree(lease_dst); > + > static int > remoteDomainBlockStatsFlags(virDomainPtr domain, > const char *path, > @@ -6678,6 +6723,145 @@ done: > 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 = NULL; > + remoteDriverLock(priv); > + > + make_nonnull_network(&args.net, net); > + args.flags = flags; > + args.need_results = !!leases; > + > + 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_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_ret, > + (char *) &ret); > + > +done: > + remoteDriverUnlock(priv); > + 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 = 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; > +} > + > + > static void > remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event) > { > @@ -6810,6 +6994,7 @@ make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDo > make_nonnull_domain(&snapshot_dst->dom, snapshot_src->domain); > } > > + > /*----------------------------------------------------------------------*/ > > unsigned long remoteVersion(void) > @@ -7038,6 +7223,8 @@ static virNetworkDriver network_driver = { > .networkSetAutostart = remoteNetworkSetAutostart, /* 0.3.0 */ > .networkIsActive = remoteNetworkIsActive, /* 0.7.3 */ > .networkIsPersistent = remoteNetworkIsPersistent, /* 0.7.3 */ > + .networkGetDHCPLeases = remoteNetworkGetDHCPLeases, /* 1.1.4 */ > + .networkGetDHCPLeasesForMAC = remoteNetworkGetDHCPLeasesForMAC, /* 1.1.4 */ > }; > > static virInterfaceDriver interface_driver = { > diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x > index f942670..60939b1 100644 > --- a/src/remote/remote_protocol.x > +++ b/src/remote/remote_protocol.x > @@ -235,6 +235,11 @@ const REMOTE_DOMAIN_JOB_STATS_MAX = 64; > /* Upper limit on number of CPU models */ > const REMOTE_CONNECT_CPU_MODELS_MAX = 8192; > > +/* > + * Upper limit on the maximum number of leases in one lease file > + */ > +const REMOTE_NETWORK_DHCP_LEASES_MAX = 32768; > + > /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ > typedef opaque remote_uuid[VIR_UUID_BUFLEN]; > > @@ -2849,6 +2854,41 @@ struct remote_connect_get_cpu_model_names_ret { > int ret; > }; > > +struct remote_network_dhcp_lease { > + remote_nonnull_string interface; > + hyper expirytime; > + int type; > + remote_string mac; > + remote_string iaid; > + remote_nonnull_string ipaddr; > + unsigned int prefix; > + remote_string hostname; > + remote_string clientid; > +}; > + > +struct remote_network_get_dhcp_leases_args { > + remote_nonnull_network net; > + int need_results; > + unsigned int flags; > +}; > + > +struct remote_network_get_dhcp_leases_ret { > + remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>; > + 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; > +}; > + Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|