[Qemu-devel] [RFC 0/3] qga: add guest-get-memory-info (for 2.5)

[Qemu-devel] [RFC 0/3] qga: add guest-get-memory-info (for 2.5)

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This series implement a new qemu guest agent command to get memory
information from the guest. This is based on ovirt-guest-agent
"memory-stats" message.

I couldn't find documentation for the ovirt message, but the list of
fields are summarized in this test
https://github.com/oVirt/ovirt-guest-agent/blob/master/tests/message_validator.py#L137
and according to how they are populated in the code, I adapted it to
the following GuestMemoryInfo structure fields:

 - mem-total: Total usable RAM.
 - mem-free:  Total of RAM that can be used without having to swap contents to disk.
 - mem-cached: In-RAM cache.
 - swap-total: Total amount of swap space available.
 - swap-free: Amount of swap space that is currently unused.
 - swap-in: Number of pages swapped-in per second.
 - swap-out: Number of pages swapped-out per second.
 - pf-major: Number of major page fault per second.
 - pf-minor: Number of minor page fault per second.

Implemented on Linux and Win32 based on ovirt implementations.

Note: the "per second" value differ between Linux and Win32. On Linux,
the value is computed based on the average since the last query,
however on win32 this seems to be an instantaneous value (they have
spikes, but often at 0). I have asked for help on SF:

http://serverfault.com/questions/709943/windows-equivalent-of-linux-vmstat-pswpin-and-pgfault

Related to RFE:
https://bugzilla.redhat.com/show_bug.cgi?id=1101915

Marc-André Lureau (3):
  qga: add guest-get-memory-info json
  qga: implement get-memory-info for Linux
  qga: implement get-memory-info on win32

 configure            |   1 +
 qga/commands-posix.c | 102 +++++++++++++++++++++++++++++++
 qga/commands-win32.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/main.c           |  28 +++++++++
 qga/qapi-schema.json |  48 +++++++++++++++
 5 files changed, 345 insertions(+)

-- 
2.4.3

[Qemu-devel] [RFC 1/3] qga: add guest-get-memory-info json

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Define a new guest agent message, and document it.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/commands-posix.c | 12 ++++++++++++
 qga/commands-win32.c |  6 ++++++
 qga/qapi-schema.json | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 675f4b4..eb4036e 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2328,8 +2328,20 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp)
     return info;
 }
 
+GuestMemoryInfo *qmp_guest_get_memory_info(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
 #else /* defined(__linux__) */
 
+GuestMemoryInfo *qmp_guest_get_memory_info(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
 void qmp_guest_suspend_disk(Error **errp)
 {
     error_setg(errp, QERR_UNSUPPORTED);
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 77d3c92..bf9cd93 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1332,3 +1332,9 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
     }
     ga_command_state_add(cs, guest_file_init, NULL);
 }
+
+GuestMemoryInfo *qmp_guest_get_memory_info(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 6b0bd16..3767896 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -929,3 +929,51 @@
 ##
 { 'command': 'guest-get-memory-block-info',
   'returns': 'GuestMemoryBlockInfo' }
+
+##
+# @GuestMemoryInfo
+#
+# Information about guest memory, in kiB.
+#
+# @mem-total: Total usable RAM.
+#
+# @mem-free:  Total of RAM that can be used without having to swap contents to disk.
+#
+# @mem-cached: In-RAM cache.
+#
+# @swap-total: Total amount of swap space available.
+#
+# @swap-free: Amount of swap space that is currently unused.
+#
+# @swap-in: Number of pages swapped-in per second.
+#
+# @swap-out: Number of pages swapped-out per second.
+#
+# @pf-major: Number of major page fault per second.
+#
+# @pf-minor: Number of minor page fault per second.
+#
+# Since: 2.5
+##
+{ 'struct': 'GuestMemoryInfo',
+  'data': { 'mem-total': 'uint64',
+            'mem-free': 'uint64',
+            'mem-cached': 'uint64',
+            'swap-total': 'uint64',
+            'swap-free': 'uint64',
+            'swap-in': 'uint64',
+            'swap-out': 'uint64',
+            'pf-major': 'uint64',
+            'pf-minor': 'uint64' } }
+
+##
+# @guest-get-memory-info:
+#
+# Get guest memory information.
+#
+# Returns: @GuestMemInfo
+#
+# Since: 2.5
+##
+{ 'command': 'guest-get-memory-info',
+  'returns': 'GuestMemoryInfo' }
-- 
2.4.3

[Qemu-devel] [RFC 2/3] qga: implement get-memory-info for Linux

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/commands-posix.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 qga/main.c           | 28 ++++++++++++++++
 2 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index eb4036e..9534c2d 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -28,6 +28,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qemu/queue.h"
 #include "qemu/host-utils.h"
+#include "glib-compat.h"
 
 #ifndef CONFIG_HAS_ENVIRON
 #ifdef __APPLE__
@@ -2328,10 +2329,99 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp)
     return info;
 }
 
