[1/2] virtio-balloon: export all balloon statistics
diff mbox

Message ID 1455954883-28858-2-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev Feb. 20, 2016, 7:54 a.m. UTC
From: Igor Redko <redkoi@virtuozzo.com>

We are making experiments with different autoballooning strategies
based on the guest behavior. Thus we need to experiment with different
guest statistics. For now every counter change requires QEMU recompilation
and dances with Libvirt.

This patch introduces transport for unrecognized counters in virtio-balloon.
This transport can be used for measuring benefits from using new
balloon counters, before submitting any patches. Current alternative
is 'guest-exec' transport which isn't made for such delicate matters
and can influence test results.

Originally all counters with tag >= VIRTIO_BALLOON_S_NR were ignored.
Instead of this we keep first (VIRTIO_BALLOON_S_NR + 32) counters from the
queue and pass unrecognized ones with the following names: 'x-stat-XXXX',
where XXXX is a tag number in hex. Defined counters are reported with their
regular names.

Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c         | 30 ++++++++++++++++++++++++------
 include/hw/virtio/virtio-balloon.h |  3 ++-
 2 files changed, 26 insertions(+), 7 deletions(-)

Comments

Eric Blake Feb. 22, 2016, 9:27 p.m. UTC | #1
On 02/20/2016 12:54 AM, Denis V. Lunev wrote:
> From: Igor Redko <redkoi@virtuozzo.com>
> 
> We are making experiments with different autoballooning strategies
> based on the guest behavior. Thus we need to experiment with different
> guest statistics. For now every counter change requires QEMU recompilation
> and dances with Libvirt.
> 
> This patch introduces transport for unrecognized counters in virtio-balloon.
> This transport can be used for measuring benefits from using new
> balloon counters, before submitting any patches. Current alternative
> is 'guest-exec' transport which isn't made for such delicate matters
> and can influence test results.
> 
> Originally all counters with tag >= VIRTIO_BALLOON_S_NR were ignored.
> Instead of this we keep first (VIRTIO_BALLOON_S_NR + 32) counters from the
> queue and pass unrecognized ones with the following names: 'x-stat-XXXX',
> where XXXX is a tag number in hex. Defined counters are reported with their
> regular names.

The name implies experimental, so apps like libvirt shouldn't rely on
it; while it does fill a gap for making development easier, so I'm in
favor of the idea.


> @@ -133,12 +132,20 @@ static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
>      if (err) {
>          goto out_end;
>      }
> -    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
> -        visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err);
> +    for (i = 0; !err && i < s->stats_cnt; i++) {

Why are you checking for err here?  You cannot enter the loop with err
set...

> +        if (s->stats[i].tag < VIRTIO_BALLOON_S_NR) {
> +            visit_type_uint64(v, balloon_stat_names[s->stats[i].tag],
> +                              &s->stats[i].val, &err);
> +        } else {
> +            gchar *str = g_strdup_printf("x-stat-%04x", s->stats[i].tag);
> +            visit_type_uint64(v, str, &s->stats[i].val, &err);
> +            g_free(str);
> +        }
>          if (err) {
>              break;
>          }

...and you cannot exit the loop with it set. Therefore the check is dead
code.

Patch
diff mbox

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index a382f43..cc551a3 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -66,8 +66,7 @@  static const char *balloon_stat_names[] = {
  */
 static inline void reset_stats(VirtIOBalloon *dev)
 {
-    int i;
-    for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1);
+    dev->stats_cnt = 0;
 }
 
 static bool balloon_stats_supported(const VirtIOBalloon *s)
@@ -133,12 +132,20 @@  static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
     if (err) {
         goto out_end;
     }
-    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
-        visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err);
+    for (i = 0; !err && i < s->stats_cnt; i++) {
+        if (s->stats[i].tag < VIRTIO_BALLOON_S_NR) {
+            visit_type_uint64(v, balloon_stat_names[s->stats[i].tag],
+                              &s->stats[i].val, &err);
+        } else {
+            gchar *str = g_strdup_printf("x-stat-%04x", s->stats[i].tag);
+            visit_type_uint64(v, str, &s->stats[i].val, &err);
+            g_free(str);
+        }
         if (err) {
             break;
         }
     }
+
     error_propagate(errp, err);
     err = NULL;
     visit_end_struct(v, &err);
@@ -273,10 +280,21 @@  static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
            == sizeof(stat)) {
         uint16_t tag = virtio_tswap16(vdev, stat.tag);
         uint64_t val = virtio_tswap64(vdev, stat.val);
+        int i;
 
         offset += sizeof(stat);
-        if (tag < VIRTIO_BALLOON_S_NR)
-            s->stats[tag] = val;
+        for (i = 0; i < s->stats_cnt; i++) {
+            if (s->stats[i].tag == tag) {
+                break;
+            }
+        }
+        if (i < ARRAY_SIZE(s->stats)) {
+            s->stats[i].tag = tag;
+            s->stats[i].val = val;
+            if (s->stats_cnt <= i) {
+                s->stats_cnt = i + 1;
+            }
+        }
     }
     s->stats_vq_offset = offset;
 
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index 35f62ac..5c8730e 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -36,7 +36,8 @@  typedef struct VirtIOBalloon {
     VirtQueue *ivq, *dvq, *svq;
     uint32_t num_pages;
     uint32_t actual;
-    uint64_t stats[VIRTIO_BALLOON_S_NR];
+    VirtIOBalloonStatModern stats[VIRTIO_BALLOON_S_NR + 32];
+    uint16_t stats_cnt;
     VirtQueueElement *stats_vq_elem;
     size_t stats_vq_offset;
     QEMUTimer *stats_timer;