Re: [libvirt] [PATCH] test_driver: implement virDomainGetCPUStats
On Thu, Jul 25, 2019 at 2:01 PM Erik Skultety <eskultet(a)redhat.com> wrote:
On Thu, Jul 18, 2019 at 12:02:43PM +0200, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
> ---
> src/test/test_driver.c | 131 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 131 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index fcb80c9e47..2907c043cb 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -3258,6 +3258,136 @@ static int testDomainSetMetadata(virDomainPtr dom,
> return ret;
> }
>
> +
> +static int
> +testDomainGetDomainTotalCpuStats(virTypedParameterPtr params,
> + int nparams)
> +{
> + if (nparams == 0) /* return supported number of params */
> + return 3;
> +
> + if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
> + VIR_TYPED_PARAM_ULLONG, 77102913900) < 0)
> + return -1;
> +
> + if (nparams > 1 &&
> + virTypedParameterAssign(¶ms[1],
> + VIR_DOMAIN_CPU_STATS_USERTIME,
> + VIR_TYPED_PARAM_ULLONG, 45650000000) < 0)
> + return -1;
> +
> + if (nparams > 2 &&
> + virTypedParameterAssign(¶ms[2],
> + VIR_DOMAIN_CPU_STATS_SYSTEMTIME,
> + VIR_TYPED_PARAM_ULLONG, 11390000000) < 0)
> + return -1;
> +
> + if (nparams > 3)
> + nparams = 3;
> +
> + return nparams;
> +}
> +
> +
> +static int
> +testDomainGetPercpuStats(virTypedParameterPtr params,
> + unsigned int nparams,
> + int start_cpu,
> + unsigned int ncpus,
> + int total_cpus)
> +{
> + size_t i;
> + int need_cpus;
> + int param_idx;
> + int ret = -1;
> +
> + /* return the number of supported params */
> + if (nparams == 0 && ncpus != 0)
> + return 2;
> +
> + /* return total number of cpus */
> + if (ncpus == 0) {
> + ret = total_cpus;
> + goto cleanup;
> + }
> +
> + if (start_cpu >= total_cpus) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("start_cpu %d larger than maximum of %d"),
> + start_cpu, total_cpus - 1);
> + goto cleanup;
> + }
> +
> + /* return percpu cputime in index 0 */
> + param_idx = 0;
> +
> + /* number of cpus to compute */
> + need_cpus = MIN(total_cpus, start_cpu + ncpus);
> +
> + for (i = 0; i < need_cpus; i++) {
> + if (i < start_cpu)
> + continue;
> + int idx = (i - start_cpu) * nparams + param_idx;
> + if (virTypedParameterAssign(¶ms[idx],
> + VIR_DOMAIN_CPU_STATS_CPUTIME,
> + VIR_TYPED_PARAM_ULLONG,
> + 202542145062 + 10 * i) < 0)
What's the reasoning behind the formula? I'm curious, wouldn't have
202542145062 + i been enough? Anyhow, the CPUTIME should be a portion of the
total cputime of all CPUs, your per-CPU time is much bigger by the total,
that doesn't sound right. I don't care about the exact numbers, but I'd like
to
see them to make sense.
Erik
The 10 * i was totally random. You are right, let's make them make sense.
We can have a total CPUTIME and then divide that equally (?) by the
number of CPUs.
The "problem" is that the number of CPUs can vary. So depending on the
number of CPUs the test VM has been configured with, this API will
return different numbers. Is that ok?
Because I thought we would prefer to return the same fixed numbers
every time. However then they won't make sense when the number of CPUs
changes.
Ilias