All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue
@ 2015-10-17 16:59 Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 01/13] build: qemu-ga: add 'qemu-ga' build target for w32 Michael Roth
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Hi Peter,

Please note that 'glib-compat: add 2.38/2.40/2.46 asserts' is also in
Marc-André's recent ivshmem PULL. The 2 versions of the patches are identical,
but let me know if you'd prefer a re-send/re-base later.

The following changes since commit 6d57410a79d51d92673c54f26624b44f27fa6214:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20151016' into staging (2015-10-17 12:31:33 +0100)

are available in the git repository at:


  git://github.com/mdroth/qemu.git tags/qga-pull-2015-10-14-v3-tag

for you to fetch changes up to ed9f1986d19c2d21667a14b875b2ac8b8a19b8a5:

  qga: fix uninitialized value warning for win32 (2015-10-17 11:24:27 -0500)

----------------------------------------------------------------
qemu-ga patch queue

* add unit tests for qemu-ga
* add guest-exec support for posix/w32 guests
* added 'qemu-ga' target for w32. this allows us to do full MSI build,
  without overloading 'qemu-ga.exe' target with uneeded dependencies.
* number of s/g_new/g_malloc/ conversions for qga

v2:
* commit message and qapi documentation spelling fixes
* rename 'inp-data' guest-exec param to 'input-data'

v3:
* fix OSX build errors for test-qga by using PRId64
  format in place of glib's, and dropping use of G_SPAWN_DEFAULT
  macro for glib 2.22 compat
* fix win32 build warnings for 32-bit builds by avoid int casts
  of process HANDLEs

----------------------------------------------------------------
Denis V. Lunev (2):
      qga: drop guest_file_init helper and replace it with static initializers
      qga: handle possible SIGPIPE in guest-file-write

Marc-André Lureau (5):
      qga: add QGA_CONF environment variable
      qga: do not override configuration verbosity
      qtest: add a few fd-level qmp helpers
      glib-compat: add 2.38/2.40/2.46 asserts
      tests: add a local test for guest agent

Markus Armbruster (1):
      qga: Use g_new() & friends where that makes obvious sense

Michael Roth (2):
      build: qemu-ga: add 'qemu-ga' build target for w32
      qga: fix uninitialized value warning for win32

Yuri Pudgorodskiy (3):
      qga: guest exec functionality
      qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()
      qga: guest-exec simple stdin/stdout/stderr redirection

 Makefile                        |  14 +-
 configure                       |   2 +-
 include/glib-compat.h           |  61 ++++
 qga/channel-posix.c             |  25 +-
 qga/channel-win32.c             |   4 +-
 qga/commands-posix.c            |  20 +-
 qga/commands-win32.c            |  20 +-
 qga/commands.c                  | 394 +++++++++++++++++++-
 qga/guest-agent-command-state.c |   4 +-
 qga/main.c                      |  13 +-
 qga/qapi-schema.json            |  67 ++++
 tests/Makefile                  |   3 +
 tests/libqtest.c                |  45 ++-
 tests/libqtest.h                |   7 +
 tests/test-qga.c                | 775 ++++++++++++++++++++++++++++++++++++++++
 15 files changed, 1396 insertions(+), 58 deletions(-)
 create mode 100644 tests/test-qga.c

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 01/13] build: qemu-ga: add 'qemu-ga' build target for w32
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 02/13] qga: Use g_new() & friends where that makes obvious sense Michael Roth
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, Michael Roth, Marc-André Lureau, Paolo Bonzini

Currently POSIX builds rely on 'qemu-ga' target to do qga-only
distributable build. On w32, as with most standalone binary targets,
we rely on 'qemu-ga.exe' target.

Unlike with POSIX, qemu-ga for w32 has a number of related targets
such as VSS DLL and MSI package. We can do the full distributable
qga-only build on w32 with:

  make qemu-ga.exe

or:

  make msi

To make that work, we tie VSS dependencies onto qemu-ga.exe.
However, in reality the DLL isn't part of the binary, so we use a
filter to pull them out of the LINK recipe, which attempts to link
against prereqs for binary targets. Additionally, it could be argued
that VSS is a separate distributable, and shouldn't be implied by
qemu-ga.exe binary target.

To avoid this, we can tie the VSS dependencies only to the 'msi'
target, but that would make it impossible to do a qga-only build of
the w32 distributable without building the 'msi' package, which was
supported in the past.

An alternative approach is to add a new target to build the whole
distributable. w32 allows us to use the same build target we use
on POSIX, 'qemu-ga', since the current binary-only target on w32
is 'qemu-ga.exe'.

To further simplify the build, we also make 'qemu-ga' build the MSI
package if the appropriate ./configure options are set, making the
full qga-only build the same on both POSIX and w32: `make qemu-ga`

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile  | 14 ++++++++------
 configure |  2 +-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index e370876..cbf252d 100644
--- a/Makefile
+++ b/Makefile
@@ -298,18 +298,15 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py)
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 
-# we require QGA_VSS_PROVIDER files to be built alongside qemu-ga
-# executable since they are shipped together, but we don't want to actually
-# link against them
-qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER)
-	$(call LINK, $(filter-out $(QGA_VSS_PROVIDER), $^))
+qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
+	$(call LINK, $^)
 
 ifdef QEMU_GA_MSI_ENABLED
 QEMU_GA_MSI=qemu-ga-$(ARCH).msi
 
 msi: $(QEMU_GA_MSI)
 
-$(QEMU_GA_MSI): qemu-ga.exe
+$(QEMU_GA_MSI): qemu-ga.exe $(QGA_VSS_PROVIDER)
 
 $(QEMU_GA_MSI): config-host.mak
 
@@ -321,6 +318,11 @@ msi:
 	@echo "MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)"
 endif
 
+ifneq ($(EXESUF),)
+.PHONY: qemu-ga
+qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI)
+endif
+
 clean:
 # avoid old build problems by removing potentially incorrect old files
 	rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
diff --git a/configure b/configure
index f08327e..cc62c38 100755
--- a/configure
+++ b/configure
@@ -4441,7 +4441,7 @@ fi
 
 if [ "$guest_agent" != "no" ]; then
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
-      tools="qemu-ga\$(EXESUF) $tools"
+      tools="qemu-ga $tools"
       guest_agent=yes
   elif [ "$guest_agent" != yes ]; then
       guest_agent=no
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 02/13] qga: Use g_new() & friends where that makes obvious sense
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 01/13] build: qemu-ga: add 'qemu-ga' build target for w32 Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 03/13] qga: add QGA_CONF environment variable Michael Roth
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Markus Armbruster, Michael Roth

From: Markus Armbruster <armbru@redhat.com>

g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
for two reasons.  One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.

This commit only touches allocations with size arguments of the form
sizeof(T).  Same Coccinelle semantic patch as in commit b45c03f.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/channel-posix.c             |  2 +-
 qga/channel-win32.c             |  2 +-
 qga/commands-posix.c            | 10 +++++-----
 qga/commands-win32.c            | 10 +++++-----
 qga/commands.c                  |  6 +++---
 qga/guest-agent-command-state.c |  4 ++--
 6 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 8aad4fe..61aa3cb 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -249,7 +249,7 @@ GIOStatus ga_channel_read(GAChannel *c, gchar *buf, gsize size, gsize *count)
 GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
                           GAChannelCallback cb, gpointer opaque)
 {
-    GAChannel *c = g_malloc0(sizeof(GAChannel));
+    GAChannel *c = g_new0(GAChannel, 1);
     c->event_cb = cb;
     c->user_data = opaque;
 
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 04fa5e4..215b15a 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -322,7 +322,7 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
 GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
                           GAChannelCallback cb, gpointer opaque)
 {
-    GAChannel *c = g_malloc0(sizeof(GAChannel));
+    GAChannel *c = g_new0(GAChannel, 1);
     SECURITY_ATTRIBUTES sec_attrs;
 
     if (!ga_channel_open(c, method, path)) {
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index b03c316..a932809 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -235,7 +235,7 @@ static int64_t guest_file_handle_add(FILE *fh, Error **errp)
         return -1;
     }
 
-    gfh = g_malloc0(sizeof(GuestFileHandle));
+    gfh = g_new0(GuestFileHandle, 1);
     gfh->id = handle;
     gfh->fh = fh;
     QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
@@ -488,7 +488,7 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
         slog("guest-file-read failed, handle: %" PRId64, handle);
     } else {
         buf[read_count] = 0;
-        read_data = g_malloc0(sizeof(GuestFileRead));
+        read_data = g_new0(GuestFileRead, 1);
         read_data->count = read_count;
         read_data->eof = feof(fh);
         if (read_count) {
@@ -533,7 +533,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
         error_setg_errno(errp, errno, "failed to write to file");
         slog("guest-file-write failed, handle: %" PRId64, handle);
     } else {
-        write_data = g_malloc0(sizeof(GuestFileWrite));
+        write_data = g_new0(GuestFileWrite, 1);
         write_data->count = write_count;
         write_data->eof = feof(fh);
     }
@@ -678,7 +678,7 @@ static void build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
             continue;
         }
 
-        mount = g_malloc0(sizeof(FsMount));
+        mount = g_new0(FsMount, 1);
         mount->dirname = g_strdup(ment->mnt_dir);
         mount->devtype = g_strdup(ment->mnt_type);
         mount->devmajor = devmajor;
@@ -757,7 +757,7 @@ static void build_fs_mount_list(FsMountList *mounts, Error **errp)
             }
         }
 
-        mount = g_malloc0(sizeof(FsMount));
+        mount = g_new0(FsMount, 1);
         mount->dirname = g_strdup(line + dir_s);
         mount->devtype = g_strdup(dash + type_s);
         mount->devmajor = devmajor;
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 41bdd3f..0481646 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -106,7 +106,7 @@ static int64_t guest_file_handle_add(HANDLE fh, Error **errp)
     if (handle < 0) {
         return -1;
     }
-    gfh = g_malloc0(sizeof(GuestFileHandle));
+    gfh = g_new0(GuestFileHandle, 1);
     gfh->id = handle;
     gfh->fh = fh;
     QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
@@ -298,7 +298,7 @@ GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
         slog("guest-file-read failed, handle %" PRId64, handle);
     } else {
         buf[read_count] = 0;
-        read_data = g_malloc0(sizeof(GuestFileRead));
+        read_data = g_new0(GuestFileRead, 1);
         read_data->count = (size_t)read_count;
         read_data->eof = read_count == 0;
 
@@ -342,7 +342,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
         error_setg_win32(errp, GetLastError(), "failed to write to file");
         slog("guest-file-write-failed, handle: %" PRId64, handle);
     } else {
-        write_data = g_malloc0(sizeof(GuestFileWrite));
+        write_data = g_new0(GuestFileWrite, 1);
         write_data->count = (size_t) write_count;
     }
 
