[libvirt] [dbus PATCH 15/15] Implement GetCPUStats method for Domain Interface

[Pavel Hrdina]

On Wed, Apr 25, 2018 at 12:20:30PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou <kkoukiou at 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
-------------- 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/20180425/b6de29f2/attachment-0001.sig>