[Qemu-devel] [PATCH 0/3] qemu: guest agent: implement guest-exec command for Linux

[Qemu-devel] [PATCH 0/3] qemu: guest agent: implement guest-exec command for Linux

[Denis V. Lunev]

These patches for guest-agent add the functionality to execute commands on
a guest UNIX machine.

These patches add the following interfaces:

guest-pipe-open
guest-exec
guest-exec-status

With these interfaces it's possible to:

* Open an anonymous pipe and work with it as with a file using already
implemented interfaces guest-file-{read,write,flush,close}.

* Execute a binary or a script on a guest machine.
We can pass arbitrary arguments and environment to a new child process.

* Pass redirected IO from/to stdin, stdout, stderr from a child process to a
local file or an anonymous pipe.

* Check the status of a child process, get its exit status if it exited or get
signal number if it was killed.

We plan to add support for Windows in the near future using the same interfaces
we introduce here.

Example of usage:

{"execute": "guest-pipe-open", "arguments":{"mode": "r"}}
{'return': 1000}

{"execute":"guest-exec", "arguments":{ "path": "id", "params": ["user"],
    "env": ["MYENV=myvalue"], "handle_stdout": 1000 }}'
{"return": 2636}

{"execute": "guest-exec-status", "arguments": {"pid": 2636}}
{"return":{"exit":0,"handle_stderr":-1,"handle_stdin":-1,"handle_stdout":1000,"signal":-1}}

{"execute": "guest-file-read", "arguments": {"handle": 1000, "count":128}}
{"return":{"count":58,"buf-b64":"dWlk...","eof":true}}

{"execute": "guest-file-close", "arguments": {"handle": 1000}}
{"return":{}}


These patches are based on the patches proposed by Michael Roth in 2011:
http://lists.gnu.org/archive/html/qemu-devel/2011-12/msg00722.html

We made several modifications to the interfaces proposed by Michael to simplify
their usage and we also added several new functions:

* Arguments to an executable file are passed as an array of strings.

    Before that we had to pass parameters like this:
    'params': [ {'param': '-b'}, {'param': '-n1'} ]

    Now we may pass them just as an array of strings:
    'params': [ '-b', '-n1' ]

* Environment can be passed to a child process.

    "env": ["MYENV=myvalue"]

* Removed "detach" argument from "guest-exec" - it never waits for a child
process to signal.  With this change it's possible to return just PID from
"guest-exec", a user must call "guest-exec-status" to get the status of a child.

* Removed "wait" argument from "guest-exec-status" - waiting for a child process
to signal is dangerous, because it may block the whole qemu-ga.  Instead, the
command polls just once a status of a child.

* "guest-exec-status" returns exit status of a child process or a signal number,
in case a process was killed.

* Simplified the command "guest-pipe-open" - the way how it stores the pipe
fd's.  No additional logic is needed about which end of pipe should be closed
with "guest-file-close".  This way we avoid modifying the interface of the
existing "guest-file-close".

In the conversation about the original patches there was a suggestion to merge
"path" and "params" into one single list.  But we didn't do so, because having
separate "path" is similar to exec*() family functions in UNIX.  That way it
looks more consistent.

Signed-off-by: Semen Zolin <szolin@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>

[Qemu-devel] [PATCH 1/3] guest agent: guest-file-open: refactoring

[Denis V. Lunev]

From: Simon Zolin <szolin@parallels.com>

Moved the code that sets non-blocking flag on fd into a separate function.

Signed-off-by: Simon Zolin <szolin@parallels.com>
Acked-by: Roman Kagan <rkagan@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index f6f3e3c..fd746db 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -376,13 +376,33 @@ safe_open_or_create(const char *path, const char *mode, Error **errp)
     return NULL;
 }
 