@@ -865,7 +865,7 @@ static DWORD WINAPI do_suspend(LPVOID opaque)
 void qmp_guest_suspend_disk(Error **errp)
 {
     Error *local_err = NULL;
-    GuestSuspendMode *mode = g_malloc(sizeof(GuestSuspendMode));
+    GuestSuspendMode *mode = g_new(GuestSuspendMode, 1);
 
     *mode = GUEST_SUSPEND_MODE_DISK;
     check_suspend_mode(*mode, &local_err);
@@ -881,7 +881,7 @@ void qmp_guest_suspend_disk(Error **errp)
 void qmp_guest_suspend_ram(Error **errp)
 {
     Error *local_err = NULL;
-    GuestSuspendMode *mode = g_malloc(sizeof(GuestSuspendMode));
+    GuestSuspendMode *mode = g_new(GuestSuspendMode, 1);
 
     *mode = GUEST_SUSPEND_MODE_RAM;
     check_suspend_mode(*mode, &local_err);
diff --git a/qga/commands.c b/qga/commands.c
index 7834967..9a765e8 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -51,12 +51,12 @@ static void qmp_command_info(QmpCommand *cmd, void *opaque)
     GuestAgentCommandInfo *cmd_info;
     GuestAgentCommandInfoList *cmd_info_list;
 
-    cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
+    cmd_info = g_new0(GuestAgentCommandInfo, 1);
     cmd_info->name = g_strdup(qmp_command_name(cmd));
     cmd_info->enabled = qmp_command_is_enabled(cmd);
     cmd_info->success_response = qmp_has_success_response(cmd);
 
-    cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
+    cmd_info_list = g_new0(GuestAgentCommandInfoList, 1);
     cmd_info_list->value = cmd_info;
     cmd_info_list->next = info->supported_commands;
     info->supported_commands = cmd_info_list;
@@ -64,7 +64,7 @@ static void qmp_command_info(QmpCommand *cmd, void *opaque)
 
 struct GuestAgentInfo *qmp_guest_info(Error **errp)
 {
-    GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
+    GuestAgentInfo *info = g_new0(GuestAgentInfo, 1);
 
     info->version = g_strdup(QEMU_VERSION);
     qmp_for_each_command(qmp_command_info, info);
diff --git a/qga/guest-agent-command-state.c b/qga/guest-agent-command-state.c
index 969da23..128c549 100644
--- a/qga/guest-agent-command-state.c
+++ b/qga/guest-agent-command-state.c
@@ -27,7 +27,7 @@ void ga_command_state_add(GACommandState *cs,
                           void (*init)(void),
                           void (*cleanup)(void))
 {
-    GACommandGroup *cg = g_malloc0(sizeof(GACommandGroup));
+    GACommandGroup *cg = g_new0(GACommandGroup, 1);
     cg->init = init;
     cg->cleanup = cleanup;
     cs->groups = g_slist_append(cs->groups, cg);
@@ -67,7 +67,7 @@ void ga_command_state_cleanup_all(GACommandState *cs)
 
 GACommandState *ga_command_state_new(void)
 {
-    GACommandState *cs = g_malloc0(sizeof(GACommandState));
+    GACommandState *cs = g_new0(GACommandState, 1);
     cs->groups = NULL;
     return cs;
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 03/13] qga: add QGA_CONF environment variable
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 01/13] build: qemu-ga: add 'qemu-ga' build target for w32 Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 02/13] qga: Use g_new() & friends where that makes obvious sense Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 04/13] qga: do not override configuration verbosity Michael Roth
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Michael Roth, Marc-André Lureau

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

Having a environment variable allows to override default configuration
path, useful for testing. Note that this can't easily be an argument,
since loading config is done before parsing the arguments.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index d8e063a..18e1e1d 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -945,10 +945,11 @@ static void config_load(GAConfig *config)
 {
     GError *gerr = NULL;
     GKeyFile *keyfile;
+    const char *conf = g_getenv("QGA_CONF") ?: QGA_CONF_DEFAULT;
 
     /* read system config */
     keyfile = g_key_file_new();
-    if (!g_key_file_load_from_file(keyfile, QGA_CONF_DEFAULT, 0, &gerr)) {
+    if (!g_key_file_load_from_file(keyfile, conf, 0, &gerr)) {
         goto end;
     }
     if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 04/13] qga: do not override configuration verbosity
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (2 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 03/13] qga: add QGA_CONF environment variable Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 05/13] qtest: add a few fd-level qmp helpers Michael Roth
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Michael Roth, Marc-André Lureau

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

Move the default verbosity settings before loading the configuration
file, or it will overwrite it. Found thanks to writing qga tests :)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 18e1e1d..aa6a063 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1083,8 +1083,6 @@ static void config_parse(GAConfig *config, int argc, char **argv)
         { NULL, 0, NULL, 0 }
     };
 
-    config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
-
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
         case 'm':
@@ -1332,6 +1330,8 @@ int main(int argc, char **argv)
     GAState *s = g_new0(GAState, 1);
     GAConfig *config = g_new0(GAConfig, 1);
 
+    config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
+
     module_call_init(MODULE_INIT_QAPI);
 
     init_dfl_pathnames();
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 05/13] qtest: add a few fd-level qmp helpers
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (3 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 04/13] qga: do not override configuration verbosity Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 06/13] glib-compat: add 2.38/2.40/2.46 asserts Michael Roth
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Michael Roth, Marc-André Lureau

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

Add a few functions to interact with qmp via a simple fd.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 tests/libqtest.c | 45 +++++++++++++++++++++++++++++++++++++++++----
 tests/libqtest.h |  7 +++++++
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 2a396ba..b6d700c 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -356,7 +356,7 @@ static void qmp_response(JSONMessageParser *parser, QList *tokens)
     qmp->response = (QDict *)obj;
 }
 
-QDict *qtest_qmp_receive(QTestState *s)
+QDict *qmp_fd_receive(int fd)
 {
     QMPResponseParser qmp;
     bool log = getenv("QTEST_LOG") != NULL;
@@ -367,7 +367,7 @@ QDict *qtest_qmp_receive(QTestState *s)
         ssize_t len;
         char c;
 
-        len = read(s->qmp_fd, &c, 1);
+        len = read(fd, &c, 1);
         if (len == -1 && errno == EINTR) {
             continue;
         }
@@ -387,12 +387,17 @@ QDict *qtest_qmp_receive(QTestState *s)
     return qmp.response;
 }
 
+QDict *qtest_qmp_receive(QTestState *s)
+{
+    return qmp_fd_receive(s->qmp_fd);
+}
+
 /**
  * Allow users to send a message without waiting for the reply,
  * in the case that they choose to discard all replies up until
  * a particular EVENT is received.
  */
-void qtest_async_qmpv(QTestState *s, const char *fmt, va_list ap)
+void qmp_fd_sendv(int fd, const char *fmt, va_list ap)
 {
     va_list ap_copy;
     QObject *qobj;
@@ -416,13 +421,25 @@ void qtest_async_qmpv(QTestState *s, const char *fmt, va_list ap)
             fprintf(stderr, "%s", str);
         }
         /* Send QMP request */
-        socket_send(s->qmp_fd, str, size);
+        socket_send(fd, str, size);
 
         QDECREF(qstr);
         qobject_decref(qobj);
     }
 }
 
+void qtest_async_qmpv(QTestState *s, const char *fmt, va_list ap)
+{
+    qmp_fd_sendv(s->qmp_fd, fmt, ap);
+}
+
+QDict *qmp_fdv(int fd, const char *fmt, va_list ap)
+{
+    qmp_fd_sendv(fd, fmt, ap);
+
+    return qmp_fd_receive(fd);
+}
+
 QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
 {
     qtest_async_qmpv(s, fmt, ap);
@@ -431,6 +448,26 @@ QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
     return qtest_qmp_receive(s);
 }
 
+QDict *qmp_fd(int fd, const char *fmt, ...)
+{
+    va_list ap;
+    QDict *response;
+
+    va_start(ap, fmt);
+    response = qmp_fdv(fd, fmt, ap);
+    va_end(ap);
+    return response;
+}
+
+void qmp_fd_send(int fd, const char *fmt, ...)
+{
+    va_list ap;
+
+    va_start(ap, fmt);
+    qmp_fd_sendv(fd, fmt, ap);
+    va_end(ap);
+}
+
 QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
 {
     va_list ap;
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 55bccbf..9818ef7 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -851,4 +851,11 @@ static inline int64_t clock_set(int64_t val)
  */
 bool qtest_big_endian(void);
 
+
+QDict *qmp_fd_receive(int fd);
+void qmp_fd_sendv(int fd, const char *fmt, va_list ap);
+void qmp_fd_send(int fd, const char *fmt, ...);
+QDict *qmp_fdv(int fd, const char *fmt, va_list ap);
+QDict *qmp_fd(int fd, const char *fmt, ...);
+
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 06/13] glib-compat: add 2.38/2.40/2.46 asserts
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (4 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 05/13] qtest: add a few fd-level qmp helpers Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 07/13] tests: add a local test for guest agent Michael Roth
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Michael Roth, Marc-André Lureau

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

Those are mostly useful for writing tests.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 include/glib-compat.h | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/include/glib-compat.h b/include/glib-compat.h
index 318e000..fb25f43 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -165,4 +165,65 @@ static inline GThread *g_thread_new(const char *name,
 #define CompatGCond GCond
 #endif /* glib 2.31 */
 
+#ifndef g_assert_true
+#define g_assert_true(expr)                                                    \
+    do {                                                                       \
+        if (G_LIKELY(expr)) {                                                  \
+        } else {                                                               \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "'" #expr "' should be TRUE");                 \
+        }                                                                      \
+    } while (0)
+#endif
+
+#ifndef g_assert_false
+#define g_assert_false(expr)                                                   \
+    do {                                                                       \
+        if (G_LIKELY(!(expr))) {                                               \
+        } else {                                                               \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "'" #expr "' should be FALSE");                \
+        }                                                                      \
+    } while (0)
+#endif
+
+#ifndef g_assert_null
+#define g_assert_null(expr)                                                    \
+    do {                                                                       \
+        if (G_LIKELY((expr) == NULL)) {                                        \
+        } else {                                                               \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "'" #expr "' should be NULL");                 \
+        }                                                                      \
+    } while (0)
+#endif
+
+#ifndef g_assert_nonnull
+#define g_assert_nonnull(expr)                                                 \
+    do {                                                                       \
+        if (G_LIKELY((expr) != NULL)) {                                        \
+        } else {                                                               \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "'" #expr "' should not be NULL");             \
+        }                                                                      \
+    } while (0)
+#endif
+
+#ifndef g_assert_cmpmem
+#define g_assert_cmpmem(m1, l1, m2, l2)                                        \
+    do {                                                                       \
+        gconstpointer __m1 = m1, __m2 = m2;                                    \
+        int __l1 = l1, __l2 = l2;                                              \
+        if (__l1 != __l2) {                                                    \
+            g_assertion_message_cmpnum(                                        \
+                G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,                   \
+                #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   \
+                __l2, 'i');                                                    \
+        } else if (memcmp(__m1, __m2, __l1) != 0) {                            \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "assertion failed (" #m1 " == " #m2 ")");      \
+        }                                                                      \
+    } while (0)
+#endif
+
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 07/13] tests: add a local test for guest agent
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (5 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 06/13] glib-compat: add 2.38/2.40/2.46 asserts Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 08/13] qga: drop guest_file_init helper and replace it with static initializers Michael Roth
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Michael Roth, Marc-André Lureau

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

