| Message ID | 1438364209-24940-3-git-send-email-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
----- 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 :| >
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");