1. Patchew
  2. Libvirt
  3. View series

[PATCH] qemu: Provide virDomainGetCPUStats() implementation for session connection

Michal Privoznik posted 1 patch 1 year, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/802feb24a9ed9191facebdc3ca47356ad744a6df.1673959484.git.mprivozn@redhat.com
There is a newer version of this series
src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 50 insertions(+), 2 deletions(-)
[PATCH] qemu: Provide virDomainGetCPUStats() implementation for session connection
Posted by Michal Privoznik 1 year, 3 months ago 
We have virDomainGetCPUStats() API which offers querying
statistics on host CPU usage by given guest. And it works in two
modes: getting overall stats (@start_cpu == -1, @ncpus == 1) or
getting per host CPU usage.

For the QEMU driver it is implemented by looking into values
stored in corresponding cpuacct CGroup controller. Well, this
works for system instances, where libvirt has permissions to
create CGroups and place QEMU process into them. But it does not
fly for session connection, where no CGroups are set up.

Fortunately, we can do something similar to v8.8.0-rc1~95 and use
virProcessGetStatInfo() to fill the overall stats. Unfortunately,
I haven't found any source of per host CPU usage, so we just
continue throwing an error in that case.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d6879175fe..a570057ecf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16009,6 +16009,50 @@ qemuDomainGetMetadata(virDomainPtr dom,
     return ret;
 }
 
+#define QEMU_CPU_STATS_PROC_TOTAL 3
+
+static int
+qemuDomainGetCPUStatsProc(virDomainObj *vm,
+                          virTypedParameterPtr params,
+                          unsigned int nparams)
+{
+    unsigned long long cpuTime = 0;
+    unsigned long long userTime = 0;
+    unsigned long long sysTime = 0;
+
+    if (nparams == 0) {
+        /* return supported number of params */
+        return QEMU_CPU_STATS_PROC_TOTAL;
+    }
+
+    if (virProcessGetStatInfo(&cpuTime, &userTime, &sysTime,
+                              NULL, NULL, vm->pid, 0) < 0) {
+        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+                       _("cannot read cputime for domain"));
+        return -1;
+    }
+
+    if (virTypedParameterAssign(&params[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
+                                VIR_TYPED_PARAM_ULLONG, cpuTime) < 0)
+        return -1;
+
+    if (nparams > 1 &&
+        virTypedParameterAssign(&params[1], VIR_DOMAIN_CPU_STATS_USERTIME,
+                                VIR_TYPED_PARAM_ULLONG, userTime) < 0)
+        return -1;
+
+    if (nparams > 2 &&
+        virTypedParameterAssign(&params[2], VIR_DOMAIN_CPU_STATS_SYSTEMTIME,
+                                VIR_TYPED_PARAM_ULLONG, sysTime) < 0)
+        return -1;
+
+    if (nparams > 3)
+        nparams = 3;
+
+    return nparams;
+}
+
+#undef QEMU_CPU_STATS_PROC_TOTAL
 
 static int
 qemuDomainGetCPUStats(virDomainPtr domain,
@@ -16037,8 +16081,12 @@ qemuDomainGetCPUStats(virDomainPtr domain,
         goto cleanup;
 
     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) {
-        virReportError(VIR_ERR_OPERATION_INVALID,
-                       "%s", _("cgroup CPUACCT controller is not mounted"));
+        if (start_cpu == -1) {
+            ret = qemuDomainGetCPUStatsProc(vm, params, nparams);
+        } else {
+            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                           _("cgroup CPUACCT controller is not mounted"));
+        }
         goto cleanup;
     }
 
-- 
2.38.2
Re: [PATCH] qemu: Provide virDomainGetCPUStats() implementation for session connection
Posted by Martin Kletzander 1 year, 3 months ago 
On Tue, Jan 17, 2023 at 01:44:44PM +0100, Michal Privoznik wrote:
>We have virDomainGetCPUStats() API which offers querying
>statistics on host CPU usage by given guest. And it works in two
>modes: getting overall stats (@start_cpu == -1, @ncpus == 1) or
>getting per host CPU usage.
>
>For the QEMU driver it is implemented by looking into values
>stored in corresponding cpuacct CGroup controller. Well, this
>works for system instances, where libvirt has permissions to
>create CGroups and place QEMU process into them. But it does not
>fly for session connection, where no CGroups are set up.
>
>Fortunately, we can do something similar to v8.8.0-rc1~95 and use
>virProcessGetStatInfo() to fill the overall stats. Unfortunately,
>I haven't found any source of per host CPU usage, so we just
>continue throwing an error in that case.
>