Add some local guest agent tests, as it is better than nothing, only
when CONFIG_POSIX (using unix sockets).

With the QGA_TEST_SIDE_EFFECTING environment variable, it will include
tests with side effects, such as freezing/thawing the FS or changing the
time.

(a better test would involve a managed VM (or container), but it might
be better to leave that off to autotest/avocado)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
* use mkdtemp() in placeof g_mkdtemp() for glib 2.22 compat
* drop redundant/conflicting compat defines for
  g_assert_{true,false}, since glib-compat has them now.
* build fixes for OSX: use PRId64 instead of glib formats, drop
  g_spawn_default usage for glib compat
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 tests/Makefile   |   3 +
 tests/test-qga.c | 775 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 778 insertions(+)
 create mode 100644 tests/test-qga.c

diff --git a/tests/Makefile b/tests/Makefile
index cb221de..afefd32 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -80,6 +80,7 @@ check-unit-$(CONFIG_GNUTLS_HASH) += tests/test-crypto-hash$(EXESUF)
 check-unit-y += tests/test-crypto-cipher$(EXESUF)
 check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)
 check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF)
+check-unit-$(CONFIG_POSIX) += tests/test-qga$(EXESUF)
 
 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
@@ -534,6 +535,8 @@ endif
 qtest-obj-y = tests/libqtest.o $(test-util-obj-y)
 $(check-qtest-y): $(qtest-obj-y)
 
+tests/test-qga: tests/test-qga.o $(qtest-obj-y)
+
 .PHONY: check-help
 check-help:
 	@echo "Regression testing targets:"
