[Qemu-devel] [PATCH v3 0/3] re-enable balloon stats

[Qemu-devel] [PATCH v3 0/3] re-enable balloon stats

[Luiz Capitulino]

This series re-enables the virtio-balloon device stats feature. It now uses
QOM properties via a polling mechanism as suggested by Anthony here:

 http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02390.html

o v3

 - return all stats at once via the guest-stats property
   (ie. kill individual querying of stats)
 - rebase on top of latest master
 - update doc

Luiz Capitulino (3):
  balloon: drop old stats code & API
  balloon: re-enable balloon stats
  docs: document virtio-balloon stats

 docs/virtio-balloon-stats.txt | 102 ++++++++++++++++++++++++
 hmp.c                         |  24 +-----
 hw/virtio-balloon.c           | 175 ++++++++++++++++++++++++++++++++++++------
 qapi-schema.json              |  20 +----
 qmp-commands.hx               |  13 ----
 5 files changed, 254 insertions(+), 80 deletions(-)
 create mode 100644 docs/virtio-balloon-stats.txt

-- 
1.8.1.GIT

[Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API

[Luiz Capitulino]

Next commit will re-enable balloon stats with a different interface, but
this old code conflicts with it. Let's drop it.

It's important to note that the QMP and HMP interfaces are also dropped
by this commit. That shouldn't be a problem though, because:

 1. All QMP fields are optional
 2. This feature has always been disabled

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp.c               | 24 +-----------------------
 hw/virtio-balloon.c | 24 ------------------------
 qapi-schema.json    | 20 +-------------------
 qmp-commands.hx     | 13 -------------
 4 files changed, 2 insertions(+), 79 deletions(-)

diff --git a/hmp.c b/hmp.c
index c7b6ba0..ae16916 100644
--- a/hmp.c
+++ b/hmp.c
@@ -465,29 +465,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    monitor_printf(mon, "balloon: actual=%" PRId64, info->actual >> 20);
-    if (info->has_mem_swapped_in) {
-        monitor_printf(mon, " mem_swapped_in=%" PRId64, info->mem_swapped_in);
-    }
-    if (info->has_mem_swapped_out) {
-        monitor_printf(mon, " mem_swapped_out=%" PRId64, info->mem_swapped_out);
-    }
-    if (info->has_major_page_faults) {
-        monitor_printf(mon, " major_page_faults=%" PRId64,
-                       info->major_page_faults);
-    }
-    if (info->has_minor_page_faults) {
-        monitor_printf(mon, " minor_page_faults=%" PRId64,
-                       info->minor_page_faults);
-    }
-    if (info->has_free_mem) {
-        monitor_printf(mon, " free_mem=%" PRId64, info->free_mem);
-    }
-    if (info->has_total_mem) {
-        monitor_printf(mon, " total_mem=%" PRId64, info->total_mem);
-    }
-
-    monitor_printf(mon, "\n");
+    monitor_printf(mon, "balloon: actual=%" PRId64 "\n", info->actual >> 20);
 
     qapi_free_BalloonInfo(info);
 }
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 3040bc6..2520cba 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -164,28 +164,6 @@ static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
 {
     VirtIOBalloon *dev = opaque;
-
-#if 0
-    /* Disable guest-provided stats for now. For more details please check:
-     * https://bugzilla.redhat.com/show_bug.cgi?id=623903
-     *
-     * If you do enable it (which is probably not going to happen as we
-     * need a new command for it), remember that you also need to fill the
-     * appropriate members of the BalloonInfo structure so that the stats
-     * are returned to the client.
-     */
-    if (dev->vdev.guest_features & (1 << VIRTIO_BALLOON_F_STATS_VQ)) {
-        virtqueue_push(dev->svq, &dev->stats_vq_elem, dev->stats_vq_offset);
-        virtio_notify(&dev->vdev, dev->svq);
-        return;
-    }
-#endif
-
-    /* Stats are not supported.  Clear out any stale values that might
-     * have been set by a more featureful guest kernel.
-     */
-    reset_stats(dev);
-
     info->actual = ram_size - ((uint64_t) dev->actual <<
                                VIRTIO_BALLOON_PFN_SHIFT);
 }
@@ -255,8 +233,6 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
     s->dvq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
     s->svq = virtio_add_queue(&s->vdev, 128, virtio_balloon_receive_stats);
 
-    reset_stats(s);
-
     s->qdev = dev;
     register_savevm(dev, "virtio-balloon", -1, 1,
                     virtio_balloon_save, virtio_balloon_load, s);
