[libvirt] [PATCH v2 1/1] qemu: add entry for balloon stat stat-disk-caches

[John Ferlan]

$SUBJ:  s/add/Add


Might be nice to add something here... You could even add your qemu
commit id that added this for QEMU 2.12, e.g.

QEMU 2.12 commit id bf1e7140e provides balloon statistics for how much
memory can be quickly reclaimed without additional I/O, so let's add
reporting that to the libvirt balloon statistics (when it exists).


On 06/08/2018 08:23 AM, Tomáš Golembiovský wrote:
> Signed-off-by: Tomáš Golembiovský <tgolembi at redhat.com>
> ---
>  include/libvirt/libvirt-domain.h | 9 ++++++++-
>  src/libvirt-domain.c             | 3 +++
>  src/qemu/qemu_monitor_json.c     | 2 ++
>  tools/virsh-domain-monitor.c     | 2 ++
>  4 files changed, 15 insertions(+), 1 deletion(-)
> 

> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index da773b76cb..b96c018a90 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -628,11 +628,18 @@ typedef enum {
>      /* Timestamp of the last update of statistics, in seconds. */
>      VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE     = 9,
>  
> +    /*
> +     * The amount of memory, in bytes, that can be quickly reclaimed without

"in bytes"... or....

> +     * additional I/O. Typically these pages are used for caching files from
> +     * disk.
> +     */
> +    VIR_DOMAIN_MEMORY_STAT_DISK_CACHES     = 10,
> +
>      /*
>       * 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              = 10,
> +    VIR_DOMAIN_MEMORY_STAT_NR              = 11,
>  
>  # ifdef VIR_ENUM_SENTINELS
>      VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index d44b553c74..ca22d13568 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -5732,6 +5732,9 @@ virDomainGetInterfaceParameters(virDomainPtr domain,
>   *     Current balloon value (in kb).
>   * VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE
>   *     Timestamp of the last statistic
> + * VIR_DOMAIN_MEMORY_STAT_DISK_CACHES
> + *     Memory that can be reclaimed without additional I/O, typically disk
> + *     caches (in kb).

..."in kb" ??

...or "in KiB"


>   *
>   * Returns: The number of stats provided or -1 in case of failure.
>   */
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index e8a46d2ec3..d62d979b5a 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2071,6 +2071,8 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
>                        VIR_DOMAIN_MEMORY_STAT_USABLE, 1024);
>      GET_BALLOON_STATS(data, "last-update",
>                        VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 1);
> +    GET_BALLOON_STATS(statsdata, "stat-disk-caches",
> +                      VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, 1024);
>      ret = got;
>   cleanup:
>      virJSONValueFree(cmd);


What about qemuDomainGetStatsBalloon?  and the STORE_MEM_RECORD?  It's
another consumer of qemuDomainMemoryStatsInternal type data.

I was looking at/for occurrences of "last*.*update" in the code

> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 8cbb3db37c..bad882e87c 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -362,6 +362,8 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
>              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);
> +        if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_DISK_CACHES)
> +            vshPrint(ctl, "disk_caches %llu\n", stats[i].val);

I think this should be after last_update too, although I can certainly
see cause for it being earlier too; however, it is possible that there
are consumers of virsh output that expect a certain order - so let's
just add it to the end to avoid any of those type complaints.

>          if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE)
>              vshPrint(ctl, "last_update %llu\n", stats[i].val);
>      }
> 

You'll need to update tools/virsh.pod too - search on last-update as
there are two places to adjust.

John