diff --git a/tests/test-qga.c b/tests/test-qga.c
new file mode 100644
index 0000000..a905c4b
--- /dev/null
+++ b/tests/test-qga.c
@@ -0,0 +1,775 @@
+#include <locale.h>
+#include <glib.h>
+#include <glib/gstdio.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <unistd.h>
+#include <inttypes.h>
+
+#include "libqtest.h"
+#include "config-host.h"
+
+typedef struct {
+    char *test_dir;
+    GMainLoop *loop;
+    int fd;
+    GPid pid;
+} TestFixture;
+
+static int connect_qga(char *path)
+{
+    int s, ret, len, i = 0;
+    struct sockaddr_un remote;
+
+    s = socket(AF_UNIX, SOCK_STREAM, 0);
+    g_assert(s != -1);
+
+    remote.sun_family = AF_UNIX;
+    do {
+        strcpy(remote.sun_path, path);
+        len = strlen(remote.sun_path) + sizeof(remote.sun_family);
+        ret = connect(s, (struct sockaddr *)&remote, len);
+        if (ret == -1) {
+            g_usleep(G_USEC_PER_SEC);
+        }
+        if (i++ == 5) {
+            return -1;
+        }
+    } while (ret == -1);
+
+    return s;
+}
+
+static void qga_watch(GPid pid, gint status, gpointer user_data)
+{
+    TestFixture *fixture = user_data;
+
+    g_assert_cmpint(status, ==, 0);
+    g_main_loop_quit(fixture->loop);
+}
+
+static void
+fixture_setup(TestFixture *fixture, gconstpointer data)
+{
+    const gchar *extra_arg = data;
+    GError *error = NULL;
+    gchar *cwd, *path, *cmd, **argv = NULL;
+
+    fixture->loop = g_main_loop_new(NULL, FALSE);
+
+    fixture->test_dir = g_strdup("/tmp/qgatest.XXXXXX");
+    g_assert_nonnull(mkdtemp(fixture->test_dir));
+
+    path = g_build_filename(fixture->test_dir, "sock", NULL);
+    cwd = g_get_current_dir();
+    cmd = g_strdup_printf("%s%cqemu-ga -m unix-listen -t %s -p %s %s %s",
+                          cwd, G_DIR_SEPARATOR,
+                          fixture->test_dir, path,
+                          getenv("QTEST_LOG") ? "-v" : "",
+                          extra_arg ?: "");
+    g_shell_parse_argv(cmd, NULL, &argv, &error);
+    g_assert_no_error(error);
+
+    g_spawn_async(fixture->test_dir, argv, NULL,
+                  G_SPAWN_SEARCH_PATH|G_SPAWN_DO_NOT_REAP_CHILD,
+                  NULL, NULL, &fixture->pid, &error);
+    g_assert_no_error(error);
+
+    g_child_watch_add(fixture->pid, qga_watch, fixture);
+
+    fixture->fd = connect_qga(path);
+
+    g_strfreev(argv);
+    g_free(cmd);
+    g_free(cwd);
+    g_free(path);
+}
+
+static void
+fixture_tear_down(TestFixture *fixture, gconstpointer data)
+{
+    gchar *tmp;
+
+    kill(fixture->pid, SIGTERM);
+
+    g_main_loop_run(fixture->loop);
+    g_main_loop_unref(fixture->loop);
+
+    g_spawn_close_pid(fixture->pid);
+
+    tmp = g_build_filename(fixture->test_dir, "foo", NULL);
+    g_unlink(tmp);
+    g_free(tmp);
+
+    tmp = g_build_filename(fixture->test_dir, "qga.state", NULL);
+    g_unlink(tmp);
+    g_free(tmp);
+
+    tmp = g_build_filename(fixture->test_dir, "sock", NULL);
+    g_unlink(tmp);
+    g_free(tmp);
+
+    g_rmdir(fixture->test_dir);
+    g_free(fixture->test_dir);
+}
+
+static void qmp_assertion_message_error(const char     *domain,
+                                        const char     *file,
+                                        int             line,
+                                        const char     *func,
+                                        const char     *expr,
+                                        QDict          *dict)
+{
+    const char *class, *desc;
+    char *s;
+    QDict *error;
+
+    error = qdict_get_qdict(dict, "error");
+    class = qdict_get_try_str(error, "class");
+    desc = qdict_get_try_str(error, "desc");
+
+    s = g_strdup_printf("assertion failed %s: %s %s", expr, class, desc);
+    g_assertion_message(domain, file, line, func, s);
+    g_free(s);
+}
+
+#define qmp_assert_no_error(err) do {                                   \
+    if (qdict_haskey(err, "error")) {                                   \
+        qmp_assertion_message_error(G_LOG_DOMAIN, __FILE__, __LINE__,   \
+                                    G_STRFUNC, #err, err);              \
+    }                                                                   \
+} while (0)
+
+static void test_qga_sync_delimited(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    guint32 v, r = g_random_int();
+    unsigned char c;
+    QDict *ret;
+    gchar *cmd;
+
+    cmd = g_strdup_printf("%c{'execute': 'guest-sync-delimited',"
+                          " 'arguments': {'id': %u } }", 0xff, r);
+    qmp_fd_send(fixture->fd, cmd);
+    g_free(cmd);
+
+    v = read(fixture->fd, &c, 1);
+    g_assert_cmpint(v, ==, 1);
+    g_assert_cmpint(c, ==, 0xff);
+
+    ret = qmp_fd_receive(fixture->fd);
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    v = qdict_get_int(ret, "return");
+    g_assert_cmpint(r, ==, v);
+
+    QDECREF(ret);
+}
+
+static void test_qga_sync(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    guint32 v, r = g_random_int();
+    QDict *ret;
+    gchar *cmd;
+
+    cmd = g_strdup_printf("%c{'execute': 'guest-sync',"
+                          " 'arguments': {'id': %u } }", 0xff, r);
+    ret = qmp_fd(fixture->fd, cmd);
+    g_free(cmd);
+
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    v = qdict_get_int(ret, "return");
+    g_assert_cmpint(r, ==, v);
+
+    QDECREF(ret);
+}
+
+static void test_qga_ping(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-ping'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    QDECREF(ret);
+}
+
+static void test_qga_invalid_cmd(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret, *error;
+    const gchar *class, *desc;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-invalid-cmd'}");
+    g_assert_nonnull(ret);
+
+    error = qdict_get_qdict(ret, "error");
+    class = qdict_get_try_str(error, "class");
+    desc = qdict_get_try_str(error, "desc");
+
+    g_assert_cmpstr(class, ==, "CommandNotFound");
+    g_assert_cmpint(strlen(desc), >, 0);
+
+    QDECREF(ret);
+}
+
+static void test_qga_info(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret, *val;
+    const gchar *version;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-info'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    val = qdict_get_qdict(ret, "return");
+    version = qdict_get_try_str(val, "version");
+    g_assert_cmpstr(version, ==, QEMU_VERSION);
+
+    QDECREF(ret);
+}
+
+static void test_qga_get_vcpus(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+    QList *list;
+    const QListEntry *entry;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-vcpus'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    /* check there is at least a cpu */
+    list = qdict_get_qlist(ret, "return");
+    entry = qlist_first(list);
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "online"));
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "logical-id"));
+
+    QDECREF(ret);
+}
+
+static void test_qga_get_fsinfo(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+    QList *list;
+    const QListEntry *entry;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-fsinfo'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    /* check there is at least a fs */
+    list = qdict_get_qlist(ret, "return");
+    entry = qlist_first(list);
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "name"));
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "mountpoint"));
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "type"));
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "disk"));
+
+    QDECREF(ret);
+}
+
+static void test_qga_get_memory_block_info(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret, *val;
+    int64_t size;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-memory-block-info'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    /* check there is at least some memory */
+    val = qdict_get_qdict(ret, "return");
+    size = qdict_get_int(val, "size");
+    g_assert_cmpint(size, >, 0);
+
+    QDECREF(ret);
+}
+
+static void test_qga_get_memory_blocks(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+    QList *list;
+    const QListEntry *entry;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-memory-blocks'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    list = qdict_get_qlist(ret, "return");
+    entry = qlist_first(list);
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "phys-index"));
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "online"));
+
+    QDECREF(ret);
+}
+
+static void test_qga_network_get_interfaces(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+    QList *list;
+    const QListEntry *entry;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-network-get-interfaces'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    /* check there is at least an interface */
+    list = qdict_get_qlist(ret, "return");
+    entry = qlist_first(list);
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "name"));
+
+    QDECREF(ret);
+}
+
+static void test_qga_file_ops(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    const guchar helloworld[] = "Hello World!\n";
+    const char *b64;
+    gchar *cmd, *path, *enc;
+    guchar *dec;
+    QDict *ret, *val;
+    int64_t id, eof;
+    gsize count;
+    FILE *f;
+    char tmp[100];
+
+    /* open */
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-file-open',"
+                 " 'arguments': { 'path': 'foo', 'mode': 'w+' } }");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    id = qdict_get_int(ret, "return");
+    QDECREF(ret);
+
+    enc = g_base64_encode(helloworld, sizeof(helloworld));
+    /* write */
+    cmd = g_strdup_printf("{'execute': 'guest-file-write',"
+                          " 'arguments': { 'handle': %" PRId64 ","
+                          " 'buf-b64': '%s' } }", id, enc);
+    ret = qmp_fd(fixture->fd, cmd);
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    val = qdict_get_qdict(ret, "return");
+    count = qdict_get_int(val, "count");
+    eof = qdict_get_bool(val, "eof");
+    g_assert_cmpint(count, ==, sizeof(helloworld));
+    g_assert_cmpint(eof, ==, 0);
+    QDECREF(ret);
+    g_free(cmd);
+
+    /* flush */
+    cmd = g_strdup_printf("{'execute': 'guest-file-flush',"
+                          " 'arguments': {'handle': %" PRId64 "} }",
+                          id);
+    ret = qmp_fd(fixture->fd, cmd);
+    QDECREF(ret);
+    g_free(cmd);
+
+    /* close */
+    cmd = g_strdup_printf("{'execute': 'guest-file-close',"
+                          " 'arguments': {'handle': %" PRId64 "} }",
+                          id);
+    ret = qmp_fd(fixture->fd, cmd);
+    QDECREF(ret);
+    g_free(cmd);
+
+    /* check content */
+    path = g_build_filename(fixture->test_dir, "foo", NULL);
+    f = fopen(path, "r");
+    g_assert_nonnull(f);
+    count = fread(tmp, 1, sizeof(tmp), f);
+    g_assert_cmpint(count, ==, sizeof(helloworld));
+    tmp[count] = 0;
+    g_assert_cmpstr(tmp, ==, (char *)helloworld);
+    fclose(f);
+
+    /* open */
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-file-open',"
+                 " 'arguments': { 'path': 'foo', 'mode': 'r' } }");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    id = qdict_get_int(ret, "return");
+    QDECREF(ret);
+
+    /* read */
+    cmd = g_strdup_printf("{'execute': 'guest-file-read',"
+                          " 'arguments': { 'handle': %" PRId64 "} }",
+                          id);
+    ret = qmp_fd(fixture->fd, cmd);
+    val = qdict_get_qdict(ret, "return");
+    count = qdict_get_int(val, "count");
+    eof = qdict_get_bool(val, "eof");
+    b64 = qdict_get_str(val, "buf-b64");
+    g_assert_cmpint(count, ==, sizeof(helloworld));
+    g_assert(eof);
+    g_assert_cmpstr(b64, ==, enc);
+
+    QDECREF(ret);
+    g_free(cmd);
+    g_free(enc);
+
+    /* read eof */
+    cmd = g_strdup_printf("{'execute': 'guest-file-read',"
+                          " 'arguments': { 'handle': %" PRId64 "} }",
+                          id);
+    ret = qmp_fd(fixture->fd, cmd);
+    val = qdict_get_qdict(ret, "return");
+    count = qdict_get_int(val, "count");
+    eof = qdict_get_bool(val, "eof");
+    b64 = qdict_get_str(val, "buf-b64");
+    g_assert_cmpint(count, ==, 0);
+    g_assert(eof);
+    g_assert_cmpstr(b64, ==, "");
+    QDECREF(ret);
+    g_free(cmd);
+
+    /* seek */
+    cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
+                          " 'arguments': { 'handle': %" PRId64 ", "
+                          " 'offset': %d, 'whence': %d } }",
+                          id, 6, SEEK_SET);
+    ret = qmp_fd(fixture->fd, cmd);
+    qmp_assert_no_error(ret);
+    val = qdict_get_qdict(ret, "return");
+    count = qdict_get_int(val, "position");
+    eof = qdict_get_bool(val, "eof");
+    g_assert_cmpint(count, ==, 6);
+    g_assert(!eof);
+    QDECREF(ret);
+    g_free(cmd);
+
+    /* partial read */
+    cmd = g_strdup_printf("{'execute': 'guest-file-read',"
+                          " 'arguments': { 'handle': %" PRId64 "} }",
+                          id);
+    ret = qmp_fd(fixture->fd, cmd);
+    val = qdict_get_qdict(ret, "return");
+    count = qdict_get_int(val, "count");
+    eof = qdict_get_bool(val, "eof");
+    b64 = qdict_get_str(val, "buf-b64");
+    g_assert_cmpint(count, ==, sizeof(helloworld) - 6);
+    g_assert(eof);
+    dec = g_base64_decode(b64, &count);
+    g_assert_cmpint(count, ==, sizeof(helloworld) - 6);
+    g_assert_cmpmem(dec, count, helloworld + 6, sizeof(helloworld) - 6);
+    g_free(dec);
+
+    QDECREF(ret);
+    g_free(cmd);
+
+    /* close */
+    cmd = g_strdup_printf("{'execute': 'guest-file-close',"
+                          " 'arguments': {'handle': %" PRId64 "} }",
+                          id);
+    ret = qmp_fd(fixture->fd, cmd);
+    QDECREF(ret);
+    g_free(cmd);
+}
+
+static void test_qga_get_time(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+    int64_t time;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    time = qdict_get_int(ret, "return");
+    g_assert_cmpint(time, >, 0);
+
+    QDECREF(ret);
+}
+
+static void test_qga_set_time(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+    int64_t current, time;
+    gchar *cmd;
+
+    /* get current time */
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    current = qdict_get_int(ret, "return");
+    g_assert_cmpint(current, >, 0);
+    QDECREF(ret);
+
+    /* set some old time */
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-set-time',"
+                 " 'arguments': { 'time': 1000 } }");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    QDECREF(ret);
+
+    /* check old time */
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    time = qdict_get_int(ret, "return");
+    g_assert_cmpint(time / 1000, <, G_USEC_PER_SEC * 10);
+    QDECREF(ret);
+
+    /* set back current time */
+    cmd = g_strdup_printf("{'execute': 'guest-set-time',"
+                          " 'arguments': { 'time': %" PRId64 " } }",
+                          current + time * 1000);
+    ret = qmp_fd(fixture->fd, cmd);
+    g_free(cmd);
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    QDECREF(ret);
+}
+
+static void test_qga_fstrim(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+    QList *list;
+    const QListEntry *entry;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-fstrim',"
+                 " arguments: { minimum: 4194304 } }");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    list = qdict_get_qlist(ret, "return");
+    entry = qlist_first(list);
+    g_assert(qdict_haskey(qobject_to_qdict(entry->value), "paths"));
+
+    QDECREF(ret);
+}
+
+static void test_qga_blacklist(gconstpointer data)
+{
+    TestFixture fix;
+    QDict *ret, *error;
+    const gchar *class, *desc;
+
+    fixture_setup(&fix, "-b guest-ping,guest-get-time");
+
+    /* check blacklist */
+    ret = qmp_fd(fix.fd, "{'execute': 'guest-ping'}");
+    g_assert_nonnull(ret);
+    error = qdict_get_qdict(ret, "error");
+    class = qdict_get_try_str(error, "class");
+    desc = qdict_get_try_str(error, "desc");
+    g_assert_cmpstr(class, ==, "GenericError");
+    g_assert_nonnull(g_strstr_len(desc, -1, "has been disabled"));
+    QDECREF(ret);
+
+    ret = qmp_fd(fix.fd, "{'execute': 'guest-get-time'}");
+    g_assert_nonnull(ret);
+    error = qdict_get_qdict(ret, "error");
+    class = qdict_get_try_str(error, "class");
+    desc = qdict_get_try_str(error, "desc");
+    g_assert_cmpstr(class, ==, "GenericError");
+    g_assert_nonnull(g_strstr_len(desc, -1, "has been disabled"));
+    QDECREF(ret);
+
+    /* check something work */
+    ret = qmp_fd(fix.fd, "{'execute': 'guest-get-fsinfo'}");
+    qmp_assert_no_error(ret);
+    QDECREF(ret);
+
+    fixture_tear_down(&fix, NULL);
+}
+
+static void test_qga_config(gconstpointer data)
+{
+    GError *error = NULL;
+    char *cwd, *cmd, *out, *err, *str, **strv, *conf, **argv = NULL;
+    char *env[2];
+    int status, tmp;
+    gsize n;
+    GKeyFile *kf;
+    const char *qga_config =
+        "[general]\n"
+        "daemon=false\n"
+        "method=virtio-serial\n"
+        "path=/path/to/org.qemu.guest_agent.0\n"
+        "pidfile=/var/foo/qemu-ga.pid\n"
+        "statedir=/var/state\n"
+        "verbose=true\n"
+        "blacklist=guest-ping;guest-get-time\n";
+
+    tmp = g_file_open_tmp(NULL, &conf, &error);
+    g_assert_no_error(error);
+    g_assert_cmpint(tmp, >=, 0);
+    g_assert_cmpstr(conf, !=, "");
+
+    g_file_set_contents(conf, qga_config, -1, &error);
+    g_assert_no_error(error);
+
+    cwd = g_get_current_dir();
+    cmd = g_strdup_printf("%s%cqemu-ga -D",
+                          cwd, G_DIR_SEPARATOR);
+    g_shell_parse_argv(cmd, NULL, &argv, &error);
+    g_assert_no_error(error);
+
+    env[0] = g_strdup_printf("QGA_CONF=%s", conf);
+    env[1] = NULL;
+    g_spawn_sync(NULL, argv, env, 0,
+                 NULL, NULL, &out, &err, &status, &error);
+    g_assert_no_error(error);
+    g_assert_cmpstr(err, ==, "");
+    g_assert_cmpint(status, ==, 0);
+
+    kf = g_key_file_new();
+    g_key_file_load_from_data(kf, out, -1, G_KEY_FILE_NONE, &error);
+    g_assert_no_error(error);
+
+    str = g_key_file_get_start_group(kf);
+    g_assert_cmpstr(str, ==, "general");
+    g_free(str);
+
+    g_assert_false(g_key_file_get_boolean(kf, "general", "daemon", &error));
+    g_assert_no_error(error);
+
+    str = g_key_file_get_string(kf, "general", "method", &error);
+    g_assert_no_error(error);
+    g_assert_cmpstr(str, ==, "virtio-serial");
+    g_free(str);
+
+    str = g_key_file_get_string(kf, "general", "path", &error);
+    g_assert_no_error(error);
+    g_assert_cmpstr(str, ==, "/path/to/org.qemu.guest_agent.0");
+    g_free(str);
+
+    str = g_key_file_get_string(kf, "general", "pidfile", &error);
+    g_assert_no_error(error);
+    g_assert_cmpstr(str, ==, "/var/foo/qemu-ga.pid");
+    g_free(str);
+
+    str = g_key_file_get_string(kf, "general", "statedir", &error);
+    g_assert_no_error(error);
+    g_assert_cmpstr(str, ==, "/var/state");
+    g_free(str);
+
+    g_assert_true(g_key_file_get_boolean(kf, "general", "verbose", &error));
+    g_assert_no_error(error);
+
+    strv = g_key_file_get_string_list(kf, "general", "blacklist", &n, &error);
+    g_assert_cmpint(n, ==, 2);
+#if GLIB_CHECK_VERSION(2, 44, 0)
+    g_assert_true(g_strv_contains((const char * const *)strv,
+                                  "guest-ping"));
+    g_assert_true(g_strv_contains((const char * const *)strv,
+                                  "guest-get-time"));
+#endif
+    g_assert_no_error(error);
+    g_strfreev(strv);
+
+    g_free(out);
+    g_free(err);
+    g_free(conf);
+    g_free(env[0]);
+    g_key_file_free(kf);
+
+    close(tmp);
+}
+
+static void test_qga_fsfreeze_status(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+    const gchar *status;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-status'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+
+    status = qdict_get_try_str(ret, "return");
+    g_assert_cmpstr(status, ==, "thawed");
+
+    QDECREF(ret);
+}
+
+static void test_qga_fsfreeze_and_thaw(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    QDict *ret;
+    const gchar *status;
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-freeze'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    QDECREF(ret);
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-status'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    status = qdict_get_try_str(ret, "return");
+    g_assert_cmpstr(status, ==, "frozen");
+    QDECREF(ret);
+
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-thaw'}");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    QDECREF(ret);
+}
+
+int main(int argc, char **argv)
+{
+    TestFixture fix;
+    int ret;
+
+    setlocale (LC_ALL, "");
+    g_test_init(&argc, &argv, NULL);
+    fixture_setup(&fix, NULL);
+
+    g_test_add_data_func("/qga/sync-delimited", &fix, test_qga_sync_delimited);
+    g_test_add_data_func("/qga/sync", &fix, test_qga_sync);
+    g_test_add_data_func("/qga/ping", &fix, test_qga_ping);
+    g_test_add_data_func("/qga/info", &fix, test_qga_info);
+    g_test_add_data_func("/qga/network-get-interfaces", &fix,
+                         test_qga_network_get_interfaces);
+    g_test_add_data_func("/qga/get-vcpus", &fix, test_qga_get_vcpus);
+    g_test_add_data_func("/qga/get-fsinfo", &fix, test_qga_get_fsinfo);
+    g_test_add_data_func("/qga/get-memory-block-info", &fix,
+                         test_qga_get_memory_block_info);
+    g_test_add_data_func("/qga/get-memory-blocks", &fix,
+                         test_qga_get_memory_blocks);
+    g_test_add_data_func("/qga/file-ops", &fix, test_qga_file_ops);
+    g_test_add_data_func("/qga/get-time", &fix, test_qga_get_time);
+    g_test_add_data_func("/qga/invalid-cmd", &fix, test_qga_invalid_cmd);
+    g_test_add_data_func("/qga/fsfreeze-status", &fix,
+                         test_qga_fsfreeze_status);
+
+    g_test_add_data_func("/qga/blacklist", NULL, test_qga_blacklist);
+    g_test_add_data_func("/qga/config", NULL, test_qga_config);
+
+    if (g_getenv("QGA_TEST_SIDE_EFFECTING")) {
+        g_test_add_data_func("/qga/fsfreeze-and-thaw", &fix,
+                             test_qga_fsfreeze_and_thaw);
+        g_test_add_data_func("/qga/set-time", &fix, test_qga_set_time);
+        g_test_add_data_func("/qga/fstrim", &fix, test_qga_fstrim);
+    }
+
+    ret = g_test_run();
+
+    fixture_tear_down(&fix, NULL);
+
+    return ret;
+}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 08/13] qga: drop guest_file_init helper and replace it with static initializers
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (6 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 07/13] tests: add a local test for guest agent Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 09/13] qga: guest exec functionality Michael Roth
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Yuri Pudgorodskiy, Michael Roth, Denis V. Lunev