+static long meminfo_value(gchar * const *col)
+{
+    int i;
+
+    g_return_val_if_fail(col && col[0], 0);
+
+    for (i = 1; col[i]; i++) {
+        if (strlen(col[i]) > 0) {
+            return g_ascii_strtoll(col[i], NULL, 10);
+        }
+    }
+
+    g_return_val_if_reached(0);
+}
+
 GuestMemoryInfo *qmp_guest_get_memory_info(Error **errp)
 {
-    error_setg(errp, QERR_UNSUPPORTED);
-    return NULL;
+    static guint64 last_time, last_swap_in, last_swap_out;
+    static guint64 last_pf_major, last_pf_minor;
+    GError *err = NULL;
+    GuestMemoryInfo *info;
+    gchar *contents = NULL;
+    gchar **lines;
+    int i;
+    guint64 time;
+
+    if (!g_file_get_contents("/proc/meminfo", &contents, NULL, &err)) {
+        error_setg(errp, "unable to read meminfo: %s", err->message);
+        g_clear_error(&err);
+        return NULL;
+    }
+
+    info = g_new0(GuestMemoryInfo, 1);
+
+    lines = g_strsplit(contents, "\n", -1);
+    for (i = 0; lines[i]; i++) {
+        gchar **col = g_strsplit(lines[i], " ", -1);
+        if (g_strcmp0(col[0], "MemTotal:") == 0) {
+            info->mem_total = meminfo_value(col);
+        } else if (g_strcmp0(col[0], "MemAvailable:") == 0) {
+            /* available since kernel 3.2 */
+            info->mem_free = meminfo_value(col);
+        } else if (g_strcmp0(col[0], "Cached:") == 0) {
+            info->mem_cached = meminfo_value(col);
+        } else if (g_strcmp0(col[0], "SwapTotal:") == 0) {
+            info->swap_total = meminfo_value(col);
+        } else if (g_strcmp0(col[0], "SwapFree:") == 0) {
+            info->swap_free = meminfo_value(col);
+        }
+        g_strfreev(col);
+    }
+    g_strfreev(lines);
+
+    g_free(contents);
+
+    if (!g_file_get_contents("/proc/vmstat", &contents, NULL, &err)) {
+        error_setg(errp, "unable to read meminfo: %s", err->message);
+        g_clear_error(&err);
+        g_free(info);
+        return NULL;
+    }
+
+    time = g_get_monotonic_time();
+    double elapsed = (time - last_time + 1) / (double)G_USEC_PER_SEC;
+
+#define UPDATE(Field) do {                              \
+    guint64 val = g_ascii_strtoll(col[1], NULL, 10);    \
+    info->Field = (val - last_ ##Field) / elapsed;      \
+    last_ ##Field = val;                                \
+} while (0)
+
+    lines = g_strsplit(contents, "\n", -1);
+    for (i = 0; lines[i]; i++) {
+        gchar **col = g_strsplit(lines[i], " ", -1);
+        if (g_strcmp0(col[0], "pswpin") == 0) {
+            UPDATE(swap_in);
+        } else if (g_strcmp0(col[0], "pswpout") == 0) {
+            UPDATE(swap_out);
+        } else if (g_strcmp0(col[0], "pgfault") == 0) {
+            UPDATE(pf_minor);
+        } else if (g_strcmp0(col[0], "pgmajfault") == 0) {
+            UPDATE(pf_major);
+        }
+        g_strfreev(col);
+    }
+    g_strfreev(lines);
+
+#undef UPDATE
+
+    last_time = time;
+    g_free(contents);
+
+    return info;
 }
 
 #else /* defined(__linux__) */
diff --git a/qga/main.c b/qga/main.c
index aef007b..4790e26 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -42,6 +42,7 @@
 #define CONFIG_FSFREEZE
 #endif
 #endif
+#include "qga-qmp-commands.h"
 
 #ifndef _WIN32
 #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
@@ -901,6 +902,31 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp)
     return handle;
 }
 