+static int guest_file_toggle_flags(int fd, long flags, bool set, Error **err)
+{
+    int ret, old_flags;
+
+    old_flags = fcntl(fd, F_GETFL);
+    if (old_flags == -1) {
+        error_set_errno(err, errno, QERR_QGA_COMMAND_FAILED,
+                        "failed to fetch filehandle flags");
+        return -1;
+    }
+
+    ret = fcntl(fd, F_SETFL, set ? (old_flags | flags) : (old_flags & ~flags));
+    if (ret == -1) {
+        error_set_errno(err, errno, QERR_QGA_COMMAND_FAILED,
+                        "failed to set filehandle flags");
+        return -1;
+    }
+
+    return ret;
+}
+
 int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
                             Error **errp)
 {
     FILE *fh;
     Error *local_err = NULL;
-    int fd;
-    int64_t ret = -1, handle;
+    int64_t handle;
 
     if (!has_mode) {
         mode = "r";
@@ -397,12 +417,7 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
     /* set fd non-blocking to avoid common use cases (like reading from a
      * named pipe) from hanging the agent
      */
-    fd = fileno(fh);
-    ret = fcntl(fd, F_GETFL);
-    ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
-    if (ret == -1) {
-        error_setg_errno(errp, errno, "failed to make file '%s' non-blocking",
-                         path);
+    if (guest_file_toggle_flags(fileno(fh), O_NONBLOCK, true, errp) < 0) {
         fclose(fh);
         return -1;
     }
-- 
1.9.1

[Qemu-devel] [PATCH 2/3] guest agent: add guest-pipe-open

[Denis V. Lunev]

From: Simon Zolin <szolin@parallels.com>

Creates a FIFO pair that can be used with existing file read/write
interfaces to communicate with processes spawned via the forthcoming
guest-file-exec interface.

Signed-off-by: Simon Zolin <szolin@parallels.com>
Acked-by: Roman Kagan <rkagan@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 qga/commands-win32.c |  8 ++++-
 qga/qapi-schema.json | 21 ++++++++++++++
 3 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index fd746db..bf14518 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -208,9 +208,11 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
     }
 }
 
+
 typedef struct GuestFileHandle {
     uint64_t id;
     FILE *fh;
+    int pipe_other_end_fd; /* if set, it's a pipe fd of the other end. */
     QTAILQ_ENTRY(GuestFileHandle) next;
 } GuestFileHandle;
 
@@ -218,7 +220,8 @@ static struct {
     QTAILQ_HEAD(, GuestFileHandle) filehandles;
 } guest_file_state;
 
-static int64_t guest_file_handle_add(FILE *fh, Error **errp)
+static int64_t guest_file_handle_add(FILE *fh, int pipe_other_end_fd,
+                                     Error **errp)
 {
     GuestFileHandle *gfh;
     int64_t handle;
@@ -231,6 +234,7 @@ static int64_t guest_file_handle_add(FILE *fh, Error **errp)
     gfh = g_malloc0(sizeof(GuestFileHandle));
     gfh->id = handle;
     gfh->fh = fh;
+    gfh->pipe_other_end_fd = pipe_other_end_fd;
     QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
 
     return handle;
@@ -422,7 +426,7 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
         return -1;
     }
 
-    handle = guest_file_handle_add(fh, errp);
+    handle = guest_file_handle_add(fh, -1, errp);
     if (handle < 0) {
         fclose(fh);
         return -1;
@@ -432,6 +436,75 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
     return handle;
 }
 
+int64_t qmp_guest_pipe_open(const char *mode, Error **err)
+{
+    FILE *f = NULL;
+    int fd[2], this_end, other_end;
+    int64_t handle;
+
+    slog("guest-pipe-open called");
+
+    if ((mode[0] != 'r' && mode[0] != 'w') || mode[1] != '\0') {
+        error_set(err, ERROR_CLASS_GENERIC_ERROR,
+                  "Only \"r\" or \"w\" are the valid modes to open a pipe");
+        return -1;
+    }
+
+    if (pipe(fd) != 0) {
+        error_set_errno(err, errno, QERR_QGA_COMMAND_FAILED, "pipe() failed");
+        return -1;
+    }
+
+    this_end = (mode[0] == 'w');
+    other_end = !this_end;
+
+    if (guest_file_toggle_flags(fd[this_end], O_NONBLOCK, true, err) == -1) {
+        goto fail;
+    }
+
+    qemu_set_cloexec(fd[this_end]);
+    qemu_set_cloexec(fd[other_end]);
+
+    f = fdopen(fd[this_end], mode);
+    if (f == NULL) {
+        error_set_errno(err, errno, QERR_QGA_COMMAND_FAILED,
+                        "fdopen() failed to open pipe handle");
+        goto fail;
+    }
+
+    handle = guest_file_handle_add(f, fd[other_end], err);
+    if (handle == -1) {
+        goto fail;
+    }
+
+    slog("guest-pipe-open: handle: %" PRId64, handle);
+
+    return handle;
+
+fail:
+    if (f != NULL) {
+        fclose(f);
+    } else {
+        close(fd[this_end]);
+    }
+    close(fd[other_end]);
+
+    return -1;
+}
+
+static int guest_pipe_close_other_end(GuestFileHandle *gfh)
+{
+    if (gfh->pipe_other_end_fd != -1) {
+        if (close(gfh->pipe_other_end_fd) != 0) {
+            return 1;
+        }
+
+        gfh->pipe_other_end_fd = -1;
+    }
+
+    return 0;
+}
+
 void qmp_guest_file_close(int64_t handle, Error **errp)
 {
     GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
@@ -442,6 +515,11 @@ void qmp_guest_file_close(int64_t handle, Error **errp)
         return;
     }
 
+    if (guest_pipe_close_other_end(gfh) != 0) {
+        error_setg_errno(errp, errno, "failed to close pipe handle");
+        return;
+    }
+
     ret = fclose(gfh->fh);
     if (ret == EOF) {
         error_setg_errno(errp, errno, "failed to close handle");
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 3bcbeae..ae4c98a 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -120,6 +120,12 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
     return 0;
 }
 
+int64_t qmp_guest_pipe_open(const char *mode, Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return 0;
+}
+
 void qmp_guest_file_close(int64_t handle, Error **errp)
 {
     error_set(errp, QERR_UNSUPPORTED);
@@ -455,7 +461,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
         "guest-suspend-hybrid", "guest-network-get-interfaces",
         "guest-get-vcpus", "guest-set-vcpus",
         "guest-fsfreeze-freeze-list", "guest-get-fsinfo",
-        "guest-fstrim", NULL};
+        "guest-fstrim", "guest-pipe-open", NULL};
     char **p = (char **)list_unsupported;
 
     while (*p) {
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 376e79f..67d3b72 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -212,12 +212,33 @@
   'returns': 'int' }
 
 ##