From: "Denis V. Lunev" <den@openvz.org>

This just makes code shorter and better.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 10 +++-------
 qga/commands-win32.c | 10 +++-------
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index a932809..03b0f27 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -223,7 +223,9 @@ typedef struct GuestFileHandle {
 
 static struct {
     QTAILQ_HEAD(, GuestFileHandle) filehandles;
-} guest_file_state;
+} guest_file_state = {
+    .filehandles = QTAILQ_HEAD_INITIALIZER(guest_file_state.filehandles),
+};
 
 static int64_t guest_file_handle_add(FILE *fh, Error **errp)
 {
@@ -586,11 +588,6 @@ void qmp_guest_file_flush(int64_t handle, Error **errp)
     }
 }
 
-static void guest_file_init(void)
-{
-    QTAILQ_INIT(&guest_file_state.filehandles);
-}
-
 /* linux-specific implementations. avoid this if at all possible. */
 #if defined(__linux__)
 
@@ -2486,5 +2483,4 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
 #if defined(CONFIG_FSFREEZE)
     ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
 #endif
-    ga_command_state_add(cs, guest_file_init, NULL);
 }
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0481646..d9de23b 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -55,7 +55,9 @@ typedef struct GuestFileHandle {
 
 static struct {
     QTAILQ_HEAD(, GuestFileHandle) filehandles;
-} guest_file_state;
+} guest_file_state = {
+    .filehandles = QTAILQ_HEAD_INITIALIZER(guest_file_state.filehandles),
+};
 
 
 typedef struct OpenFlags {
@@ -390,11 +392,6 @@ void qmp_guest_file_flush(int64_t handle, Error **errp)
     }
 }
 
-static void guest_file_init(void)
-{
-    QTAILQ_INIT(&guest_file_state.filehandles);
-}
-
 #ifdef CONFIG_QGA_NTDDSCSI
 
 static STORAGE_BUS_TYPE win2qemu[] = {
@@ -1330,5 +1327,4 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
     if (!vss_initialized()) {
         ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
     }
-    ga_command_state_add(cs, guest_file_init, NULL);
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 09/13] qga: guest exec functionality
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (7 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 08/13] qga: drop guest_file_init helper and replace it with static initializers Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 10/13] qga: handle possible SIGPIPE in guest-file-write Michael Roth
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Yuri Pudgorodskiy, Michael Roth, Denis V. Lunev

From: Yuri Pudgorodskiy <yur@virtuozzo.com>

Guest-exec rewritten in platform-independent style with glib spawn.

Child process is spawn asynchronously and exit status can later
be picked up by guest-exec-status command.

stdin/stdout/stderr of the child now is redirected to /dev/null
Later we will add ability to specify stdin in guest-exec command
and to get collected stdout/stderr with guest-exec-status.

Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
* use g_new0 in place of g_malloc for GuestExec struct
* commit msg spelling fixes
* s/inp-data/input-data
* document capture-input mode as false by default
* use GetProcessId() for pids on w32 instead of casting HANDLE
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands.c       | 199 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/qapi-schema.json |  62 ++++++++++++++++
 2 files changed, 261 insertions(+)

diff --git a/qga/commands.c b/qga/commands.c
index 9a765e8..ced72d5 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -70,3 +70,202 @@ struct GuestAgentInfo *qmp_guest_info(Error **errp)
     qmp_for_each_command(qmp_command_info, info);
     return info;
 }
