Add helper function virGetCpuHaltPollTime 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(a)huawei.com&gt; --- src/libvirt_private.syms | 1 + src/util/virutil.c | 43 ++++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 4 ++++ 3 files changed, 48 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 68e4b6aab8..64aff4eca4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3479,6 +3479,7 @@ virDoesUserExist; virDoubleToStr; virFormatIntDecimal; virFormatIntPretty; +virGetCpuHaltPollTime; virGetDeviceID; virGetDeviceUnprivSGIO; virGetGroupID; diff --git a/src/util/virutil.c b/src/util/virutil.c index 311cbbf93a..f5304644c0 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1936,3 +1936,46 @@ virPipeNonBlock(int fds[2]) { return virPipeImpl(fds, true, true); } + +int +virGetCpuHaltPollTime(pid_t pid, + unsigned long long *haltPollSuccess, + unsigned long long *haltPollFail) +{ + g_autofree char *pidToStr = NULL; + g_autofree char *debugFsPath = NULL; + g_autofree char *completePath = NULL; + struct dirent *ent = NULL; + g_autoptr(DIR) dir = NULL; + int ret = -1; + bool found = false; + + if (!(debugFsPath = virFileFindMountPoint("debugfs"))) + return ret; + + completePath = g_strdup_printf("%s/%s", debugFsPath, "kvm"); + if (virDirOpenIfExists(&dir, completePath) != 1) + return ret; + + pidToStr = g_strdup_printf("%d%c", pid, '-'); + while (virDirRead(dir, &ent, NULL) > 0) { + if (STRPREFIX(ent->d_name, pidToStr)) { + found = true; + break; + } + } + + if (!found) + return ret; + + if (virFileReadValueUllong(haltPollSuccess, "%s/%s/%s", completePath, + ent->d_name, "halt_poll_success_ns") < 0 + || virFileReadValueUllong(haltPollFail, "%s/%s/%s", completePath, + ent->d_name, "halt_poll_fail_ns") < 0) { + return ret; + } + + ret = 0; + + return ret; +} diff --git a/src/util/virutil.h b/src/util/virutil.h index 854b494890..03b225185f 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -225,3 +225,7 @@ int virPipeQuiet(int fds[2]); * Returns: -1 on error, 0 on success */ int virPipeNonBlock(int fds[2]); + +int virGetCpuHaltPollTime(pid_t pid, + unsigned long long *haltPollSuccess, + unsigned long long *haltPollFail); -- 2.23.0

On 2021/7/19 17:05, Peter Krempa wrote: In the last version, I try to use %lld but failed in the ninja test. How about forcibly converts the variable pid to long long? Like this: g_strdup_printf("%lld%c", (long long)pid, '-') I find that this is used elsewhere in the code. The full directory name format is "pid-fd", The fd is assigned during VM creation. As I know, we can find it in /proc/(qemu-kvm pid)/fd, but also need to look up which one points to "anon_inode:kvm-vm". That may not be any more efficient than look up it in the kvm directory. Thanks, Fei.

On 7/16/21 12:42 PM, Yang Fei wrote: Apart from Peter's findings: I'm not sure virutil.c is the best placement for this function. I mean, virutil.c became dump of functions we did not find a better place for. How about src/util/virhostcpu.c ? That seems like a better fit, doesn't it? This variable is pretty much useless. The reason we have it in other functions is that they have the following pattern: int ret = -1; if () goto clenaup; if () goto cleanup; ret = 0; cleanup: something(); return ret; But this is not really the case for this function. You can do 'return -1' instead of 'return ret' and then plain 'return 0' at the end. We like to format this as: if (condition1 || condition2) 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(a)huawei.com&gt; --- src/libvirt-domain.c | 7 +++++++ src/qemu/qemu_driver.c | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 750e32f0ca..8e58c1b43f 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. + * "haltpollsuccess.time" - halt-polling cpu usage about the VCPU polled + * until a virtual interrupt was delivered in + * nanoseconds as unsigned long long. + * "haltpollfail.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 235f575901..adb4628558 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17839,6 +17839,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 (virGetCpuHaltPollTime(pid, &haltPollSuccess, &haltPollFail) < 0) + return 0; + + if (virTypedParamListAddULLong(params, haltPollSuccess, "haltpollsuccess.time") < 0 || + virTypedParamListAddULLong(params, haltPollFail, "haltpollfail.time") < 0) + return -1; + + return 0; +} static int qemuDomainGetStatsCpu(virQEMUDriver *driver, @@ -17852,6 +17869,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 2021/7/19 17:07, Peter Krempa wrote: OK, I'll modify it in next version. calling this function(now the virFileReadValueUllong() and virDirOpenIfExists() will record errors), just let it execute, even if it fails. Have I got that right? Thanks, Fei.