5:02 a.m.
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)
+ goto cleanup;
+ }
+
+ /* return percpu vcputime in index 1 */
+ param_idx = 1;
+
+ if (param_idx < nparams) {
+ for (i = start_cpu; i < need_cpus; i++) {
+ int idx = (i - start_cpu) * nparams + param_idx;
+ if (virTypedParameterAssign(¶ms[idx],
+ VIR_DOMAIN_CPU_STATS_VCPUTIME,
+ VIR_TYPED_PARAM_ULLONG,
+ 26254023765 + 10 * i) < 0)
+ goto cleanup;
+ }
+ param_idx++;
+ }
+
+ ret = param_idx;
+ cleanup:
+ return ret;
+}
+
+
+static int
+testDomainGetCPUStats(virDomainPtr dom,
+ virTypedParameterPtr params,
+ unsigned int nparams,
+ int start_cpu,
+ unsigned int ncpus,
+ unsigned int flags)
+{
+ virDomainObjPtr vm = NULL;
+ testDriverPtr privconn = dom->conn->privateData;
+ int ret = -1;
+
+ virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
+
+ if (virDomainObjCheckActive(vm) < 0)
+ goto cleanup;
+
+ if (start_cpu == -1)
+ ret = testDomainGetDomainTotalCpuStats(params, nparams);
+ else
+ ret = testDomainGetPercpuStats(params, nparams, start_cpu, ncpus,
+ privconn->nodeInfo.cores);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
+
static int
testDomainSendProcessSignal(virDomainPtr dom,
long long pid_value,
@@ -7794,6 +7924,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainSendKey = testDomainSendKey, /* 5.5.0 */
.domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */
.domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */
+ .domainGetCPUStats = testDomainGetCPUStats, /* 5.6.0 */
.domainSendProcessSignal = testDomainSendProcessSignal, /* 5.5.0 */
.connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */
.domainManagedSave = testDomainManagedSave, /* 1.1.4 */
--
2.22.0
7 a.m.
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
7:08 a.m.
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(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.
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 :|
6:24 a.m.
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
7:44 a.m.
On Fri, Jul 26, 2019 at 01:24:39PM +0200, Ilias Stamatis wrote:
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.
I didn't want to do division really (in an ideal world we'd be okay with bit
shifts, but that's not the case unfortunately), but it's test driver, so
doesn't matter at all.
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?
Yes it is, that is expected, it would be weird if a configuration with 4 vCPUs
returns data for 2. CPUTIME needs to be static though.
Erik
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
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
1951
days inactive
1959
days old
4 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Erik Skultety -
Ilias Stamatis