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

Martin Kletzander mkletzan at redhat.com
Thu Jan 19 14:18:39 UTC 2023


On Wed, Jan 18, 2023 at 10:58:19AM +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.
>
>Signed-off-by: Michal Privoznik <mprivozn at 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 25c681bfd2..cc10dd87e2 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -16006,6 +16006,50 @@ qemuDomainGetMetadata(virDomainPtr dom,
>     return ret;
> }
>
>+#define QEMU_CPU_STATS_PROC_TOTAL 3
>+
>+static int
>+qemuDomainGetCPUStatsProc(virDomainObj *vm,
>+                          virTypedParameterPtr params,
>+                          unsigned int nparams)

The only thing I do not like about this patch is that we would now have
both:

     static int
     qemuDomainGetCPUStatsProc(virDomainObj *vm,
                               virTypedParameterPtr params,
                               unsigned int nparams)

and:

     static int
     qemuDomainGetStatsCpuProc(virDomainObj *vm,
                               virTypedParamList *params)

doing almost, but not completely, the same thing.  And there is no nice
way to avoid it.  I just haven't noticed in v1, sorry.  Or is there a
way to converge at least some of these?

>+{
>+    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,
>@@ -16034,8 +16078,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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230119/02b3f33e/attachment-0001.sig>


More information about the libvir-list mailing list