Re: [PATCH] test_driver: implement virDomainGetCPUStats

[Daniel P. Berrangé <berrange@xxxxxxxxxx>]

On Thu, Jul 25, 2019 at 02:00:56PM +0200, Erik Skultety wrote:
> On Thu, Jul 18, 2019 at 12:02:43PM +0200, Ilias Stamatis wrote:
> > Signed-off-by: Ilias Stamatis <stamatis.iliass@xxxxxxxxx>
> > ---
> >  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(&params[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
> > +                                VIR_TYPED_PARAM_ULLONG, 77102913900) < 0)
> > +        return -1;
> > +
> > +    if (nparams > 1 &&
> > +        virTypedParameterAssign(&params[1],
> > +                                VIR_DOMAIN_CPU_STATS_USERTIME,
> > +                                VIR_TYPED_PARAM_ULLONG, 45650000000) < 0)
> > +        return -1;
> > +
> > +    if (nparams > 2 &&
> > +        virTypedParameterAssign(&params[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(&params[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.

Note that this calculation  is going to be overflowing 32-bit ints,
so the 202542145062 constant needs a LL suffix to force 64-bit
arthimetic too, so we don't break on 32-bit hosts.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list