[Qemu-devel] [PATCH 6/6] qmp: add balloon-get-memory-stats & event

[Luiz Capitulino]

This commit adds a QMP API for the guest provided memory statistics
(long disabled by commit 07b0403dfc2b2ac179ae5b48105096cc2d03375a).

The approach taken by the original commit
(625a5befc2e3200b396594f002218d235e375da5) was to extend the
query-balloon command. It introduced a severe bug though: query-balloon
would hang if the guest didn't respond.

The approach taken by this commit is asynchronous and thus avoids
any QMP hangs.

First, a client has to issue the balloon-get-memory-stats command.
That command gets the process started by only sending a request to
the guest, it doesn't block. When the memory stats are made available
by the guest, they are returned to the client as an QMP event.

Signed-off-by: Luiz Capitulino <address@hidden>
---
 QMP/qmp-events.txt  |   36 ++++++++++++++++++++++++
 balloon.c           |   30 +++++++++++++++++++-
 balloon.h           |    4 ++-
 hw/virtio-balloon.c |   76 ++++++++++++++++++++++++++++++++++-----------------
 monitor.c           |    2 +
 monitor.h           |    1 +
 qapi-schema.json    |   21 ++++++++++++++
 qmp-commands.hx     |    5 +++
 8 files changed, 147 insertions(+), 28 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 06cb404..b34b289 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -1,6 +1,42 @@
                    QEMU Monitor Protocol Events
                    ============================
 
+BALLOON_MEMORY_STATS
+--------------------
+
+Emitted when memory statistics information is made available by the guest.
+
+Data:
+
+- "memory-swapped-in": number of pages swapped in within the guest
+  (json-int, optional)
+- "memory-swapped-out": number of pages swapped out within the guest
+  (json-int, optional)
+- "major-page-faults": number of major page faults within the guest
+  (json-int, optional)
+- "minor-page-faults": number of minor page faults within the guest
+  (json-int, optional)
+- "memory-free": amount of memory (in bytes) free in the guest
+  (json-int, optional)
+- "memory-total": amount of memory (in bytes) visible to the guest
+  (json-int, optional)
+
+Example:
+
+{ "event": "BALLOON_MEMORY_STATS",
+    "data": { "memory-free": 847941632,
+              "major-page-faults": 225,
+              "memory-swapped-in": 0,
+              "minor-page-faults": 222317,
+              "memory-total": 1045516288,
+              "memory-swapped-out": 0 },
+    "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
+
+Notes: o The balloon-get-memory-stats command must be issued first, to tell
+         the guest to make the memory statistics available.
+       o The event may not contain all data members when emitted
+
+
 BLOCK_IO_ERROR
 --------------
 
diff --git a/balloon.c b/balloon.c
index d340ae3..1c1a3c1 100644
--- a/balloon.c
+++ b/balloon.c
@@ -33,12 +33,15 @@
 
 static QEMUBalloonEvent *balloon_event_fn;
 static QEMUBalloonInfo *balloon_info_fn;
+static QEMUBalloonStats *balloon_stats_fn;
 static void *balloon_opaque;
 
 int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
-                             QEMUBalloonInfo *info_func, void *opaque)
+                             QEMUBalloonInfo *info_func,
+                             QEMUBalloonStats *stats_func, void *opaque)
 {
-    if (balloon_event_fn || balloon_info_fn || balloon_opaque) {
+    if (balloon_event_fn || balloon_info_fn || balloon_stats_fn ||
+        balloon_opaque) {
         /* We're already registered one balloon handler.  How many can
          * a guest really have?
          */
@@ -47,6 +50,7 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
     }
     balloon_event_fn = event_func;
     balloon_info_fn = info_func;
+    balloon_stats_fn = stats_func;
     balloon_opaque = opaque;
     return 0;
 }
@@ -58,6 +62,7 @@ void qemu_remove_balloon_handler(void *opaque)
     }
     balloon_event_fn = NULL;
     balloon_info_fn = NULL;
+    balloon_stats_fn = NULL;
     balloon_opaque = NULL;
 }
 