+static void initialize_memory_stats(void)
+{
+    GuestMemoryInfo *info = qmp_guest_get_memory_info(NULL);
+
+    if (!info) {
+        return;
+    }
+
+    /* just for checking at start if everything looks ok */
+    g_debug("mem-total: %" G_GUINT64_FORMAT " kB\n"
+            "mem-free: %" G_GUINT64_FORMAT " kB\n"
+            "mem-cached: %" G_GUINT64_FORMAT " kB\n"
+            "swap-total: %" G_GUINT64_FORMAT " kB\n"
+            "swap-free: %" G_GUINT64_FORMAT " kB\n"
+            "swap-in: %" G_GUINT64_FORMAT " kB\n"
+            "swap-out: %" G_GUINT64_FORMAT " kB\n"
+            "pf-major: %" G_GUINT64_FORMAT " kB\n"
+            "pf-minor: %" G_GUINT64_FORMAT " kB\n",
+            info->mem_total, info->mem_free, info->mem_cached,
+            info->swap_total, info->swap_free, info->swap_in, info->swap_out,
+            info->pf_major, info->pf_minor);
+
+    g_free(info);
+}
+
 static void ga_print_cmd(QmpCommand *cmd, void *opaque)
 {
     printf("%s\n", qmp_command_name(cmd));
@@ -1256,6 +1282,8 @@ static int run_agent(GAState *s)
     }
 #endif
 
+    initialize_memory_stats();
+
     s->main_loop = g_main_loop_new(NULL, false);
     if (!channel_init(ga_state, method, device_path)) {
         g_critical("failed to initialize guest agent channel");
-- 
2.4.3

[Qemu-devel] [RFC 3/3] qga: implement get-memory-info on win32

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 configure            |   1 +
 qga/commands-win32.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 163 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index c5c4b82..82ee185 100755
--- a/configure
+++ b/configure
@@ -734,6 +734,7 @@ if test "$mingw32" = "yes" ; then
   local_statedir=
   confsuffix=""
   libs_qga="-lws2_32 -lwinmm -lpowrprof -liphlpapi -lnetapi32 $libs_qga"
+  libs_qga="-lole32 -loleaut32 -lwbemuuid -lpsapi $libs_qga"
 fi
 
 werror=""
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index bf9cd93..be11221 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -27,6 +27,8 @@
 #include <initguid.h>
 #endif
 #include <lm.h>
+#include <wbemcli.h>
+#include <psapi.h>
 
 #include "qga/guest-agent-core.h"
 #include "qga/vss-win32.h"
@@ -1333,8 +1335,166 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
     ga_command_state_add(cs, guest_file_init, NULL);
 }
 
