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

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

[Luiz Capitulino]

This new try to re-enable the virtio-balloon driver stats uses QOM properties
via a polling mechanism as suggested by Anthony here:

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

The second patch contains the implementation, the last patch contains
documentation and examples.

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

 docs/virtio-balloon-stats.txt |  73 +++++++++++++++++
 hw/virtio-balloon.c           | 178 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 228 insertions(+), 23 deletions(-)
 create mode 100644 docs/virtio-balloon-stats.txt

-- 
1.8.0

[Qemu-devel] [RFC 1/3] virtio-balloon: drop old stats code

[Luiz Capitulino]

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

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/virtio-balloon.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index dd1a650..4398025 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);
 }
-- 
1.8.0

[Qemu-devel] [RFC 2/3] virtio-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 the stats.

The following control properties are introduced:

 o stats-polling-interval: a value greater than zero enables polling
   in the specified interval (in seconds). When value equals zero,
   polling is disabled. If polling is already enabled and a value
   greater than zero is written, the polling interval time is changed

 o stats-last-update: last stats update timestamp, in seconds.

The following stats properties are introduced:

 o stat-swap-in
 o stat-swap-out
 o stat-major-faults
 o stat-minor-faults
 o stat-free-memory
 o stat-total-memory

All values are in bytes. A value of -1 means that the statistic isn't
available right now.

FIXME: Can balloon_stats_poll_cb(), balloon_stats_set_poll_interval(),
       virtio_balloon_handle_output() can run in parallel?

XXX: Should we return an error instead of -1? Might require a specific
     error. Although this is not exactly a failure...

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---