+
+struct GuestExecInfo {
+    GPid pid;
+    int64_t pid_numeric;
+    gint status;
+    bool finished;
+    QTAILQ_ENTRY(GuestExecInfo) next;
+};
+typedef struct GuestExecInfo GuestExecInfo;
+
+static struct {
+    QTAILQ_HEAD(, GuestExecInfo) processes;
+} guest_exec_state = {
+    .processes = QTAILQ_HEAD_INITIALIZER(guest_exec_state.processes),
+};
+
+static int64_t gpid_to_int64(GPid pid)
+{
+#ifdef G_OS_WIN32
+    return GetProcessId(pid);
+#else
+    return (int64_t)pid;
+#endif
+}
+
+static GuestExecInfo *guest_exec_info_add(GPid pid)
+{
+    GuestExecInfo *gei;
+
+    gei = g_new0(GuestExecInfo, 1);
+    gei->pid = pid;
+    gei->pid_numeric = gpid_to_int64(pid);
+    QTAILQ_INSERT_TAIL(&guest_exec_state.processes, gei, next);
+
+    return gei;
+}
+
+static GuestExecInfo *guest_exec_info_find(int64_t pid_numeric)
+{
+    GuestExecInfo *gei;
+
+    QTAILQ_FOREACH(gei, &guest_exec_state.processes, next) {
+        if (gei->pid_numeric == pid_numeric) {
+            return gei;
+        }
+    }
+
+    return NULL;
+}
+
+GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
+{
+    GuestExecInfo *gei;
+    GuestExecStatus *ges;
+
+    slog("guest-exec-status called, pid: %u", (uint32_t)pid);
+
+    gei = guest_exec_info_find(pid);
+    if (gei == NULL) {
+        error_setg(err, QERR_INVALID_PARAMETER, "pid");
+        return NULL;
+    }
+
+    ges = g_new0(GuestExecStatus, 1);
+    ges->exited = gei->finished;
+
+    if (gei->finished) {
+        /* Glib has no portable way to parse exit status.
+         * On UNIX, we can get either exit code from normal termination
+         * or signal number.
+         * On Windows, it is either the same exit code or the exception
+         * value for an unhandled exception that caused the process
+         * to terminate.
+         * See MSDN for GetExitCodeProcess() and ntstatus.h for possible
+         * well-known codes, e.g. C0000005 ACCESS_DENIED - analog of SIGSEGV
+         * References:
+         *   https://msdn.microsoft.com/en-us/library/windows/desktop/ms683189(v=vs.85).aspx
+         *   https://msdn.microsoft.com/en-us/library/aa260331(v=vs.60).aspx
+         */
+#ifdef G_OS_WIN32
+        /* Additionally WIN32 does not provide any additional information
+         * on whetherthe child exited or terminated via signal.
+         * We use this simple range check to distingish application exit code
+         * (usually value less then 256) and unhandled exception code with
+         * ntstatus (always value greater then 0xC0000005). */
+        if ((uint32_t)gei->status < 0xC0000000U) {
+            ges->has_exitcode = true;
+            ges->exitcode = gei->status;
+        } else {
+            ges->has_signal = true;
+            ges->signal = gei->status;
+        }
+#else
+        if (WIFEXITED(gei->status)) {
+            ges->has_exitcode = true;
+            ges->exitcode = WEXITSTATUS(gei->status);
+        } else if (WIFSIGNALED(gei->status)) {
+            ges->has_signal = true;
+            ges->signal = WTERMSIG(gei->status);
+        }
+#endif
+        QTAILQ_REMOVE(&guest_exec_state.processes, gei, next);
+        g_free(gei);
+    }
+
+    return ges;
+}
+
+/* Get environment variables or arguments array for execve(). */
+static char **guest_exec_get_args(const strList *entry, bool log)
+{
+    const strList *it;
+    int count = 1, i = 0;  /* reserve for NULL terminator */
+    char **args;
+    char *str; /* for logging array of arguments */
+    size_t str_size = 1;
+
+    for (it = entry; it != NULL; it = it->next) {
+        count++;
+        str_size += 1 + strlen(it->value);
+    }
+
+    str = g_malloc(str_size);
+    *str = 0;
+    args = g_malloc(count * sizeof(char *));
+    for (it = entry; it != NULL; it = it->next) {
+        args[i++] = it->value;
+        pstrcat(str, str_size, it->value);
+        if (it->next) {
+            pstrcat(str, str_size, " ");
+        }
+    }
+    args[i] = NULL;
+
+    if (log) {
+        slog("guest-exec called: \"%s\"", str);
+    }
+    g_free(str);
+
+    return args;
+}
+
+static void guest_exec_child_watch(GPid pid, gint status, gpointer data)
+{
+    GuestExecInfo *gei = (GuestExecInfo *)data;
+
+    g_debug("guest_exec_child_watch called, pid: %d, status: %u",
+            (int32_t)gpid_to_int64(pid), (uint32_t)status);
+
+    gei->status = status;
+    gei->finished = true;
+
+    g_spawn_close_pid(pid);
+}
+
+GuestExec *qmp_guest_exec(const char *path,
+                       bool has_arg, strList *arg,
+                       bool has_env, strList *env,
+                       bool has_input_data, const char *input_data,
+                       bool has_capture_output, bool capture_output,
+                       Error **err)
+{
+    GPid pid;
+    GuestExec *ge = NULL;
+    GuestExecInfo *gei;
+    char **argv, **envp;
+    strList arglist;
+    gboolean ret;
+    GError *gerr = NULL;
+
+    arglist.value = (char *)path;
+    arglist.next = has_arg ? arg : NULL;
+
+    argv = guest_exec_get_args(&arglist, true);
+    envp = guest_exec_get_args(has_env ? env : NULL, false);
+
+    ret = g_spawn_async_with_pipes(NULL, argv, envp,
+            G_SPAWN_SEARCH_PATH |
+            G_SPAWN_DO_NOT_REAP_CHILD |
+            G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
+            NULL, NULL, &pid, NULL, NULL, NULL, &gerr);
+    if (!ret) {
+        error_setg(err, QERR_QGA_COMMAND_FAILED, gerr->message);
+        g_error_free(gerr);
+        goto done;
+    }
+
+    ge = g_new0(GuestExec, 1);
+    ge->pid = gpid_to_int64(pid);
+
+    gei = guest_exec_info_add(pid);
+    g_child_watch_add(pid, guest_exec_child_watch, gei);
+
+done:
+    g_free(argv);
+    g_free(envp);
+
+    return ge;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 82894c6..4900b5e 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -930,3 +930,65 @@
 ##
 { 'command': 'guest-get-memory-block-info',
   'returns': 'GuestMemoryBlockInfo' }
+
+# @GuestExecStatus:
+#
+# @exited: true if process has already terminated.
+# @exitcode: #optional process exit code if it was normally terminated.
+# @signal: #optional signal number (linux) or unhandled exception code
+#       (windows) if the process was abnormally terminated.
+# @out-data: #optional base64-encoded stdout of the process
+# @err-data: #optional base64-encoded stderr of the process
+#       Note: @out-data and @err-data are present only
+#       if 'capture-output' was specified for 'guest-exec'
+#
+# Since: 2.5
+##
+{ 'struct': 'GuestExecStatus',
+  'data': { 'exited': 'bool', '*exitcode': 'int', '*signal': 'int',
+            '*out-data': 'str', '*err-data': 'str' }}
+##
+# @guest-exec-status
+#
+# Check status of process associated with PID retrieved via guest-exec.
+# Reap the process and associated metadata if it has exited.
+#
+# @pid: pid returned from guest-exec
+#
+# Returns: GuestExecStatus on success.
+#
+# Since 2.5
+##
+{ 'command': 'guest-exec-status',
+  'data':    { 'pid': 'int' },
+  'returns': 'GuestExecStatus' }
+
+##
+# @GuestExec:
+# @pid: pid of child process in guest OS
+#
+#Since: 2.5
+##
+{ 'struct': 'GuestExec',
+  'data': { 'pid': 'int'} }
+
+##
+# @guest-exec:
+#
+# Execute a command in the guest
+#
+# @path: path or executable name to execute
+# @arg: #optional argument list to pass to executable
+# @env: #optional environment variables to pass to executable
+# @input-data: #optional data to be passed to process stdin (base64 encoded)
+# @capture-output: #optional bool flag to enable capture of
+#                  stdout/stderr of running process. defaults to false.
+#
+# Returns: PID on success.
+#
+# Since: 2.5
+##
+{ 'command': 'guest-exec',
+  'data':    { 'path': 'str', '*arg': ['str'], '*env': ['str'],
+               '*input-data': 'str', '*capture-output': 'bool' },
+  'returns': 'GuestExec' }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 10/13] qga: handle possible SIGPIPE in guest-file-write
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (8 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 09/13] qga: guest exec functionality Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 11/13] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all() Michael Roth
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Yuri Pudgorodskiy, Michael Roth, Denis V. Lunev

From: "Denis V. Lunev" <den@openvz.org>

qemu-ga should not exit on guest-file-write to pipe without read end
but proper error code should be returned. The behavior of the
spawned process should be default thus SIGPIPE processing should be
reset to default after fork() but before exec().

Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands.c | 18 +++++++++++++++++-
 qga/main.c     |  6 ++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/qga/commands.c b/qga/commands.c
index ced72d5..68e8cfa 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -225,6 +225,22 @@ static void guest_exec_child_watch(GPid pid, gint status, gpointer data)
     g_spawn_close_pid(pid);
 }
 
+/** Reset ignored signals back to default. */
+static void guest_exec_task_setup(gpointer data)
+{
+#if !defined(G_OS_WIN32)
+    struct sigaction sigact;
+
+    memset(&sigact, 0, sizeof(struct sigaction));
+    sigact.sa_handler = SIG_DFL;
+
+    if (sigaction(SIGPIPE, &sigact, NULL) != 0) {
+        slog("sigaction() failed to reset child process's SIGPIPE: %s",
+             strerror(errno));
+    }
+#endif
+}
+
 GuestExec *qmp_guest_exec(const char *path,
                        bool has_arg, strList *arg,
                        bool has_env, strList *env,
@@ -250,7 +266,7 @@ GuestExec *qmp_guest_exec(const char *path,
             G_SPAWN_SEARCH_PATH |
             G_SPAWN_DO_NOT_REAP_CHILD |
             G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
-            NULL, NULL, &pid, NULL, NULL, NULL, &gerr);
+            guest_exec_task_setup, NULL, &pid, NULL, NULL, NULL, &gerr);
     if (!ret) {
         error_setg(err, QERR_QGA_COMMAND_FAILED, gerr->message);
         g_error_free(gerr);
diff --git a/qga/main.c b/qga/main.c
index aa6a063..068169f 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -161,6 +161,12 @@ static gboolean register_signal_handlers(void)
         g_error("error configuring signal handler: %s", strerror(errno));
     }
 
+    sigact.sa_handler = SIG_IGN;
+    if (sigaction(SIGPIPE, &sigact, NULL) != 0) {
+        g_error("error configuring SIGPIPE signal handler: %s",
+                strerror(errno));
+    }
+
     return true;
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 11/13] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (9 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 10/13] qga: handle possible SIGPIPE in guest-file-write Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 12/13] qga: guest-exec simple stdin/stdout/stderr redirection Michael Roth
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Yuri Pudgorodskiy, Michael Roth, Denis V. Lunev

From: Yuri Pudgorodskiy <yur@virtuozzo.com>

