[Qemu-devel] [PATCH 0/5] simplified QEMU guest exec

[Qemu-devel] [PATCH 0/5] simplified QEMU guest exec

[Denis V. Lunev]

This patchset provides simplified guest-exec functionality. The
idea is simple. We drop original guest-pipe-open etc stuff and provides
simple and dumb API:
- spawn process (originally with stdin/stdout/stderr as /dev/null)
- later simple buffer is added for this purpose

That is all for now.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>

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

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

 qga/channel-posix.c  |  23 ++--
 qga/commands-posix.c |  10 +-
 qga/commands-win32.c |  10 +-
 qga/commands.c       | 343 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/main.c           |   6 +
 qga/qapi-schema.json |  60 +++++++++
 6 files changed, 426 insertions(+), 26 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers

[Denis V. Lunev]

This just makes code shorter and better.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
CC: 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 b03c316..8989912 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 41bdd3f..3374678 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);
 }
-- 
2.1.4

[Qemu-devel] [PATCH 2/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()

[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>
CC: 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 8aad4fe..7be92cc 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;
-- 
2.1.4

[Qemu-devel] [PATCH 3/5] qga: guest exec functionality

[Denis V. Lunev]

From: Yuri Pudgorodskiy <yur@virtuozzo.com>

Guest-exec rewriten in platform-independant style with glib spawn.

Child process is spawn asynchroneously 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>
---
 qga/commands.c       | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/qapi-schema.json |  57 +++++++++++++++++
 2 files changed, 225 insertions(+)

diff --git a/qga/commands.c b/qga/commands.c
index 7834967..6efd6aa 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -70,3 +70,171 @@ struct GuestAgentInfo *qmp_guest_info(Error **errp)
     qmp_for_each_command(qmp_command_info, info);
     return info;
 }
+
+struct GuestExecInfo {
+    GPid pid;
+    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 GuestExecInfo *guest_exec_info_add(GPid pid)
+{
+    GuestExecInfo *gei;
+
+    gei = g_malloc0(sizeof(*gei));
+    gei->pid = pid;
+    QTAILQ_INSERT_TAIL(&guest_exec_state.processes, gei, next);
+
+    return gei;
+}
+
+static GuestExecInfo *guest_exec_info_find(GPid pid)
+{
+    GuestExecInfo *gei;
+
+    QTAILQ_FOREACH(gei, &guest_exec_state.processes, next) {
+        if (gei->pid == pid) {
+            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((GPid)pid);
+    if (gei == NULL) {
+        error_setg(err, QERR_INVALID_PARAMETER, "pid");
+        return NULL;
+    }
+
+    ges = g_malloc0(sizeof(GuestExecStatus));
+    ges->exit = ges->signal = -1;
+
+    if (gei->finished) {
+        /* glib has no platform independent way to parse exit status */
+#ifdef G_OS_WIN32
+        if ((uint32_t)gei->status < 0xC0000000U) {
+            ges->exit = gei->status;
+        } else {
+            ges->signal = gei->status;
+        }
+#else
+        if (WIFEXITED(gei->status)) {
+            ges->exit = WEXITSTATUS(gei->status);
+        } else if (WIFSIGNALED(gei->status)) {
+            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: %u, status: %u",
+            (uint32_t)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_inp_data, const char *inp_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_malloc(sizeof(*ge));
+    ge->pid = (int64_t)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..ca9a633 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -930,3 +930,60 @@
 ##
 { 'command': 'guest-get-memory-block-info',
   'returns': 'GuestMemoryBlockInfo' }
+
+##
+# @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.  If a child process exited, "exit" is set
+#          to the exit code.  If a child process was killed by a signal,
+#          "signal" is set to the signal number. For Windows guest, "signal" is
+#          actually an unhandled exception code. If a child process is still
+#          running, both "exit" and "signal" are set to -1. If a guest cannot
+#          reliably detect exit signals, "signal" will be -1.
+#          'out-data' and 'err-data' contains captured data when guest-exec was
+#          called with 'capture-output' flag.
+#
+# Since: 2.5
+##
+{ 'struct': 'GuestExecStatus',
+  'data': { 'exit': 'int', 'signal': 'int',
+            '*out-data': 'str', '*err-data': 'str' }}
+
+{ '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
+# @inp-data: #optional data to be passed to process stdin (base64 encoded)
+# @capture-output: #optional bool flags to enable capture of
+#                  stdout/stderr of running process
+#
+# Returns: PID on success.
+#
+# Since: 2.5
+##
+{ 'command': 'guest-exec',
+  'data':    { 'path': 'str', '*arg': ['str'], '*env': ['str'],
+               '*inp-data': 'str', '*capture-output': 'bool' },
+  'returns': 'GuestExec' }
-- 
2.1.4

[Qemu-devel] [PATCH 4/5] qga: handle possible SIGPIPE in guest-file-write

[Denis V. Lunev]

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>
CC: 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 6efd6aa..199c7c3 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -194,6 +194,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,
@@ -219,7 +235,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 d8e063a..07e3c1c 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;
 }
 
-- 
2.1.4

[Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection

[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>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands.c       | 173 ++++++++++++++++++++++++++++++++++++++++++++++++---
 qga/qapi-schema.json |   9 ++-
 2 files changed, 172 insertions(+), 10 deletions(-)

diff --git a/qga/commands.c b/qga/commands.c
index 199c7c3..a85ec77 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,10 +76,23 @@ struct GuestAgentInfo *qmp_guest_info(Error **errp)
     return info;
 }
 
+struct GuestExecIOData {
+    guchar *data;
+    gsize size;
+    gsize length;
+    gint closed;
+    bool truncated;
+    const char *name;
+};
+
 struct GuestExecInfo {
     GPid pid;
     gint status;
-    bool finished;
+    bool has_output;
+    gint finished;
+    struct GuestExecIOData in;
+    struct GuestExecIOData out;
+    struct GuestExecIOData err;
     QTAILQ_ENTRY(GuestExecInfo) next;
 };
 typedef struct GuestExecInfo GuestExecInfo;
@@ -125,7 +143,16 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
     ges = g_malloc0(sizeof(GuestExecStatus));
     ges->exit = ges->signal = -1;
 
-    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);
+    }
+
+    if (finished) {
         /* glib has no platform independent way to parse exit status */
 #ifdef G_OS_WIN32
         if ((uint32_t)gei->status < 0xC0000000U) {
@@ -140,6 +167,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);
     }
@@ -210,6 +251,85 @@ static void guest_exec_task_setup(gpointer data)
 #endif
 }
 
+static gboolean guest_exec_input_watch(GIOChannel *ch,
+        GIOCondition cond, gpointer p_)
+{
+    struct GuestExecIOData *p = (struct 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 inp_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_)
+{
+    struct GuestExecIOData *p = (struct GuestExecIOData *)p_;
+    gsize bytes_read = 0;
+
+    if (cond == G_IO_HUP) {
+        g_io_channel_unref(ch);
+        g_atomic_int_set(&p->closed, 1);
+        return FALSE;
+    }
+
+    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) {
+            p->truncated = true;
+            /* ignore truncated output */
+            gchar buf[GUEST_EXEC_IO_SIZE];
+            g_io_channel_read_chars(ch, buf, sizeof(buf), &bytes_read, NULL);
+            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 */
+    g_io_channel_read_chars(ch, (gchar *)p->data + p->length,
+            p->size - p->length, &bytes_read, NULL);
+
+    p->length += bytes_read;
+
+    return TRUE;
+}
+
 GuestExec *qmp_guest_exec(const char *path,
                        bool has_arg, strList *arg,
                        bool has_env, strList *env,
@@ -224,6 +344,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;
@@ -231,11 +355,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_inp_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);
@@ -246,8 +373,40 @@ GuestExec *qmp_guest_exec(const char *path,
     ge->pid = (int64_t)pid;
 
     gei = guest_exec_info_add(pid);
+    gei->has_output = has_output;
     g_child_watch_add(pid, guest_exec_child_watch, gei);
 
+    if (has_inp_data) {
+        gei->in.data = g_base64_decode(inp_data, &gei->in.size);
+#ifdef G_OS_WIN32
+        in_ch = g_io_channel_win32_new_fd(in_ch);
+#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 ca9a633..d980360 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -945,14 +945,17 @@
 #          actually an unhandled exception code. If a child process is still
 #          running, both "exit" and "signal" are set to -1. If a guest cannot
 #          reliably detect exit signals, "signal" will be -1.
-#          'out-data' and 'err-data' contains captured data when guest-exec was
-#          called with 'capture-output' flag.
+#          'out-data' and 'err-data' contains captured stdout/stderr
+#          when guest-exec was called with 'capture-output' flag.
+#          If size of the output exceeds hardcoded limit, corresponding boolean
+#          'truncated' flag is set.
 #
 # Since: 2.5
 ##
 { 'struct': 'GuestExecStatus',
   'data': { 'exit': 'int', 'signal': 'int',
-            '*out-data': 'str', '*err-data': 'str' }}
+            '*out-data': 'str', '*err-data': 'str',
+            '*out-truncated': 'bool', '*err-truncated': 'bool' }}
 
 { 'command': 'guest-exec-status',
   'data':    { 'pid': 'int' },
-- 
2.1.4

Re: [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers

[Michael Roth]

Quoting Denis V. Lunev (2015-10-01 02:37:59)
> This just makes code shorter and better.

Can't complain with that.

> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>

Reviewed-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 b03c316..8989912 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 41bdd3f..3374678 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);
>  }
> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCH 2/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()

[Michael Roth]

Quoting Denis V. Lunev (2015-10-01 02:38:00)
> 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>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>

Reviewed-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 8aad4fe..7be92cc 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;
> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCH 3/5] qga: guest exec functionality

[Michael Roth]

Quoting Denis V. Lunev (2015-10-01 02:38:01)
> From: Yuri Pudgorodskiy <yur@virtuozzo.com>
> 
> Guest-exec rewriten in platform-independant style with glib spawn.
> 
> Child process is spawn asynchroneously 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>
> ---
>  qga/commands.c       | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  qga/qapi-schema.json |  57 +++++++++++++++++
>  2 files changed, 225 insertions(+)
> 
> diff --git a/qga/commands.c b/qga/commands.c
> index 7834967..6efd6aa 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -70,3 +70,171 @@ struct GuestAgentInfo *qmp_guest_info(Error **errp)
>      qmp_for_each_command(qmp_command_info, info);
>      return info;
>  }
> +
> +struct GuestExecInfo {
> +    GPid pid;
> +    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 GuestExecInfo *guest_exec_info_add(GPid pid)
> +{
> +    GuestExecInfo *gei;
> +
> +    gei = g_malloc0(sizeof(*gei));

gei = g_new0(GuestExecInfo, 1);

and same for all other g_malloc*(sizeof(...)) callers.

(Markus has been trying to get all prior g_malloc users converted over)

> +    gei->pid = pid;
> +    QTAILQ_INSERT_TAIL(&guest_exec_state.processes, gei, next);
> +
> +    return gei;
> +}
> +
> +static GuestExecInfo *guest_exec_info_find(GPid pid)
> +{
> +    GuestExecInfo *gei;
> +
> +    QTAILQ_FOREACH(gei, &guest_exec_state.processes, next) {
> +        if (gei->pid == pid) {
> +            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((GPid)pid);
> +    if (gei == NULL) {
> +        error_setg(err, QERR_INVALID_PARAMETER, "pid");
> +        return NULL;
> +    }
> +
> +    ges = g_malloc0(sizeof(GuestExecStatus));
> +    ges->exit = ges->signal = -1;
> +
> +    if (gei->finished) {
> +        /* glib has no platform independent way to parse exit status */
> +#ifdef G_OS_WIN32
> +        if ((uint32_t)gei->status < 0xC0000000U) {

Can you add a comment with a link to the documentation you referenced
for this? I assume this is actually for exceptions rather than normal
signals?

> +            ges->exit = gei->status;
> +        } else {
> +            ges->signal = gei->status;
> +        }
> +#else
> +        if (WIFEXITED(gei->status)) {
> +            ges->exit = WEXITSTATUS(gei->status);
> +        } else if (WIFSIGNALED(gei->status)) {
> +            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: %u, status: %u",
> +            (uint32_t)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_inp_data, const char *inp_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_malloc(sizeof(*ge));
> +    ge->pid = (int64_t)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..ca9a633 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -930,3 +930,60 @@
>  ##
>  { 'command': 'guest-get-memory-block-info',
>    'returns': 'GuestMemoryBlockInfo' }
> +
> +##
> +# @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.  If a child process exited, "exit" is set
> +#          to the exit code.  If a child process was killed by a signal,
> +#          "signal" is set to the signal number. For Windows guest, "signal" is
> +#          actually an unhandled exception code. If a child process is still
> +#          running, both "exit" and "signal" are set to -1. If a guest cannot
> +#          reliably detect exit signals, "signal" will be -1.
> +#          'out-data' and 'err-data' contains captured data when guest-exec was
> +#          called with 'capture-output' flag.
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'GuestExecStatus',
> +  'data': { 'exit': 'int', 'signal': 'int',
> +            '*out-data': 'str', '*err-data': 'str' }}

This structure should be documented separately, with descriptions for
it's individual fields.

exit/signal are somewhat ambiguous in terms of the running status of the
process. I think we should have an explicit 'exited': bool that users
can check to determine whether that process is still running or not.

> +
> +{ 'command': 'guest-exec-status',
> +  'data':    { 'pid': 'int' },

I would call this 'handle' since it aligns with the other interfaces and
gives us a bit more freedom if we decide not to expose actual
pids/HANDLEs.

> +  'returns': 'GuestExecStatus' }
> +
> +##
> +# @GuestExec:
> +# @pid: pid of child process in guest OS
> +#
> +#Since: 2.5
> +##
> +{ 'struct': 'GuestExec',
> +  'data': { 'pid': 'int'} }

Same here.

> +
> +##
> +# @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
> +# @inp-data: #optional data to be passed to process stdin (base64 encoded)
> +# @capture-output: #optional bool flags to enable capture of
> +#                  stdout/stderr of running process
> +#
> +# Returns: PID on success.

Returns GuestExec on success

> +#
> +# Since: 2.5
> +##
> +{ 'command': 'guest-exec',
> +  'data':    { 'path': 'str', '*arg': ['str'], '*env': ['str'],
> +               '*inp-data': 'str', '*capture-output': 'bool' },
> +  'returns': 'GuestExec' }

Would it make sense to just add handle (pid) to GuestExecStatus, and
have this return GuestExecStatus just the same as guest-exec-status
does? I'm not sure either way personally, just thought I'd mention it.

> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCH 4/5] qga: handle possible SIGPIPE in guest-file-write

[Michael Roth]

Quoting Denis V. Lunev (2015-10-01 02:38:02)
> 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>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>

Reviewed-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 6efd6aa..199c7c3 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -194,6 +194,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,
> @@ -219,7 +235,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 d8e063a..07e3c1c 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;
>  }
> 
> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCH 3/5] qga: guest exec functionality

[Yuri Pudgorodskiy]

On 10/2/2015 1:59 AM, Michael Roth wrote:
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'command': 'guest-exec',
>> +  'data':    { 'path': 'str', '*arg': ['str'], '*env': ['str'],
>> +               '*inp-data': 'str', '*capture-output': 'bool' },
>> +  'returns': 'GuestExec' }
> Would it make sense to just add handle (pid) to GuestExecStatus, and
> have this return GuestExecStatus just the same as guest-exec-status
> does? I'm not sure either way personally, just thought I'd mention it.
>

I do not think it's a good idea. GuestExecStatus contains mostly the 
data about
the finished exec. Having GuestExec returns same struct may make wrong
assumption that it can be synchronous - wait for exec to complete and 
return all
data in a single call.

Implementing synchronous GuestExec is not and easy job - either we occupy
host-guest channel for all time until task finished, which is bad or we 
need to
implement multiplexed messages for concurrent qga commands.