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@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); } + +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.
+ 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) {
As you can see here, this is one of the functions having terrible semantics. It sometimes does set the libvirt error object and sometimes doesn't, thats why we try to avoid those.
+ return ret; + } + + ret = 0; + + return ret; +}
.
Thanks, Fei.