+#define chk(msg)                                                        \
+    do {                                                                \
+        if (FAILED(hr)) {                                               \
+            gchar *emsg = g_win32_error_message(GetLastError());        \
+            slog("Failed to %s: %s", (msg), emsg);                      \
+            g_free(emsg);                                               \
+            goto out;                                                   \
+        }                                                               \
+    } while (0)
+
+static gboolean get_memory_wmi_info(GuestMemoryInfo *info)
+{
+    HRESULT hr = 0;
+    IWbemLocator *locator  = NULL;
+    IWbemServices *services = NULL;
+    IEnumWbemClassObject *results  = NULL;
+    BSTR resource, language, query_perf, query_swap;
+    gboolean success = FALSE;
+
+    g_return_val_if_fail(info != NULL, FALSE);
+
+    resource = SysAllocString(L"ROOT\\CIMV2");
+    language = SysAllocString(L"WQL");
+    query_perf = SysAllocString(L"SELECT * FROM "
+                                "Win32_PerfFormattedData_PerfOS_Memory");
+    query_swap = SysAllocString(L"SELECT * FROM "
+                                "Win32_PageFileUsage");
+
+    /* initialize COM */
+    hr = CoInitializeEx(0, COINIT_MULTITHREADED);
+    chk("initialize COM");
+
+    hr = CoInitializeSecurity(NULL, -1, NULL, NULL,
+                              RPC_C_AUTHN_LEVEL_DEFAULT,
+                              RPC_C_IMP_LEVEL_IMPERSONATE,
+                              NULL, EOAC_NONE, NULL);
+    chk("initialize COM security");
+
+    /* connect to WMI */
+    hr = CoCreateInstance(&CLSID_WbemLocator, 0, CLSCTX_INPROC_SERVER,
+                          &IID_IWbemLocator, (LPVOID *) &locator);
+    chk("create WbemLocator instance");
+
+    hr = locator->lpVtbl->ConnectServer(locator, resource, NULL, NULL, NULL, 0,
+                                        NULL, NULL, &services);
+    chk("connect to CIMV2 server");
+
+    /* issue a WMI perf query */
+    hr = services->lpVtbl->ExecQuery(services, language, query_perf,
+                                     WBEM_FLAG_BIDIRECTIONAL, NULL, &results);
+    chk("execute perf query");
+
+    if (results != NULL) {
+        IWbemClassObject *result = NULL;
+        ULONG returnedCount = 0;
+
+        /* enumerate the retrieved objects */
+        while ((hr = results->lpVtbl->Next(results, WBEM_INFINITE, 1,
+                                           &result, &returnedCount)) == S_OK) {
+            VARIANT pages_in, pages_out, pf, pfr;
+
+            hr = result->lpVtbl->Get(result, L"PagesInputPersec", 0,
+                                     &pages_in, 0, 0);
+            hr = result->lpVtbl->Get(result, L"PagesOutputPersec", 0,
+                                     &pages_out, 0, 0);
+            hr = result->lpVtbl->Get(result, L"PageFaultsPersec", 0,
+                                     &pf, 0, 0);
+            hr = result->lpVtbl->Get(result, L"PageReadsPersec", 0,
+                                     &pfr, 0, 0);
+
+            info->swap_in += pages_in.ulVal;
+            info->swap_out += pages_out.ulVal;
+            info->pf_minor += pf.ulVal - pfr.ulVal;
+            info->pf_major += pfr.ulVal;
+
+            result->lpVtbl->Release(result);
+        }
+
+        results->lpVtbl->Release(results);
+    }
+
+    /* issue a WMI swap query */
+    hr = services->lpVtbl->ExecQuery(services, language, query_swap,
+                                     WBEM_FLAG_BIDIRECTIONAL, NULL, &results);
+    chk("execute swap query");
+
+    if (results != NULL) {
+        IWbemClassObject *result = NULL;
+        ULONG returnedCount = 0;
+
+        /* enumerate the retrieved objects */
+        while ((hr = results->lpVtbl->Next(results, WBEM_INFINITE, 1,
+                                           &result, &returnedCount)) == S_OK) {
+            VARIANT usage, alloc;
+
+            hr = result->lpVtbl->Get(result, L"AllocatedBaseSize", 0,
+                                     &alloc, 0, 0);
+            hr = result->lpVtbl->Get(result, L"CurrentUsage", 0,
+                                     &usage, 0, 0);
+
+            /* MiB to kiB */
+            info->swap_total += alloc.ulVal * 1024;
+            info->swap_free += (alloc.ulVal - usage.ulVal) * 1024;
+
+            result->lpVtbl->Release(result);
+        }
+
+        results->lpVtbl->Release(results);
+    }
+
+    success = TRUE;
+
+out:
+    /* release WMI COM interfaces */
+    if (services) {
+        services->lpVtbl->Release(services);
+    }
+    if (locator) {
+        locator->lpVtbl->Release(locator);
+    }
+
+    /* unwind everything else we've allocated */
+    CoUninitialize();
+
+    if (query_perf) {
+        SysFreeString(query_perf);
+    }
+    if (query_swap) {
+        SysFreeString(query_swap);
+    }
+    if (language) {
+        SysFreeString(language);
+    }
+    if (resource) {
+        SysFreeString(resource);
+    }
+
+    return success;
+}
+
 GuestMemoryInfo *qmp_guest_get_memory_info(Error **errp)
 {
-    error_setg(errp, QERR_UNSUPPORTED);
-    return NULL;
+    GuestMemoryInfo *info = g_new0(GuestMemoryInfo, 1);
+    PERFORMANCE_INFORMATION perf = { .cb = sizeof(PERFORMANCE_INFORMATION) };
+
+    if (!get_memory_wmi_info(info)) {
+        error_setg(errp, "Failed to get WMI info");
+        g_free(info);
+        return NULL;
+    }
+
+    if (!GetPerformanceInfo(&perf, perf.cb)) {
+        error_setg(errp, "Failed to get performance info");
+        g_free(info);
+        return NULL;
+    }
+
+    info->mem_total = (perf.PhysicalTotal * perf.PageSize) / 1024;
+    info->mem_free = (perf.PhysicalAvailable * perf.PageSize) / 1024;
+    info->mem_cached = (perf.SystemCache * perf.PageSize) / 1024;
+
+    return info;
 }
-- 
2.4.3

Re: [Qemu-devel] [RFC 2/3] qga: implement get-memory-info for Linux

[Daniel P. Berrange]

On Fri, Jul 31, 2015 at 07:36:48PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qga/commands-posix.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  qga/main.c           | 28 ++++++++++++++++
>  2 files changed, 120 insertions(+), 2 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index eb4036e..9534c2d 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -28,6 +28,7 @@
>  #include "qapi/qmp/qerror.h"
>  #include "qemu/queue.h"
>  #include "qemu/host-utils.h"
> +#include "glib-compat.h"
>  
>  #ifndef CONFIG_HAS_ENVIRON
>  #ifdef __APPLE__
> @@ -2328,10 +2329,99 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp)
>      return info;
>  }
>  
> +static long meminfo_value(gchar * const *col)

Don't we need to use uint64 here not long, since we're assigning to
struct fields which are uint64 and on 32-bit host using long might
result in wraparound.