diff --git a/qapi-schema.json b/qapi-schema.json
index 6d7252b..a4c6eca 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -977,28 +977,10 @@
 #
 # @actual: the number of bytes the balloon currently contains
 #
-# @mem_swapped_in: #optional number of pages swapped in within the guest
-#
-# @mem_swapped_out: #optional number of pages swapped out within the guest
-#
-# @major_page_faults: #optional number of major page faults within the guest
-#
-# @minor_page_faults: #optional number of minor page faults within the guest
-#
-# @free_mem: #optional amount of memory (in bytes) free in the guest
-#
-# @total_mem: #optional amount of memory (in bytes) visible to the guest
-#
 # Since: 0.14.0
 #
-# Notes: all current versions of QEMU do not fill out optional information in
-#        this structure.
 ##
-{ 'type': 'BalloonInfo',
-  'data': {'actual': 'int', '*mem_swapped_in': 'int',
-           '*mem_swapped_out': 'int', '*major_page_faults': 'int',
-           '*minor_page_faults': 'int', '*free_mem': 'int',
-           '*total_mem': 'int'} }
+{ 'type': 'BalloonInfo', 'data': {'actual': 'int' } }
 
 ##
 # @query-balloon:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cbf1280..3be5330 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2549,13 +2549,6 @@ Make an asynchronous request for balloon info. When the request completes a
 json-object will be returned containing the following data:
 
 - "actual": current balloon value in bytes (json-int)
-- "mem_swapped_in": Amount of memory swapped in bytes (json-int, optional)
-- "mem_swapped_out": Amount of memory swapped out in bytes (json-int, optional)
-- "major_page_faults": Number of major faults (json-int, optional)
-- "minor_page_faults": Number of minor faults (json-int, optional)
-- "free_mem": Total amount of free and unused memory in
-              bytes (json-int, optional)
-- "total_mem": Total amount of available memory in bytes (json-int, optional)
 
 Example:
 
@@ -2563,12 +2556,6 @@ Example:
 <- {
       "return":{
          "actual":1073741824,
-         "mem_swapped_in":0,
-         "mem_swapped_out":0,
-         "major_page_faults":142,
-         "minor_page_faults":239245,
-         "free_mem":1014185984,
-         "total_mem":1044668416
       }
    }
 
-- 
1.8.1.GIT

[Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats

[Luiz Capitulino]

The statistics are now available through device properties via a
polling mechanism. First a client has to enable polling, then it
can query available stats.

Polling is enabled by setting an update interval (in seconds)
to a property named guest-stats-polling-interval, like this:

{ "execute": "qom-set",
  "arguments": { "path": "/machine/peripheral-anon/device[1]",
                 "property": "guest-stats-polling-interval", "value": 4 } }

Then the available stats can be retrieved by querying the
guest-stats property. The returned object is a dict containing
all available stats. Example:

{ "execute": "qom-get",
  "arguments": { "path": "/machine/peripheral-anon/device[1]",
  "property": "guest-stats" } }

{
    "return": {
        "stats": {
            "stat-swap-out": 0,
            "stat-free-memory": 844943360,
            "stat-minor-faults": 219028,
            "stat-major-faults": 235,
            "stat-total-memory": 1044406272,
            "stat-swap-in": 0
        },
        "last-update": 1358529861
    }
}

Please, check the next commit for full documentation.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/virtio-balloon.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 150 insertions(+), 1 deletion(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 2520cba..f4605fb 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -14,6 +14,7 @@
  */
 
 #include "qemu/iov.h"
+#include "qemu/timer.h"
 #include "qemu-common.h"
 #include "virtio.h"
 #include "pc.h"
@@ -22,6 +23,7 @@
 #include "virtio-balloon.h"
 #include "sysemu/kvm.h"
 #include "exec/address-spaces.h"
+#include "qapi/visitor.h"
 
 #if defined(__linux__)
 #include <sys/mman.h>
@@ -36,6 +38,9 @@ typedef struct VirtIOBalloon
     uint64_t stats[VIRTIO_BALLOON_S_NR];
     VirtQueueElement stats_vq_elem;
     size_t stats_vq_offset;
+    QEMUTimer *stats_timer;
+    int64_t stats_last_update;
+    int64_t stats_poll_interval;
     DeviceState *qdev;
 } VirtIOBalloon;
 
@@ -53,6 +58,16 @@ static void balloon_page(void *addr, int deflate)
 #endif
 }
 
