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@xxxxxxxxxx> > --- > 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