Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
---
data/org.libvirt.Domain.xml | 8 ++++++
src/domain.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+)
diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index 8d027dd..9292a91 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -191,6 +191,14 @@
<arg name="flags" type="u" direction="in"/>
<arg name="controlInfo" type="(sst)" direction="out"/>
</method>
+ <method name="GetCPUStats">
+ <annotation name="org.gtk.GDBus.DocString"
+ value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetCPUStats
+ @total should be set to get total stats instead of per-cpu stats."/>
+ <arg name="total" type="b" direction="in"/>
+ <arg name="flags" type="u" direction="in"/>
+ <arg name="stats" type="a(sa{sv})" direction="out"/>
+ </method>
<method name="GetDiskErrors">
<annotation name="org.gtk.GDBus.DocString"
value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetDiskErrors"/>
diff --git a/src/domain.c b/src/domain.c
index 608f1d1..1c09cc9 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -3,6 +3,8 @@
#include <libvirt/libvirt.h>
+#define VIRT_DBUS_DOMAIN_MAX_CPUS 128
+
VIRT_DBUS_ENUM_DECL(virtDBusDomainBlockJob)
VIRT_DBUS_ENUM_IMPL(virtDBusDomainBlockJob,
VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
@@ -1041,6 +1043,71 @@ virtDBusDomainGetControlInfo(GVariant *inArgs,
errorReasonStr, controlInfo->stateTime);
}
+static void
+virtDBusDomainGetCPUStats(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath,
+ gpointer userData,
+ GVariant **outArgs,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+ virtDBusConnect *connect = userData;
+ g_autoptr(virDomain) domain = NULL;
+ g_auto(virtDBusUtilTypedParams) params = { 0 };
+ gint startCPU = 0;
+ guint ncpus = VIRT_DBUS_DOMAIN_MAX_CPUS;
+ gboolean total;
+ guint flags;
+ GVariant *gret;
+ GVariantBuilder builder;
+ gint ret;
+
+ g_variant_get(inArgs, "(bu)", &total, &flags);
+
+ domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
+ if (!domain)
+ return;
+
+ if (total) {
+ startCPU = -1;
+ ncpus = 1;
+ }
+ params.nparams = virDomainGetCPUStats(domain, NULL, 0, 0, 1, flags);
+ if (params.nparams < 0)
+ return virtDBusUtilSetLastVirtError(error);
+ params.params = g_new0(virTypedParameter, ncpus * params.nparams + 1);
+
+ ret = virDomainGetCPUStats(domain, params.params, params.nparams,
+ startCPU, ncpus, flags);
+ if (ret < 0)
+ return virtDBusUtilSetLastVirtError(error);
+
+ g_variant_builder_init(&builder, G_VARIANT_TYPE("a(sa{sv})"));
+
+ for (guint i = 0; i < ncpus; i++) {
+ GVariant *grecords;
+ g_autofree gchar *name = NULL;
+
+ if (params.params[i * params.nparams].type == 0)
+ continue;
+
+ g_variant_builder_open(&builder, G_VARIANT_TYPE("(sa{sv})"));
+ grecords = virtDBusUtilTypedParamsToGVariant(params.params + i*params.nparams,
+ params.nparams);
+ if (!total)
+ name = g_strdup_printf("CPU%d", i + startCPU);
+ else
+ name = g_strdup_printf("Total");
+ g_variant_builder_add(&builder, "s", name);
+ g_variant_builder_add_value(&builder, grecords);
+ g_variant_builder_close(&builder);
+ }
+ gret = g_variant_builder_end(&builder);
+
+ *outArgs = g_variant_new_tuple(&gret, 1);
+}
+
static void
virtDBusDomainGetDiskErrors(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -2516,6 +2583,7 @@ static virtDBusGDBusMethodTable virtDBusDomainMethodTable[] = {
{ "GetBlockIoTune", virtDBusDomainGetBlockIoTune },
{ "GetBlockJobInfo", virtDBusDomainGetBlockJobInfo },
{ "GetControlInfo", virtDBusDomainGetControlInfo },
+ { "GetCPUStats", virtDBusDomainGetCPUStats },
{ "GetDiskErrors", virtDBusDomainGetDiskErrors },
{ "GetFSInfo", virtDBusDomainGetFSInfo },
{ "GetGuestVcpus", virtDBusDomainGetGuestVcpus },
--
2.15.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Apr 25, 2018 at 12:20:30PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
> ---
> data/org.libvirt.Domain.xml | 8 ++++++
> src/domain.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index 8d027dd..9292a91 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -191,6 +191,14 @@
> <arg name="flags" type="u" direction="in"/>
> <arg name="controlInfo" type="(sst)" direction="out"/>
> </method>
> + <method name="GetCPUStats">
> + <annotation name="org.gtk.GDBus.DocString"
> + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetCPUStats
> + @total should be set to get total stats instead of per-cpu stats."/>
I would rephrase it:
"If @total is True, it returns total cpu stats, otherwise it returns
per-cpu stats."
> + <arg name="total" type="b" direction="in"/>
> + <arg name="flags" type="u" direction="in"/>
> + <arg name="stats" type="a(sa{sv})" direction="out"/>
> + </method>
> <method name="GetDiskErrors">
> <annotation name="org.gtk.GDBus.DocString"
> value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetDiskErrors"/>
> diff --git a/src/domain.c b/src/domain.c
> index 608f1d1..1c09cc9 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -3,6 +3,8 @@
>
> #include <libvirt/libvirt.h>
>
> +#define VIRT_DBUS_DOMAIN_MAX_CPUS 128
> +
> VIRT_DBUS_ENUM_DECL(virtDBusDomainBlockJob)
> VIRT_DBUS_ENUM_IMPL(virtDBusDomainBlockJob,
> VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
> @@ -1041,6 +1043,71 @@ virtDBusDomainGetControlInfo(GVariant *inArgs,
> errorReasonStr, controlInfo->stateTime);
> }
>
> +static void
> +virtDBusDomainGetCPUStats(GVariant *inArgs,
> + GUnixFDList *inFDs G_GNUC_UNUSED,
> + const gchar *objectPath,
> + gpointer userData,
> + GVariant **outArgs,
> + GUnixFDList **outFDs G_GNUC_UNUSED,
> + GError **error)
> +{
> + virtDBusConnect *connect = userData;
> + g_autoptr(virDomain) domain = NULL;
> + g_auto(virtDBusUtilTypedParams) params = { 0 };
> + gint startCPU = 0;
> + guint ncpus = VIRT_DBUS_DOMAIN_MAX_CPUS;
> + gboolean total;
> + guint flags;
> + GVariant *gret;
> + GVariantBuilder builder;
> + gint ret;
> +
> + g_variant_get(inArgs, "(bu)", &total, &flags);
> +
> + domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
> + if (!domain)
> + return;
So this libvirt API has really stupid design and it's super complicated,
I'm considering dropping it completely since the only benefit of this
API is that you can get per host cpu statistic for specific domain.
However, if we decide to keep it, it has to be rewritten. Currently the
code would report only first 128 host CPUs which is not correct.
You can look at libvirt-python implementation [1] of this API to get the
idea of how it needs to be done, basically if there is more than 128
host CPUs, the virDomainGetCPUStats() needs to be called multiple times
to get all statistics.
[1] libvirt-override.c (libvirt_virDomainGetCPUStats)
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2018-04-25 at 18:33 +0200, Pavel Hrdina wrote:
> On Wed, Apr 25, 2018 at 12:20:30PM +0200, Katerina Koukiou wrote:
> > Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
> > ---
> > data/org.libvirt.Domain.xml | 8 ++++++
> > src/domain.c | 68
> > +++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 76 insertions(+)
> >
> > diff --git a/data/org.libvirt.Domain.xml
> > b/data/org.libvirt.Domain.xml
> > index 8d027dd..9292a91 100644
> > --- a/data/org.libvirt.Domain.xml
> > +++ b/data/org.libvirt.Domain.xml
> > @@ -191,6 +191,14 @@
> > <arg name="flags" type="u" direction="in"/>
> > <arg name="controlInfo" type="(sst)" direction="out"/>
> > </method>
> > + <method name="GetCPUStats">
> > + <annotation name="org.gtk.GDBus.DocString"
> > + value="See https://libvirt.org/html/libvirt-libvirt-domain
> > .html#virDomainGetCPUStats
> > + @total should be set to get total stats instead of
> > per-cpu stats."/>
>
> I would rephrase it:
>
> "If @total is True, it returns total cpu stats, otherwise it returns
> per-cpu stats."
>
> > + <arg name="total" type="b" direction="in"/>
> > + <arg name="flags" type="u" direction="in"/>
> > + <arg name="stats" type="a(sa{sv})" direction="out"/>
> > + </method>
> > <method name="GetDiskErrors">
> > <annotation name="org.gtk.GDBus.DocString"
> > value="See https://libvirt.org/html/libvirt-libvirt-domain
> > .html#virDomainGetDiskErrors"/>;
> > diff --git a/src/domain.c b/src/domain.c
> > index 608f1d1..1c09cc9 100644
> > --- a/src/domain.c
> > +++ b/src/domain.c
> > @@ -3,6 +3,8 @@
> >
> > #include <libvirt/libvirt.h>
> >
> > +#define VIRT_DBUS_DOMAIN_MAX_CPUS 128
> > +
> > VIRT_DBUS_ENUM_DECL(virtDBusDomainBlockJob)
> > VIRT_DBUS_ENUM_IMPL(virtDBusDomainBlockJob,
> > VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
> > @@ -1041,6 +1043,71 @@ virtDBusDomainGetControlInfo(GVariant
> > *inArgs,
> > errorReasonStr, controlInfo-
> > >stateTime);
> > }
> >
> > +static void
> > +virtDBusDomainGetCPUStats(GVariant *inArgs,
> > + GUnixFDList *inFDs G_GNUC_UNUSED,
> > + const gchar *objectPath,
> > + gpointer userData,
> > + GVariant **outArgs,
> > + GUnixFDList **outFDs G_GNUC_UNUSED,
> > + GError **error)
> > +{
> > + virtDBusConnect *connect = userData;
> > + g_autoptr(virDomain) domain = NULL;
> > + g_auto(virtDBusUtilTypedParams) params = { 0 };
> > + gint startCPU = 0;
> > + guint ncpus = VIRT_DBUS_DOMAIN_MAX_CPUS;
> > + gboolean total;
> > + guint flags;
> > + GVariant *gret;
> > + GVariantBuilder builder;
> > + gint ret;
> > +
> > + g_variant_get(inArgs, "(bu)", &total, &flags);
> > +
> > + domain = virtDBusDomainGetVirDomain(connect, objectPath,
> > error);
> > + if (!domain)
> > + return;
>
> So this libvirt API has really stupid design and it's super
> complicated,
> I'm considering dropping it completely since the only benefit of this
> API is that you can get per host cpu statistic for specific domain.
>
> However, if we decide to keep it, it has to be rewritten. Currently
> the
> code would report only first 128 host CPUs which is not correct.
>
> You can look at libvirt-python implementation [1] of this API to get
> the
> idea of how it needs to be done, basically if there is more than 128
> host CPUs, the virDomainGetCPUStats() needs to be called multiple
> times
> to get all statistics.
>
> [1] libvirt-override.c (libvirt_virDomainGetCPUStats)
>
> Pavel
Thanks for the heads up. So, I 'll drop it for now and I 'll leave it
for future.
Katerina
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.