> +{
> +    int i;
> +
> +    g_return_val_if_fail(col && col[0], 0);
> +
> +    for (i = 1; col[i]; i++) {
> +        if (strlen(col[i]) > 0) {
> +            return g_ascii_strtoll(col[i], NULL, 10);
> +        }
> +    }
> +
> +    g_return_val_if_reached(0);
> +}
> +
>  GuestMemoryInfo *qmp_guest_get_memory_info(Error **errp)
>  {
> -    error_setg(errp, QERR_UNSUPPORTED);
> -    return NULL;
> +    static guint64 last_time, last_swap_in, last_swap_out;
> +    static guint64 last_pf_major, last_pf_minor;
> +    GError *err = NULL;
> +    GuestMemoryInfo *info;
> +    gchar *contents = NULL;
> +    gchar **lines;
> +    int i;
> +    guint64 time;
> +
> +    if (!g_file_get_contents("/proc/meminfo", &contents, NULL, &err)) {
> +        error_setg(errp, "unable to read meminfo: %s", err->message);
> +        g_clear_error(&err);
> +        return NULL;
> +    }
> +
> +    info = g_new0(GuestMemoryInfo, 1);
> +
> +    lines = g_strsplit(contents, "\n", -1);
> +    for (i = 0; lines[i]; i++) {
> +        gchar **col = g_strsplit(lines[i], " ", -1);
> +        if (g_strcmp0(col[0], "MemTotal:") == 0) {
> +            info->mem_total = meminfo_value(col);
> +        } else if (g_strcmp0(col[0], "MemAvailable:") == 0) {
> +            /* available since kernel 3.2 */
> +            info->mem_free = meminfo_value(col);
> +        } else if (g_strcmp0(col[0], "Cached:") == 0) {
> +            info->mem_cached = meminfo_value(col);
> +        } else if (g_strcmp0(col[0], "SwapTotal:") == 0) {
> +            info->swap_total = meminfo_value(col);
> +        } else if (g_strcmp0(col[0], "SwapFree:") == 0) {
> +            info->swap_free = meminfo_value(col);
> +        }
> +        g_strfreev(col);
> +    }
> +    g_strfreev(lines);
> +
> +    g_free(contents);
> +
> +    if (!g_file_get_contents("/proc/vmstat", &contents, NULL, &err)) {
> +        error_setg(errp, "unable to read meminfo: %s", err->message);
> +        g_clear_error(&err);
> +        g_free(info);
> +        return NULL;
> +    }
> +
> +    time = g_get_monotonic_time();
> +    double elapsed = (time - last_time + 1) / (double)G_USEC_PER_SEC;
> +
> +#define UPDATE(Field) do {                              \
> +    guint64 val = g_ascii_strtoll(col[1], NULL, 10);    \
> +    info->Field = (val - last_ ##Field) / elapsed;      \
> +    last_ ##Field = val;                                \
> +} while (0)
> +
> +    lines = g_strsplit(contents, "\n", -1);
> +    for (i = 0; lines[i]; i++) {
> +        gchar **col = g_strsplit(lines[i], " ", -1);
> +        if (g_strcmp0(col[0], "pswpin") == 0) {
> +            UPDATE(swap_in);
> +        } else if (g_strcmp0(col[0], "pswpout") == 0) {
> +            UPDATE(swap_out);
> +        } else if (g_strcmp0(col[0], "pgfault") == 0) {
> +            UPDATE(pf_minor);
> +        } else if (g_strcmp0(col[0], "pgmajfault") == 0) {
> +            UPDATE(pf_major);
> +        }
> +        g_strfreev(col);
> +    }
> +    g_strfreev(lines);
> +
> +#undef UPDATE
> +
> +    last_time = time;
> +    g_free(contents);
> +
> +    return info;
>  }
>  
>  #else /* defined(__linux__) */
> diff --git a/qga/main.c b/qga/main.c
> index aef007b..4790e26 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -42,6 +42,7 @@
>  #define CONFIG_FSFREEZE
>  #endif
>  #endif
> +#include "qga-qmp-commands.h"
>  
>  #ifndef _WIN32
>  #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
> @@ -901,6 +902,31 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp)
>      return handle;
>  }
>  
> +static void initialize_memory_stats(void)
> +{
> +    GuestMemoryInfo *info = qmp_guest_get_memory_info(NULL);
> +
> +    if (!info) {
> +        return;
> +    }
> +
> +    /* just for checking at start if everything looks ok */
> +    g_debug("mem-total: %" G_GUINT64_FORMAT " kB\n"
> +            "mem-free: %" G_GUINT64_FORMAT " kB\n"
> +            "mem-cached: %" G_GUINT64_FORMAT " kB\n"
> +            "swap-total: %" G_GUINT64_FORMAT " kB\n"
> +            "swap-free: %" G_GUINT64_FORMAT " kB\n"
> +            "swap-in: %" G_GUINT64_FORMAT " kB\n"
> +            "swap-out: %" G_GUINT64_FORMAT " kB\n"
> +            "pf-major: %" G_GUINT64_FORMAT " kB\n"
> +            "pf-minor: %" G_GUINT64_FORMAT " kB\n",
> +            info->mem_total, info->mem_free, info->mem_cached,
> +            info->swap_total, info->swap_free, info->swap_in, info->swap_out,
> +            info->pf_major, info->pf_minor);
> +
> +    g_free(info);
> +}