+# @guest-pipe-open
+#
+# Open a pipe to in the guest to associated with a qga-spawned processes
+# for communication.
+#
+# Returns: Guest file handle on success, as per guest-file-open. This
+# handle is useable with the same interfaces as a handle returned by
+# guest-file-open.
+#
+# Since: 2.3
+##
+{ 'command': 'guest-pipe-open',
+  'data':    { 'mode': 'str' },
+  'returns': 'int' }
+
+##
 # @guest-file-close:
 #
 # Close an open file in the guest
 #
 # @handle: filehandle returned by guest-file-open
 #
+# Please note that closing the write side of a pipe will block until the read
+# side is closed.  If you passed the read-side of the pipe to a qga-spawned
+# process, make sure the process has exited before attempting to close the
+# write side.
+#
 # Returns: Nothing on success.
 #
 # Since: 0.15.0
-- 
1.9.1

[Qemu-devel] [PATCH 3/3] guest agent: add guest-exec and guest-exec-status interfaces

[Denis V. Lunev]

From: Simon Zolin <szolin@parallels.com>

Interfaces to execute/manage processes in the guest. Child process'
stdin/stdout/stderr can be associated with handles for communication
via read/write interfaces.

Signed-off-by: Simon Zolin <szolin@parallels.com>
Acked-by: Roman Kagan <rkagan@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 259 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/commands-win32.c |  21 ++++-
 qga/qapi-schema.json |  47 ++++++++++
 3 files changed, 326 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index bf14518..5872196 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -842,6 +842,264 @@ static void build_fs_mount_list(FsMountList *mounts, Error **errp)
 }
 #endif
 
