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@xxxxxxxxxx>
> ---
> 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 }
> };
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list