Looks ok, but there's one thing that's bothering me a bit.  On non-Linux
platforms this API would start to work and returning zeros.  Could that
be fixed somehow in a nice fashion or should I treat that as a non-issue?

>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 50 insertions(+), 2 deletions(-)
>
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index d6879175fe..a570057ecf 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -16009,6 +16009,50 @@ qemuDomainGetMetadata(virDomainPtr dom,
>     return ret;
> }
>
>+#define QEMU_CPU_STATS_PROC_TOTAL 3
>+
>+static int
>+qemuDomainGetCPUStatsProc(virDomainObj *vm,
>+                          virTypedParameterPtr params,
>+                          unsigned int nparams)
>+{
>+    unsigned long long cpuTime = 0;
>+    unsigned long long userTime = 0;
>+    unsigned long long sysTime = 0;
>+
>+    if (nparams == 0) {
>+        /* return supported number of params */
>+        return QEMU_CPU_STATS_PROC_TOTAL;
>+    }
>+
>+    if (virProcessGetStatInfo(&cpuTime, &userTime, &sysTime,
>+                              NULL, NULL, vm->pid, 0) < 0) {
>+        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>+                       _("cannot read cputime for domain"));
>+        return -1;
>+    }
>+
>+    if (virTypedParameterAssign(&params[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
>+                                VIR_TYPED_PARAM_ULLONG, cpuTime) < 0)
>+        return -1;
>+
>+    if (nparams > 1 &&
>+        virTypedParameterAssign(&params[1], VIR_DOMAIN_CPU_STATS_USERTIME,
>+                                VIR_TYPED_PARAM_ULLONG, userTime) < 0)
>+        return -1;
>+
>+    if (nparams > 2 &&
>+        virTypedParameterAssign(&params[2], VIR_DOMAIN_CPU_STATS_SYSTEMTIME,
>+                                VIR_TYPED_PARAM_ULLONG, sysTime) < 0)
>+        return -1;
>+
>+    if (nparams > 3)
>+        nparams = 3;
>+
>+    return nparams;
>+}
>+
>+#undef QEMU_CPU_STATS_PROC_TOTAL
>
> static int
> qemuDomainGetCPUStats(virDomainPtr domain,
>@@ -16037,8 +16081,12 @@ qemuDomainGetCPUStats(virDomainPtr domain,
>         goto cleanup;
>
>     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) {
>-        virReportError(VIR_ERR_OPERATION_INVALID,
>-                       "%s", _("cgroup CPUACCT controller is not mounted"));
>+        if (start_cpu == -1) {
>+            ret = qemuDomainGetCPUStatsProc(vm, params, nparams);
>+        } else {
>+            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>+                           _("cgroup CPUACCT controller is not mounted"));
>+        }
>         goto cleanup;
>     }
>
>-- 
>2.38.2
>
Re: [PATCH] qemu: Provide virDomainGetCPUStats() implementation for session connection
Posted by Michal Prívozník 1 year, 3 months ago 
On 1/17/23 15:15, Martin Kletzander wrote:
> On Tue, Jan 17, 2023 at 01:44:44PM +0100, Michal Privoznik wrote:
>> We have virDomainGetCPUStats() API which offers querying
>> statistics on host CPU usage by given guest. And it works in two
>> modes: getting overall stats (@start_cpu == -1, @ncpus == 1) or
>> getting per host CPU usage.
>>
>> For the QEMU driver it is implemented by looking into values
>> stored in corresponding cpuacct CGroup controller. Well, this
>> works for system instances, where libvirt has permissions to
>> create CGroups and place QEMU process into them. But it does not
>> fly for session connection, where no CGroups are set up.
>>
>> Fortunately, we can do something similar to v8.8.0-rc1~95 and use
>> virProcessGetStatInfo() to fill the overall stats. Unfortunately,
>> I haven't found any source of per host CPU usage, so we just
>> continue throwing an error in that case.
>>
> 
> Looks ok, but there's one thing that's bothering me a bit.  On non-Linux
> platforms this API would start to work and returning zeros.  Could that
> be fixed somehow in a nice fashion or should I treat that as a non-issue?

Good point, I've never realized that non-Linux version of
virProcessGetStatInfo() doesn't fail. Let me post a v2 in which I'll
change the function to return error followed by this patch.

Michal

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.