On 7/16/21 12:42 PM, Yang Fei wrote:
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@huawei.com> --- 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); } +
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?
+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;
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.
+ 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) {
We like to format this as: if (condition1 || condition2)
+ return ret; + } + + ret = 0; + + return ret; +}
Michal