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

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




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)


> +            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()?

Thanks,
Li Wei

> +            goto cleanup;
> +
> +        for (i = 0; i < ret.retStats.retStats_len; i++) {
> +            virDomainStatsRecordPtr elem;
> +            remote_domain_stats_record *rec = ret.retStats.retStats_val + i;
> +
> +            if (VIR_ALLOC(elem) < 0)
> +                goto cleanup;
> +
> +            if (!(elem->dom = get_nonnull_domain(conn, rec->dom)))
> +                goto cleanup;
> +
> +            if (remoteDeserializeTypedParameters(rec->params.params_val,
> +                                                 rec->params.params_len,
> +                                                 REMOTE_PROC_DOMAIN_LIST_GET_STATS,
> +                                                 &elem->params,
> +                                                 &elem->nparams))
> +                goto cleanup;
> +
> +            tmpret[i] = elem;
> +        }
> +
> +        *retStats = tmpret;
> +        tmpret = NULL;
> +    }
> +
> +    rv = ret.ret;
> +
> + cleanup:
> +    if (tmpret)
> +        virDomainStatsRecordListFree(tmpret);
> +    xdr_free((xdrproc_t)xdr_remote_domain_list_get_stats_ret,
> +             (char *) &ret);
> +
> +    return rv;
> +}
> +
>  /* get_nonnull_domain and get_nonnull_network turn an on-wire
>   * (name, uuid) pair into virDomainPtr or virNetworkPtr object.
>   * These can return NULL if underlying memory allocations fail,
> @@ -8008,6 +8095,7 @@ static virDriver remote_driver = {
>      .domainSetTime = remoteDomainSetTime, /* 1.2.5 */
>      .nodeGetFreePages = remoteNodeGetFreePages, /* 1.2.6 */
>      .connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 1.2.7 */
> +    .domainListGetStats = remoteDomainListGetStats, /* 1.2.8 */
>  };
> 
>  static virNetworkDriver network_driver = {
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index 5c316fb..9729392 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -241,6 +241,9 @@ const REMOTE_DOMAIN_FSFREEZE_MOUNTPOINTS_MAX = 256;
>  /* Upper limit on the maximum number of leases in one lease file */
>  const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;
> 
> +/* Upper limit on count of parameters returned via bulk stats API */
> +const REMOTE_DOMAIN_STATS_PARAMS_MAX = 2048;
> +
>  /* UUID.  VIR_UUID_BUFLEN definition comes from libvirt.h */
>  typedef opaque remote_uuid[VIR_UUID_BUFLEN];
> 
> @@ -3057,6 +3060,21 @@ struct remote_network_get_dhcp_leases_ret {
>      unsigned int ret;
>  };
> 
> +struct remote_domain_stats_record {
> +    remote_nonnull_domain dom;
> +    remote_typed_param params<REMOTE_DOMAIN_STATS_PARAMS_MAX>;
> +};
> +
> +struct remote_domain_list_get_stats_args {
> +    remote_nonnull_domain doms<REMOTE_DOMAIN_LIST_MAX>;
> +    unsigned int stats;
> +    unsigned int flags;
> +};
> +
> +struct remote_domain_list_get_stats_ret {
> +    remote_domain_stats_record retStats<REMOTE_DOMAIN_LIST_MAX>;
> +    int ret;
> +};
>  /*----- Protocol. -----*/
> 
>  /* Define the program number, protocol version and procedure numbers here. */
> @@ -5420,5 +5438,11 @@ enum remote_procedure {
>       * @generate: both
>       * @acl: connect:write
>       */
> -    REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342
> +    REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342,
> +
> +    /**
> +     * @generate: none
> +     * @acl: domain:read
> +     */
> +    REMOTE_PROC_DOMAIN_LIST_GET_STATS = 343
>  };
> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
> index 9bf09b8..3d9f5e7 100644
> --- a/src/remote_protocol-structs
> +++ b/src/remote_protocol-structs
> @@ -2519,6 +2519,28 @@ struct remote_network_get_dhcp_leases_ret {
>          } leases;
>          u_int                      ret;
>  };
> +struct remote_domain_stats_record {
> +        remote_nonnull_domain      dom;
> +        struct {
> +                u_int              params_len;
> +                remote_typed_param * params_val;
> +        } params;
> +};
> +struct remote_domain_list_get_stats_args {
> +        struct {
> +                u_int              doms_len;
> +                remote_nonnull_domain * doms_val;
> +        } doms;
> +        u_int                      stats;
> +        u_int                      flags;
> +};
> +struct remote_domain_list_get_stats_ret {
> +        struct {
> +                u_int              retStats_len;
> +                remote_domain_stats_record * retStats_val;
> +        } retStats;
> +        int                        ret;
> +};
>  enum remote_procedure {
>          REMOTE_PROC_CONNECT_OPEN = 1,
>          REMOTE_PROC_CONNECT_CLOSE = 2,
> @@ -2862,4 +2884,5 @@ enum remote_procedure {
>          REMOTE_PROC_NODE_GET_FREE_PAGES = 340,
>          REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 341,
>          REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342,
> +        REMOTE_PROC_DOMAIN_LIST_GET_STATS = 343,
>  };
> 


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