[libvirt] [PATCH 0/2] add user and system times to domain cpu stats

See also https://bugzilla.redhat.com/show_bug.cgi?id=800366 Eric Blake (2): cpustats: collect VM user and sys times cpustats: report user and sys times include/libvirt/libvirt.h.in | 12 ++++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++------ src/util/cgroup.c | 51 ++++++++++++++++++++++++++++++++++++++++- src/util/cgroup.h | 4 ++- src/util/virtypedparam.h | 5 ++- tools/virsh.c | 12 +++++---- 7 files changed, 112 insertions(+), 18 deletions(-) -- 1.7.7.6

As documented in linux.git/Documentation/cgroups/cpuacct.txt, cpuacct.stat returns user and system time in ticks (the same unit used in times(2)). It would be a bit nicer if it were like getrusage(2) and reported timeval contents, or like cpuacct.usage and in nanoseconds, but we can't be picky. * src/util/cgroup.h (virCgroupGetCpuacctStat): New function. * src/util/cgroup.c (virCgroupGetCpuacctStat): Implement it. (virCgroupGetValueStr): Allow for multi-line files. * src/libvirt_private.syms (cgroup.h): Export it. --- src/libvirt_private.syms | 1 + src/util/cgroup.c | 51 ++++++++++++++++++++++++++++++++++++++++++++- src/util/cgroup.h | 4 ++- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d7ec221..1f55f5d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -78,6 +78,7 @@ virCgroupGetCpuCfsPeriod; virCgroupGetCpuCfsQuota; virCgroupGetCpuShares; virCgroupGetCpuacctPercpuUsage; +virCgroupGetCpuacctStat; virCgroupGetCpuacctUsage; virCgroupGetCpusetMems; virCgroupGetFreezerState; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index c150fbb..ad49bc2 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -355,8 +355,8 @@ static int virCgroupGetValueStr(virCgroupPtr group, VIR_DEBUG("Failed to read %s: %m\n", keypath); } else { /* Terminated with '\n' has sometimes harmful effects to the caller */ - char *p = strchr(*value, '\n'); - if (p) *p = '\0'; + if ((*value)[rc - 1] == '\n') + (*value)[rc - 1] = '\0'; rc = 0; } @@ -1561,6 +1561,53 @@ int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage) "cpuacct.usage_percpu", usage); } +#ifdef _SC_CLK_TCK +int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, + unsigned long long *sys) +{ + char *str; + char *p; + int ret; + static double scale = -1.0; + + if ((ret = virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, + "cpuacct.stat", &str)) < 0) + return ret; + if (!(p = STRSKIP(str, "user ")) || + virStrToLong_ull(p, &p, 10, user) < 0 || + !(p = STRSKIP(p, "\nsystem ")) || + virStrToLong_ull(p, NULL, 10, sys) < 0) { + ret = -EINVAL; + goto cleanup; + } + /* times reported are in system ticks (generally 100 Hz), but that + * rate can theoretically vary between machines. Scale things + * into approximate nanoseconds. */ + if (scale < 0) { + long ticks_per_sec = sysconf(_SC_CLK_TCK); + if (ticks_per_sec == -1) { + ret = -errno; + goto cleanup; + } + scale = 1000000000.0 / ticks_per_sec; + } + *user *= scale; + *sys *= scale; + + ret = 0; +cleanup: + VIR_FREE(str); + return ret; +} +#else +int virCgroupGetCpuacctStat(virCgroupPtr group ATTRIBUTE_UNUSED, + unsigned long long *user ATTRIBUTE_UNUSED, + unsigned long long *sys ATTRIBUTE_UNUSED) +{ + return -ENOSYS; +} +#endif + int virCgroupSetFreezerState(virCgroupPtr group, const char *state) { return virCgroupSetValueStr(group, diff --git a/src/util/cgroup.h b/src/util/cgroup.h index b4e0f37..8486c42 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -1,7 +1,7 @@ /* * cgroup.h: Interface to tools for managing cgroups * - * Copyright (C) 2011 Red Hat, Inc. + * Copyright (C) 2011-2012 Red Hat, Inc. * Copyright IBM Corp. 2008 * * See COPYING.LIB for the License of this software @@ -116,6 +116,8 @@ int virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota); int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage); int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage); +int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, + unsigned long long *sys); int virCgroupSetFreezerState(virCgroupPtr group, const char *state); int virCgroupGetFreezerState(virCgroupPtr group, char **state); -- 1.7.7.6

