[PATCH v4 0/3] domstats:add haltpolling time statistic interface
This series add the ability to statistic the halt polling time when VM execute HLT(arm is WFI). v1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00029.html v2: https://listman.redhat.com/archives/libvir-list/2021-July/msg00339.html v3: https://listman.redhat.com/archives/libvir-list/2021-July/msg00445.html changes from v1: - Move virGetCgroupValueRaw to utils.c and rename it virGetValueRaw. So that we can call it to obtain halt polling time. - Helper function virGetCpuHaltPollTime and virGetDebugFsKvmValue are added in a separate patch - Use STRPREFIX to match the path prefix. - Fix the logic that domstats will break when platform is non-linux, debugfs isn't mounted and so on. change from v2: - Drop patch 1, use virFileReadValueUllong() to get halt polling data. - Delete unnecessary error report in logs. - Remove the qemuDomainGetStatsCpuHaltPollTime function conditionally compiled on linux. - Document the new parameters in src/libvirt-domain.c. change from v3: - Add function virFileReadValueUllongQuiet without error report. - Move virGetCpuHaltPollTime to src/util/virhostcpu.c and change the name to virHostCPUGetHaltPollTime. - Replace the function which will report errors: virDirOpenIfExists -> virDirOpenQuiet virFileReadValueUllong -> virFileReadValueUllongQuiet Yang Fei (3): util: Add virFileReadValueUllongQuiet util: Add virHostCPUGetHaltPollTime qemu: Introduce qemuDomainGetStatsCpuHaltPollTime docs/manpages/virsh.rst | 4 ++++ src/libvirt-domain.c | 7 +++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 20 ++++++++++++++++++++ src/util/virfile.c | 24 ++++++++++++++++++++++++ src/util/virfile.h | 2 ++ src/util/virhostcpu.c | 39 +++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 4 ++++ 8 files changed, 102 insertions(+) -- 2.23.0
Use function virFileReadValueUllongQuiet to read unsigned long long value without error report. Signed-off-by: Yang Fei <yangfei85@huawei.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 24 ++++++++++++++++++++++++ src/util/virfile.h | 2 ++ 3 files changed, 27 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4bc2974e7f..5429a82a7c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2242,6 +2242,7 @@ virFileReadValueScaledInt; virFileReadValueString; virFileReadValueUint; virFileReadValueUllong; +virFileReadValueUllongQuiet; virFileRelLinkPointsTo; virFileRemove; virFileRemoveLastComponent; diff --git a/src/util/virfile.c b/src/util/virfile.c index 723e1ca6e5..70737f4751 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -4065,6 +4065,30 @@ virFileReadValueUllong(unsigned long long *value, const char *format, ...) return 0; } +int +virFileReadValueUllongQuiet(unsigned long long *value, const char *format, ...) +{ + g_autofree char *str = NULL; + g_autofree char *path = NULL; + va_list ap; + + va_start(ap, format); + path = g_strdup_vprintf(format, ap); + va_end(ap); + + if (!virFileExists(path)) + return -2; + + if (virFileReadAllQuiet(path, VIR_INT64_STR_BUFLEN, &str) < 0) + return -1; + + virStringTrimOptionalNewline(str); + + if (virStrToLong_ullp(str, NULL, 10, value) < 0) + return -1; + + return 0; +} /** * virFileReadValueScaledInt: diff --git a/src/util/virfile.h b/src/util/virfile.h index 72368495bf..967c9a9b4f 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -339,6 +339,8 @@ int virFileReadValueUint(unsigned int *value, const char *format, ...) G_GNUC_PRINTF(2, 3); int virFileReadValueUllong(unsigned long long *value, const char *format, ...) G_GNUC_PRINTF(2, 3); +int virFileReadValueUllongQuiet(unsigned long long *value, const char *format, ...) + G_GNUC_PRINTF(2, 3); int virFileReadValueBitmap(virBitmap **value, const char *format, ...) G_GNUC_PRINTF(2, 3); int virFileReadValueScaledInt(unsigned long long *value, const char *format, ...) -- 2.23.0
Add helper function virHostCPUGetHaltPollTime to obtain halt polling time. If the kernel support halt polling time statistic, and mount debugfs. This function will take effect on KVM VMs. Signed-off-by: Yang Fei <yangfei85@huawei.com> --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 39 +++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 4 ++++ 3 files changed, 44 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5429a82a7c..531ba03cf7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2374,6 +2374,7 @@ virHookPresent; # util/virhostcpu.h virHostCPUGetAvailableCPUsBitmap; virHostCPUGetCount; +virHostCPUGetHaltPollTime; virHostCPUGetInfo; virHostCPUGetKVMMaxVCPUs; virHostCPUGetMap; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index bf7fda23af..7f577c3e3e 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1535,3 +1535,42 @@ virHostCPUGetSignature(char **signature) } #endif /* __linux__ */ + +int +virHostCPUGetHaltPollTime(pid_t pid, + unsigned long long *haltPollSuccess, + unsigned long long *haltPollFail) +{ + g_autofree char *pidToStr = NULL; + g_autofree char *debugFsPath = NULL; + g_autofree char *kvmPath = NULL; + struct dirent *ent = NULL; + g_autoptr(DIR) dir = NULL; + bool found = false; + + if (!(debugFsPath = virFileFindMountPoint("debugfs"))) + return -1; + + kvmPath = g_strdup_printf("%s/%s", debugFsPath, "kvm"); + if (virDirOpenQuiet(&dir, kvmPath) != 1) + return -1; + + pidToStr = g_strdup_printf("%lld%c", (long long)pid, '-'); + while (virDirRead(dir, &ent, NULL) > 0) { + if (STRPREFIX(ent->d_name, pidToStr)) { + found = true; + break; + } + } + + if (!found) + return -1; + + if (virFileReadValueUllongQuiet(haltPollSuccess, "%s/%s/%s", kvmPath, + ent->d_name, "halt_poll_success_ns") < 0 || + virFileReadValueUllongQuiet(haltPollFail, "%s/%s/%s", kvmPath, + ent->d_name, "halt_poll_fail_ns") < 0) + return -1; + + return 0; +} diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h index fc717250d9..d98385d53f 100644 --- a/src/util/virhostcpu.h +++ b/src/util/virhostcpu.h @@ -83,3 +83,7 @@ int virHostCPUGetMSR(unsigned long index, virHostCPUTscInfo *virHostCPUGetTscInfo(void); int virHostCPUGetSignature(char **signature); + +int virHostCPUGetHaltPollTime(pid_t pid, + unsigned long long *haltPollSuccess, + unsigned long long *haltPollFail); -- 2.23.0
On 7/22/21 10:05 AM, Yang Fei wrote:
Add helper function virHostCPUGetHaltPollTime to obtain halt polling time. If the kernel support halt polling time statistic, and mount debugfs. This function will take effect on KVM VMs.
Signed-off-by: Yang Fei <yangfei85@huawei.com> --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 39 +++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 4 ++++ 3 files changed, 44 insertions(+)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index bf7fda23af..7f577c3e3e 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1535,3 +1535,42 @@ virHostCPUGetSignature(char **signature) }
#endif /* __linux__ */ + +int +virHostCPUGetHaltPollTime(pid_t pid, + unsigned long long *haltPollSuccess, + unsigned long long *haltPollFail) +{ + g_autofree char *pidToStr = NULL; + g_autofree char *debugFsPath = NULL; + g_autofree char *kvmPath = NULL; + struct dirent *ent = NULL; + g_autoptr(DIR) dir = NULL; + bool found = false; + + if (!(debugFsPath = virFileFindMountPoint("debugfs"))) + return -1; + + kvmPath = g_strdup_printf("%s/%s", debugFsPath, "kvm"); + if (virDirOpenQuiet(&dir, kvmPath) != 1) + return -1; + + pidToStr = g_strdup_printf("%lld%c", (long long)pid, '-');
Just a tiny nit: this could be "%lld-".
+ while (virDirRead(dir, &ent, NULL) > 0) { + if (STRPREFIX(ent->d_name, pidToStr)) { + found = true; + break; + } + } + + if (!found) + return -1; + + if (virFileReadValueUllongQuiet(haltPollSuccess, "%s/%s/%s", kvmPath, + ent->d_name, "halt_poll_success_ns") < 0 || + virFileReadValueUllongQuiet(haltPollFail, "%s/%s/%s", kvmPath, + ent->d_name, "halt_poll_fail_ns") < 0) + return -1; + + return 0; +}
Michal
On 2021/7/27 16:31, Michal Prívozník wrote:
On 7/22/21 10:05 AM, Yang Fei wrote:
Add helper function virHostCPUGetHaltPollTime to obtain halt polling time. If the kernel support halt polling time statistic, and mount debugfs. This function will take effect on KVM VMs.
Signed-off-by: Yang Fei <yangfei85@huawei.com> --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 39 +++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 4 ++++ 3 files changed, 44 insertions(+)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index bf7fda23af..7f577c3e3e 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1535,3 +1535,42 @@ virHostCPUGetSignature(char **signature) }
#endif /* __linux__ */ + +int +virHostCPUGetHaltPollTime(pid_t pid, + unsigned long long *haltPollSuccess, + unsigned long long *haltPollFail) +{ + g_autofree char *pidToStr = NULL; + g_autofree char *debugFsPath = NULL; + g_autofree char *kvmPath = NULL; + struct dirent *ent = NULL; + g_autoptr(DIR) dir = NULL; + bool found = false; + + if (!(debugFsPath = virFileFindMountPoint("debugfs"))) + return -1; + + kvmPath = g_strdup_printf("%s/%s", debugFsPath, "kvm"); + if (virDirOpenQuiet(&dir, kvmPath) != 1) + return -1; + + pidToStr = g_strdup_printf("%lld%c", (long long)pid, '-');
Just a tiny nit: this could be "%lld-".
Thanks for your review Michal. And do I need to send another version to fix it now?
+ while (virDirRead(dir, &ent, NULL) > 0) { + if (STRPREFIX(ent->d_name, pidToStr)) { + found = true; + break; + } + } + + if (!found) + return -1; + + if (virFileReadValueUllongQuiet(haltPollSuccess, "%s/%s/%s", kvmPath, + ent->d_name, "halt_poll_success_ns") < 0 || + virFileReadValueUllongQuiet(haltPollFail, "%s/%s/%s", kvmPath, + ent->d_name, "halt_poll_fail_ns") < 0) + return -1; + + return 0; +}
Michal
.
Thanks, Fei.
On 7/27/21 11:02 AM, Yang Fei wrote:
On 2021/7/27 16:31, Michal Prívozník wrote:
On 7/22/21 10:05 AM, Yang Fei wrote:
Add helper function virHostCPUGetHaltPollTime to obtain halt polling time. If the kernel support halt polling time statistic, and mount debugfs. This function will take effect on KVM VMs.
Signed-off-by: Yang Fei <yangfei85@huawei.com> --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 39 +++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 4 ++++ 3 files changed, 44 insertions(+)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index bf7fda23af..7f577c3e3e 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1535,3 +1535,42 @@ virHostCPUGetSignature(char **signature) }
#endif /* __linux__ */ + +int +virHostCPUGetHaltPollTime(pid_t pid, + unsigned long long *haltPollSuccess, + unsigned long long *haltPollFail) +{ + g_autofree char *pidToStr = NULL; + g_autofree char *debugFsPath = NULL; + g_autofree char *kvmPath = NULL; + struct dirent *ent = NULL; + g_autoptr(DIR) dir = NULL; + bool found = false; + + if (!(debugFsPath = virFileFindMountPoint("debugfs"))) + return -1; + + kvmPath = g_strdup_printf("%s/%s", debugFsPath, "kvm"); + if (virDirOpenQuiet(&dir, kvmPath) != 1) + return -1; + + pidToStr = g_strdup_printf("%lld%c", (long long)pid, '-');
Just a tiny nit: this could be "%lld-".
Thanks for your review Michal. And do I need to send another version to fix it now?
No, it's pushed. I've fixed it just before pushing. Michal
This function add halt polling time interface in domstats. So that we can use command 'virsh domstats VM' to get the data if system support. Signed-off-by: Yang Fei <yangfei85@huawei.com> --- docs/manpages/virsh.rst | 4 ++++ src/libvirt-domain.c | 7 +++++++ src/qemu/qemu_driver.c | 20 ++++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 87668f2b9a..20936994ce 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2260,6 +2260,10 @@ When selecting the *--state* group the following fields are returned: * ``cpu.time`` - total cpu time spent for this domain in nanoseconds * ``cpu.user`` - user cpu time spent in nanoseconds * ``cpu.system`` - system cpu time spent in nanoseconds +* ``cpu.haltpoll.success.time`` - cpu halt polling success time spent in + nanoseconds +* ``cpu.haltpoll.fail.time`` - cpu halt polling fail time spent in + nanoseconds * ``cpu.cache.monitor.count`` - the number of cache monitors for this domain * ``cpu.cache.monitor.<num>.name`` - the name of cache monitor <num> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 35c0df0ebc..4eb14d4176 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11625,6 +11625,13 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * "cpu.user" - user cpu time spent in nanoseconds as unsigned long long. * "cpu.system" - system cpu time spent in nanoseconds as unsigned long * long. + * "cpu.haltpoll.success.time" - halt-polling cpu usage about the VCPU polled + * until a virtual interrupt was delivered in + * nanoseconds as unsigned long long. + * "cpu.haltpoll.fail.time" - halt-polling cpu usage about the VCPU had to schedule + * out (either because the maximum poll time was reached + * or it needed to yield the CPU) in nanoseconds as + * unsigned long long. * "cpu.cache.monitor.count" - the number of cache monitors for this domain * "cpu.cache.monitor.<num>.name" - the name of cache monitor <num> * "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 521063d438..79308b7157 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17857,6 +17857,23 @@ qemuDomainGetStatsCpuCgroup(virDomainObj *dom, return 0; } +static int +qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom, + virTypedParamList *params) +{ + unsigned long long haltPollSuccess = 0; + unsigned long long haltPollFail = 0; + pid_t pid = dom->pid; + + if (virHostCPUGetHaltPollTime(pid, &haltPollSuccess, &haltPollFail) < 0) + return 0; + + if (virTypedParamListAddULLong(params, haltPollSuccess, "cpu.haltpoll.success.time") < 0 || + virTypedParamListAddULLong(params, haltPollFail, "cpu.haltpoll.fail.time") < 0) + return -1; + + return 0; +} static int qemuDomainGetStatsCpu(virQEMUDriver *driver, @@ -17870,6 +17887,9 @@ qemuDomainGetStatsCpu(virQEMUDriver *driver, if (qemuDomainGetStatsCpuCache(driver, dom, params) < 0) return -1; + if (qemuDomainGetStatsCpuHaltPollTime(dom, params) < 0) + return -1; + return 0; } -- 2.23.0
On 7/22/21 10:04 AM, Yang Fei wrote:
This series add the ability to statistic the halt polling time when VM execute HLT(arm is WFI).
v1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00029.html v2: https://listman.redhat.com/archives/libvir-list/2021-July/msg00339.html v3: https://listman.redhat.com/archives/libvir-list/2021-July/msg00445.html
changes from v1: - Move virGetCgroupValueRaw to utils.c and rename it virGetValueRaw. So that we can call it to obtain halt polling time. - Helper function virGetCpuHaltPollTime and virGetDebugFsKvmValue are added in a separate patch - Use STRPREFIX to match the path prefix. - Fix the logic that domstats will break when platform is non-linux, debugfs isn't mounted and so on.
change from v2: - Drop patch 1, use virFileReadValueUllong() to get halt polling data. - Delete unnecessary error report in logs. - Remove the qemuDomainGetStatsCpuHaltPollTime function conditionally compiled on linux. - Document the new parameters in src/libvirt-domain.c.
change from v3: - Add function virFileReadValueUllongQuiet without error report. - Move virGetCpuHaltPollTime to src/util/virhostcpu.c and change the name to virHostCPUGetHaltPollTime. - Replace the function which will report errors: virDirOpenIfExists -> virDirOpenQuiet virFileReadValueUllong -> virFileReadValueUllongQuiet
Yang Fei (3): util: Add virFileReadValueUllongQuiet util: Add virHostCPUGetHaltPollTime qemu: Introduce qemuDomainGetStatsCpuHaltPollTime
docs/manpages/virsh.rst | 4 ++++ src/libvirt-domain.c | 7 +++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 20 ++++++++++++++++++++ src/util/virfile.c | 24 ++++++++++++++++++++++++ src/util/virfile.h | 2 ++ src/util/virhostcpu.c | 39 +++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 4 ++++ 8 files changed, 102 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Congratulations on your first libvirt contribution! Michal
On 2021/7/27 16:31, Michal Prívozník wrote:
On 7/22/21 10:04 AM, Yang Fei wrote:
This series add the ability to statistic the halt polling time when VM execute HLT(arm is WFI).
v1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00029.html v2: https://listman.redhat.com/archives/libvir-list/2021-July/msg00339.html v3: https://listman.redhat.com/archives/libvir-list/2021-July/msg00445.html
changes from v1: - Move virGetCgroupValueRaw to utils.c and rename it virGetValueRaw. So that we can call it to obtain halt polling time. - Helper function virGetCpuHaltPollTime and virGetDebugFsKvmValue are added in a separate patch - Use STRPREFIX to match the path prefix. - Fix the logic that domstats will break when platform is non-linux, debugfs isn't mounted and so on.
change from v2: - Drop patch 1, use virFileReadValueUllong() to get halt polling data. - Delete unnecessary error report in logs. - Remove the qemuDomainGetStatsCpuHaltPollTime function conditionally compiled on linux. - Document the new parameters in src/libvirt-domain.c.
change from v3: - Add function virFileReadValueUllongQuiet without error report. - Move virGetCpuHaltPollTime to src/util/virhostcpu.c and change the name to virHostCPUGetHaltPollTime. - Replace the function which will report errors: virDirOpenIfExists -> virDirOpenQuiet virFileReadValueUllong -> virFileReadValueUllongQuiet
Yang Fei (3): util: Add virFileReadValueUllongQuiet util: Add virHostCPUGetHaltPollTime qemu: Introduce qemuDomainGetStatsCpuHaltPollTime
docs/manpages/virsh.rst | 4 ++++ src/libvirt-domain.c | 7 +++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 20 ++++++++++++++++++++ src/util/virfile.c | 24 ++++++++++++++++++++++++ src/util/virfile.h | 2 ++ src/util/virhostcpu.c | 39 +++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 4 ++++ 8 files changed, 102 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
and pushed. Congratulations on your first libvirt contribution!
Michal
.
Hi Michal. Do I need to add a description to the file NEWS.rst, because the content displayed by the domstats interface is changed? Thanks, Fei.
participants (2)
-
Michal Prívozník -
Yang Fei