NOTE: Anthony suggested having a bool to enable/disable polling, but I prefer
      to let the client specify the polling interval. I can revisit this,
	  though.

 hw/virtio-balloon.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 155 insertions(+), 1 deletion(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 4398025..06af18f 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -22,6 +22,8 @@
 #include "virtio-balloon.h"
 #include "kvm.h"
 #include "exec-memory.h"
+#include "qemu-timer.h"
+#include "qapi/qapi-visit-core.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,119 @@ 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_disable_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;
+
+    virtqueue_push(s->svq, &s->stats_vq_elem, s->stats_vq_offset);
+    virtio_notify(&s->vdev, s->svq);
+
+    balloon_stats_change_timer(s, s->stats_poll_interval);
+}
+
+static void balloon_stats_get_last_update(Object *obj, struct Visitor *v,
+                                          void *opaque, const char *name,
+                                          Error **errp)
+{
+    VirtIOBalloon *s = opaque;
+    visit_type_int(v, &s->stats_last_update, name, errp);
+}
+
+static void balloon_stats_get_stat(Object *obj, struct Visitor *v,
+                                   void *opaque, const char *name, Error **errp)
+{
+    VirtIOBalloon *s = opaque;
+    int i;
+
+    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
+        if (!strcmp(balloon_stat_names[i], name)) {
+            visit_type_int(v, (int64_t *) &s->stats[i], name, errp);
+            return;
+        }
+    }
+
+    error_setg(errp, "invalid property name '%s'", name);
+}
+
+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;
+
+    if (!balloon_stats_supported(s)) {
+        error_setg(errp, "guest doesn\'t support balloon stats");
+        return;
+    }
+
+    visit_type_int(v, &value, name, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (value < 0) {
+        error_setg(errp, "timer value must be positive");
+        return;
+    }
+
+    if (value == s->stats_poll_interval) {
+        return;
+    }
+
+    if (value == 0) {
+        /* timer=0 disables the timer */
+        balloon_stats_disable_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,11 +235,16 @@ 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;
     }
 
+    if (!balloon_stats_enabled(s)) {
+        return;
+    }
+
     /* Initialize the stats to get rid of any stale values.  This is only
      * needed to handle the case where a guest supports fewer stats than it
      * used to (ie. it has booted into an old kernel).
@@ -128,6 +261,13 @@ 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__);
+        return;
+    }
+
+    s->stats_last_update = tv.tv_sec;
 }
 
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
@@ -212,7 +352,7 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
 VirtIODevice *virtio_balloon_init(DeviceState *dev)
 {
     VirtIOBalloon *s;
-    int ret;
+    int i, ret;
 
     s = (VirtIOBalloon *)virtio_common_init("virtio-balloon",
                                             VIRTIO_ID_BALLOON,
@@ -239,6 +379,19 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
     register_savevm(dev, "virtio-balloon", -1, 1,
                     virtio_balloon_save, virtio_balloon_load, s);
 
+    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
+        object_property_add(OBJECT(dev), balloon_stat_names[i], "int",
+                            balloon_stats_get_stat, NULL, NULL, s, NULL);
+    }
+
+    object_property_add(OBJECT(dev), "stats-last-update", "int",
+                        balloon_stats_get_last_update, NULL, NULL, s, NULL);
+
+    object_property_add(OBJECT(dev), "stats-polling-interval", "int",
+                        balloon_stats_get_poll_interval,
+                        balloon_stats_set_poll_interval,
+                        NULL, s, NULL);
+
     return &s->vdev;
 }
 
@@ -246,6 +399,7 @@ void virtio_balloon_exit(VirtIODevice *vdev)
 {
     VirtIOBalloon *s = DO_UPCAST(VirtIOBalloon, vdev, vdev);
 
+    balloon_stats_disable_timer(s);
     qemu_remove_balloon_handler(s);
     unregister_savevm(s->qdev, "virtio-balloon", s);
     virtio_cleanup(vdev);
-- 
1.8.0

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

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 docs/virtio-balloon-stats.txt | 73 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 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..7e7ddc4
--- /dev/null
+++ b/docs/virtio-balloon-stats.txt
@@ -0,0 +1,73 @@
+virtio balloon memory statistics
+================================
+
+The virtio balloon driver supports guest memory statistics reporting. These
+statistics are available to QEMU users as QOM (QEMU Obejct Model) device
+properties via a polling mechanism.
+
+Basically, clients have to enable polling. Then they can query the available
+statistics.
+
+There are two control properties and six memory statistics from the guest.
+
+The control properties are:
+
+ o stats-polling-interval: a value greater than zero enables polling
+   in the specified interval (in seconds). When value equals zero,
+   polling is disabled. If polling is already enabled and a value
+   greater than zero is written, the polling interval time is changed
+
+ o stats-last-update: last stats update timestamp, in seconds
+
+The memory statistics are:
+
+ o stat-swap-in
+ o stat-swap-out
+ o stat-major-faults
+ o stat-minor-faults
+ o stat-free-memory
+ o stat-total-memory
+
+All values are in bytes. A value of -1 means that the statistic isn't
+available right now.
+
+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": "stats-polling-interval", "value": 2 } }
+
+{ "return": {} }
+
+Change polling to 10 seconds:
+
+{ "execute": "qom-set",
+             "arguments": { "path": "/machine/peripheral-anon/device[1]",
+			 "property": "stats-polling-interval", "value": 10 } }
+
+{ "return": {} }
+
+Get last update timestamp and free memory stat:
+
+{ "execute": "qom-get",
+  "arguments": { "path": "/machine/peripheral-anon/device[1]",
+  "property": "stats-last-update" } }
+
+{ "return": 1354629634 }
+
+{ "execute": "qom-get",
+  "arguments": { "path": "/machine/peripheral-anon/device[1]",
+  "property": "stat-free-memory" } }
+
+{ "return": 845115392 }
+
+Disable polling:
+
+{ "execute": "qom-set",
+             "arguments": { "path": "/machine/peripheral-anon/device[1]",
+			 "property": "stats-polling-interval", "value": 0 } }
+
+{ "return": {} }
-- 
1.8.0

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

[Daniel P. Berrange]

On Tue, Dec 04, 2012 at 01:04:48PM -0200, Luiz Capitulino wrote:
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  docs/virtio-balloon-stats.txt | 73 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 73 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..7e7ddc4
> --- /dev/null
> +++ b/docs/virtio-balloon-stats.txt
> @@ -0,0 +1,73 @@
> +virtio balloon memory statistics
> +================================
> +
> +The virtio balloon driver supports guest memory statistics reporting. These
> +statistics are available to QEMU users as QOM (QEMU Obejct Model) device
> +properties via a polling mechanism.
> +
> +Basically, clients have to enable polling. Then they can query the available
> +statistics.
> +
> +There are two control properties and six memory statistics from the guest.
> +
> +The control properties are:
> +
> + o stats-polling-interval: a value greater than zero enables polling
> +   in the specified interval (in seconds). When value equals zero,
> +   polling is disabled. If polling is already enabled and a value
> +   greater than zero is written, the polling interval time is changed
> +
> + o stats-last-update: last stats update timestamp, in seconds
> +
> +The memory statistics are:
> +
> + o stat-swap-in
> + o stat-swap-out
> + o stat-major-faults
> + o stat-minor-faults
> + o stat-free-memory
> + o stat-total-memory
> +
> +All values are in bytes. A value of -1 means that the statistic isn't
> +available right now.
> +
> +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": "stats-polling-interval", "value": 2 } }
> +
> +{ "return": {} }
> +
> +Change polling to 10 seconds:
> +
> +{ "execute": "qom-set",
> +             "arguments": { "path": "/machine/peripheral-anon/device[1]",
> +			 "property": "stats-polling-interval", "value": 10 } }
> +
> +{ "return": {} }
> +
> +Get last update timestamp and free memory stat:
> +
> +{ "execute": "qom-get",
> +  "arguments": { "path": "/machine/peripheral-anon/device[1]",
> +  "property": "stats-last-update" } }
> +
> +{ "return": 1354629634 }
> +
> +{ "execute": "qom-get",
> +  "arguments": { "path": "/machine/peripheral-anon/device[1]",
> +  "property": "stat-free-memory" } }
> +
> +{ "return": 845115392 }
> +
> +Disable polling:
> +
> +{ "execute": "qom-set",
> +             "arguments": { "path": "/machine/peripheral-anon/device[1]",
> +			 "property": "stats-polling-interval", "value": 0 } }
> +
> +{ "return": {} }


What sort of performance implications are there for enabling polling of
virtio stats. Is it the kind of thing that it is reasonable to just
enable for all VMs on a 10 second interval, so we'll always have stats
available without having to have thought to enable them ahead of time ?

eg, the use case I'm wondering is that someone comes along and just
runs   'virsh memstats $DOMAIN' and wants to see the latest data
right away. 

I'm not suggesting that libvirt would be actually asking QEMU for the
stats every 10 seconds. Only that libvirt tells QEMU to collect them.
Then libvirt can just ask for them whenver someone wants them.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

[Luiz Capitulino]

On Thu, 6 Dec 2012 13:24:11 +0000
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Tue, Dec 04, 2012 at 01:04:48PM -0200, Luiz Capitulino wrote:
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  docs/virtio-balloon-stats.txt | 73 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 73 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..7e7ddc4
> > --- /dev/null
> > +++ b/docs/virtio-balloon-stats.txt
> > @@ -0,0 +1,73 @@
> > +virtio balloon memory statistics
> > +================================
> > +
> > +The virtio balloon driver supports guest memory statistics reporting. These
> > +statistics are available to QEMU users as QOM (QEMU Obejct Model) device
> > +properties via a polling mechanism.
> > +
> > +Basically, clients have to enable polling. Then they can query the available
> > +statistics.
> > +
> > +There are two control properties and six memory statistics from the guest.
> > +
> > +The control properties are:
> > +
> > + o stats-polling-interval: a value greater than zero enables polling
> > +   in the specified interval (in seconds). When value equals zero,
> > +   polling is disabled. If polling is already enabled and a value
> > +   greater than zero is written, the polling interval time is changed
> > +
> > + o stats-last-update: last stats update timestamp, in seconds
> > +
> > +The memory statistics are:
> > +
> > + o stat-swap-in
> > + o stat-swap-out
> > + o stat-major-faults
> > + o stat-minor-faults
> > + o stat-free-memory
> > + o stat-total-memory
> > +
> > +All values are in bytes. A value of -1 means that the statistic isn't
> > +available right now.
> > +
> > +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": "stats-polling-interval", "value": 2 } }
> > +
> > +{ "return": {} }
> > +
> > +Change polling to 10 seconds:
> > +
> > +{ "execute": "qom-set",
> > +             "arguments": { "path": "/machine/peripheral-anon/device[1]",
> > +			 "property": "stats-polling-interval", "value": 10 } }
> > +
> > +{ "return": {} }
> > +
> > +Get last update timestamp and free memory stat:
> > +
> > +{ "execute": "qom-get",
> > +  "arguments": { "path": "/machine/peripheral-anon/device[1]",
> > +  "property": "stats-last-update" } }
> > +
> > +{ "return": 1354629634 }
> > +
> > +{ "execute": "qom-get",
> > +  "arguments": { "path": "/machine/peripheral-anon/device[1]",
> > +  "property": "stat-free-memory" } }
> > +
> > +{ "return": 845115392 }
> > +
> > +Disable polling:
> > +
> > +{ "execute": "qom-set",
> > +             "arguments": { "path": "/machine/peripheral-anon/device[1]",
> > +			 "property": "stats-polling-interval", "value": 0 } }
> > +
> > +{ "return": {} }
> 
> 
> What sort of performance implications are there for enabling polling of
> virtio stats. Is it the kind of thing that it is reasonable to just
> enable for all VMs on a 10 second interval, so we'll always have stats
> available without having to have thought to enable them ahead of time ?

I can't think of any performance implications. Would be nice to get a
second opinion from the CC'ed people though.

> eg, the use case I'm wondering is that someone comes along and just
> runs   'virsh memstats $DOMAIN' and wants to see the latest data
> right away. 
> 
> I'm not suggesting that libvirt would be actually asking QEMU for the
> stats every 10 seconds. Only that libvirt tells QEMU to collect them.
> Then libvirt can just ask for them whenver someone wants them.

Note that once you enable polling, the balloon driver will immediately make
a request to the guest, that is, it won't wait the specified time interval to
send the first request.

So, the first call to virsh memstats could start polling and also poll for it
(although you do need to be prepared for the case where the guest doesn't
respond).

Also, you could consider adding the time interval in libvirt's API and
virsh memstats.

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

[mdroth]

On Thu, Dec 06, 2012 at 01:31:09PM -0200, Luiz Capitulino wrote:
> On Thu, 6 Dec 2012 13:24:11 +0000
> "Daniel P. Berrange" <berrange@redhat.com> wrote:
> 
> > On Tue, Dec 04, 2012 at 01:04:48PM -0200, Luiz Capitulino wrote:
> > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > > ---
> > >  docs/virtio-balloon-stats.txt | 73 +++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 73 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..7e7ddc4
> > > --- /dev/null
> > > +++ b/docs/virtio-balloon-stats.txt
> > > @@ -0,0 +1,73 @@
> > > +virtio balloon memory statistics
> > > +================================
> > > +
> > > +The virtio balloon driver supports guest memory statistics reporting. These
> > > +statistics are available to QEMU users as QOM (QEMU Obejct Model) device
> > > +properties via a polling mechanism.
> > > +
> > > +Basically, clients have to enable polling. Then they can query the available
> > > +statistics.
> > > +
> > > +There are two control properties and six memory statistics from the guest.
> > > +
> > > +The control properties are:
> > > +
> > > + o stats-polling-interval: a value greater than zero enables polling
> > > +   in the specified interval (in seconds). When value equals zero,
> > > +   polling is disabled. If polling is already enabled and a value
> > > +   greater than zero is written, the polling interval time is changed
> > > +
> > > + o stats-last-update: last stats update timestamp, in seconds
> > > +
> > > +The memory statistics are:
> > > +
> > > + o stat-swap-in
> > > + o stat-swap-out
> > > + o stat-major-faults
> > > + o stat-minor-faults
> > > + o stat-free-memory
> > > + o stat-total-memory
> > > +
> > > +All values are in bytes. A value of -1 means that the statistic isn't
> > > +available right now.
> > > +
> > > +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": "stats-polling-interval", "value": 2 } }
> > > +
> > > +{ "return": {} }
> > > +
> > > +Change polling to 10 seconds:
> > > +
> > > +{ "execute": "qom-set",
> > > +             "arguments": { "path": "/machine/peripheral-anon/device[1]",
> > > +			 "property": "stats-polling-interval", "value": 10 } }
> > > +
> > > +{ "return": {} }
> > > +
> > > +Get last update timestamp and free memory stat:
> > > +
> > > +{ "execute": "qom-get",
> > > +  "arguments": { "path": "/machine/peripheral-anon/device[1]",
> > > +  "property": "stats-last-update" } }
> > > +
> > > +{ "return": 1354629634 }
> > > +
> > > +{ "execute": "qom-get",
> > > +  "arguments": { "path": "/machine/peripheral-anon/device[1]",
> > > +  "property": "stat-free-memory" } }
> > > +
> > > +{ "return": 845115392 }
> > > +
> > > +Disable polling:
> > > +
> > > +{ "execute": "qom-set",
> > > +             "arguments": { "path": "/machine/peripheral-anon/device[1]",
> > > +			 "property": "stats-polling-interval", "value": 0 } }
> > > +
> > > +{ "return": {} }
> > 
> > 
> > What sort of performance implications are there for enabling polling of
> > virtio stats. Is it the kind of thing that it is reasonable to just
> > enable for all VMs on a 10 second interval, so we'll always have stats
> > available without having to have thought to enable them ahead of time ?
> 
> I can't think of any performance implications. Would be nice to get a
> second opinion from the CC'ed people though.

Pushing/popping/processing one vq entry every 10 seconds should be
virtually unnoticeable given that virtio-net/blk do this much more frequently
with much more processing overhead per entry on a relatively idle guest. So
performance-wise, I don't think it's an issue. As to whether or not it
*should* be enabled by default I'm not so sure, but if it actually simplifies
things on that end I'd say it's worth it if the alternatives are
cumbersome.

> 
> > eg, the use case I'm wondering is that someone comes along and just
> > runs   'virsh memstats $DOMAIN' and wants to see the latest data
> > right away. 
> > 
> > I'm not suggesting that libvirt would be actually asking QEMU for the
> > stats every 10 seconds. Only that libvirt tells QEMU to collect them.
> > Then libvirt can just ask for them whenver someone wants them.
> 
> Note that once you enable polling, the balloon driver will immediately make
> a request to the guest, that is, it won't wait the specified time interval to
> send the first request.
> 
> So, the first call to virsh memstats could start polling and also poll for it
> (although you do need to be prepared for the case where the guest doesn't
> respond).
> 
> Also, you could consider adding the time interval in libvirt's API and
> virsh memstats.
> 

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

[mdroth]

On Tue, Dec 04, 2012 at 01:04:47PM -0200, 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 the stats.
> 
> The following control properties are introduced:
> 
>  o stats-polling-interval: a value greater than zero enables polling
>    in the specified interval (in seconds). When value equals zero,
>    polling is disabled. If polling is already enabled and a value
>    greater than zero is written, the polling interval time is changed
> 
>  o stats-last-update: last stats update timestamp, in seconds.
> 
> The following stats properties are introduced:
> 
>  o stat-swap-in
>  o stat-swap-out
>  o stat-major-faults
>  o stat-minor-faults
>  o stat-free-memory
>  o stat-total-memory
> 
> All values are in bytes. A value of -1 means that the statistic isn't
> available right now.
> 
> FIXME: Can balloon_stats_poll_cb(), balloon_stats_set_poll_interval(),
>        virtio_balloon_handle_output() can run in parallel?
> 
> XXX: Should we return an error instead of -1? Might require a specific
>      error. Although this is not exactly a failure...
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> 
> NOTE: Anthony suggested having a bool to enable/disable polling, but I prefer
>       to let the client specify the polling interval. I can revisit this,
> 	  though.
> 
>  hw/virtio-balloon.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 155 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> index 4398025..06af18f 100644
> --- a/hw/virtio-balloon.c
> +++ b/hw/virtio-balloon.c
> @@ -22,6 +22,8 @@
>  #include "virtio-balloon.h"
>  #include "kvm.h"
>  #include "exec-memory.h"
> +#include "qemu-timer.h"
> +#include "qapi/qapi-visit-core.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,119 @@ 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_disable_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;
> +
> +    virtqueue_push(s->svq, &s->stats_vq_elem, s->stats_vq_offset);
> +    virtio_notify(&s->vdev, s->svq);

I think we'll want to add some logic to avoid the potential for
re-pushing an elem we've already processed, as I think that violates the
virtio spec. In the past the monitor blocked us from doing this but with
timer-driven requests I think it's a possibility now.

But the general approach seems sane to me and the code looks to
be in decent shape.

> -- 
> 1.8.0
> 
> 

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

[Luiz Capitulino]

On Thu, 6 Dec 2012 11:03:21 -0600
mdroth <mdroth@linux.vnet.ibm.com> wrote:

> On Tue, Dec 04, 2012 at 01:04:47PM -0200, 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 the stats.
> > 
> > The following control properties are introduced:
> > 
> >  o stats-polling-interval: a value greater than zero enables polling
> >    in the specified interval (in seconds). When value equals zero,
> >    polling is disabled. If polling is already enabled and a value
> >    greater than zero is written, the polling interval time is changed
> > 
> >  o stats-last-update: last stats update timestamp, in seconds.
> > 
> > The following stats properties are introduced:
> > 
> >  o stat-swap-in
> >  o stat-swap-out
> >  o stat-major-faults
> >  o stat-minor-faults
> >  o stat-free-memory
> >  o stat-total-memory
> > 
> > All values are in bytes. A value of -1 means that the statistic isn't
> > available right now.
> > 
> > FIXME: Can balloon_stats_poll_cb(), balloon_stats_set_poll_interval(),
> >        virtio_balloon_handle_output() can run in parallel?
> > 
> > XXX: Should we return an error instead of -1? Might require a specific
> >      error. Although this is not exactly a failure...
> > 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> > 
> > NOTE: Anthony suggested having a bool to enable/disable polling, but I prefer
> >       to let the client specify the polling interval. I can revisit this,
> > 	  though.
> > 
> >  hw/virtio-balloon.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 155 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> > index 4398025..06af18f 100644
> > --- a/hw/virtio-balloon.c
> > +++ b/hw/virtio-balloon.c
> > @@ -22,6 +22,8 @@
> >  #include "virtio-balloon.h"
> >  #include "kvm.h"
> >  #include "exec-memory.h"
> > +#include "qemu-timer.h"
> > +#include "qapi/qapi-visit-core.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,119 @@ 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_disable_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;
> > +
> > +    virtqueue_push(s->svq, &s->stats_vq_elem, s->stats_vq_offset);
> > +    virtio_notify(&s->vdev, s->svq);
> 
> I think we'll want to add some logic to avoid the potential for
> re-pushing an elem we've already processed, as I think that violates the
> virtio spec. In the past the monitor blocked us from doing this but with
> timer-driven requests I think it's a possibility now.

I'll check that, thanks for the feedback Mike.

> 
> But the general approach seems sane to me and the code looks to
> be in decent shape.
> 
> > -- 
> > 1.8.0
> > 
> > 
> 

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

[Dietmar Maurer]

> +The memory statistics are:
> +
> + o stat-swap-in
> + o stat-swap-out
> + o stat-major-faults
> + o stat-minor-faults
> + o stat-free-memory
> + o stat-total-memory

I want to implement an automatic ballooning policy, so I need to
know the amount of free RAM inside the guest.

But it seem that the balloon driver reports free ram without accounting
buffers and cache.

Is it possible to extent those statistic to include buffer/cache values?

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

[Luiz Capitulino]

On Fri, 7 Dec 2012 05:30:17 +0000
Dietmar Maurer <dietmar@proxmox.com> wrote:

> > +The memory statistics are:
> > +
> > + o stat-swap-in
> > + o stat-swap-out
> > + o stat-major-faults
> > + o stat-minor-faults
> > + o stat-free-memory
> > + o stat-total-memory
> 
> I want to implement an automatic ballooning policy, so I need to
> know the amount of free RAM inside the guest.
> 
> But it seem that the balloon driver reports free ram without accounting
> buffers and cache.
> 
> Is it possible to extent those statistic to include buffer/cache values?

Yes, should be possible but the kernel virtio-balloon driver has to be
extended too (and I suspect this kind of change goes into the kernel first).

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

[Dietmar Maurer]

> > Is it possible to extent those statistic to include buffer/cache values?
> 
> Yes, should be possible but the kernel virtio-balloon driver has to be
> extended too (and I suspect this kind of change goes into the kernel first).

The current statistics are quite useless without that information, and
I guess it is impossible to such changes upstream within a reasonable time?

So what is the purpose of that 'stat-free-memory'? Can we change that to
return the amount of free RAM including buffers?

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

[Luiz Capitulino]

On Fri, 7 Dec 2012 15:01:36 +0000
Dietmar Maurer <dietmar@proxmox.com> wrote:

> > > Is it possible to extent those statistic to include buffer/cache values?
> > 
> > Yes, should be possible but the kernel virtio-balloon driver has to be
> > extended too (and I suspect this kind of change goes into the kernel first).
> 
> The current statistics are quite useless without that information, and

Maybe it's not possible to implement the feature you want to implement, but
I do get a ping once a while from people asking for these stats. Some of them
are people running virsh dommemstat which also makes use of that information.

> I guess it is impossible to such changes upstream within a reasonable time?

In the kernel, yes it would take a while.

> So what is the purpose of that 'stat-free-memory'? Can we change that to
> return the amount of free RAM including buffers?

That's kernel work, qemu is merely reporting what the kernel is making
available. But giving that you'll touch the kernel anyway, it's better
to add a new stat.

There are two other alternatives:

 1. add a query-memory-stats command to qemu-ga

     Pros: should be quicker and easier to modify/extend
     Cons: has the implications of running this from the guest user-space

 2. wait for the (kernel-based) auto-ballooning feature, which is on the
    works

     Pros: you do nothing and it's going to be completely automatic
     Cons: this is unlikely to be ready for the short term

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

[Dietmar Maurer]

>  2. wait for the (kernel-based) auto-ballooning feature, which is on the
>     works

Do you have more information on that? Any links?

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

[Alexandre DERUMIER]

>>That's kernel work, qemu is merely reporting what the kernel is making
>>available. But giving that you'll touch the kernel anyway, it's better
>>to add a new stat.
>>
>>There are two other alternatives:
>>
>>1. add a query-memory-stats command to qemu-ga
>>
>>Pros: should be quicker and easier to modify/extend
>>Cons: has the implications of running this from the guest user-space

Hello,
I would like also extend balloon stats to retrieve guest buffer memory, It can help a lot for doing accurate balloning.

Using qemu-ga is easy for linux (we can read /proc/meminfo), but with windows guest is impossible currently.
(we need to query wmi counter, and executing process is no possible with qga)
Adding the new wmi counter ("System Cache Resident Bytes") is quite easy in windows balloon driver.

Best Regards,

Alexandre Derumier


--- Mail original -----

De: "Luiz Capitulino" <lcapitulino@redhat.com>
À: "Dietmar Maurer" <dietmar@proxmox.com>
Cc: aliguori@us.ibm.com, agl@us.ibm.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com
Envoyé: Vendredi 7 Décembre 2012 19:54:44
Objet: Re: [Qemu-devel] [RFC 3/3] docs: document virtio-balloon stats

On Fri, 7 Dec 2012 15:01:36 +0000
Dietmar Maurer <dietmar@proxmox.com> wrote:

> > > Is it possible to extent those statistic to include buffer/cache values?
> >
> > Yes, should be possible but the kernel virtio-balloon driver has to be 
> > extended too (and I suspect this kind of change goes into the kernel first).
>
> The current statistics are quite useless without that information, and

Maybe it's not possible to implement the feature you want to implement, but
I do get a ping once a while from people asking for these stats. Some of them
are people running virsh dommemstat which also makes use of that information.

> I guess it is impossible to such changes upstream within a reasonable time?

In the kernel, yes it would take a while.

> So what is the purpose of that 'stat-free-memory'? Can we change that to 
> return the amount of free RAM including buffers?

That's kernel work, qemu is merely reporting what the kernel is making
available. But giving that you'll touch the kernel anyway, it's better
to add a new stat.

There are two other alternatives:

1. add a query-memory-stats command to qemu-ga

Pros: should be quicker and easier to modify/extend
Cons: has the implications of running this from the guest user-space

2. wait for the (kernel-based) auto-ballooning feature, which is on the
works

Pros: you do nothing and it's going to be completely automatic
Cons: this is unlikely to be ready for the short term

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

[Luiz Capitulino]

On Sat, 8 Dec 2012 06:26:05 +0000
Dietmar Maurer <dietmar@proxmox.com> wrote:

> >  2. wait for the (kernel-based) auto-ballooning feature, which is on the
> >     works
> 
> Do you have more information on that? Any links?

No, no links. What's being worked right now is a notification mechanism for
the kernel to inform user-space about memory pressure:

 http://marc.info/?l=linux-mm&m=135513372205134&w=2

Then we can add that to qemu for auto-inflate and change the virtio-balloon
driver to do auto-deflate.

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

[Dietmar Maurer]

> > >  2. wait for the (kernel-based) auto-ballooning feature, which is on the
> > >     works
> >
> > Do you have more information on that? Any links?
> 
> No, no links. What's being worked right now is a notification mechanism for
> the kernel to inform user-space about memory pressure:
> 
>  http://marc.info/?l=linux-mm&m=135513372205134&w=2
> 
> Then we can add that to qemu for auto-inflate and change the virtio-balloon
> driver to do auto-deflate.

Many thanks for that information.