From c6ec021b3c19c3ecc97d60d35b12eaa0b94da701 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Date: Sat, 28 Jan 2012 15:21:31 +0900
Subject: [PATCH] remote handler for virDomainGetCPUStats()

Unlike other users of virTypedParameter with RPC, this interface
can return zero-filled entries because the interface assumes
2 dimensional array. We compress these entries out from the
server when generating the over-the-wire contents, then reconstitute
them in the client.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 daemon/remote.c              | 76 ++++++++++++++++++++++++++-
 src/libvirt.c                | 15 +++---
 src/remote/remote_driver.c   | 99 +++++++++++++++++++++++++++++++++++-
 src/remote/remote_protocol.x | 27 +++++++++-
 src/remote_protocol-structs  | 15 ++++++
 5 files changed, 220 insertions(+), 12 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index d2150bf545..cb8423a33e 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -704,8 +704,11 @@ remoteSerializeTypedParameters(virTypedParameterPtr params,
     }
 
     for (i = 0, j = 0; i < nparams; ++i) {
-        if (!(flags & VIR_TYPED_PARAM_STRING_OKAY) &&
-            params[i].type == VIR_TYPED_PARAM_STRING) {
+        /* virDomainGetCPUStats can return a sparse array; also, we
+         * can't pass back strings to older clients.  */
+        if (!params[i].type ||
+            (!(flags & VIR_TYPED_PARAM_STRING_OKAY) &&
+             params[i].type == VIR_TYPED_PARAM_STRING)) {
             --*ret_params_len;
             continue;
         }
@@ -3523,6 +3526,75 @@ cleanup:
     return rv;
 }
 
+static int
+remoteDispatchDomainGetCPUStats(virNetServerPtr server ATTRIBUTE_UNUSED,
+                                virNetServerClientPtr client ATTRIBUTE_UNUSED,
+                                virNetMessagePtr hdr ATTRIBUTE_UNUSED,
+                                virNetMessageErrorPtr rerr,
+                                remote_domain_get_cpu_stats_args *args,
+                                remote_domain_get_cpu_stats_ret *ret)
+{
+    virDomainPtr dom = NULL;
+    struct daemonClientPrivate *priv;
+    virTypedParameterPtr params = NULL;
+    int rv = -1;
+    int percpu_len = 0;
+
+    priv = virNetServerClientGetPrivateData(client);
+    if (!priv->conn) {
+        virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
+        goto cleanup;
+    }
+
+    if (args->nparams > REMOTE_NODE_CPU_STATS_MAX) {
+        virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
+        goto cleanup;
+    }
+    if (args->ncpus > REMOTE_DOMAIN_GET_CPU_STATS_NCPUS_MAX) {
+        virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("ncpus too large"));
+        goto cleanup;
+    }
+
+    if (args->nparams > 0 &&
+        VIR_ALLOC_N(params, args->ncpus * args->nparams) < 0) {
+        virReportOOMError();
+        goto cleanup;
+    }
+
+    if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+        goto cleanup;
+
+    percpu_len = virDomainGetCPUStats(dom, params, args->nparams,
+                                      args->start_cpu, args->ncpus,
+                                      args->flags);
+    if (percpu_len < 0)
+        goto cleanup;
+    /* If nparams == 0, the function returns a single value */
+    if (args->nparams == 0)
+        goto success;
+
+    if (remoteSerializeTypedParameters(params, args->nparams * args->ncpus,
+                                       &ret->params.params_val,
+                                       &ret->params.params_len,
+                                       args->flags) < 0)
+        goto cleanup;
+
+    percpu_len = ret->params.params_len / args->ncpus;
+
+success:
+    rv = 0;
+    ret->nparams = percpu_len;
+
+cleanup:
+    if (rv < 0)
+         virNetMessageSaveError(rerr);
+    virTypedParameterArrayClear(params, args->ncpus * args->nparams);
+    VIR_FREE(params);
+    if (dom)
+        virDomainFree(dom);
+    return rv;
+}
+
 /*----- Helpers. -----*/
 
 /* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/src/libvirt.c b/src/libvirt.c
index 7060f5a879..e702a3421b 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -18162,7 +18162,7 @@ error:
  * @nparams: number of parameters per cpu
  * @start_cpu: which cpu to start with, or -1 for summary
  * @ncpus: how many cpus to query
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virTypedParameterFlags
  *
  * Get statistics relating to CPU usage attributable to a single
  * domain (in contrast to the statistics returned by
@@ -18251,20 +18251,19 @@ int virDomainGetCPUStats(virDomainPtr domain,
      * if start_cpu is -1, ncpus must be 1
      * params == NULL must match nparams == 0
      * ncpus must be non-zero unless params == NULL
+     * nparams * ncpus must not overflow (RPC may restrict it even more)
      */
     if (start_cpu < -1 ||
         (start_cpu == -1 && ncpus != 1) ||
         ((params == NULL) != (nparams == 0)) ||
-        (ncpus == 0 && params != NULL)) {
-        virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
-        goto error;
-    }
-
-    /* remote protocol doesn't welcome big args in one shot */
-    if ((nparams > 16) || (ncpus > 128)) {
+        (ncpus == 0 && params != NULL) ||
+        ncpus < UINT_MAX / nparams) {
         virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
+    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
+        flags |= VIR_TYPED_PARAM_STRING_OKAY;
 
     if (conn->driver->domainGetCPUStats) {
         int ret;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index ead0192107..61b96e9a43 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2,7 +2,7 @@
  * remote_driver.c: driver to provide access to libvirtd running
  *   on a remote machine
  *
- * Copyright (C) 2007-2011 Red Hat, Inc.
+ * Copyright (C) 2007-2012 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -2305,6 +2305,102 @@ done:
     return rv;
 }
 
+static int remoteDomainGetCPUStats(virDomainPtr domain,
+                                   virTypedParameterPtr params,
+                                   unsigned int nparams,
+                                   int start_cpu,
+                                   unsigned int ncpus,
+                                   unsigned int flags)
+{
+    struct private_data *priv = domain->conn->privateData;
+    remote_domain_get_cpu_stats_args args;
+    remote_domain_get_cpu_stats_ret ret;
+    int rv = -1;
+    int cpu;
+
+    remoteDriverLock(priv);
+
+    if (nparams > REMOTE_NODE_CPU_STATS_MAX) {
+        remoteError(VIR_ERR_RPC,
+                    _("nparams count exceeds maximum: %u > %u"),
+                    nparams, REMOTE_NODE_CPU_STATS_MAX);
+        goto done;
+    }
+    if (ncpus > REMOTE_DOMAIN_GET_CPU_STATS_NCPUS_MAX) {
+        remoteError(VIR_ERR_RPC,
+                    _("ncpus count exceeds maximum: %u > %u"),
+                    ncpus, REMOTE_DOMAIN_GET_CPU_STATS_NCPUS_MAX);
+        goto done;
+    }
+
+    make_nonnull_domain(&args.dom, domain);
+    args.nparams = nparams;
+    args.start_cpu = start_cpu;
+    args.ncpus = ncpus;
+    args.flags = flags;
+
+    memset(&ret, 0, sizeof(ret));
+
+    if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_CPU_STATS,
+             (xdrproc_t) xdr_remote_domain_get_cpu_stats_args,
+             (char *) &args,
+             (xdrproc_t) xdr_remote_domain_get_cpu_stats_ret,
+             (char *) &ret) == -1)
+        goto done;
+
+    /* Check the length of the returned list carefully. */
+    if (ret.params.params_len > nparams * ncpus ||
+        (ret.params.params_len &&
+         ret.nparams * ncpus != ret.params.params_len)) {
+        remoteError(VIR_ERR_RPC, "%s",
+                    _("remoteDomainGetCPUStats: "
+                      "returned number of stats exceeds limit"));
+        memset(params, 0, sizeof(*params) * nparams * ncpus);
+        goto cleanup;
+    }
+
+    /* Handle the case when the caller does not know the number of stats
+     * and is asking for the number of stats supported
+     */
+    if (nparams == 0) {
+        rv = ret.nparams;
+        goto cleanup;
+    }
+
+    /* The remote side did not send back any zero entries, so we have
+     * to expand things back into a possibly sparse array.
+     */
+    memset(params, 0, sizeof(*params) * nparams * ncpus);
+    for (cpu = 0; cpu < ncpus; cpu++) {
+        int tmp = nparams;
+        remote_typed_param *stride = &ret.params.params_val[cpu * ret.nparams];
+
+        if (remoteDeserializeTypedParameters(stride, ret.nparams,
+                                             REMOTE_NODE_CPU_STATS_MAX,
+                                             &params[cpu * nparams],
+                                             &tmp) < 0)
+            goto cleanup;
+    }
+
+    rv = ret.nparams;
+cleanup:
+    if (rv < 0) {
+        int max = nparams * ncpus;
+        int i;
+
+        for (i = 0; i < max; i++) {
+            if (params[i].type == VIR_TYPED_PARAM_STRING)
+                VIR_FREE(params[i].value.s);
+        }
+    }
+    xdr_free ((xdrproc_t) xdr_remote_domain_get_cpu_stats_ret,
+              (char *) &ret);
+done:
+    remoteDriverUnlock(priv);
+    return rv;
+}
+
+
 /*----------------------------------------------------------------------*/
 
 static virDrvOpenStatus ATTRIBUTE_NONNULL (1)