On 2012年03月10日 01:37, Eric Blake wrote:
As documented in linux.git/Documentation/cgroups/cpuacct.txt, cpuacct.stat returns user and system time in ticks (the same unit used in times(2)). It would be a bit nicer if it were like getrusage(2) and reported timeval contents, or like cpuacct.usage and in nanoseconds, but we can't be picky.
* src/util/cgroup.h (virCgroupGetCpuacctStat): New function. * src/util/cgroup.c (virCgroupGetCpuacctStat): Implement it. (virCgroupGetValueStr): Allow for multi-line files. * src/libvirt_private.syms (cgroup.h): Export it. --- src/libvirt_private.syms | 1 + src/util/cgroup.c | 51 ++++++++++++++++++++++++++++++++++++++++++++- src/util/cgroup.h | 4 ++- 3 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d7ec221..1f55f5d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -78,6 +78,7 @@ virCgroupGetCpuCfsPeriod; virCgroupGetCpuCfsQuota; virCgroupGetCpuShares; virCgroupGetCpuacctPercpuUsage; +virCgroupGetCpuacctStat; virCgroupGetCpuacctUsage; virCgroupGetCpusetMems; virCgroupGetFreezerState; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index c150fbb..ad49bc2 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -355,8 +355,8 @@ static int virCgroupGetValueStr(virCgroupPtr group, VIR_DEBUG("Failed to read %s: %m\n", keypath); } else { /* Terminated with '\n' has sometimes harmful effects to the caller */ - char *p = strchr(*value, '\n'); - if (p) *p = '\0'; + if ((*value)[rc - 1] == '\n') + (*value)[rc - 1] = '\0';
rc = 0; } @@ -1561,6 +1561,53 @@ int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage) "cpuacct.usage_percpu", usage); }
+#ifdef _SC_CLK_TCK +int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, + unsigned long long *sys) +{ + char *str; + char *p; + int ret; + static double scale = -1.0; + + if ((ret = virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, + "cpuacct.stat",&str))< 0) + return ret; + if (!(p = STRSKIP(str, "user ")) || + virStrToLong_ull(p,&p, 10, user)< 0 || + !(p = STRSKIP(p, "\nsystem ")) || + virStrToLong_ull(p, NULL, 10, sys)< 0) { + ret = -EINVAL; + goto cleanup; + } + /* times reported are in system ticks (generally 100 Hz), but that + * rate can theoretically vary between machines. Scale things + * into approximate nanoseconds. */ + if (scale< 0) { + long ticks_per_sec = sysconf(_SC_CLK_TCK); + if (ticks_per_sec == -1) { + ret = -errno; + goto cleanup; + } + scale = 1000000000.0 / ticks_per_sec; + } + *user *= scale; + *sys *= scale; + + ret = 0; +cleanup: + VIR_FREE(str); + return ret; +} +#else +int virCgroupGetCpuacctStat(virCgroupPtr group ATTRIBUTE_UNUSED, + unsigned long long *user ATTRIBUTE_UNUSED, + unsigned long long *sys ATTRIBUTE_UNUSED) +{ + return -ENOSYS; +} +#endif + int virCgroupSetFreezerState(virCgroupPtr group, const char *state) { return virCgroupSetValueStr(group, diff --git a/src/util/cgroup.h b/src/util/cgroup.h index b4e0f37..8486c42 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -1,7 +1,7 @@ /* * cgroup.h: Interface to tools for managing cgroups * - * Copyright (C) 2011 Red Hat, Inc. + * Copyright (C) 2011-2012 Red Hat, Inc. * Copyright IBM Corp. 2008 * * See COPYING.LIB for the License of this software @@ -116,6 +116,8 @@ int virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota);
int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage); int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage); +int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, + unsigned long long *sys);
int virCgroupSetFreezerState(virCgroupPtr group, const char *state); int virCgroupGetFreezerState(virCgroupPtr group, char **state);
ACK