+typedef struct GuestExecInfo {
+    pid_t pid;
+    GuestFileHandle *gfh_stdin;
+    GuestFileHandle *gfh_stdout;
+    GuestFileHandle *gfh_stderr;
+    QTAILQ_ENTRY(GuestExecInfo) next;
+} GuestExecInfo;
+
+static struct {
+    QTAILQ_HEAD(, GuestExecInfo) processes;
+} guest_exec_state;
+
+static void guest_exec_info_add(pid_t pid,
+                                GuestFileHandle *in, GuestFileHandle *out,
+                                GuestFileHandle *error)
+{
+    GuestExecInfo *gei;
+
+    gei = g_malloc0(sizeof(*gei));
+    gei->pid = pid;
+    gei->gfh_stdin = in;
+    gei->gfh_stdout = out;
+    gei->gfh_stderr = error;
+    QTAILQ_INSERT_TAIL(&guest_exec_state.processes, gei, next);
+}
+
+static GuestExecInfo *guest_exec_info_find(pid_t 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;
+    int status, ret;
+
+    slog("guest-exec-status called, pid: %u", (uint32_t)pid);
+
+    gei = guest_exec_info_find(pid);
+    if (gei == NULL) {
+        error_set(err, QERR_INVALID_PARAMETER, "pid");
+        return NULL;
+    }
+
+    ret = waitpid(gei->pid, &status, WNOHANG);
+    if (ret == -1) {
+        error_setg_errno(err, errno, "waitpid() failed, pid: %u", gei->pid);
+        return NULL;
+    }
+
+    ges = g_malloc0(sizeof(GuestExecStatus));
+    ges->handle_stdin = gei->gfh_stdin ? gei->gfh_stdin->id : -1;
+    ges->handle_stdout = gei->gfh_stdout ? gei->gfh_stdout->id : -1;
+    ges->handle_stderr = gei->gfh_stderr ? gei->gfh_stderr->id : -1;
+    ges->exit = -1;
+    ges->signal = -1;
+
+    if (ret != 0) {
+        if (WIFEXITED(status)) {
+            ges->exit = WEXITSTATUS(status);
+
+        } else if (WIFSIGNALED(status)) {
+            ges->signal = WTERMSIG(status);
+        }
+
+        QTAILQ_REMOVE(&guest_exec_state.processes, gei, next);
+        g_free(gei);
+    }
+
+    return ges;
+}
+
+/* Get environment variables array for execve(). */
+static char **guest_exec_get_envp(const strList *env)
+{
+    const strList *it;
+    char **envp;
+    size_t i = 0, count = 1;
+
+    for (it = env; it != NULL; it = it->next) {
+        count++;
+    }
+
+    envp = g_malloc(count * sizeof(char *));
+
+    for (it = env; it != NULL; it = it->next) {
+        envp[i++] = it->value;
+    }
+
+    envp[i] = NULL;
+    return envp;
+}
+
+/* Get array of arguments for execve().
+ * @argv_str: arguments in one line separated by space. */
+static char **guest_exec_get_argv(const char *path, const strList *entry,
+                                  char **argv_str)
+{
+    const strList *it;
+    int i = 0, count = 2; /* reserve 2 for path and NULL terminator */
+    size_t argv_str_size;
+    char **argv;
+
+    argv_str_size = strlen(path) + 1;
+    for (it = entry; it != NULL; it = it->next) {
+        count++;
+        argv_str_size += sizeof(" ") - 1 + strlen(it->value);
+    }
+
+    *argv_str = g_malloc(argv_str_size);
+    pstrcpy(*argv_str, argv_str_size, path);
+
+    argv = g_malloc(count * sizeof(char *));
+    argv[i++] = (char *)path;
+
+    for (it = entry; it != NULL; it = it->next) {
+        argv[i++] = it->value;
+        pstrcat(*argv_str, argv_str_size, " ");
+        pstrcat(*argv_str, argv_str_size, it->value);
+    }
+
+    argv[i] = NULL;
+    return argv;
+}
+
+static int guest_exec_set_std(GuestFileHandle *gfh, int std_fd, int fd_null)
+{
+    int fd;
+
+    if (gfh == NULL) {
+        fd = fd_null;
+
+    } else if (gfh->pipe_other_end_fd != -1) {
+        fd = gfh->pipe_other_end_fd;
+
+    } else {
+        fd = fileno(gfh->fh);
+    }
+
+    if (dup2(fd, std_fd) == -1) {
+        slog("dup2() failed: %s", strerror(errno));
+        return 1;
+    }
+
+    return 0;
+}
+
+int64_t qmp_guest_exec(const char *path,
+                       bool has_params, strList *params,
+                       bool has_env, strList *env,
+                       bool has_handle_stdin, int64_t handle_stdin,
+                       bool has_handle_stdout, int64_t handle_stdout,
+                       bool has_handle_stderr, int64_t handle_stderr,
+                       Error **err)
+{
+    pid_t pid = -1;
+    int fd_null;
+    GuestFileHandle *gfh_stdin = NULL, *gfh_stdout = NULL, *gfh_stderr = NULL;
+    char **argv, *argv_str, **envp;
+
+    argv = guest_exec_get_argv(path, has_params ? params : NULL, &argv_str);
+
+    slog("guest-exec called: \"%s\"", argv_str);
+    g_free(argv_str);
+
+    envp = guest_exec_get_envp(has_env ? env : NULL);
+
+    if (has_handle_stdin) {
+        gfh_stdin = guest_file_handle_find(handle_stdin, err);
+        if (gfh_stdin == NULL) {
+            goto done;
+        }
+    }
+
+    if (has_handle_stdout) {
+        gfh_stdout = guest_file_handle_find(handle_stdout, err);
+        if (gfh_stdout == NULL) {
+            goto done;
+        }
+    }
+
+    if (has_handle_stderr) {
+        gfh_stderr = guest_file_handle_find(handle_stderr, err);
+        if (gfh_stderr == NULL) {
+            goto done;
+        }
+    }
+
+    pid = fork();
+    if (pid < 0) {
+        error_set(err, QERR_UNDEFINED_ERROR);
+        goto done;
+
+    } else if (pid == 0) {
+
+        setsid();
+
+        fd_null = -1;
+        if (!has_handle_stdin || !has_handle_stdout || !has_handle_stderr) {
+            fd_null = open("/dev/null", O_RDWR);
+            if (fd_null == -1) {
+                slog("guest-exec: couldn't open /dev/null: %s",
+                     strerror(errno));
+                exit(1);
+            }
+        }
+
+        if (guest_exec_set_std(gfh_stdin, STDIN_FILENO, fd_null) != 0 ||
+            guest_exec_set_std(gfh_stdout, STDOUT_FILENO, fd_null) != 0 ||
+            guest_exec_set_std(gfh_stderr, STDERR_FILENO, fd_null) != 0) {
+
+            exit(1);
+        }
+
+        if (fd_null != -1 && close(fd_null) != 0) {
+            slog("guest-exec: couldn't close /dev/null: %s", strerror(errno));
+            /* exit(1); */
+        }
+
+        execvpe(path, (char * const *)argv, (char * const *)envp);
+        slog("guest-exec child failed: %s", strerror(errno));
+        exit(1);
+    }
+
+    if (gfh_stdin != NULL && guest_pipe_close_other_end(gfh_stdin) != 0) {
+        slog("close() failed to close stdin pipe handle: %s", strerror(errno));
+    }
+
+    if (gfh_stdout != NULL && guest_pipe_close_other_end(gfh_stdout) != 0) {
+        slog("close() failed to close stdout pipe handle: %s", strerror(errno));
+    }
+
+    if (gfh_stderr != NULL && guest_pipe_close_other_end(gfh_stderr) != 0) {
+        slog("close() failed to close stderr pipe handle: %s", strerror(errno));
+    }
+
+    guest_exec_info_add(pid, gfh_stdin, gfh_stdout, gfh_stderr);
+
+done:
+    g_free(argv);
+    g_free(envp);
+    return pid;
+}
+
+static void guest_exec_init(void)
+{
+    QTAILQ_INIT(&guest_exec_state.processes);
+}
+
 #if defined(CONFIG_FSFREEZE)
 
 static char *get_pci_driver(char const *syspath, int pathlen, Error **errp)