@@ -4752,6 +4848,7 @@ static virDriver remote_driver = {
     .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */
     .domainSetNumaParameters = remoteDomainSetNumaParameters, /* 0.9.9 */
     .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */
+    .domainGetCPUStats = remoteDomainGetCPUStats, /* 0.9.10 */
 };
 
 static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index d0f75bbae9..b58925a235 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -208,6 +208,17 @@ const REMOTE_DOMAIN_SEND_KEY_MAX = 16;
  */
 const REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX = 16;
 
+/*
+ * Upper limit on cpus involved in per-cpu stats
+ */
+const REMOTE_DOMAIN_GET_CPU_STATS_NCPUS_MAX = 128;
+
+/*
+ * Upper limit on list of per-cpu stats:
+ *  REMOTE_NODE_CPU_STATS_MAX * REMOTE_DOMAIN_GET_CPU_STATS_MAX
+ */
+const REMOTE_DOMAIN_GET_CPU_STATS_MAX = 2048;
+
 /* UUID.  VIR_UUID_BUFLEN definition comes from libvirt.h */
 typedef opaque remote_uuid[VIR_UUID_BUFLEN];
 
@@ -1156,6 +1167,19 @@ struct remote_domain_get_block_io_tune_ret {
     int nparams;
 };
 
