Re: [PATCH v3 1/2] util: Add virGetCpuHaltPollTime

On 2021/7/19 17:05, Peter Krempa wrote: > On Fri, Jul 16, 2021 at 18:42:22 +0800, 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(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; > > Here '-1' is returned, but no libvirt error is reported ... > >> + >> + completePath = g_strdup_printf("%s/%s", debugFsPath, "kvm"); >> + if (virDirOpenIfExists(&dir, completePath) != 1) >> + return ret; > > While here -1 is reported and an error may be reported (e.g on EACCESS). > We generally avoid this pattern because the caller cant properly handle > that. > >> + >> + pidToStr = g_strdup_printf("%d%c", pid, '-'); > > I'm fairly certain this will fail to compile on mingw since %d isn't the > proper formatting modifier for pid_t. > 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.

>> + while (virDirRead(dir, &ent, NULL) > 0) { >> + if (STRPREFIX(ent->d_name, pidToStr)) { > > Any idea what the suffix after '-' in the debugfs entry is? If we can > figure it out we won't have to look up the correct file. > 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.