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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 23, 2023 at 02:12:06PM +0100, Michal Prívozník wrote:
On 1/19/23 15:18, Martin Kletzander wrote:
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@xxxxxxxxxx>
---
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?

Yeah, I realized this too. But, there's a difference between
virTypedParameter and virTypedParamList. The former is allocated by user
and we fill it out, while the latter is allocated by us. For instance,
users interested in just the very first stat, might allocate space for
it and call us like:

 qemuDomainGetCPUStatsProc(vm, &params, 1);

(obviously, they would call the public API, but in the end this is how
the function would be called). That is not possible with virTypedParamList.

Another difference is in the semantics: when the function is called with
nparams == 0, then it's supposed to return the number of supported stats.

We could invent some glue function that would cover these differences,
but I worry that it would end up being longer than this new function I'm
inventing (and less readable too).


As I said I think there is no nice way to avoid it.  Even if we figure
out something there is no need to hold this back just for a few lines.

Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux