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

[Nehal J Wani]

> +static int
> +remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,
> +                                 int nleases,
> +                                 remote_network_get_dhcp_leases_ret *ret)
> +{
> +    size_t i;
> +
> +    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 (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0)
> +        return -1;
> +
> +    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:
> +    for (i = 0; i < nleases; i++) {
> +        remote_network_dhcp_lease *lease_ret =
&(ret->leases.leases_val[i]);
> +        virNetworkDHCPLeaseFree((virNetworkDHCPLeasesPtr)lease_ret);

[1]

> +    }
> +    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]);
> +        }
> +    }
> +    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;
> +    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)
[2]

> +        goto cleanup;
> +
> +    rv = nleases;
> +


I am a little skeptical about the typecasts involved in [1] and [2]

Specifically for [2], although the APIs are different, the struct is same,
only the name is different. But, what would be the other options?

One option: (Suggested by Osier)
Change remoteSerializeNetworkDHCPLeases to

+remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,
+                                 int nleases,
+                                 (void *) ret,
+                                 int method)

where the datatype for variable in which ret will be copied, will be
decided based on the method (method's value will be taken from an enum
or something similar, e.g: 1->DHCPLeases, 2->DHCPLeasesForMAC)

OR

Second Option: Since the APIs should be independent, make another function:
remoteSerializeNetworkDHCPLeasesForMAC(...), but that will lead to code
redundancy.

Eric, what would you do in these cases?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130916/1812831f/attachment-0001.htm>