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

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

 



On Wed, Apr 25, 2018 at 12:20:30PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou <kkoukiou@xxxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[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