@@ -80,6 +85,15 @@ static int qemu_balloon_info(BalloonInfo *info)
     return 1;
 }
 
+static int qemu_balloon_stats(void)
+{
+    if (!balloon_stats_fn) {
+        return 0;
+    }
+    balloon_stats_fn(balloon_opaque);
+    return 1;
+}
+
 static bool check_kvm_sync_mmu(Error **errp)
 {
     if (kvm_enabled() && !kvm_has_sync_mmu()) {
@@ -89,6 +103,18 @@ static bool check_kvm_sync_mmu(Error **errp)
     return true;
 }
 
+void qmp_balloon_get_memory_stats(Error **errp)
+{
+    if (!check_kvm_sync_mmu(errp)) {
+        return;
+    }
+
+    if (qemu_balloon_stats() == 0) {
+        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
+        return;
+    }
+}
+
 BalloonInfo *qmp_query_balloon(Error **errp)
 {
     BalloonInfo *info;
diff --git a/balloon.h b/balloon.h
index a539354..509e477 100644
--- a/balloon.h
+++ b/balloon.h
@@ -18,9 +18,11 @@
 
 typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t target);
 typedef void (QEMUBalloonInfo)(void *opaque, BalloonInfo *info);
+typedef void (QEMUBalloonStats)(void *opaque);
 
 int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
-                            QEMUBalloonInfo *info_func, void *opaque);
+                            QEMUBalloonInfo *info_func, QEMUBalloonStats 
*stats_func,
+                 void *opaque);
 void qemu_remove_balloon_handler(void *opaque);
 
 #endif
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 4307f4c..8bc842c 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 "monitor.h"
+#include "qemu-objects.h"
 
 #if defined(__linux__)
 #include <sys/mman.h>
@@ -34,6 +36,7 @@ typedef struct VirtIOBalloon
     uint32_t num_pages;
     uint32_t actual;
     uint64_t stats[VIRTIO_BALLOON_S_NR];
+    bool stats_requested;
     VirtQueueElement stats_vq_elem;
     size_t stats_vq_offset;
     DeviceState *qdev;
@@ -56,12 +59,10 @@ static void balloon_page(void *addr, int deflate)
 /*
  * reset_stats - Mark all items in the stats array as unset
  *
- * This function needs to be called at device intialization and before
- * before updating to a set of newly-generated stats.  This will ensure that no
- * stale values stick around in case the guest reports a subset of the 
supported
- * statistics.
+ * This function ensures that no stale values stick around in case the guest
+ * reports a subset of the supported statistics.
  */
-static inline void reset_stats(VirtIOBalloon *dev)
+static void reset_stats(VirtIOBalloon *dev)
 {
     int i;
     for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1);
@@ -101,6 +102,38 @@ static void virtio_balloon_handle_output(VirtIODevice 
*vdev, VirtQueue *vq)
     }
 }
 
+static void emit_qmp_balloon_event(const VirtIOBalloon *dev)
+{
+    int i;
+    QDict *stats;
+    const struct stats_strings {
+        int stat_idx;
+        const char *stat_name;
+    } stats_strings[] = {
+        { VIRTIO_BALLOON_S_SWAP_IN, "memory-swapped-in" },
+        { VIRTIO_BALLOON_S_SWAP_OUT, "memory-swapped-out" },
+        { VIRTIO_BALLOON_S_MAJFLT, "major-page-faults" },
+        { VIRTIO_BALLOON_S_MINFLT, "minor-page-faults" },
+        { VIRTIO_BALLOON_S_MEMFREE, "memory-free" },
+        { VIRTIO_BALLOON_S_MEMTOT, "memory-total" },
+        { 0, NULL },
+    };
+
+    stats = qdict_new();
+    for (i = 0; stats_strings[i].stat_name != NULL; i++) {
+        if (dev->stats[i] != -1) {
+            qdict_put(stats, stats_strings[i].stat_name,
+                      qint_from_int(dev->stats[i]));
+        }
+    }
+
+    if (qdict_size(stats) > 0) {
+        monitor_protocol_event(QEVENT_BALLOON_STATS, QOBJECT(stats));
+    }
+
+    QDECREF(stats);
+}
+
 static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOBalloon *s = DO_UPCAST(VirtIOBalloon, vdev, vdev);