Wouldn't we be better off just adding a proper unit test for
the code. We could copy a few same /proc/meminfo commands from
various different Linux architectures and versions and then
test parsing of them.

> +
>  static void ga_print_cmd(QmpCommand *cmd, void *opaque)
>  {
>      printf("%s\n", qmp_command_name(cmd));
> @@ -1256,6 +1282,8 @@ static int run_agent(GAState *s)
>      }
>  #endif
>  
> +    initialize_memory_stats();
> +
>      s->main_loop = g_main_loop_new(NULL, false);
>      if (!channel_init(ga_state, method, device_path)) {
>          g_critical("failed to initialize guest agent channel");


Regards,
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 0/3] qga: add guest-get-memory-info (for 2.5)

[Daniel P. Berrange]

On Fri, Jul 31, 2015 at 07:36:46PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This series implement a new qemu guest agent command to get memory
> information from the guest. This is based on ovirt-guest-agent
> "memory-stats" message.
> 
> I couldn't find documentation for the ovirt message, but the list of
> fields are summarized in this test
> https://github.com/oVirt/ovirt-guest-agent/blob/master/tests/message_validator.py#L137
> and according to how they are populated in the code, I adapted it to
> the following GuestMemoryInfo structure fields:
> 
>  - mem-total: Total usable RAM.
>  - mem-free:  Total of RAM that can be used without having to swap contents to disk.
>  - mem-cached: In-RAM cache.
>  - swap-total: Total amount of swap space available.
>  - swap-free: Amount of swap space that is currently unused.
>  - swap-in: Number of pages swapped-in per second.
>  - swap-out: Number of pages swapped-out per second.
>  - pf-major: Number of major page fault per second.
>  - pf-minor: Number of minor page fault per second.
> 
> Implemented on Linux and Win32 based on ovirt implementations.

Interesting, currently the libvirt virDomainGetMemoryStats() API is backed
by data we obtain from the virtio-balloon driver via QEMU monitor. For Linux
at least this provides equiv of your mem-total, mem-free, swap-in, swap-out
pf-major and pf-minor. So it lacks mem-cached, swap-total and swap-free
I'm unclear in the Windows balloon driver supports this data or not too.

> Note: the "per second" value differ between Linux and Win32. On Linux,
> the value is computed based on the average since the last query,
> however on win32 this seems to be an instantaneous value (they have
> spikes, but often at 0). I have asked for help on SF:
> 
> http://serverfault.com/questions/709943/windows-equivalent-of-linux-vmstat-pswpin-and-pgfault
> 
> Related to RFE:
> https://bugzilla.redhat.com/show_bug.cgi?id=1101915

I wonder if we would be better off extending the balloon driver to fill
in the gaps we have there vs this guest agent impl. With the balloon
agent we set things up so that the guest device periodically pushes the
updated stats to QEMU. So when we're querying QEMU for the stats we don't
actually block waiting on the guest OS at all, QEMU can answer directly.
This feels more appealing that querying the guest agent where we have
no reasonable expectation of prompt response. With a large enough number
of guests, I think the balloon driver push approach will scale better
than a guest agent approach.

Regards,
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 2/3] qga: implement get-memory-info for Linux

[Marc-André Lureau]

----- Original Message -----
> On Fri, Jul 31, 2015 at 07:36:48PM +0200, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  qga/commands-posix.c | 94
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  qga/main.c           | 28 ++++++++++++++++
> >  2 files changed, 120 insertions(+), 2 deletions(-)
> > 
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index eb4036e..9534c2d 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -28,6 +28,7 @@
> >  #include "qapi/qmp/qerror.h"
> >  #include "qemu/queue.h"
> >  #include "qemu/host-utils.h"
> > +#include "glib-compat.h"
> >  
> >  #ifndef CONFIG_HAS_ENVIRON
> >  #ifdef __APPLE__
> > @@ -2328,10 +2329,99 @@ GuestMemoryBlockInfo
> > *qmp_guest_get_memory_block_info(Error **errp)
> >      return info;
> >  }
> >  
> > +static long meminfo_value(gchar * const *col)
> 
> Don't we need to use uint64 here not long, since we're assigning to
> struct fields which are uint64 and on 32-bit host using long might
> result in wraparound.

Yes, thanks

