[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 2/3] qga: implement get-memory-info for Linux
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [RFC 2/3] qga: implement get-memory-info for Linux |
Date: |
Fri, 31 Jul 2015 14:02:36 -0400 (EDT) |
----- Original Message -----
> On Fri, Jul 31, 2015 at 07:36:48PM +0200, address@hidden wrote:
> > From: Marc-André Lureau <address@hidden>
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > 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 :|
>