@@ -108,7 +141,7 @@ static void virtio_balloon_receive_stats(VirtIODevice 
*vdev, VirtQueue *vq)
     VirtIOBalloonStat stat;
     size_t offset = 0;
 
-    if (!virtqueue_pop(vq, elem)) {
+    if (!virtqueue_pop(vq, elem) || !s->stats_requested) {
         return;
     }
 
@@ -128,6 +161,9 @@ static void virtio_balloon_receive_stats(VirtIODevice 
*vdev, VirtQueue *vq)
             s->stats[tag] = val;
     }
     s->stats_vq_offset = offset;
+    s->stats_requested = false;
+
+    emit_qmp_balloon_event(s);
 }
 
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
@@ -156,31 +192,21 @@ static uint32_t virtio_balloon_get_features(VirtIODevice 
*vdev, uint32_t f)
     return f;
 }
 
-static void virtio_balloon_info(void *opaque, BalloonInfo *info)
+static void virtio_balloon_stats(void *opaque)
 {
     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)) {
+        dev->stats_requested = true;
         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);
+}
 
+static void virtio_balloon_info(void *opaque, BalloonInfo *info)
+{
+    VirtIOBalloon *dev = opaque;
     info->actual = ram_size - ((uint64_t) dev->actual <<
                                VIRTIO_BALLOON_PFN_SHIFT);
 }
@@ -236,18 +262,18 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
     s->vdev.get_features = virtio_balloon_get_features;
 
     ret = qemu_add_balloon_handler(virtio_balloon_to_target,
-                                   virtio_balloon_info, s);
+                                   virtio_balloon_info,
+                                   virtio_balloon_stats, s);
     if (ret < 0) {
         virtio_cleanup(&s->vdev);
         return NULL;
     }
 
+    s->stats_requested = false;
     s->ivq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
     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/monitor.c b/monitor.c
index aadbdcb..868f2a0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -484,6 +484,8 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
             break;
         case QEVENT_BLOCK_JOB_CANCELLED:
             event_name = "BLOCK_JOB_CANCELLED";
+        case QEVENT_BALLOON_STATS:
+            event_name = "BALLOON_MEMORY_STATS";
             break;
         default:
             abort();
diff --git a/monitor.h b/monitor.h
index b72ea07..76e26c7 100644
--- a/monitor.h
+++ b/monitor.h
@@ -38,6 +38,7 @@ typedef enum MonitorEvent {
     QEVENT_SPICE_DISCONNECTED,
     QEVENT_BLOCK_JOB_COMPLETED,
     QEVENT_BLOCK_JOB_CANCELLED,
+    QEVENT_BALLOON_STATS,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 24a42e3..c4d8d0c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1563,3 +1563,24 @@
 { 'command': 'qom-list-types',
   'data': { '*implements': 'str', '*abstract': 'bool' },
   'returns': [ 'ObjectTypeInfo' ] }
+
+##
+# @balloon-get-memory-stats
+#
+# Ask the guest's balloon driver for guest memory statistics.
+#
+# This command will only get the process started and will return immediately.
+# The BALLOON_MEMORY_STATS event will be emitted when the statistics
+# information is returned by the guest.
+#
+# Returns: nothing on success
+#          If the balloon driver is enabled but not functional because the KVM
+#          kernel module cannot support it, KvmMissingCap
+#          If no balloon device is present, DeviceNotActive
+#
+# Notes: There's no guarantees the guest will ever respond, thus the
+#        BALLOON_MEMORY_STATS event may never be emitted.
+#
+# Since: 1.1
+##
+{ 'command': 'balloon-get-memory-stats' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b5e2ab8..52c1fc3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2047,3 +2047,8 @@ EQMP
         .args_type  = "implements:s?,abstract:b?",
         .mhandler.cmd_new = qmp_marshal_input_qom_list_types,
     },
+    {
+        .name       = "balloon-get-memory-stats",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_balloon_get_memory_stats,
+    },
-- 
1.7.9.111.gf3fb0.dirty