> 
> > +{
> > +    int i;
> > +
> > +    g_return_val_if_fail(col && col[0], 0);
> > +
> > +    for (i = 1; col[i]; i++) {
> > +        if (strlen(col[i]) > 0) {
> > +            return g_ascii_strtoll(col[i], NULL, 10);
> > +        }
> > +    }
> > +
> > +    g_return_val_if_reached(0);
> > +}
> > +
> >  GuestMemoryInfo *qmp_guest_get_memory_info(Error **errp)
> >  {
> > -    error_setg(errp, QERR_UNSUPPORTED);
> > -    return NULL;
> > +    static guint64 last_time, last_swap_in, last_swap_out;
> > +    static guint64 last_pf_major, last_pf_minor;
> > +    GError *err = NULL;
> > +    GuestMemoryInfo *info;
> > +    gchar *contents = NULL;
> > +    gchar **lines;
> > +    int i;
> > +    guint64 time;
> > +
> > +    if (!g_file_get_contents("/proc/meminfo", &contents, NULL, &err)) {
> > +        error_setg(errp, "unable to read meminfo: %s", err->message);
> > +        g_clear_error(&err);
> > +        return NULL;
> > +    }
> > +
> > +    info = g_new0(GuestMemoryInfo, 1);
> > +
> > +    lines = g_strsplit(contents, "\n", -1);
> > +    for (i = 0; lines[i]; i++) {
> > +        gchar **col = g_strsplit(lines[i], " ", -1);
> > +        if (g_strcmp0(col[0], "MemTotal:") == 0) {
> > +            info->mem_total = meminfo_value(col);
> > +        } else if (g_strcmp0(col[0], "MemAvailable:") == 0) {
> > +            /* available since kernel 3.2 */
> > +            info->mem_free = meminfo_value(col);
> > +        } else if (g_strcmp0(col[0], "Cached:") == 0) {
> > +            info->mem_cached = meminfo_value(col);
> > +        } else if (g_strcmp0(col[0], "SwapTotal:") == 0) {
> > +            info->swap_total = meminfo_value(col);
> > +        } else if (g_strcmp0(col[0], "SwapFree:") == 0) {
> > +            info->swap_free = meminfo_value(col);
> > +        }
> > +        g_strfreev(col);
> > +    }
> > +    g_strfreev(lines);
> > +
> > +    g_free(contents);
> > +
> > +    if (!g_file_get_contents("/proc/vmstat", &contents, NULL, &err)) {
> > +        error_setg(errp, "unable to read meminfo: %s", err->message);
> > +        g_clear_error(&err);
> > +        g_free(info);
> > +        return NULL;
> > +    }
> > +
> > +    time = g_get_monotonic_time();
> > +    double elapsed = (time - last_time + 1) / (double)G_USEC_PER_SEC;
> > +
> > +#define UPDATE(Field) do {                              \
> > +    guint64 val = g_ascii_strtoll(col[1], NULL, 10);    \
> > +    info->Field = (val - last_ ##Field) / elapsed;      \
> > +    last_ ##Field = val;                                \
> > +} while (0)
> > +
> > +    lines = g_strsplit(contents, "\n", -1);
> > +    for (i = 0; lines[i]; i++) {
> > +        gchar **col = g_strsplit(lines[i], " ", -1);
> > +        if (g_strcmp0(col[0], "pswpin") == 0) {
> > +            UPDATE(swap_in);
> > +        } else if (g_strcmp0(col[0], "pswpout") == 0) {
> > +            UPDATE(swap_out);
> > +        } else if (g_strcmp0(col[0], "pgfault") == 0) {
> > +            UPDATE(pf_minor);
> > +        } else if (g_strcmp0(col[0], "pgmajfault") == 0) {
> > +            UPDATE(pf_major);
> > +        }
> > +        g_strfreev(col);
> > +    }
> > +    g_strfreev(lines);
> > +
> > +#undef UPDATE
> > +
> > +    last_time = time;
> > +    g_free(contents);
> > +
> > +    return info;
> >  }
> >  
> >  #else /* defined(__linux__) */
> > diff --git a/qga/main.c b/qga/main.c
> > index aef007b..4790e26 100644
> > --- a/qga/main.c
> > +++ b/qga/main.c
> > @@ -42,6 +42,7 @@
> >  #define CONFIG_FSFREEZE
> >  #endif
> >  #endif
> > +#include "qga-qmp-commands.h"
> >  
> >  #ifndef _WIN32
> >  #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
> > @@ -901,6 +902,31 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp)
> >      return handle;
> >  }
> >  
> > +static void initialize_memory_stats(void)
> > +{
> > +    GuestMemoryInfo *info = qmp_guest_get_memory_info(NULL);
> > +
> > +    if (!info) {
> > +        return;
> > +    }
> > +
> > +    /* just for checking at start if everything looks ok */
> > +    g_debug("mem-total: %" G_GUINT64_FORMAT " kB\n"
> > +            "mem-free: %" G_GUINT64_FORMAT " kB\n"
> > +            "mem-cached: %" G_GUINT64_FORMAT " kB\n"
> > +            "swap-total: %" G_GUINT64_FORMAT " kB\n"
> > +            "swap-free: %" G_GUINT64_FORMAT " kB\n"
> > +            "swap-in: %" G_GUINT64_FORMAT " kB\n"
> > +            "swap-out: %" G_GUINT64_FORMAT " kB\n"
> > +            "pf-major: %" G_GUINT64_FORMAT " kB\n"
> > +            "pf-minor: %" G_GUINT64_FORMAT " kB\n",
> > +            info->mem_total, info->mem_free, info->mem_cached,
> > +            info->swap_total, info->swap_free, info->swap_in,
> > info->swap_out,
> > +            info->pf_major, info->pf_minor);
> > +
> > +    g_free(info);
> > +}
> 
> Wouldn't we be better off just adding a proper unit test for
> the code. We could copy a few same /proc/meminfo commands from
> various different Linux architectures and versions and then
> test parsing of them.

