Re: [PATCH v3 1/2] util: Add virGetCpuHaltPollTime
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(a)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