+static const char *balloon_stat_names[] = {
+   [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in", 
+   [VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
+   [VIRTIO_BALLOON_S_MAJFLT] = "stat-major-faults",
+   [VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults",
+   [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
+   [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
+   [VIRTIO_BALLOON_S_NR] = NULL
+};
+
 /*
  * reset_stats - Mark all items in the stats array as unset
  *
@@ -67,6 +82,118 @@ static inline void reset_stats(VirtIOBalloon *dev)
     for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1);
 }
 
+static bool balloon_stats_supported(const VirtIOBalloon *s)
+{
+    return s->vdev.guest_features & (1 << VIRTIO_BALLOON_F_STATS_VQ);
+}
+
+static bool balloon_stats_enabled(const VirtIOBalloon *s)
+{
+    return s->stats_poll_interval > 0;
+}
+
+static void balloon_stats_destroy_timer(VirtIOBalloon *s)
+{
+    if (balloon_stats_enabled(s)) {
+        qemu_del_timer(s->stats_timer);
+        qemu_free_timer(s->stats_timer);
+        s->stats_timer = NULL;
+        s->stats_poll_interval = 0;
+    }
+}
+
+static void balloon_stats_change_timer(VirtIOBalloon *s, int secs)
+{
+    qemu_mod_timer(s->stats_timer, qemu_get_clock_ms(vm_clock) + secs * 1000);
+}
+
+static void balloon_stats_poll_cb(void *opaque)
+{
+    VirtIOBalloon *s = opaque;
+
+    if (!balloon_stats_supported(s)) {
+        /* re-schedule */
+        balloon_stats_change_timer(s, s->stats_poll_interval);
+        return;
+    }
+
+    virtqueue_push(s->svq, &s->stats_vq_elem, s->stats_vq_offset);
+    virtio_notify(&s->vdev, s->svq);
+}
+
+static void balloon_stats_get_all(Object *obj, struct Visitor *v,
+                                  void *opaque, const char *name, Error **errp)
+{
+    VirtIOBalloon *s = opaque;
+    int i;
+
+    if (!s->stats_last_update) {
+        error_setg(errp, "guest hasn't updated any stats yet");
+        return;
+    }
+
+    visit_start_struct(v, NULL, "guest-stats", name, 0, errp);
+    visit_type_int(v, &s->stats_last_update, "last-update", errp);
+
+    visit_start_struct(v, NULL, NULL, "stats", 0, errp);
+    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
+        visit_type_int64(v, (int64_t *) &s->stats[i], balloon_stat_names[i],
+                         errp);
+    }
+    visit_end_struct(v, errp);
+
+    visit_end_struct(v, errp);
+}
+
+static void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v,
+                                            void *opaque, const char *name,
+                                            Error **errp)
+{
+    VirtIOBalloon *s = opaque;
+    visit_type_int(v, &s->stats_poll_interval, name, errp);
+}
+
+static void balloon_stats_set_poll_interval(Object *obj, struct Visitor *v,
+                                            void *opaque, const char *name,
+                                            Error **errp)
+{
+    VirtIOBalloon *s = opaque;
+    int64_t value;
+
+    visit_type_int(v, &value, name, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (value < 0) {
+        error_setg(errp, "timer value must be greater than zero");
+        return;
+    }
+
+    if (value == s->stats_poll_interval) {
+        return;
+    }
+
+    if (value == 0) {
+        /* timer=0 disables the timer */
+        balloon_stats_destroy_timer(s);
+        return;
+    }
+
+    if (balloon_stats_enabled(s)) {
+        /* timer interval change */
+        s->stats_poll_interval = value;
+        balloon_stats_change_timer(s, value);
+        return;
+    }
+
+    /* create a new timer */
+    g_assert(s->stats_timer == NULL);
+    s->stats_timer = qemu_new_timer_ms(vm_clock, balloon_stats_poll_cb, s);
+    s->stats_poll_interval = value;
+    balloon_stats_change_timer(s, 0);
+}
+
 static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOBalloon *s = to_virtio_balloon(vdev);
@@ -107,9 +234,10 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
     VirtQueueElement *elem = &s->stats_vq_elem;
     VirtIOBalloonStat stat;
     size_t offset = 0;
+    qemu_timeval tv;
 
     if (!virtqueue_pop(vq, elem)) {
-        return;
+        goto out;
     }
 
     /* Initialize the stats to get rid of any stale values.  This is only
@@ -128,6 +256,18 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
             s->stats[tag] = val;
     }
     s->stats_vq_offset = offset;
+
+    if (qemu_gettimeofday(&tv) < 0) {
+        fprintf(stderr, "warning: %s: failed to get time of day\n", __func__);
+        goto out;
+    }
+
+    s->stats_last_update = tv.tv_sec;
+
+out:
+    if (balloon_stats_enabled(s)) {
+        balloon_stats_change_timer(s, s->stats_poll_interval);
+    }
 }
 
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
@@ -237,6 +377,14 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
     register_savevm(dev, "virtio-balloon", -1, 1,
                     virtio_balloon_save, virtio_balloon_load, s);
 
+    object_property_add(OBJECT(dev), "guest-stats", "guest statistics",
+                        balloon_stats_get_all, NULL, NULL, s, NULL);
+
+    object_property_add(OBJECT(dev), "guest-stats-polling-interval", "int",
+                        balloon_stats_get_poll_interval,
+                        balloon_stats_set_poll_interval,
+                        NULL, s, NULL);
+
     return &s->vdev;
 }
 
@@ -244,6 +392,7 @@ void virtio_balloon_exit(VirtIODevice *vdev)
 {
     VirtIOBalloon *s = DO_UPCAST(VirtIOBalloon, vdev, vdev);
 
+    balloon_stats_destroy_timer(s);
     qemu_remove_balloon_handler(s);
     unregister_savevm(s->qdev, "virtio-balloon", s);
     virtio_cleanup(vdev);
-- 
1.8.1.GIT

[Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 docs/virtio-balloon-stats.txt | 102 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)
 create mode 100644 docs/virtio-balloon-stats.txt

diff --git a/docs/virtio-balloon-stats.txt b/docs/virtio-balloon-stats.txt
new file mode 100644
index 0000000..621145e
--- /dev/null
+++ b/docs/virtio-balloon-stats.txt
@@ -0,0 +1,102 @@
+virtio balloon memory statistics
+================================
+
+The virtio balloon driver supports guest memory statistics reporting. These
+statistics are available to QEMU users as QOM (QEMU Object Model) device
+properties via a polling mechanism.
+
+Before querying the available stats, clients first have to enable polling.
+This is done by writing a time interval value (in seconds) to the
+guest-stats-polling-interval property. This value can be:
+
+  > 0  enables polling in the specified interval. If polling is already
+       enabled, the polling time interval is changed to the new value
+  
+  0    disables polling. Previous polled statistics are still valid and
+       can be queried.
+
+Once polling is enabled, the virtio-balloon device in QEMU will start
+polling the guest's balloon driver for new stats in the specified time
+interval.
+
+To retrieve those stats, clients have to query the guest-stats property,
+which will return a dictionary containing:
+
+  o A key named 'stats', containing all avaiable stats. If the guest
+    doesn't support a particular stat, its value will be -1. Currently,
+    the following stats are supported:
+
+      - stat-swap-in
+      - stat-swap-out
+      - stat-major-faults
+      - stat-minor-faults
+      - stat-free-memory
+      - stat-total-memory
+
+  o A key named last-update, which contains the last stats update
+    timestamp in seconds
+
+It's also important to note the following:
+
+ - Previously polled statistics remain available even if the polling is
+   later disabled
+
+ - As noted above, if a guest doesn't support a particular stat it
+   will always be -1. However, it's also possible that a guest couldn't
+   temporarily update one or even all stats. If this happens, just wait
+   for the next update
+
+ - If polling is enabled but the guest doesn't have stats support or
+   the balloon driver is not loaded, an error will be returned when
+   querying stats
+
+ - The polling timer is only re-armed when the guest responds to the
+   statistics request. This means that if a (buggy) guest doesn't ever
+   respond to the request the timer will never be re-armed, which has
+   the same effect as disabling polling
+
+Here are a few examples. The virtio-balloon device is assumed to be in the
+'/machine/peripheral-anon/device[1]' QOM path.
+
+Enable polling with 2 seconds interval:
+
+{ "execute": "qom-set",
+             "arguments": { "path": "/machine/peripheral-anon/device[1]",
+			 "property": "guest-stats-polling-interval", "value": 2 } }
+
+{ "return": {} }
+
+Change polling to 10 seconds:
+
+{ "execute": "qom-set",
+             "arguments": { "path": "/machine/peripheral-anon/device[1]",
+			 "property": "guest-stats-polling-interval", "value": 10 } }
+
+{ "return": {} }
+
+Get stats:
+
+{ "execute": "qom-get",
+  "arguments": { "path": "/machine/peripheral-anon/device[1]",
+  "property": "guest-stats" } }
+{
+    "return": {
+        "stats": {
+            "stat-swap-out": 0, 
+            "stat-free-memory": 844943360, 
+            "stat-minor-faults": 219028, 
+            "stat-major-faults": 235, 
+            "stat-total-memory": 1044406272, 
+            "stat-swap-in": 0
+        }, 
+        "last-update": 1358529861
+    }
+}
+
+Disable polling:
+
+{ "execute": "qom-set",
+             "arguments": { "path": "/machine/peripheral-anon/device[1]",
+			 "property": "stats-polling-interval", "value": 0 } }
+
+{ "return": {} }
-- 
1.8.1.GIT

Re: [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]

On 01/18/2013 12:29 PM, Luiz Capitulino wrote:
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  docs/virtio-balloon-stats.txt | 102 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>  create mode 100644 docs/virtio-balloon-stats.txt
> 

> +
> +  o A key named 'stats', containing all avaiable stats. If the guest

s/avaiable/available/

> +    doesn't support a particular stat, its value will be -1. Currently,
> +    the following stats are supported:
> +
> +      - stat-swap-in
> +      - stat-swap-out
> +      - stat-major-faults
> +      - stat-minor-faults
> +      - stat-free-memory
> +      - stat-total-memory
> +
> +  o A key named last-update, which contains the last stats update
> +    timestamp in seconds

Is it worth mentioning that this is a timestamp relative to the Unix
epoch?  For that matter, does it even matter what the timestamp is
relative to, or just that it increases when a new poll completes?  Is it
worth mentioning that the timestamp is computed by the host (that is, a
broken guest can't fake the timestamp, even if it can provide bogus data
for all the stats)?

> +
> + - As noted above, if a guest doesn't support a particular stat it
> +   will always be -1. However, it's also possible that a guest couldn't
> +   temporarily update one or even all stats. If this happens, just wait

s/couldn't temporarily/temporarily couldn't/

> +
> +Here are a few examples. The virtio-balloon device is assumed to be in the
> +'/machine/peripheral-anon/device[1]' QOM path.

Is this QOM path stable, or can it change depending on target
architecture and/or command-line arguments used to install the guest?
It might be worth showing which command line arguments set up this
particular QOM path.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On 01/18/2013 12:29 PM, Luiz Capitulino wrote:
> Next commit will re-enable balloon stats with a different interface, but
> this old code conflicts with it. Let's drop it.
> 
> It's important to note that the QMP and HMP interfaces are also dropped
> by this commit. That shouldn't be a problem though, because:
> 
>  1. All QMP fields are optional
>  2. This feature has always been disabled
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  hmp.c               | 24 +-----------------------
>  hw/virtio-balloon.c | 24 ------------------------
>  qapi-schema.json    | 20 +-------------------
>  qmp-commands.hx     | 13 -------------
>  4 files changed, 2 insertions(+), 79 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

On 01/18/2013 12:29 PM, Luiz Capitulino wrote:
> The statistics are now available through device properties via a
> polling mechanism. First a client has to enable polling, then it
> can query available stats.
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats

[Luiz Capitulino]

On Fri, 18 Jan 2013 13:00:28 -0700
Eric Blake <eblake@redhat.com> wrote:

> On 01/18/2013 12:29 PM, Luiz Capitulino wrote:
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  docs/virtio-balloon-stats.txt | 102 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 102 insertions(+)
> >  create mode 100644 docs/virtio-balloon-stats.txt
> > 
> 
> > +
> > +  o A key named 'stats', containing all avaiable stats. If the guest
> 
> s/avaiable/available/

OK.

> > +    doesn't support a particular stat, its value will be -1. Currently,
> > +    the following stats are supported:
> > +
> > +      - stat-swap-in
> > +      - stat-swap-out
> > +      - stat-major-faults
> > +      - stat-minor-faults
> > +      - stat-free-memory
> > +      - stat-total-memory
> > +
> > +  o A key named last-update, which contains the last stats update
> > +    timestamp in seconds
> 
> Is it worth mentioning that this is a timestamp relative to the Unix
> epoch?  For that matter, does it even matter what the timestamp is
> relative to, or just that it increases when a new poll completes?

Yes, I think this field is only important to calculate the delta between
updates.

>  Is it
> worth mentioning that the timestamp is computed by the host (that is, a
> broken guest can't fake the timestamp, even if it can provide bogus data
> for all the stats)?

I can mention that.

> > +
> > + - As noted above, if a guest doesn't support a particular stat it
> > +   will always be -1. However, it's also possible that a guest couldn't
> > +   temporarily update one or even all stats. If this happens, just wait
> 
> s/couldn't temporarily/temporarily couldn't/

OK.

> > +
> > +Here are a few examples. The virtio-balloon device is assumed to be in the
> > +'/machine/peripheral-anon/device[1]' QOM path.
> 
> Is this QOM path stable, or can it change depending on target
> architecture and/or command-line arguments used to install the guest?

I think it can change.

> It might be worth showing which command line arguments set up this
> particular QOM path.

Will do.