Sure, but a debug dump is still useful, doesn't hurt much. It doesn' look like there are unit tests for qga. It'd something worth to look at.

> > +
> >  static void ga_print_cmd(QmpCommand *cmd, void *opaque)
> >  {
> >      printf("%s\n", qmp_command_name(cmd));
> > @@ -1256,6 +1282,8 @@ static int run_agent(GAState *s)
> >      }
> >  #endif
> >  
> > +    initialize_memory_stats();
> > +
> >      s->main_loop = g_main_loop_new(NULL, false);
> >      if (!channel_init(ga_state, method, device_path)) {
> >          g_critical("failed to initialize guest agent channel");
> 
> 
> Regards,
> 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 0/3] qga: add guest-get-memory-info (for 2.5)

[Marc-André Lureau]

Hi

----- Original Message -----
> On Fri, Jul 31, 2015 at 07:36:46PM +0200, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > 
> > This series implement a new qemu guest agent command to get memory
> > information from the guest. This is based on ovirt-guest-agent
> > "memory-stats" message.
> > 
> > I couldn't find documentation for the ovirt message, but the list of
> > fields are summarized in this test
> > https://github.com/oVirt/ovirt-guest-agent/blob/master/tests/message_validator.py#L137
> > and according to how they are populated in the code, I adapted it to
> > the following GuestMemoryInfo structure fields:
> > 
> >  - mem-total: Total usable RAM.
> >  - mem-free:  Total of RAM that can be used without having to swap contents
> >  to disk.
> >  - mem-cached: In-RAM cache.
> >  - swap-total: Total amount of swap space available.
> >  - swap-free: Amount of swap space that is currently unused.
> >  - swap-in: Number of pages swapped-in per second.
> >  - swap-out: Number of pages swapped-out per second.
> >  - pf-major: Number of major page fault per second.
> >  - pf-minor: Number of minor page fault per second.
> > 
> > Implemented on Linux and Win32 based on ovirt implementations.
> 
> Interesting, currently the libvirt virDomainGetMemoryStats() API is backed
> by data we obtain from the virtio-balloon driver via QEMU monitor. For Linux
> at least this provides equiv of your mem-total, mem-free, swap-in, swap-out
> pf-major and pf-minor. So it lacks mem-cached, swap-total and swap-free
> I'm unclear in the Windows balloon driver supports this data or not too.

Ah, I should have thought about it! However, many guests don't have a balloon driver. So it could still be useful as a fallback imho

> > Note: the "per second" value differ between Linux and Win32. On Linux,
> > the value is computed based on the average since the last query,
> > however on win32 this seems to be an instantaneous value (they have
> > spikes, but often at 0). I have asked for help on SF:
> > 
> > http://serverfault.com/questions/709943/windows-equivalent-of-linux-vmstat-pswpin-and-pgfault
> > 
> > Related to RFE:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1101915
> 
> I wonder if we would be better off extending the balloon driver to fill
> in the gaps we have there vs this guest agent impl. With the balloon
> agent we set things up so that the guest device periodically pushes the
> updated stats to QEMU. So when we're querying QEMU for the stats we don't
> actually block waiting on the guest OS at all, QEMU can answer directly.
> This feels more appealing that querying the guest agent where we have
> no reasonable expectation of prompt response. With a large enough number
> of guests, I think the balloon driver push approach will scale better
> than a guest agent approach.

I see, I think it really depends on the use case. Clearly for debugging, it doesn't need to be push-based. For load-balancing, I don't know either.

Please Jiri give some details on how these fields are used by ovirt. Thanks