glib may return G_IO_STATUS_AGAIN which is actually not an error.
Also fixed a bug when on incomplete write buf pointer was not adjusted.

Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/channel-posix.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 61aa3cb..50d9dd3 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -217,25 +217,24 @@ GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size)
     GIOStatus status = G_IO_STATUS_NORMAL;
 
     while (size) {
+        g_debug("sending data, count: %d", (int)size);
         status = g_io_channel_write_chars(c->client_channel, buf, size,
                                           &written, &err);
-        g_debug("sending data, count: %d", (int)size);
-        if (err != NULL) {
+        if (status == G_IO_STATUS_NORMAL) {
+            size -= written;
+            buf += written;
+        } else if (status != G_IO_STATUS_AGAIN) {
             g_warning("error writing to channel: %s", err->message);
-            return G_IO_STATUS_ERROR;
-        }
-        if (status != G_IO_STATUS_NORMAL) {
-            break;
+            return status;
         }
-        size -= written;
     }
 
-    if (status == G_IO_STATUS_NORMAL) {
+    do {
         status = g_io_channel_flush(c->client_channel, &err);
-        if (err != NULL) {
-            g_warning("error flushing channel: %s", err->message);
-            return G_IO_STATUS_ERROR;
-        }
+    } while (status == G_IO_STATUS_AGAIN);
+
+    if (status != G_IO_STATUS_NORMAL) {
+        g_warning("error flushing channel: %s", err->message);
     }
 
     return status;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 12/13] qga: guest-exec simple stdin/stdout/stderr redirection
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (10 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 11/13] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all() Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 13/13] qga: fix uninitialized value warning for win32 Michael Roth
  2015-10-18 21:21 ` [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Peter Maydell
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Yuri Pudgorodskiy, Michael Roth, Denis V. Lunev

From: Yuri Pudgorodskiy <yur@virtuozzo.com>

Implemented with base64-encoded strings in qga json protocol.
Glib portable GIOChannel is used for data I/O.

Optinal stdin parameter of guest-exec command is now used as
stdin content for spawned subprocess.

If capture-output bool flag is specified, guest-exec redirects out/err
file descriptiors internally to pipes and collects subprocess
output.

Guest-exe-status is modified to return this collected data to requestor
in base64 encoding.

Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
* switch from 'struct GuestIOExecData' to 'GuestIOExecData'
* s/TRUE/true/g, s/FALSE/false/g for gboolean return values
* s/inp_data/input_data/
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands.c       | 189 ++++++++++++++++++++++++++++++++++++++++++++++++---
 qga/qapi-schema.json |   7 +-
 2 files changed, 187 insertions(+), 9 deletions(-)

diff --git a/qga/commands.c b/qga/commands.c
index 68e8cfa..0f80ce6 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -15,6 +15,11 @@
 #include "qga-qmp-commands.h"
 #include "qapi/qmp/qerror.h"
 
+/* Maximum captured guest-exec out_data/err_data - 16MB */
+#define GUEST_EXEC_MAX_OUTPUT (16*1024*1024)
+/* Allocation and I/O buffer for reading guest-exec out_data/err_data - 4KB */
+#define GUEST_EXEC_IO_SIZE (4*1024)
+
 /* Note: in some situations, like with the fsfreeze, logging may be
  * temporarilly disabled. if it is necessary that a command be able
  * to log for accounting purposes, check ga_logging_enabled() beforehand,
@@ -71,11 +76,25 @@ struct GuestAgentInfo *qmp_guest_info(Error **errp)
     return info;
 }
 
+struct GuestExecIOData {
+    guchar *data;
+    gsize size;
+    gsize length;
+    gint closed;
+    bool truncated;
+    const char *name;
+};
+typedef struct GuestExecIOData GuestExecIOData;
+
 struct GuestExecInfo {
     GPid pid;
     int64_t pid_numeric;
     gint status;
-    bool finished;
+    bool has_output;
+    gint finished;
+    GuestExecIOData in;
+    GuestExecIOData out;
+    GuestExecIOData err;
     QTAILQ_ENTRY(GuestExecInfo) next;
 };
 typedef struct GuestExecInfo GuestExecInfo;
@@ -134,9 +153,18 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
     }
 
     ges = g_new0(GuestExecStatus, 1);
-    ges->exited = gei->finished;
 
-    if (gei->finished) {
+    bool finished = g_atomic_int_get(&gei->finished);
+
+    /* need to wait till output channels are closed
+     * to be sure we captured all output at this point */
+    if (gei->has_output) {
+        finished = finished && g_atomic_int_get(&gei->out.closed);
+        finished = finished && g_atomic_int_get(&gei->err.closed);
+    }
+
+    ges->exited = finished;
+    if (finished) {
         /* Glib has no portable way to parse exit status.
          * On UNIX, we can get either exit code from normal termination
          * or signal number.
@@ -171,6 +199,20 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
             ges->signal = WTERMSIG(gei->status);
         }
 #endif
+        if (gei->out.length > 0) {
+            ges->has_out_data = true;
+            ges->out_data = g_base64_encode(gei->out.data, gei->out.length);
+            g_free(gei->out.data);
+            ges->has_out_truncated = gei->out.truncated;
+        }
+
+        if (gei->err.length > 0) {
+            ges->has_err_data = true;
+            ges->err_data = g_base64_encode(gei->err.data, gei->err.length);
+            g_free(gei->err.data);
+            ges->has_err_truncated = gei->err.truncated;
+        }
+
         QTAILQ_REMOVE(&guest_exec_state.processes, gei, next);
         g_free(gei);
     }
@@ -241,6 +283,98 @@ static void guest_exec_task_setup(gpointer data)
 #endif
 }
 
+static gboolean guest_exec_input_watch(GIOChannel *ch,
+        GIOCondition cond, gpointer p_)
+{
+    GuestExecIOData *p = (GuestExecIOData *)p_;
+    gsize bytes_written = 0;
+    GIOStatus status;
+    GError *gerr = NULL;
+
+    /* nothing left to write */
+    if (p->size == p->length) {
+        goto done;
+    }
+
+    status = g_io_channel_write_chars(ch, (gchar *)p->data + p->length,
+            p->size - p->length, &bytes_written, &gerr);
+
+    /* can be not 0 even if not G_IO_STATUS_NORMAL */
+    if (bytes_written != 0) {
+        p->length += bytes_written;
+    }
+
+    /* continue write, our callback will be called again */
+    if (status == G_IO_STATUS_NORMAL || status == G_IO_STATUS_AGAIN) {
+        return true;
+    }
+
+    if (gerr) {
+        g_warning("qga: i/o error writing to input_data channel: %s",
+                gerr->message);
+        g_error_free(gerr);
+    }
+
+done:
+    g_io_channel_shutdown(ch, true, NULL);
+    g_io_channel_unref(ch);
+    g_atomic_int_set(&p->closed, 1);
+    g_free(p->data);
+
+    return false;
+}
+
+static gboolean guest_exec_output_watch(GIOChannel *ch,
+        GIOCondition cond, gpointer p_)
+{
+    GuestExecIOData *p = (GuestExecIOData *)p_;
+    gsize bytes_read;
+    GIOStatus gstatus;
+
+    if (cond == G_IO_HUP || cond == G_IO_ERR) {
+        goto close;
+    }
+
+    if (p->size == p->length) {
+        gpointer t = NULL;
+        if (!p->truncated && p->size < GUEST_EXEC_MAX_OUTPUT) {
+            t = g_try_realloc(p->data, p->size + GUEST_EXEC_IO_SIZE);
+        }
+        if (t == NULL) {
+            /* ignore truncated output */
+            gchar buf[GUEST_EXEC_IO_SIZE];
+
+            p->truncated = true;
+            gstatus = g_io_channel_read_chars(ch, buf, sizeof(buf),
+                                              &bytes_read, NULL);
+            if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
+                goto close;
+            }
+
+            return true;
+        }
+        p->size += GUEST_EXEC_IO_SIZE;
+        p->data = t;
+    }
+
+    /* Calling read API once.
+     * On next available data our callback will be called again */
+    gstatus = g_io_channel_read_chars(ch, (gchar *)p->data + p->length,
+            p->size - p->length, &bytes_read, NULL);
+    if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
+        goto close;
+    }
+
+    p->length += bytes_read;
+
+    return true;
+
+close:
+    g_io_channel_unref(ch);
+    g_atomic_int_set(&p->closed, 1);
+    return false;
+}
+
 GuestExec *qmp_guest_exec(const char *path,
                        bool has_arg, strList *arg,
                        bool has_env, strList *env,
@@ -255,6 +389,10 @@ GuestExec *qmp_guest_exec(const char *path,
     strList arglist;
     gboolean ret;
     GError *gerr = NULL;
+    gint in_fd, out_fd, err_fd;
+    GIOChannel *in_ch, *out_ch, *err_ch;
+    GSpawnFlags flags;
+    bool has_output = (has_capture_output && capture_output);
 
     arglist.value = (char *)path;
     arglist.next = has_arg ? arg : NULL;
@@ -262,11 +400,14 @@ GuestExec *qmp_guest_exec(const char *path,
     argv = guest_exec_get_args(&arglist, true);
     envp = guest_exec_get_args(has_env ? env : NULL, false);
 
-    ret = g_spawn_async_with_pipes(NULL, argv, envp,
-            G_SPAWN_SEARCH_PATH |
-            G_SPAWN_DO_NOT_REAP_CHILD |
-            G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
-            guest_exec_task_setup, NULL, &pid, NULL, NULL, NULL, &gerr);
+    flags = G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD;
+    if (!has_output) {
+        flags |= G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL;
+    }
+
+    ret = g_spawn_async_with_pipes(NULL, argv, envp, flags,
+            guest_exec_task_setup, NULL, &pid, has_input_data ? &in_fd : NULL,
+            has_output ? &out_fd : NULL, has_output ? &err_fd : NULL, &gerr);
     if (!ret) {
         error_setg(err, QERR_QGA_COMMAND_FAILED, gerr->message);
         g_error_free(gerr);
@@ -277,8 +418,40 @@ GuestExec *qmp_guest_exec(const char *path,
     ge->pid = gpid_to_int64(pid);
 
     gei = guest_exec_info_add(pid);
+    gei->has_output = has_output;
     g_child_watch_add(pid, guest_exec_child_watch, gei);
 
+    if (has_input_data) {
+        gei->in.data = g_base64_decode(input_data, &gei->in.size);
+#ifdef G_OS_WIN32
+        in_ch = g_io_channel_win32_new_fd(in_fd);
+#else
+        in_ch = g_io_channel_unix_new(in_fd);
+#endif
+        g_io_channel_set_encoding(in_ch, NULL, NULL);
+        g_io_channel_set_buffered(in_ch, false);
+        g_io_channel_set_flags(in_ch, G_IO_FLAG_NONBLOCK, NULL);
+        g_io_add_watch(in_ch, G_IO_OUT, guest_exec_input_watch, &gei->in);
+    }
+
+    if (has_output) {
+#ifdef G_OS_WIN32
+        out_ch = g_io_channel_win32_new_fd(out_fd);
+        err_ch = g_io_channel_win32_new_fd(err_fd);
+#else
+        out_ch = g_io_channel_unix_new(out_fd);
+        err_ch = g_io_channel_unix_new(err_fd);
+#endif
+        g_io_channel_set_encoding(out_ch, NULL, NULL);
+        g_io_channel_set_encoding(err_ch, NULL, NULL);
+        g_io_channel_set_buffered(out_ch, false);
+        g_io_channel_set_buffered(err_ch, false);
+        g_io_add_watch(out_ch, G_IO_IN | G_IO_HUP,
+                guest_exec_output_watch, &gei->out);
+        g_io_add_watch(err_ch, G_IO_IN | G_IO_HUP,
+                guest_exec_output_watch, &gei->err);
+    }
+
 done:
     g_free(argv);
     g_free(envp);
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 4900b5e..78362e0 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -941,12 +941,17 @@
 # @err-data: #optional base64-encoded stderr of the process
 #       Note: @out-data and @err-data are present only
 #       if 'capture-output' was specified for 'guest-exec'
+# @out-truncated: #optional true if stdout was not fully captured
+#       due to size limitation.
+# @err-truncated: #optional true if stderr was not fully captured
+#       due to size limitation.
 #
 # Since: 2.5
 ##
 { 'struct': 'GuestExecStatus',
   'data': { 'exited': 'bool', '*exitcode': 'int', '*signal': 'int',
-            '*out-data': 'str', '*err-data': 'str' }}
+            '*out-data': 'str', '*err-data': 'str',
+            '*out-truncated': 'bool', '*err-truncated': 'bool' }}
 ##
 # @guest-exec-status
 #
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PULL v3 13/13] qga: fix uninitialized value warning for win32
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (11 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 12/13] qga: guest-exec simple stdin/stdout/stderr redirection Michael Roth
@ 2015-10-17 16:59 ` Michael Roth
  2015-10-18 21:21 ` [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Peter Maydell
  13 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-17 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Michael Roth

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/channel-win32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 215b15a..0452b9f 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -269,7 +269,7 @@ static GIOStatus ga_channel_write(GAChannel *c, const char *buf, size_t size,
 GIOStatus ga_channel_write_all(GAChannel *c, const char *buf, size_t size)
 {
     GIOStatus status = G_IO_STATUS_NORMAL;
-    size_t count;
+    size_t count = 0;
 
     while (size) {
         status = ga_channel_write(c, buf, size, &count);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue
  2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
                   ` (12 preceding siblings ...)
  2015-10-17 16:59 ` [Qemu-devel] [PULL v3 13/13] qga: fix uninitialized value warning for win32 Michael Roth
@ 2015-10-18 21:21 ` Peter Maydell
  2015-10-19 12:44   ` Michael Roth
  13 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2015-10-18 21:21 UTC (permalink / raw)
  To: Michael Roth; +Cc: QEMU Developers

On 17 October 2015 at 17:59, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> Hi Peter,
>
> Please note that 'glib-compat: add 2.38/2.40/2.46 asserts' is also in
> Marc-André's recent ivshmem PULL. The 2 versions of the patches are identical,
> but let me know if you'd prefer a re-send/re-base later.
>
> The following changes since commit 6d57410a79d51d92673c54f26624b44f27fa6214:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20151016' into staging (2015-10-17 12:31:33 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/mdroth/qemu.git tags/qga-pull-2015-10-14-v3-tag
>
> for you to fetch changes up to ed9f1986d19c2d21667a14b875b2ac8b8a19b8a5:
>
>   qga: fix uninitialized value warning for win32 (2015-10-17 11:24:27 -0500)
>
> ----------------------------------------------------------------
> qemu-ga patch queue
>
> * add unit tests for qemu-ga
> * add guest-exec support for posix/w32 guests
> * added 'qemu-ga' target for w32. this allows us to do full MSI build,
>   without overloading 'qemu-ga.exe' target with uneeded dependencies.
> * number of s/g_new/g_malloc/ conversions for qga
>
> v2:
> * commit message and qapi documentation spelling fixes
> * rename 'inp-data' guest-exec param to 'input-data'
>
> v3:
> * fix OSX build errors for test-qga by using PRId64
>   format in place of glib's, and dropping use of G_SPAWN_DEFAULT
>   macro for glib 2.22 compat
> * fix win32 build warnings for 32-bit builds by avoid int casts
>   of process HANDLEs

Still seeing failures, I'm afraid.

OSX assertion failures:

GTESTER tests/test-qga
**
ERROR:/Users/pm215/src/qemu-for-merges/tests/libqtest.c:238:void
socket_send(int, const char *, size_t): assertion failed (len != -1):
(-1 != -1)
GTester: last random seed: R02S96655a200709f74b72ea42792cd65e8e

(repeated about 10 times).

Test failures on 64-bit ARM:
TEST: tests/test-qga... (pid=22454)
  /qga/sync-delimited:                                                 OK
  /qga/sync:                                                           OK
  /qga/ping:                                                           OK
  /qga/info:                                                           OK
  /qga/network-get-interfaces:                                         OK
  /qga/get-vcpus:                                                      OK
  /qga/get-fsinfo:                                                     OK
  /qga/get-memory-block-info:                                          **
ERROR:/home/petmay01/qemu/tests/test-qga.c:294:test_qga_get_memory_block_info:
assertion failed ret: GenericError
open("/sys/devices/system/memory/"): No such file or directory
FAIL
GTester: last random seed: R02S6aa3e1f8b691a9900d2ea9945e79869d
(pid=22458)
  /qga/get-memory-blocks:                                              **
ERROR:/home/petmay01/qemu/tests/test-qga.c:313:test_qga_get_memory_blocks:
assertion failed ret: GenericError Can't open
directory"/sys/devices/system/memory/"
: No such file or directory
FAIL
GTester: last random seed: R02S978afb04187410dc690a8b7d6d236793
(pid=22461)
  /qga/file-ops:                                                       OK
  /qga/get-time:                                                       OK
  /qga/invalid-cmd:                                                    OK
  /qga/fsfreeze-status:                                                OK
  /qga/blacklist:                                                      OK
  /qga/config:                                                         OK
FAIL: tests/test-qga
make: *** [check-tests/test-qga] Error 1

Not sure why it's complaining, /sys/devices/system/memory/ does
exist on this box.

Ditto on 32-bit ARM, though that's not surprising as it's just
a chroot on an equivalently-configured machine to the 64-bit build.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue
  2015-10-18 21:21 ` [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Peter Maydell
@ 2015-10-19 12:44   ` Michael Roth
  0 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-19 12:44 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

Quoting Peter Maydell (2015-10-18 16:21:30)
> On 17 October 2015 at 17:59, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > Hi Peter,
> >
> > Please note that 'glib-compat: add 2.38/2.40/2.46 asserts' is also in
> > Marc-André's recent ivshmem PULL. The 2 versions of the patches are identical,
> > but let me know if you'd prefer a re-send/re-base later.
> >
> > The following changes since commit 6d57410a79d51d92673c54f26624b44f27fa6214:
> >
> >   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20151016' into staging (2015-10-17 12:31:33 +0100)
> >
> > are available in the git repository at:
> >
> >
> >   git://github.com/mdroth/qemu.git tags/qga-pull-2015-10-14-v3-tag
> >
> > for you to fetch changes up to ed9f1986d19c2d21667a14b875b2ac8b8a19b8a5:
> >
> >   qga: fix uninitialized value warning for win32 (2015-10-17 11:24:27 -0500)
> >
> > ----------------------------------------------------------------
> > qemu-ga patch queue
> >
> > * add unit tests for qemu-ga
> > * add guest-exec support for posix/w32 guests
> > * added 'qemu-ga' target for w32. this allows us to do full MSI build,
> >   without overloading 'qemu-ga.exe' target with uneeded dependencies.
> > * number of s/g_new/g_malloc/ conversions for qga
> >
> > v2:
> > * commit message and qapi documentation spelling fixes
> > * rename 'inp-data' guest-exec param to 'input-data'
> >
> > v3:
> > * fix OSX build errors for test-qga by using PRId64
> >   format in place of glib's, and dropping use of G_SPAWN_DEFAULT
> >   macro for glib 2.22 compat
> > * fix win32 build warnings for 32-bit builds by avoid int casts
> >   of process HANDLEs
> 
> Still seeing failures, I'm afraid.
> 
> OSX assertion failures:
> 
> GTESTER tests/test-qga
> **
> ERROR:/Users/pm215/src/qemu-for-merges/tests/libqtest.c:238:void
> socket_send(int, const char *, size_t): assertion failed (len != -1):
> (-1 != -1)
> GTester: last random seed: R02S96655a200709f74b72ea42792cd65e8e

Argh, sorry for all this noise.

I suspect what's happening is qemu-ga itself might be failing to load or
bind to the socket, and there appears to be a bug in the test case where
we don't check for an fd == -1 return value in connect_qga(), so probably
end up trying to write to it later.

As far as why qemu-ga doesn't appear to be loading properly under OSX,
I think I need to get an environment set up somewhere to debug. I tried
to get an OSX guest going but apparently you need to own an OSX box to
get a AppleSMC OSK key so I'll have to hold off on that for now.

This isn't really something I think has been tested or deployed very
often outside of linux/w32 guests (and w32 isn't enabled since
interacting with a qemu-ga under wine/binfmt requires a specialized
wine environment), so for now I think the best option is to run test-qga
under CONFIG_LINUX instead of CONFIG_POSIX.

> 
> (repeated about 10 times).
> 
> Test failures on 64-bit ARM:
> TEST: tests/test-qga... (pid=22454)
>   /qga/sync-delimited:                                                 OK
>   /qga/sync:                                                           OK
>   /qga/ping:                                                           OK
>   /qga/info:                                                           OK
>   /qga/network-get-interfaces:                                         OK
>   /qga/get-vcpus:                                                      OK
>   /qga/get-fsinfo:                                                     OK
>   /qga/get-memory-block-info:                                          **
> ERROR:/home/petmay01/qemu/tests/test-qga.c:294:test_qga_get_memory_block_info:
> assertion failed ret: GenericError
> open("/sys/devices/system/memory/"): No such file or directory
> FAIL
> GTester: last random seed: R02S6aa3e1f8b691a9900d2ea9945e79869d
> (pid=22458)
>   /qga/get-memory-blocks:                                              **
> ERROR:/home/petmay01/qemu/tests/test-qga.c:313:test_qga_get_memory_blocks:
> assertion failed ret: GenericError Can't open
> directory"/sys/devices/system/memory/"
> : No such file or directory
> FAIL
> GTester: last random seed: R02S978afb04187410dc690a8b7d6d236793
> (pid=22461)
>   /qga/file-ops:                                                       OK
>   /qga/get-time:                                                       OK
>   /qga/invalid-cmd:                                                    OK
>   /qga/fsfreeze-status:                                                OK
>   /qga/blacklist:                                                      OK
>   /qga/config:                                                         OK
> FAIL: tests/test-qga
> make: *** [check-tests/test-qga] Error 1
> 
> Not sure why it's complaining, /sys/devices/system/memory/ does
> exist on this box.
> 
> Ditto on 32-bit ARM, though that's not surprising as it's just
> a chroot on an equivalently-configured machine to the 64-bit build.

Hmm, I'm have FC22 running via -machine vexpress-a15 and I see the same
error, but in my case /sys/devices/system/memory *is* actually missing:

[mdroth@localhost qemu-build]$ uname -a
Linux localhost 4.0.4-301.fc22.armv7hl #1 SMP Thu May 21 14:37:41 UTC 2015 armv7l armv7l armv7l GNU/Linux
[mdroth@localhost qemu-build]$ ls /sys/devices/
armv7_cortex_a15  breakpoint  platform  software  system  tracepoint virtual
[mdroth@localhost qemu-build]$ ls /sys/devices/system/
clockevents  clocksource  container  cpu
[mdroth@localhost qemu-build]$

I didn't see any config option to disable creation of that sysfs node
so I'm a bit confused...

But given such systems exist I think the right fix is to have
guest-get-memory-block-info return an empty-list rather than error in this
case.

I think that would fix the case you're describing, but only by luck.
Probably the best we can do for now though...


Will send a v4 with these changes unless anyone has any other
suggestions.

> 
> thanks
> -- PMM
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-10-19 12:44 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-17 16:59 [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 01/13] build: qemu-ga: add 'qemu-ga' build target for w32 Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 02/13] qga: Use g_new() & friends where that makes obvious sense Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 03/13] qga: add QGA_CONF environment variable Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 04/13] qga: do not override configuration verbosity Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 05/13] qtest: add a few fd-level qmp helpers Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 06/13] glib-compat: add 2.38/2.40/2.46 asserts Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 07/13] tests: add a local test for guest agent Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 08/13] qga: drop guest_file_init helper and replace it with static initializers Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 09/13] qga: guest exec functionality Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 10/13] qga: handle possible SIGPIPE in guest-file-write Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 11/13] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all() Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 12/13] qga: guest-exec simple stdin/stdout/stderr redirection Michael Roth
2015-10-17 16:59 ` [Qemu-devel] [PULL v3 13/13] qga: fix uninitialized value warning for win32 Michael Roth
2015-10-18 21:21 ` [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue Peter Maydell
2015-10-19 12:44   ` Michael Roth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.