@@ -2096,4 +2354,5 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
     ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
 #endif
     ga_command_state_add(cs, guest_file_init, NULL);
+    ga_command_state_add(cs, guest_exec_init, NULL);
 }
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index ae4c98a..b2a6647 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -158,6 +158,24 @@ void qmp_guest_file_flush(int64_t handle, Error **errp)
     error_set(errp, QERR_UNSUPPORTED);
 }
 
+GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return 0;
+}
+
+int64_t qmp_guest_exec(const char *path,
+                       bool has_params, strList *params,
+                       bool has_env, strList *env,
+                       bool has_handle_stdin, int64_t handle_stdin,
+                       bool has_handle_stdout, int64_t handle_stdout,
+                       bool has_handle_stderr, int64_t handle_stderr,
+                       Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return 0;
+}
+
 GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
 {
     error_set(errp, QERR_UNSUPPORTED);
@@ -461,7 +479,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
         "guest-suspend-hybrid", "guest-network-get-interfaces",
         "guest-get-vcpus", "guest-set-vcpus",
         "guest-fsfreeze-freeze-list", "guest-get-fsinfo",
-        "guest-fstrim", "guest-pipe-open", NULL};
+        "guest-fstrim", "guest-pipe-open", "guest-exec-status",
+        "guest-exec", NULL};
     char **p = (char **)list_unsupported;
 
     while (*p) {
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 67d3b72..ea09565 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -759,3 +759,50 @@
 ##
 { 'command': 'guest-get-fsinfo',
   'returns': ['GuestFilesystemInfo'] }
+
+##
+# @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.  If a child process is still
+#          running, both "exit" and "signal" are set to -1.  On Windows,
+#          "signal" is always set to -1.
+#
+# Since: 2.3
+##
+{ 'type': 'GuestExecStatus',
+  'data': { 'exit': 'int', 'signal': 'int',
+            'handle_stdin': 'int', 'handle_stdout': 'int',
+            'handle_stderr': 'int' } }
+
+{ 'command': 'guest-exec-status',
+  'data':    { 'pid': 'int' },
+  'returns': 'GuestExecStatus' }
+
+##
+# @guest-exec:
+#
+# Execute a command in the guest
+#
+# @path: path or executable name to execute
+# @params: #optional parameter list to pass to executable
+# @env: #optional environment variables to pass to executable
+# @handle_stdin: #optional handle to associate with process' stdin.
+# @handle_stdout: #optional handle to associate with process' stdout
+# @handle_stderr: #optional handle to associate with process' stderr
+#
+# Returns: PID on success.
+#
+# Since: 2.3
+##
+{ 'command': 'guest-exec',
+  'data':    { 'path': 'str', '*params': ['str'], '*env': ['str'],
+               '*handle_stdin': 'int', '*handle_stdout': 'int',
+               '*handle_stderr': 'int' },
+  'returns': 'int' }
-- 
1.9.1

Re: [Qemu-devel] [PATCH 2/3] guest agent: add guest-pipe-open

[Denis V. Lunev]

On 22/12/14 12:06, Denis V. Lunev wrote:
> From: Simon Zolin <szolin@parallels.com>
>
> Creates a FIFO pair that can be used with existing file read/write
> interfaces to communicate with processes spawned via the forthcoming
> guest-file-exec interface.
>
> Signed-off-by: Simon Zolin <szolin@parallels.com>
> Acked-by: Roman Kagan <rkagan@parallels.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>

hmm, it looks like we have missed SIGPIPE handling,
which could kill the agent. Though the same applies
for the agent even without this patch if someone
will open named pipe on the filesystem and perform
writing.

[Qemu-devel] [PATCH 4/3] guest agent: ignore SIGPIPE signal

[Denis V. Lunev]

From: Simon Zolin <szolin@parallels.com>

If write operation fails on a pipe whose reading end is closed, qemu-ga
won't be terminated, but instead write() will fail with error EPIPE.

execve() inherits signals that are ignored, so reset SIGPIPE to its default
handler before calling execve() in a forked process.

Signed-off-by: Simon Zolin <szolin@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 16 ++++++++++++++++
 qga/main.c           |  6 ++++++
 2 files changed, 22 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 5872196..93aed2e 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -998,6 +998,20 @@ static int guest_exec_set_std(GuestFileHandle *gfh, int std_fd, int fd_null)
     return 0;
 }
 
+/** Reset ignored signals back to default. */
+static void guest_exec_reset_child_sig(void)
+{
+    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));
+    }
+}
+
 int64_t qmp_guest_exec(const char *path,
                        bool has_params, strList *params,
                        bool has_env, strList *env,
@@ -1070,6 +1084,8 @@ int64_t qmp_guest_exec(const char *path,
             /* exit(1); */
         }
 
+        guest_exec_reset_child_sig();
+
         execvpe(path, (char * const *)argv, (char * const *)envp);
         slog("guest-exec child failed: %s", strerror(errno));
         exit(1);
diff --git a/qga/main.c b/qga/main.c
index 9939a2b..bc6414c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -160,6 +160,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