Thanks to cgroups, providing user vs. system time of the overall guest is easy to add to our existing API. * include/libvirt/libvirt.h.in (VIR_DOMAIN_CPU_STATS_USERTIME) (VIR_DOMAIN_CPU_STATS_SYSTEMTIME): New constants. * src/util/virtypedparam.h (virTypedParameterArrayValidate) (virTypedParameterAssign): Enforce checking the result. * src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Fix offender. (qemuDomainGetTotalcpuStats): Implement new parameters. * tools/virsh.c (cmdCPUStats): Tweak output accordingly. --- include/libvirt/libvirt.h.in | 12 +++++++++++ src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++------- src/util/virtypedparam.h | 5 ++- tools/virsh.c | 12 ++++++---- 4 files changed, 59 insertions(+), 15 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 66883fb..7d41642 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1332,6 +1332,18 @@ int virDomainGetState (virDomainPtr domain, */ #define VIR_DOMAIN_CPU_STATS_CPUTIME "cpu_time" +/** + * VIR_DOMAIN_CPU_STATS_USERTIME: + * cpu time charged to user instructions in nanoseconds, as a ullong + */ +#define VIR_DOMAIN_CPU_STATS_USERTIME "user_time" + +/** + * VIR_DOMAIN_CPU_STATS_SYSTEMTIME: + * cpu time charged to system instructions in nanoseconds, as a ullong + */ +#define VIR_DOMAIN_CPU_STATS_SYSTEMTIME "system_time" + int virDomainGetCPUStats(virDomainPtr domain, virTypedParameterPtr params, unsigned int nparams, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index be678f3..3147a9b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -101,6 +101,9 @@ #define QEMU_NB_NUMA_PARAM 2 +#define QEMU_NB_TOTAL_CPU_STAT_PARAM 3 +#define QEMU_NB_PER_CPU_STAT_PARAM 1 + #if HAVE_LINUX_KVM_H # include <linux/kvm.h> #endif @@ -12145,11 +12148,10 @@ qemuDomainGetTotalcpuStats(virCgroupPtr group, int nparams) { unsigned long long cpu_time; - int param_idx = 0; int ret; if (nparams == 0) /* return supported number of params */ - return 1; + return QEMU_NB_TOTAL_CPU_STAT_PARAM; /* entry 0 is cputime */ ret = virCgroupGetCpuacctUsage(group, &cpu_time); if (ret < 0) { @@ -12157,9 +12159,35 @@ qemuDomainGetTotalcpuStats(virCgroupPtr group, return -1; } - virTypedParameterAssign(¶ms[param_idx], VIR_DOMAIN_CPU_STATS_CPUTIME, - VIR_TYPED_PARAM_ULLONG, cpu_time); - return 1; + if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_CPU_STATS_CPUTIME, + VIR_TYPED_PARAM_ULLONG, cpu_time) < 0) + return -1; + + if (nparams > 1) { + unsigned long long user; + unsigned long long sys; + + ret = virCgroupGetCpuacctStat(group, &user, &sys); + if (ret < 0) { + virReportSystemError(-ret, "%s", _("unable to get cpu account")); + return -1; + } + + if (virTypedParameterAssign(¶ms[1], + VIR_DOMAIN_CPU_STATS_USERTIME, + VIR_TYPED_PARAM_ULLONG, user) < 0) + return -1; + if (nparams > 2 && + virTypedParameterAssign(¶ms[2], + VIR_DOMAIN_CPU_STATS_SYSTEMTIME, + VIR_TYPED_PARAM_ULLONG, sys) < 0) + return -1; + + if (nparams > QEMU_NB_TOTAL_CPU_STAT_PARAM) + nparams = QEMU_NB_TOTAL_CPU_STAT_PARAM; + } + + return nparams; } static int @@ -12180,7 +12208,7 @@ qemuDomainGetPercpuStats(virDomainPtr domain, /* return the number of supported params */ if (nparams == 0 && ncpus != 0) - return 1; /* only cpu_time is supported */ + return QEMU_NB_PER_CPU_STAT_PARAM; /* only cpu_time is supported */ /* return percpu cputime in index 0 */ param_idx = 0; @@ -12223,8 +12251,9 @@ qemuDomainGetPercpuStats(virDomainPtr domain, if (i < start_cpu) continue; ent = ¶ms[ (i - start_cpu) * nparams + param_idx]; - virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME, - VIR_TYPED_PARAM_ULLONG, cpu_time); + if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME, + VIR_TYPED_PARAM_ULLONG, cpu_time) < 0) + goto cleanup; } rv = param_idx + 1; cleanup: diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 52cbe78..7c513ed 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -29,9 +29,10 @@ void virTypedParameterArrayClear(virTypedParameterPtr params, int nparams); int virTypedParameterArrayValidate(virTypedParameterPtr params, int nparams, /* const char *name, int type ... */ ...) - ATTRIBUTE_SENTINEL; + ATTRIBUTE_SENTINEL ATTRIBUTE_RETURN_CHECK; int virTypedParameterAssign(virTypedParameterPtr param, const char *name, - int type, /* TYPE arg */ ...); + int type, /* TYPE arg */ ...) + ATTRIBUTE_RETURN_CHECK; #endif /* __VIR_TYPED_PARAM_H */ diff --git a/tools/virsh.c b/tools/virsh.c index 9a16ef8..630b77f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5630,10 +5630,10 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) for (j = 0; j < nparams; j++) { pos = i * nparams + j; - vshPrint(ctl, "\t%-10s ", params[pos].field); + vshPrint(ctl, "\t%-12s ", params[pos].field); if (STREQ(params[pos].field, VIR_DOMAIN_CPU_STATS_CPUTIME) && params[j].type == VIR_TYPED_PARAM_ULLONG) { - vshPrint(ctl, "%lld.%09lld seconds\n", + vshPrint(ctl, "%9lld.%09lld seconds\n", params[pos].value.ul / 1000000000, params[pos].value.ul % 1000000000); } else { @@ -5671,10 +5671,12 @@ do_show_total: vshPrint(ctl, _("Total:\n")); for (i = 0; i < nparams; i++) { - vshPrint(ctl, "\t%-10s ", params[i].field); - if (STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME) && + vshPrint(ctl, "\t%-12s ", params[i].field); + if ((STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME) || + STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_USERTIME) || + STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_SYSTEMTIME)) && params[i].type == VIR_TYPED_PARAM_ULLONG) { - vshPrint(ctl, "%llu.%09llu seconds\n", + vshPrint(ctl, "%9lld.%09lld seconds\n", params[i].value.ul / 1000000000, params[i].value.ul % 1000000000); } else { -- 1.7.7.6

On 2012年03月10日 01:37, Eric Blake wrote:
Thanks to cgroups, providing user vs. system time of the overall guest is easy to add to our existing API.
* include/libvirt/libvirt.h.in (VIR_DOMAIN_CPU_STATS_USERTIME) (VIR_DOMAIN_CPU_STATS_SYSTEMTIME): New constants. * src/util/virtypedparam.h (virTypedParameterArrayValidate) (virTypedParameterAssign): Enforce checking the result. * src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Fix offender. (qemuDomainGetTotalcpuStats): Implement new parameters. * tools/virsh.c (cmdCPUStats): Tweak output accordingly. --- include/libvirt/libvirt.h.in | 12 +++++++++++ src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++------- src/util/virtypedparam.h | 5 ++- tools/virsh.c | 12 ++++++---- 4 files changed, 59 insertions(+), 15 deletions(-)
ACK

On 03/10/2012 04:25 AM, Osier Yang wrote:
On 2012年03月10日 01:37, Eric Blake wrote:
Thanks to cgroups, providing user vs. system time of the overall guest is easy to add to our existing API.
* include/libvirt/libvirt.h.in (VIR_DOMAIN_CPU_STATS_USERTIME) (VIR_DOMAIN_CPU_STATS_SYSTEMTIME): New constants. * src/util/virtypedparam.h (virTypedParameterArrayValidate) (virTypedParameterAssign): Enforce checking the result. * src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Fix offender. (qemuDomainGetTotalcpuStats): Implement new parameters. * tools/virsh.c (cmdCPUStats): Tweak output accordingly. --- include/libvirt/libvirt.h.in | 12 +++++++++++ src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++------- src/util/virtypedparam.h | 5 ++- tools/virsh.c | 12 ++++++---- 4 files changed, 59 insertions(+), 15 deletions(-)
ACK
Thanks; series pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Osier Yang