Re: [PATCH v2 2/2] Added RSS reporting

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

 



On Tue, Jan 24, 2012 at 02:25:05PM +0100, Martin Kletzander wrote:
> Added RSS information gathering into qemuMemoryStats into qemu driver
> and the reporting into virsh dommemstat.
> ---
> v2:
>  - fixed sign for the ret variable (can be negative)
> 
>  include/libvirt/libvirt.h.in |    7 ++++++-
>  src/qemu/qemu_driver.c       |   23 ++++++++++++++++++-----
>  tools/virsh.c                |    2 ++
>  3 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 958e5a6..d5ef061 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -884,11 +884,16 @@ typedef enum {
> 
>      /* Current balloon value (in KB). */
>      VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON  = 6,
> +
> +    /* Resident Set Size of the process running the domain. This value
> +     * is in kB */
> +    VIR_DOMAIN_MEMORY_STAT_RSS             = 7,
> +
>      /*
>       * The number of statistics supported by this version of the interface.
>       * To add new statistics, add them to the enum and increase this value.
>       */
> -    VIR_DOMAIN_MEMORY_STAT_NR              = 7,
> +    VIR_DOMAIN_MEMORY_STAT_NR              = 8,
> 
>  #ifdef VIR_ENUM_SENTINELS
>      VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR

 Change to the API is just fine

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6600afd..20d3d84 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7978,7 +7978,7 @@ qemudDomainMemoryStats (virDomainPtr dom,
>  {
>      struct qemud_driver *driver = dom->conn->privateData;
>      virDomainObjPtr vm;
> -    unsigned int ret = -1;
> +    int ret = -1;

  I was a bit surprized by the original statement actually, that looks
better to me, and needed now. Hopefully I got that right

>      virCheckFlags(0, -1);
> 
> @@ -7997,14 +7997,27 @@ qemudDomainMemoryStats (virDomainPtr dom,
>      if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
>          goto cleanup;
> 
> -    if (virDomainObjIsActive(vm)) {
> +    if (!virDomainObjIsActive(vm)) {
> +        qemuReportError(VIR_ERR_OPERATION_INVALID,
> +                        "%s", _("domain is not running"));
> +    } else {
>          qemuDomainObjPrivatePtr priv = vm->privateData;
>          qemuDomainObjEnterMonitor(driver, vm);
>          ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats);
>          qemuDomainObjExitMonitor(driver, vm);
> -    } else {
> -        qemuReportError(VIR_ERR_OPERATION_INVALID,
> -                        "%s", _("domain is not running"));
> +
> +        if (ret >= 0 && ret < nr_stats) {
> +            long rss;
> +            if (qemudGetProcessInfo(NULL, NULL, &rss, vm->pid, 0) < 0) {
> +                qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                            _("cannot get RSS for domain"));
> +            } else {
> +                stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
> +                stats[ret].val = rss;
> +                ret++;
> +            }
> +
> +        }
>      }
> 
>      if (qemuDomainObjEndJob(driver, vm) == 0)
> diff --git a/tools/virsh.c b/tools/virsh.c
> index d635b56..d5bbabf 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -1871,6 +1871,8 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
>              vshPrint (ctl, "available %llu\n", stats[i].val);
>          if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON)
>              vshPrint (ctl, "actual %llu\n", stats[i].val);
> +        if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_RSS)
> +            vshPrint (ctl, "rss %llu\n", stats[i].val);
>      }
> 
>      virDomainFree(dom);

  Looks fine, and works for me, ACK too

I'm pushing the 2 patches but please provide one missing item:
the python bindings, basically in python/libvirt-override.c
you need to add the new information in libvirt_virDomainMemoryStats()
it's an easy patch and we can add it while in feature freeze next week

  thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
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]