On 2014/9/8 21:05, Francesco Romani wrote:
This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL group of statistics.
Signed-off-by: Francesco Romani <fromani@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 9 ++++++++ src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) [...] diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2950a4b..cfc5941 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -96,6 +96,7 @@ #include "storage/storage_driver.h" #include "virhostdev.h" #include "domain_capabilities.h" +#include "vircgroup.h"
Hi, Francesco. I see the file including relationship. 'qemu_driver.c' includes 'qemu_cgroup.h' which includes 'vircgroup.h'. There are other virCgroupGet* functions called in qemu_driver.c now. So I think here "include vircgroup.h" is not necessary.
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -17338,6 +17339,55 @@ qemuDomainGetStatsState(virConnectPtr conn ATTRIBUTE_UNUSED, }
+static int +qemuDomainGetStatsCpu(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + qemuDomainObjPrivatePtr priv = dom->privateData; + unsigned long long cpu_time = 0; + unsigned long long user_time = 0; + unsigned long long sys_time = 0; + int ncpus = 0; + int err; + + ncpus = nodeGetCPUCount(); + if (ncpus > 0 && + virTypedParamsAddUInt(&record->params, + &record->nparams, + maxparams, + "cpu.count", + (unsigned int)ncpus) < 0) + return -1; + + err = virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time); + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.time", + cpu_time) < 0) + return -1; + err = virCgroupGetCpuacctStat(priv->cgroup, &user_time, &sys_time); + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.user", + user_time) < 0) + return -1; + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.system", + sys_time) < 0) + return -1;
1. If any of the 'err's is not zero, the function may returns 0 as success. Is this the expected return? Or at least we can give a warning that we miss some parameters. 2. I think it's better to report an error or warning log before return -1.
+ return 0; +} + + typedef int (*qemuDomainGetStatsFunc)(virConnectPtr conn, virDomainObjPtr dom, @@ -17353,6 +17403,7 @@ struct qemuDomainGetStatsWorker {
static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false }, + { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false }, { NULL, 0, false } };