+struct remote_domain_get_cpu_stats_args {
+    remote_nonnull_domain dom;
+    unsigned int nparams;
+    int          start_cpu;
+    unsigned int ncpus;
+    unsigned int flags;
+};
+
+struct remote_domain_get_cpu_stats_ret {
+    remote_typed_param params<REMOTE_DOMAIN_GET_CPU_STATS_MAX>;
+    int nparams;
+};
+
 /* Network calls: */
 
 struct remote_num_of_networks_ret {
@@ -2683,7 +2707,8 @@ enum remote_procedure {
     REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 259, /* autogen autogen */
     REMOTE_PROC_STORAGE_VOL_RESIZE = 260, /* autogen autogen */
 
-    REMOTE_PROC_DOMAIN_PM_SUSPEND_FOR_DURATION = 261 /* autogen autogen */
+    REMOTE_PROC_DOMAIN_PM_SUSPEND_FOR_DURATION = 261, /* autogen autogen */
+    REMOTE_PROC_DOMAIN_GET_CPU_STATS = 262 /* skipgen skipgen */
 
     /*
      * Notice how the entries are grouped in sets of 10 ?
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index ad08fd5774..5eac9bf9f0 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -835,6 +835,20 @@ struct remote_domain_get_block_io_tune_ret {
         } params;
         int                        nparams;
 };
+struct remote_domain_get_cpu_stats_args {
+        remote_nonnull_domain      dom;
+        u_int                      nparams;
+        int                        start_cpu;
+        u_int                      ncpus;
+        u_int                      flags;
+};
+struct remote_domain_get_cpu_stats_ret {
+        struct {
+                u_int              params_len;
+                remote_typed_param * params_val;
+        } params;
+        int                        nparams;
+};
 struct remote_num_of_networks_ret {
         int                        num;
 };
@@ -2114,4 +2128,5 @@ enum remote_procedure {
         REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 259,
         REMOTE_PROC_STORAGE_VOL_RESIZE = 260,
         REMOTE_PROC_DOMAIN_PM_SUSPEND_FOR_DURATION = 261,
+        REMOTE_PROC_DOMAIN_GET_CPU_STATS = 262,
 };
-- 
GitLab