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

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

 



On Tue, Nov 26, 2013 at 02:35:59AM +0530, Nehal J Wani wrote:
> diff --git a/daemon/remote.c b/daemon/remote.c
> index decaecc..c2f13a8 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -5215,6 +5215,182 @@ cleanup:
>      return rv;
>  }
>  
> +/* Copy contents of virNetworkDHCPLeasesPtr to remote_network_dhcp_lease */
> +static int
> +remoteSerializeDHCPLease(remote_network_dhcp_lease *lease_dst, virNetworkDHCPLeasesPtr lease_src)
> +{
> +    char **mac_tmp = NULL;
> +    char **iaid_tmp = NULL;
> +    char **hostname_tmp = NULL;
> +    char **clientid_tmp = NULL;
> +
> +    if (VIR_ALLOC(mac_tmp) < 0 ||
> +        VIR_ALLOC(iaid_tmp) < 0 ||
> +        VIR_ALLOC(hostname_tmp) < 0 ||
> +        VIR_ALLOC(clientid_tmp) < 0)
> +        goto error;
> +
> +    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 ||
> +        VIR_STRDUP(lease_dst->ipaddr, lease_src->ipaddr) < 0 ||
> +        VIR_STRDUP(*mac_tmp, lease_src->mac) < 0 ||
> +        VIR_STRDUP(*iaid_tmp, lease_src->iaid) < 0 ||
> +        VIR_STRDUP(*hostname_tmp, lease_src->hostname) < 0 ||
> +        VIR_STRDUP(*clientid_tmp, lease_src->clientid) < 0)
> +        goto error;
> +
> +    lease_dst->mac = *mac_tmp ? mac_tmp : NULL;
> +    lease_dst->iaid = *iaid_tmp ? iaid_tmp : NULL;
> +    lease_dst->hostname = *hostname_tmp ? hostname_tmp : NULL;
> +    lease_dst->clientid = *clientid_tmp ? clientid_tmp : NULL;
> +
> +    return 0;
> +
> +error:

What about lease_dist->interface and lease_dst->ipaddr - shouldn't we
free them too ?

> +    VIR_FREE(mac_tmp);
> +    VIR_FREE(iaid_tmp);
> +    VIR_FREE(hostname_tmp);
> +    VIR_FREE(clientid_tmp);

Leaking the contents of each of these ie you need VIR_FREE(*mac_tmp)
before here too

> +    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;
> +    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 = virNetworkGetDHCPLeases(net,
> +                                           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;

'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);
> +    return -1;
> +}
> +
>  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 :|

--
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]