On 7/14/21 2:28 PM, Yang Fei wrote:
Add helper function virGetCpuHaltPollTime and virGetDebugFsKvmValue to obtain the halt polling time. If system mount debugfs, and the kernel support halt polling time statistic, it will work.
Signed-off-by: Yang Fei <yangfei85@huawei.com> --- src/libvirt_private.syms | 2 ++ src/util/virutil.c | 78 ++++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 9 +++++ 3 files changed, 89 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 68e4b6aab8..f92213b8c2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3479,6 +3479,8 @@ virDoesUserExist; virDoubleToStr; virFormatIntDecimal; virFormatIntPretty; +virGetCpuHaltPollTime; +virGetDebugFsKvmValue; virGetDeviceID; virGetDeviceUnprivSGIO; virGetGroupID; diff --git a/src/util/virutil.c b/src/util/virutil.c index c0d25fe247..7e4531b4b4 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1959,3 +1959,81 @@ virGetValueRaw(const char *path,
return 0; } + +int +virGetDebugFsKvmValue(struct dirent *ent, + const char *path, + const char *filename, + unsigned long long *value) +{ + g_autofree char *valToStr = NULL; + g_autofree char *valPath = NULL; + + valPath = g_strdup_printf("%s/%s/%s", path, ent->d_name, filename); + + if (virGetValueRaw(valPath, &valToStr) < 0) { + return -1; + } + + /* 10 is a Cardinality, must be between 2 and 36 inclusive, + * or special value 0. Used in fuction strtoull() + */ + if (virStrToLong_ull(valToStr, NULL, 10, value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), valToStr); + return -1; + } + + return 0; +}
This looks very close to what virFileReadValueUllong() does.
+ +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; + DIR *dir = NULL; + int ret = -1; + int flag = 0; + + if (!(debugFsPath = virFileFindMountPoint("debugfs"))) { + virReportSystemError(errno, "%s", + _("unable to find debugfs mountpoint")); + return ret; + }
Is this something worth polluting logs with error? What if we run under kernel that doesn't have debugfs?
+ + completePath = g_strdup_printf("%s/%s", debugFsPath, "kvm"); + if (virDirOpen(&dir, completePath) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s %s", "Can not open directory", completePath); + return ret; + }
virDirOpen() reports an error on failure. No need to rewrite it. And in the light of previous comment maybe virDirOpenIfExists() instead?
+ + pidToStr = g_strdup_printf("%lld%c", (unsigned long long)pid, '-'); + while (virDirRead(dir, &ent, NULL) > 0) { + if (STRPREFIX(ent->d_name, pidToStr)) { + flag = 1; + break; + } + } + + if (flag == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not find VM(Pid %s) in '%s'"), pidToStr, completePath); + goto cleanup; + }
Again, not big fan. What it my domain runs TCG? Also, please rename 'flag' to something more specific, e.g. "found".
+ + if (virGetDebugFsKvmValue(ent, completePath, "halt_poll_success_ns", haltPollSuccess) < 0 || + virGetDebugFsKvmValue(ent, completePath, "halt_poll_fail_ns", haltPollFail) < 0) { + goto cleanup; + } + + ret = 0; +cleanup: + closedir(dir);
Again, please make sure 'ninja test' passes. We have a syntax-check rule that forbids direct call of closedir(). The proper way is to define dir variable like this: g_autoptr(DIR) dir = NULL;
+ return ret; +} diff --git a/src/util/virutil.h b/src/util/virutil.h index 851b421476..00cef47208 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -228,3 +228,12 @@ int virPipeNonBlock(int fds[2]);
int virGetValueRaw(const char *path, char **value); + +int virGetDebugFsKvmValue(struct dirent *ent, + const char *path, + const char *filename, + unsigned long long *value); + +int virGetCpuHaltPollTime(pid_t pid, + unsigned long long *haltPollSuccess, + unsigned long long *haltPollFail);
Michal