[libvirt] [PATCH 2/2] remote: Implement bulk domain stats APIs in the remote driver

[Peter Krempa]

On 08/26/14 11:47, Li Wei wrote:
> 
> 
> On 08/26/2014 01:05 AM, Peter Krempa wrote:
>> Implement the remote driver support for shuffling the domain stats
>> around.
>> ---
>>  daemon/remote.c              | 91 ++++++++++++++++++++++++++++++++++++++++++++
>>  src/remote/remote_driver.c   | 88 ++++++++++++++++++++++++++++++++++++++++++
>>  src/remote/remote_protocol.x | 26 ++++++++++++-
>>  src/remote_protocol-structs  | 23 +++++++++++
>>  4 files changed, 227 insertions(+), 1 deletion(-)
>>
>> diff --git a/daemon/remote.c b/daemon/remote.c
>> index ea16789..34e6950 100644
>> --- a/daemon/remote.c
>> +++ b/daemon/remote.c
>> @@ -6337,6 +6337,97 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,
>>  }
>>
>>
>> +static int
>> +remoteDispatchDomainListGetStats(virNetServerPtr server ATTRIBUTE_UNUSED,
>> +                                 virNetServerClientPtr client,
>> +                                 virNetMessagePtr msg ATTRIBUTE_UNUSED,
>> +                                 virNetMessageErrorPtr rerr,
>> +                                 remote_domain_list_get_stats_args *args,
>> +                                 remote_domain_list_get_stats_ret *ret)
>> +{
>> +    int rv = -1;
>> +    size_t i;
>> +    struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
>> +    virDomainStatsRecordPtr *retStats = NULL;
>> +    int nrecords = 0;
>> +    virDomainPtr *doms = NULL;
>> +
>> +    if (!priv->conn) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
>> +        goto cleanup;
>> +    }
>> +
>> +    if (args->doms.doms_len) {
>> +        if (VIR_ALLOC_N(doms, args->doms.doms_len + 1) < 0)
>> +            goto cleanup;
>> +
>> +        for (i = 0; i < args->doms.doms_len; i++) {
>> +            if (!(doms[i] = get_nonnull_domain(priv->conn, args->doms.doms_val[i])))
>> +                goto cleanup;
>> +        }
>> +
>> +        if ((nrecords = virDomainListGetStats(doms,
>> +                                              args->stats,
>> +                                              &retStats,
>> +                                              args->flags)) < 0)
>> +            goto cleanup;
>> +    } else {
>> +        if (!(virConnectGetAllDomainStats(priv->conn,
>> +                                          args->stats,
>> +                                          &retStats,
>> +                                          args->flags)) < 0)
> 
> if ((nrecords = virConnectGetAllDomainStats(priv->conn,
> 					    args->stats,
> 					    &retStats,
> 					    args->flags)) < 0)

Good catch.

> 
> 
>> +            goto cleanup;
>> +    }
>> +
>> +    if (nrecords > REMOTE_DOMAIN_LIST_MAX) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Number of domain stats records is %d, "
>> +                         "which exceeds max limit: %d"),
>> +                       nrecords, REMOTE_DOMAIN_LIST_MAX);
>> +        goto cleanup;
>> +    }
>> +
>> +    if (retStats && nrecords) {
>> +        ret->retStats.retStats_len = nrecords;
>> +
>> +        if (VIR_ALLOC_N(ret->retStats.retStats_val, nrecords) < 0)
>> +            goto cleanup;
>> +
>> +        for (i = 0; i < nrecords; i++) {
>> +            virDomainStatsRecordPtr src = retStats[i];
>> +            remote_domain_stats_record *dst = ret->retStats.retStats_val + i;
>> +
>> +            make_nonnull_domain(&dst->dom, src->dom);
>> +
>> +            if (remoteSerializeTypedParameters(src->params, src->nparams,
>> +                                               &dst->params.params_val,
>> +                                               &dst->params.params_len,
>> +                                               VIR_TYPED_PARAM_STRING_OKAY) < 0)
>> +                goto cleanup;
>> +        }
>> +    } else {
>> +        ret->retStats.retStats_len = 0;
>> +        ret->retStats.retStats_val = NULL;
>> +    }
>> +
>> +    ret->ret = nrecords;
>> +    rv = 0;
>> +
>> + cleanup:
>> +    if (rv < 0)
>> +        virNetMessageSaveError(rerr);
>> +    if (retStats)
>> +        virDomainStatsRecordListFree(retStats);
>> +    if (args->doms.doms_len) {
>> +        for (i = 0; i < args->doms.doms_len; i++)
>> +            virDomainFree(doms[i]);
>> +
>> +        VIR_FREE(doms);
>> +    }
>> +    return rv;
>> +}
>> +
>> +
>>  /*----- Helpers. -----*/
>>
>>  /* get_nonnull_domain and get_nonnull_network turn an on-wire
>> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
>> index 9a1d78f..41c807a 100644
>> --- a/src/remote/remote_driver.c
>> +++ b/src/remote/remote_driver.c
>> @@ -7670,6 +7670,93 @@ remoteNetworkGetDHCPLeases(virNetworkPtr net,
>>  }
>>
>>
>> +static int
>> +remoteDomainListGetStats(virConnectPtr conn,
>> +                         virDomainPtr *doms,
>> +                         unsigned int ndoms,
>> +                         unsigned int stats,
>> +                         virDomainStatsRecordPtr **retStats,
>> +                         unsigned int flags)
>> +{
>> +    struct private_data *priv = conn->networkPrivateData;
>> +    int rv = -1;
>> +    size_t i;
>> +    remote_domain_list_get_stats_args args;
>> +    remote_domain_list_get_stats_ret ret;
>> +
>> +    virDomainStatsRecordPtr *tmpret = NULL;
>> +
>> +    if (ndoms) {
>> +        if (VIR_ALLOC_N(args.doms.doms_val, ndoms) < 0)
>> +            goto cleanup;
>> +
>> +        for (i = 0; i < ndoms; i++)
>> +            make_nonnull_domain(args.doms.doms_val + i, doms[i]);
>> +    }
>> +    args.doms.doms_len = ndoms;
>> +
>> +    args.stats = stats;
>> +    args.flags = flags;
>> +
>> +    memset(&ret, 0, sizeof(ret));
>> +
>> +    remoteDriverLock(priv);
>> +    if (call(conn, priv, 0, REMOTE_PROC_DOMAIN_LIST_GET_STATS,
>> +             (xdrproc_t)xdr_remote_domain_list_get_stats_args, (char *)&args,
>> +             (xdrproc_t)xdr_remote_domain_list_get_stats_ret, (char *)&ret) == -1) {
>> +        remoteDriverUnlock(priv);
>> +        goto cleanup;
>> +    }
>> +    remoteDriverUnlock(priv);
>> +
>> +    if (ret.retStats.retStats_len > REMOTE_DOMAIN_LIST_MAX) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Number of stats entries is %d, which exceeds max limit: %d"),
>> +                       ret.retStats.retStats_len, REMOTE_DOMAIN_LIST_MAX);
>> +        goto cleanup;
>> +    }
>> +
>> +    *retStats = NULL;
>> +
>> +    if (ret.retStats.retStats_len) {
>> +        if (VIR_ALLOC_N(tmpret, ret.retStats.retStats_len) < 0)
> 
> Shouldn't we need to allocate one plus pointer here to act as a NULL sentinel
> for virDomainStatsRecordListFree()?

Definitely. Thanks for pointing this out.

> 
> Thanks,
> Li Wei
> 

Both problems are fixed in v2.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140826/880ffc41/attachment-0001.sig>