[libvirt] [PATCH v5 2/3] qemu: expand domain memory statistics with 'last-update' timestamp

[John Ferlan]

On 06/16/2016 02:38 PM, Maxim Nestratov wrote:
> From: Derbyshev Dmitry <dderbyshev at virtuozzo.com>
> 
> QEMU reports timestamp along with other memory statistics, but this
> information is not reported by libvirt statistics API.
> It could be useful to determine if the data reported is fresh or not.
> 
> Signed-off-by: Derbyshev Dmitry <dderbyshev at virtuozzo.com>
> ---
>  include/libvirt/libvirt-domain.h |  5 ++++-
>  src/libvirt-domain.c             |  2 ++
>  src/qemu/qemu_monitor_json.c     | 22 ++++++++++++----------
>  tools/virsh-domain-monitor.c     |  2 ++
>  4 files changed, 20 insertions(+), 11 deletions(-)
> 

Likewise from my 1/3 comments - virsh.pod updates.

Personally, I'm not so sure just a plain timestamp is all that useful to
print out as the number displayed doesn't have much meaning without
format or knowing what the current timestamp is. That is, what visual
cue is there that the timestamp displayed is from within 10 seconds, 10
minutes, 10 hours, or 10 days ago?

For someone that's using the API (and not virsh) - having a timestamp
would allow for the ability to generate "more real" statistics.

I'm not opposed to it being displayed, but unlike other stats provided
that are essentially "sized" - this one is less a stat and more of
something that "could" be used. It also only has meaning for certain
values (rss not being one of those IIRC).

John

> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 1554198..f953897 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -609,11 +609,14 @@ typedef enum {
>       */
>      VIR_DOMAIN_MEMORY_STAT_USABLE          = 8,
>  
> +    /* Timestamp of the last update of statistics */
> +    VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE     = 9,
> +
>      /*
>       * 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              = 9,
> +    VIR_DOMAIN_MEMORY_STAT_NR              = 10,
>  
>  # 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 fabd4a6..29e0c72 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -5991,6 +5991,8 @@ virDomainGetInterfaceParameters(virDomainPtr domain,
>   *     to swap, corresponds to 'Available' in /proc/meminfo
>   * VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON:
>   *     Current balloon value (in kb).
> + * VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE
> + *     Timestamp of the last statistic
>   *
>   * 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 d10e758..6f0d187 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1633,10 +1633,10 @@ qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon,
>   * rates and/or whether data has been collected since a previous cycle.
>   * It's currently unused.
>   */
> -#define GET_BALLOON_STATS(FIELD, TAG, DIVISOR)                                \
> -    if (virJSONValueObjectHasKey(statsdata, FIELD) &&                         \
> +#define GET_BALLOON_STATS(OBJECT, FIELD, TAG, DIVISOR)                        \
> +    if (virJSONValueObjectHasKey(OBJECT, FIELD) &&                            \
>         (got < nr_stats)) {                                                    \
> -        if (virJSONValueObjectGetNumberUlong(statsdata, FIELD, &mem) < 0) {   \
> +        if (virJSONValueObjectGetNumberUlong(OBJECT, FIELD, &mem) < 0) {      \
>              VIR_DEBUG("Failed to get '%s' value", FIELD);                     \
>          } else {                                                              \
>              /* Not being collected? No point in providing bad data */         \
> @@ -1707,20 +1707,22 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
>          goto cleanup;
>      }
>  
> -    GET_BALLOON_STATS("stat-swap-in",
> +    GET_BALLOON_STATS(statsdata, "stat-swap-in",
>                        VIR_DOMAIN_MEMORY_STAT_SWAP_IN, 1024);
> -    GET_BALLOON_STATS("stat-swap-out",
> +    GET_BALLOON_STATS(statsdata, "stat-swap-out",
>                        VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, 1024);
> -    GET_BALLOON_STATS("stat-major-faults",
> +    GET_BALLOON_STATS(statsdata, "stat-major-faults",
>                        VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT, 1);
> -    GET_BALLOON_STATS("stat-minor-faults",
> +    GET_BALLOON_STATS(statsdata, "stat-minor-faults",
>                        VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, 1);
> -    GET_BALLOON_STATS("stat-free-memory",
> +    GET_BALLOON_STATS(statsdata, "stat-free-memory",
>                        VIR_DOMAIN_MEMORY_STAT_UNUSED, 1024);
> -    GET_BALLOON_STATS("stat-total-memory",
> +    GET_BALLOON_STATS(statsdata, "stat-total-memory",
>                        VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 1024);
> -    GET_BALLOON_STATS("stat-available-memory",
> +    GET_BALLOON_STATS(statsdata, "stat-available-memory",
>                        VIR_DOMAIN_MEMORY_STAT_USABLE, 1024);
> +    GET_BALLOON_STATS(data, "last-update",
> +                      VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 1);
>  
>      ret = got;
>  
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 1921ff5..b4ccbf6 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -375,6 +375,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_LAST_UPDATE)
> +            vshPrint(ctl, "last_update %llu\n", stats[i].val);
>      }
>  
>      ret = true;
>