[Qemu-devel] [PATCH 0/2] [RFC] qemu-ga: add support for guest command execution

[Qemu-devel] [PATCH 0/2] [RFC] qemu-ga: add support for guest command execution

[Michael Roth]

The code is still in rough shape, but while we're on the topic of guest agents
I wanted to put out a working example of how exec functionality can be added
to qemu-ga to provide a mechansim for building arbitrarilly high-level
interfaces.

The hope is that by allowing qemu-ga to execute commands in the guest, paired
with file read/write access, we can instrument a guest "on the fly" to support
any type of hyperviser functionality, and do so without dramatically enlarging
the role qemu-ga plays as a small, QEMU-specific agent that is tightly
integrated with QEMU/QMP/libvirt.

These patches add the following interfaces:

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

The guest-file-open-pipe interface is analagous to the existing guest-file-open
interface (it might be best to roll it into it actually): it returns a handle
that can be handled via the existing guest-file-{read,write,flush,close}
interface. Internally it creates a FIFO pair that we can use to associate
handles to the stdin/stdout/stderr of a guest-exec spawned process. We can also
also use them to redirect output into other processes, giving us the basic
tools to build a basic shell (or a full-blown one if we add TTY support) using
a single qemu-ga.

Theoretically we can even deploy other agents, including session-level agents,
and communicate with them via these same handles. Thus, ovirt could deploy and
run an agent via qemu-ga, Spice could deploy vdagent, etc. Since the interface
is somewhat tedious, I'm working on a wrapper script to try out some of
these scenarios, but a basic use case using the raw QMP interface is included
below.

Any thoughts/comments on this approach are appreciated.

EXAMPLE USAGE (execute `top -b -n1`):

{'execute': 'guest-file-open-pipe'}
{'return': 6}

{'execute': 'guest-exec',                    \
 'arguments': {'detach': True,               \
               'handle_stdout': 6,           \
               'params': [{'param': '-b'},   \
                          {'param': '-n1'}], \
               'path': 'top'}}
{'return': {'exit-code': 0,                  \
            'exited': False,                 \
            'handle_stderr': -1,             \
            'handle_stdin': -1,              \
            'handle_stdout': 6,              \
            'pid': 14267}}

{'execute': 'guest-file-read',               \
 'arguments': {'count': 65536,               \
               'handle': 6}}
{'return': {'buf-b64': '',                   \
            'count': 0,                      \
            'eof': False}}

{'execute': 'guest-file-read',               \
 'arguments': {'count': 65536,               \
               'handle': 6}}
{'return': {'buf-b64': 'dG9wIC0gMjI6N...',   \
            'count': 11064,                  \
            'eof': True}}

/*
top - 22:41:49 up 1 day,  4:30,  3 users,  load average: 0.00, 0.00, 0.00
Tasks: 114 total,   1 running, 113 sleeping,   0 stopped,   0 zombie
Cpu(s):  0.2%us,  0.2%sy,  0.0%ni, 99.6%id,  0.0%wa,  0.0%hi,  0.0%si,  0.0%st
Mem:    504848k total,   445664k used,    59184k free,    49100k buffers
Swap:   323580k total,      224k used,   323356k free,   256392k cached

  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND
14267 root      20   0 19272 1248  924 R    2  0.2   0:00.02 top
    1 root      20   0 24008 2048 1280 S    0  0.4   0:00.85 init
    2 root      20   0     0    0    0 S    0  0.0   0:00.30 kthreadd
    3 root      20   0     0    0    0 S    0  0.0   0:01.09 ksoftirqd/0
...
*/

{'execute': 'guest-exec-status',             \
 'arguments': {'pid': 14267}}
{'return': {'exit-code': 0,                  \
            'exited': True,                  \
            'handle_stderr': -1,             \
            'handle_stdin': -1,              \
            'handle_stdout': 6,              \
            'pid': 14267}}

{'execute': 'guest-file-close'}              \
 'arguments': {'handle': 6}}
{'return': {}}

Michael Roth (2):
  guest agent: add guest-file-open-pipe
  guest agent: add guest-exec and guest-exec-status interfaces

 qapi-schema-guest.json     |   79 +++++++-
 qga/guest-agent-commands.c |  478 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 531 insertions(+), 26 deletions(-)

-- 
1.7.4.1

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

[Michael Roth]

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: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi-schema-guest.json     |   24 ++++++-
 qga/guest-agent-commands.c |  179 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 177 insertions(+), 26 deletions(-)

diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index fde5971..4c9f063 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -80,18 +80,40 @@
   'returns': 'int' }
 
 ##
+# @guest-file-open-pipe
+#
+# 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: 1.0.50
+##
+{ 'command': 'guest-file-open-pipe',
+  'returns': 'int' }
+
+##
 # @guest-file-close:
 #
 # Close an open file in the guest
 #
 # @handle: filehandle returned by guest-file-open
+# @pipe-end: #optional GuestFilePipeEnd value ("rw"/"w"/"r") to specify
+# which end of the pipe to close. Please note that closing the write
+# side of a pipe will block until the read side is closed. If you've
+# passed the read-side of the pipe to a qga-spawned process, make sure
+# the process as exited before attempting to close the write side.
 #
 # Returns: Nothing on success.
 #
 # Since: 0.15.0
 ##
+{ 'enum': 'GuestFilePipeEnd',
+  'data': [ 'r', 'w', 'rw' ] }
 { 'command': 'guest-file-close',
-  'data': { 'handle': 'int' } }
+  'data': { 'handle': 'int', '*pipe-end': 'GuestFilePipeEnd' } }
 
 ##
 # @guest-file-read:
diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
index 6da9904..ae77ee4 100644
--- a/qga/guest-agent-commands.c
+++ b/qga/guest-agent-commands.c
@@ -44,6 +44,34 @@ static void slog(const char *fmt, ...)
     va_end(ap);
 }
 
+static void 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(err, QERR_QGA_COMMAND_FAILED,
+                  "failed to fetch filehandle flags");
+        return;
+    }
+    ret = fcntl(fd, F_SETFL, set ? old_flags | flags : old_flags & ~flags);
+    if (ret == -1) {
+        error_set(err, QERR_QGA_COMMAND_FAILED,
+                  "failed to set filehandle flags");
+        return;
+    }
+}
+
+static void ftoggle_flags(FILE *fh, long flags, bool set, Error **err)
+{
+    int fd;
+    if (!fh || (fd = fileno(fh)) == -1) {
+        error_set(err, QERR_QGA_COMMAND_FAILED, "invalid filehandle");
+        return;
+    }
+    toggle_flags(fd, flags, set, err);
+}
+
 int64_t qmp_guest_sync(int64_t id, Error **errp)
 {
     return id;
@@ -102,7 +130,14 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
 
 typedef struct GuestFileHandle {
     uint64_t id;
-    FILE *fh;
+    bool is_pipe;
+    union {
+        FILE *fh;
+        struct {
+            FILE *in;
+            FILE *out;
+        } pipe;
+    } stream;
     QTAILQ_ENTRY(GuestFileHandle) next;
 } GuestFileHandle;
 
@@ -110,14 +145,31 @@ static struct {
     QTAILQ_HEAD(, GuestFileHandle) filehandles;
 } guest_file_state;
 
-static void guest_file_handle_add(FILE *fh)
+static uint64_t guest_file_handle_add(FILE *fh)
 {
     GuestFileHandle *gfh;
 
     gfh = g_malloc0(sizeof(GuestFileHandle));
     gfh->id = fileno(fh);
-    gfh->fh = fh;
+    gfh->is_pipe = false;
+    gfh->stream.fh = fh;
+
+    QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
+    return gfh->id;
+}
+
+static uint64_t guest_file_handle_add_pipe(FILE *in, FILE *out)
+{
+    GuestFileHandle *gfh;
+
+    gfh = g_malloc0(sizeof(GuestFileHandle));
+    gfh->id = fileno(in);
+    gfh->is_pipe = true;
+    gfh->stream.pipe.in = in;
+    gfh->stream.pipe.out = out;
+
     QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
+    return gfh->id;
 }
 
 static GuestFileHandle *guest_file_handle_find(int64_t id)
@@ -137,7 +189,6 @@ static GuestFileHandle *guest_file_handle_find(int64_t id)
 int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err)
 {
     FILE *fh;
-    int fd;
     int64_t ret = -1;
 
     if (!has_mode) {
@@ -153,39 +204,112 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, E
     /* 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_set(err, QERR_QGA_COMMAND_FAILED, "fcntl() failed");
+    ftoggle_flags(fh, O_NONBLOCK, true, err);
+    if (error_is_set(err)) {
         fclose(fh);
         return -1;
     }
 
-    guest_file_handle_add(fh);
-    slog("guest-file-open, handle: %d", fd);
-    return fd;
+    ret = guest_file_handle_add(fh);
+    slog("guest-file-open, handle: %ld", ret);
+    return ret;
 }
 
-void qmp_guest_file_close(int64_t handle, Error **err)
+int64_t qmp_guest_file_open_pipe(Error **err)
+{
+    FILE *fh[2];
+    int fd[2], i;
+    int64_t ret = -1;
+
+    slog("guest-file-open-pipe called");
+
+    ret = pipe(fd);
+    if (ret == -1) {
+        error_set(err, QERR_QGA_COMMAND_FAILED, "pipe() failed");
+        return -1;
+    }
+    for (i = 0; i < 2; i++) {
+        toggle_flags(fd[i], O_NONBLOCK, true, err);
+        if (error_is_set(err)) {
+            close(fd[i]);
+            return -1;
+        }
+    }
+
+    fh[0] = fdopen(fd[0], "r");
+    if (!fh[0]) {
+        error_set(err, QERR_OPEN_FILE_FAILED, "pipe (read-side)");
+        return -1;
+    }
+    fh[1] = fdopen(fd[1], "w");
+    if (!fh[1]) {
+        fclose(fh[0]);
+        error_set(err, QERR_OPEN_FILE_FAILED, "pipe (write-side)");
+        return -1;
+    }
+    ret = guest_file_handle_add_pipe(fh[1], fh[0]);
+    slog("guest-file-open-pipe, handle: %ld", ret);
+    return ret;
+}
+
+void qmp_guest_file_close(int64_t handle, bool has_pipe_end,
+                          GuestFilePipeEnd pipe_end, Error **err)
 {
     GuestFileHandle *gfh = guest_file_handle_find(handle);
     int ret;
+    bool remove = false;
 
-    slog("guest-file-close called, handle: %ld", handle);
+    slog("guest-file-close called, handle: %ld, pipe: %d, pipe_end: %d", handle,
+         has_pipe_end, pipe_end);
     if (!gfh) {
         error_set(err, QERR_FD_NOT_FOUND, "handle");
         return;
     }
 
-    ret = fclose(gfh->fh);
-    if (ret == -1) {
-        error_set(err, QERR_QGA_COMMAND_FAILED, "fclose() failed");
-        return;
+    if (gfh->is_pipe) {
+        if (!has_pipe_end) {
+            pipe_end = GUEST_FILE_PIPE_END_RW;
+        }
+        if ((pipe_end == GUEST_FILE_PIPE_END_RW ||
+             pipe_end == GUEST_FILE_PIPE_END_R) &&
+            gfh->stream.pipe.out) {
+            g_debug("closing pipe (read-side)...");
+            ret = fclose(gfh->stream.pipe.out);
+            g_debug("done closing pipe.");
+            if (ret == -1) {
+                error_set(err, QERR_INVALID_PARAMETER, "pipe (read-side)");
+                goto out_err;
+            }
+            gfh->stream.pipe.out = NULL;
+        }
+        if ((pipe_end == GUEST_FILE_PIPE_END_RW ||
+             pipe_end == GUEST_FILE_PIPE_END_W) &&
+            gfh->stream.pipe.in) {
+            g_debug("closing pipe (write-side)...");
+            ret = fclose(gfh->stream.pipe.in);
+            g_debug("done closing pipe.");
+            if (ret == -1) {
+                error_set(err, QERR_INVALID_PARAMETER, "pipe (write-side)");
+                goto out_err;
+            }
+            gfh->stream.pipe.in = NULL;
+        }
+        remove = !gfh->stream.pipe.in && !gfh->stream.pipe.out;
+    } else {
+        ret = fclose(gfh->stream.fh);
+        if (ret == -1) {
+            error_set(err, QERR_INVALID_PARAMETER, "filehandle");
+            goto out_err;
+        }
+        gfh->stream.fh = NULL;
+        remove = true;
     }
 
-    QTAILQ_REMOVE(&guest_file_state.filehandles, gfh, next);
-    g_free(gfh);
+out_err:
+    if (remove) {
+        QTAILQ_REMOVE(&guest_file_state.filehandles, gfh, next);
+        g_free(gfh);
+    }
 }
 
 struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
@@ -209,10 +333,10 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
         return NULL;
     }
 
-    fh = gfh->fh;
+    fh = gfh->is_pipe ? gfh->stream.pipe.out : gfh->stream.fh;
     buf = g_malloc0(count+1);
     read_count = fread(buf, 1, count, fh);
-    if (ferror(fh)) {
+    if (0 && read_count == 0 && !feof(fh) && ferror(fh)) {
         slog("guest-file-read failed, handle: %ld", handle);
         error_set(err, QERR_QGA_COMMAND_FAILED, "fread() failed");
     } else {
@@ -245,7 +369,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
         return NULL;
     }
 
-    fh = gfh->fh;
+    fh = gfh->is_pipe ? gfh->stream.pipe.in : gfh->stream.fh;
     buf = g_base64_decode(buf_b64, &buf_len);
 
     if (!has_count) {
@@ -284,7 +408,12 @@ struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset,
         return NULL;
     }
 
-    fh = gfh->fh;
+    if (gfh->is_pipe) {
+        error_set(err, QERR_QGA_COMMAND_FAILED, "cannot seek on a pipe");
+        return NULL;
+    }
+
+    fh = gfh->stream.fh;
     ret = fseek(fh, offset, whence);
     if (ret == -1) {
         error_set(err, QERR_QGA_COMMAND_FAILED, strerror(errno));
@@ -309,7 +438,7 @@ void qmp_guest_file_flush(int64_t handle, Error **err)
         return;
     }
 
-    fh = gfh->fh;
+    fh = gfh->is_pipe ? gfh->stream.pipe.in : gfh->stream.fh;
     ret = fflush(fh);
     if (ret == EOF) {
         error_set(err, QERR_QGA_COMMAND_FAILED, strerror(errno));
-- 
1.7.4.1

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

[Michael Roth]

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: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi-schema-guest.json     |   55 ++++++++
 qga/guest-agent-commands.c |  299 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 354 insertions(+), 0 deletions(-)

diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index 4c9f063..7bf3086 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -237,3 +237,58 @@
 ##
 { 'command': 'guest-fsfreeze-thaw',
   'returns': 'int' }
+
+##
+# @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
+# @wait: #optional whether to wait for completion or simply poll once.
+# Waiting is disallowed if a stdin/stdout/stderr handle was supplied
+# to guest-exec
+#
+# Returns: GuestExecStatus on success
+#
+{ 'type': 'GuestExecStatus',
+  'data': { 'pid': 'int', 'exited': 'bool', 'exit-code': 'int',
+            'handle_stdin': 'int', 'handle_stdout': 'int',
+            'handle_stderr': 'int' } }
+{ 'command': 'guest-exec-status',
+  'data':    { 'pid': 'int', '*wait': 'bool' },
+  'returns': 'GuestExecStatus' }
+
+##
+# @guest-exec:
+#
+# Execute a command in the guest
+#
+# If a pipe is associated with the resulting process, the
+# read/write/write sides of the process' stdin/stdout/stderr will
+# be transferred automatically, so no need to close them from the
+# client. If no handle is passed in for stdin/stdout/stderr, they
+# will be closed before executing the command.
+#
+# @path: path or executable name to execute
+# @params: #optional parameter list 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
+# @detach: #optional whether to detach the process or execute it
+# synchronously. If synchronous, passing of stdin/stdout/stderr handles
+# will be disallowed, since process can block on closing them and cause
+# a deadlock in the guest agent.
+#
+# Returns: GuestExecStatus on success.
+#
+# Since: 1.0.50
+##
+{ 'type': 'GuestExecParam',
+  'data': { 'param': 'str' } }
+{ 'command': 'guest-exec',
+  'data':    { 'path': 'str', '*params': ['GuestExecParam'],
+               '*handle_stdin': 'int', '*handle_stdout': 'int',
+               '*handle_stderr': 'int',
+               '*detach': 'bool' },
+  'returns': 'GuestExecStatus' }
diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
index ae77ee4..11f9d00 100644
--- a/qga/guest-agent-commands.c
+++ b/qga/guest-agent-commands.c
@@ -450,6 +450,304 @@ static void guest_file_init(void)
     QTAILQ_INIT(&guest_file_state.filehandles);
 }
 
+typedef struct GuestExecInfo {
+    pid_t pid;
+    char **params;
+    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, char **params,
+                                GuestFileHandle *in, GuestFileHandle *out,
+                                GuestFileHandle *error, Error **err)
+{
+    GuestExecInfo *gei;
+
+    gei = g_malloc0(sizeof(*gei));
+    gei->pid = pid;
+    gei->params = params;
+    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;
+}
+
+#include <sys/wait.h>
+#include <sys/types.h>
+GuestExecStatus *qmp_guest_exec_status(int64_t pid, bool has_wait,
+                                       bool wait, Error **err)
+{
+    GuestExecInfo *gei;
+    GuestExecStatus *ges;
+    int status, ret;
+    char **ptr;
+
+    slog("guest-exec-status called");
+
+    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_set(err, QERR_UNDEFINED_ERROR);
+        return NULL;
+    }
+
+    ges = g_malloc0(sizeof(*ges));
+    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->pid = gei->pid;
+    if (ret == 0) {
+        ges->exited = false;
+    } else {
+        ges->exited = true;
+        /* reap child info once user has successfully wait()'d */
+        QTAILQ_REMOVE(&guest_exec_state.processes, gei, next);
+        for (ptr = gei->params; ptr && *ptr != NULL; ptr++) {
+            g_free(*ptr);
+        }
+        g_free(gei->params);
+        g_free(gei);
+    }
+    return ges;
+}
+
+static char **extract_param_array(const char *path,
+                                  const GuestExecParamList *entry)
+{
+    const GuestExecParamList *head;
+    GuestExecParam *param_container;
+    int count = 2; /* reserve 2 for path and NULL terminator */
+    int i = 0;
+    char **param_array;
+
+    for (head = entry; head != NULL; head = head->next) {
+        param_container = head->value;
+        printf("value: %s\n", param_container->param);
+        count++;
+    }
+
+    param_array = g_malloc0((count) * sizeof(*param_array));
+    param_array[i++] = strdup(path);
+
+    for (head = entry; head != NULL; head = head->next) {
+        param_container = head->value;
+        /* NULL-terminated list, so if they passed us a NULL param dup
+         * in an emptry string to avoid client-induced memory leak
+         */
+        if (param_container->param) {
+            param_array[i] = strdup(param_container->param);
+        } else {
+            param_array[i] = strdup("");
+        }
+        i++;
+    }
+
+    return param_array;
+}
+
+GuestExecStatus *qmp_guest_exec(const char *path,
+                                bool has_params,
+                                GuestExecParamList *params,
+                                bool has_handle_stdin,
+                                int64_t handle_stdin,
+                                bool has_handle_stdout,
+                                int64_t handle_stdout,
+                                bool has_handle_stderr,
+                                int64_t handle_stderr,
+                                bool has_detach, bool detach,
+                                Error **err)
+{
+    int ret, fd;
+    Error *local_err = NULL;
+    GuestExecStatus *ges;
+    GuestFileHandle *gfh_stdin = NULL;
+    GuestFileHandle *gfh_stdout = NULL;
+    GuestFileHandle *gfh_stderr = NULL;
+    char **param_array;
+
+    slog("guest-exec called");
+
+    if (!has_detach) {
+        detach = false;
+    }
+
+    param_array = extract_param_array(path, has_params ? params : NULL);
+
+    /* processes can block writing to a pipe, so don't allow redirect
+     * to a pipe unless we're doing detached/asynchronous exec()
+     */
+    if (has_handle_stdin) {
+        gfh_stdin = guest_file_handle_find(handle_stdin);
+        if (!gfh_stdin) {
+            error_set(err, QERR_FD_NOT_FOUND, "handle_stdin");
+            return NULL;
+        }
+        if (!detach && gfh_stdin->is_pipe) {
+            error_set(err, QERR_QGA_COMMAND_FAILED,
+                      "can't use stdin pipe for non-detached exec()");
+            return NULL;
+        }
+    } else {
+        handle_stdin = -1;
+    }
+    if (has_handle_stdout) {
+        gfh_stdout = guest_file_handle_find(handle_stdout);
+        if (!gfh_stdout) {
+            error_set(err, QERR_FD_NOT_FOUND, "handle_stdout");
+            return NULL;
+        }
+        if (!detach && gfh_stdout->is_pipe) {
+            error_set(err, QERR_QGA_COMMAND_FAILED,
+                      "can't use stdout pipe for non-detached exec()");
+            return NULL;
+        }
+    } else {
+        handle_stdin = -1;
+    }
+    if (has_handle_stderr) {
+        gfh_stderr = guest_file_handle_find(handle_stderr);
+        if (!gfh_stderr) {
+            error_set(err, QERR_FD_NOT_FOUND, "handle_stderr");
+            return NULL;
+        }
+        if (!detach && gfh_stderr->is_pipe) {
+            error_set(err, QERR_QGA_COMMAND_FAILED,
+                      "can't use stderr pipe for non-detached exec()");
+            return NULL;
+        }
+    } else {
+        handle_stderr = -1;
+    }
+
+    ret = fork();
+    if (ret < 0) {
+        error_set(err, QERR_UNDEFINED_ERROR);
+        return NULL;
+    } else if (ret == 0) {
+        slog("guest-exec child spawned");
+        /* exec the command */
+        setsid();
+        if (has_handle_stdin) {
+            if (gfh_stdin->is_pipe) {
+                fclose(gfh_stdin->stream.pipe.in); /* parent writes to this */
+                fd = fileno(gfh_stdin->stream.pipe.out);
+            } else {
+                fd = fileno(gfh_stdin->stream.fh);
+            }
+            dup2(fd, STDIN_FILENO);
+            /* processes don't seem to like O_NONBLOCK std in/out/err */
+            toggle_flags(fd, O_NONBLOCK, false, err);
+            if (error_is_set(err)) {
+                return NULL;
+            }
+        } else {
+            fclose(stdin);
+        }
+        if (has_handle_stdout) {
+            if (gfh_stdout->is_pipe) {
+                fclose(gfh_stdout->stream.pipe.out); /* parent reads this end */
+                fd = fileno(gfh_stdout->stream.pipe.in);
+            } else {
+                fd = fileno(gfh_stdout->stream.fh);
+            }
+            dup2(fd, STDOUT_FILENO);
+            toggle_flags(fd, O_NONBLOCK, false, err);
+            if (error_is_set(err)) {
+                return NULL;
+            }
+        } else {
+            fclose(stdout);
+        }
+        if (has_handle_stderr) {
+            if (gfh_stderr->is_pipe) {
+                fclose(gfh_stderr->stream.pipe.out); /* parent reads this end */
+                fd = fileno(gfh_stderr->stream.pipe.in);
+            } else {
+                fd = fileno(gfh_stderr->stream.fh);
+            }
+            dup2(fd, STDERR_FILENO);
+            toggle_flags(fd, O_NONBLOCK, false, err);
+            if (error_is_set(err)) {
+                return NULL;
+            }
+        } else {
+            fclose(stderr);
+        }
+
+        ret = execvp(path, (char * const*)param_array);
+        if (ret) {
+            slog("guest-exec child failed: %s", strerror(errno));
+        }
+        exit(!!ret);
+    }
+
+    if (has_handle_stdin && gfh_stdin->is_pipe) {
+        /* child reads from this end, not us */
+        qmp_guest_file_close(gfh_stdin->id, true, GUEST_FILE_PIPE_END_R, err);
+        if (error_is_set(err)) {
+            return NULL;
+        }
+    }
+    if (has_handle_stdout && gfh_stdout->is_pipe) {
+        /* child writes to this end, not us */
+        qmp_guest_file_close(gfh_stdout->id, true, GUEST_FILE_PIPE_END_W, err);
+        if (error_is_set(err)) {
+            return NULL;
+        }
+    }
+    if (has_handle_stderr && gfh_stderr->is_pipe) {
+        /* child writes to this end, not us */
+        qmp_guest_file_close(gfh_stderr->id, true, GUEST_FILE_PIPE_END_W, err);
+        if (error_is_set(err)) {
+            return NULL;
+        }
+    }
+    guest_exec_info_add(ret, param_array, gfh_stdin, gfh_stdout, gfh_stderr,
+                        &local_err);
+    if (local_err) {
+        error_propagate(err, local_err);
+        return NULL;
+    }
+
+    /* return initial status, or wait for completion if !detach */
+    if (!detach) {
+        ges = qmp_guest_exec_status(ret, true, true, err);
+    } else {
+        ges = qmp_guest_exec_status(ret, true, false, err);
+    }
+    return ges;
+}
+
+static void guest_exec_init(void)
+{
+    QTAILQ_INIT(&guest_exec_state.processes);
+}
+
 #if defined(CONFIG_FSFREEZE)
 static void disable_logging(void)
 {
@@ -687,4 +985,5 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
     ga_command_state_add(cs, guest_fsfreeze_init, guest_fsfreeze_cleanup);
 #endif
     ga_command_state_add(cs, guest_file_init, NULL);
+    ga_command_state_add(cs, guest_exec_init, NULL);
 }
-- 
1.7.4.1

Re: [Qemu-devel] [PATCH 0/2] [RFC] qemu-ga: add support for guest command execution

[Daniel P. Berrange]

On Tue, Dec 06, 2011 at 08:34:06AM -0600, Michael Roth wrote:
> The code is still in rough shape, but while we're on the topic of guest agents
> I wanted to put out a working example of how exec functionality can be added
> to qemu-ga to provide a mechansim for building arbitrarilly high-level
> interfaces.
> 
> The hope is that by allowing qemu-ga to execute commands in the guest, paired
> with file read/write access, we can instrument a guest "on the fly" to support
> any type of hyperviser functionality, and do so without dramatically enlarging
> the role qemu-ga plays as a small, QEMU-specific agent that is tightly
> integrated with QEMU/QMP/libvirt.
> 
> These patches add the following interfaces:
> 
> guest-file-open-pipe
> guest-exec
> guest-exec-status
> 
> The guest-file-open-pipe interface is analagous to the existing guest-file-open
> interface (it might be best to roll it into it actually): it returns a handle
> that can be handled via the existing guest-file-{read,write,flush,close}
> interface. Internally it creates a FIFO pair that we can use to associate
> handles to the stdin/stdout/stderr of a guest-exec spawned process. We can also
> also use them to redirect output into other processes, giving us the basic
> tools to build a basic shell (or a full-blown one if we add TTY support) using
> a single qemu-ga.
> 
> Theoretically we can even deploy other agents, including session-level agents,
> and communicate with them via these same handles. Thus, ovirt could deploy and
> run an agent via qemu-ga, Spice could deploy vdagent, etc. Since the interface
> is somewhat tedious, I'm working on a wrapper script to try out some of
> these scenarios, but a basic use case using the raw QMP interface is included
> below.
> 
> Any thoughts/comments on this approach are appreciated.
> 
> EXAMPLE USAGE (execute `top -b -n1`):
> 
> {'execute': 'guest-file-open-pipe'}
> {'return': 6}
> 
> {'execute': 'guest-exec',                    \
>  'arguments': {'detach': True,               \
>                'handle_stdout': 6,           \
>                'params': [{'param': '-b'},   \
>                           {'param': '-n1'}], \
>                'path': 'top'}}

This feels like a rather verbose way of specifying
the ARGV. Why not just allow

  {'execute': 'guest-exec',                    \
   'arguments': {'detach': True,               \
                 'handle_stdout': 6,           \
                 'params': ['-b', '-n1'],      \
                 'path': 'top'}}

Or even


  {'execute': 'guest-exec',                    \
   'arguments': {'detach': True,               \
                 'handle_stdout': 6,           \
                 'argv': ['top', '-b', '-n1']}} \

and just use the first element of argv as the binary to
execute. Also you might need to set env variables for
some tools, so we'd want

  {'execute': 'guest-exec',                    \
   'arguments': {'detach': True,               \
                 'handle_stdout': 6,           \
                 'argv': ['top', '-b', '-n1'], \
                 'env' : ['TMPDIR=/wibble']}}

and perhaps also you might want to run as a non-root
user, so allow a username/groupname ?

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 0/2] [RFC] qemu-ga: add support for guest command execution

[Michael Roth]

On 12/06/2011 08:44 AM, Daniel P. Berrange wrote:
> On Tue, Dec 06, 2011 at 08:34:06AM -0600, Michael Roth wrote:
>> The code is still in rough shape, but while we're on the topic of guest agents
>> I wanted to put out a working example of how exec functionality can be added
>> to qemu-ga to provide a mechansim for building arbitrarilly high-level
>> interfaces.
>>
>> The hope is that by allowing qemu-ga to execute commands in the guest, paired
>> with file read/write access, we can instrument a guest "on the fly" to support
>> any type of hyperviser functionality, and do so without dramatically enlarging
>> the role qemu-ga plays as a small, QEMU-specific agent that is tightly
>> integrated with QEMU/QMP/libvirt.
>>
>> These patches add the following interfaces:
>>
>> guest-file-open-pipe
>> guest-exec
>> guest-exec-status
>>
>> The guest-file-open-pipe interface is analagous to the existing guest-file-open
>> interface (it might be best to roll it into it actually): it returns a handle
>> that can be handled via the existing guest-file-{read,write,flush,close}
>> interface. Internally it creates a FIFO pair that we can use to associate
>> handles to the stdin/stdout/stderr of a guest-exec spawned process. We can also
>> also use them to redirect output into other processes, giving us the basic
>> tools to build a basic shell (or a full-blown one if we add TTY support) using
>> a single qemu-ga.
>>
>> Theoretically we can even deploy other agents, including session-level agents,
>> and communicate with them via these same handles. Thus, ovirt could deploy and
>> run an agent via qemu-ga, Spice could deploy vdagent, etc. Since the interface
>> is somewhat tedious, I'm working on a wrapper script to try out some of
>> these scenarios, but a basic use case using the raw QMP interface is included
>> below.
>>
>> Any thoughts/comments on this approach are appreciated.
>>
>> EXAMPLE USAGE (execute `top -b -n1`):
>>
>> {'execute': 'guest-file-open-pipe'}
>> {'return': 6}
>>
>> {'execute': 'guest-exec',                    \
>>   'arguments': {'detach': True,               \
>>                 'handle_stdout': 6,           \
>>                 'params': [{'param': '-b'},   \
>>                            {'param': '-n1'}], \
>>                 'path': 'top'}}
>
> This feels like a rather verbose way of specifying
> the ARGV. Why not just allow
>
>    {'execute': 'guest-exec',                    \
>     'arguments': {'detach': True,               \
>                   'handle_stdout': 6,           \
>                   'params': ['-b', '-n1'],      \
>                   'path': 'top'}}
>
> Or even
>
>
>    {'execute': 'guest-exec',                    \
>     'arguments': {'detach': True,               \
>                   'handle_stdout': 6,           \
>                   'argv': ['top', '-b', '-n1']}} \
>

Agreed, this would look way nicer and is what I was shooting for 
initially. Unfortunately, the QAPI-generated QMP marshalling code, and 
the visitor interfaces it uses, expects lists to be of structured types. 
We might be able to special-case it for int and char* arrays though and 
open-code it though...

The problem I ran into when I looked at it though is that the QMP 
request is a QObject at that point that's contained inside a Visitor. So 
to manipulate it we'd have to extract the QList and manipulate it 
outside the visitor, which would be difficult since the Visitor is using 
a stack to manage it's traversal of the QObject based on the visitor 
calls we make, so it's difficult to pull it out of there...

Although, maybe we could just add a qmp_input_vistor_stack_top() that 
gives us the top of the stack, then call it at the appropriate point in 
the marshalling code and key into it to get qobj{'argv'} and whatnot. 
It's kinda hacky, but it's QMP-specific, and useful, so it might be 
worth it.

Alternatively, we could introduce another Visitor interface for handling 
lists of int64_t and char*.

So maybe it's not so bad, I'll look into it more.

> and just use the first element of argv as the binary to
> execute. Also you might need to set env variables for
> some tools, so we'd want
>
>    {'execute': 'guest-exec',                    \
>     'arguments': {'detach': True,               \
>                   'handle_stdout': 6,           \
>                   'argv': ['top', '-b', '-n1'], \
>                   'env' : ['TMPDIR=/wibble']}}
>
> and perhaps also you might want to run as a non-root
> user, so allow a username/groupname ?
>

Good idea, this would open up a lot of potential use cases (X 
session/display management, copy/paste, etc).

> Regards,
> Daniel

Re: [Qemu-devel] [PATCH 0/2] [RFC] qemu-ga: add support for guest command execution

[Alon Levy]

On Tue, Dec 06, 2011 at 10:43:42AM -0600, Michael Roth wrote:
> On 12/06/2011 08:44 AM, Daniel P. Berrange wrote:
> >On Tue, Dec 06, 2011 at 08:34:06AM -0600, Michael Roth wrote:
> >>The code is still in rough shape, but while we're on the topic of guest agents
> >>I wanted to put out a working example of how exec functionality can be added
> >>to qemu-ga to provide a mechansim for building arbitrarilly high-level
> >>interfaces.
> >>
> >>The hope is that by allowing qemu-ga to execute commands in the guest, paired
> >>with file read/write access, we can instrument a guest "on the fly" to support
> >>any type of hyperviser functionality, and do so without dramatically enlarging
> >>the role qemu-ga plays as a small, QEMU-specific agent that is tightly
> >>integrated with QEMU/QMP/libvirt.
> >>
> >>These patches add the following interfaces:
> >>
> >>guest-file-open-pipe
> >>guest-exec
> >>guest-exec-status
> >>
> >>The guest-file-open-pipe interface is analagous to the existing guest-file-open
> >>interface (it might be best to roll it into it actually): it returns a handle
> >>that can be handled via the existing guest-file-{read,write,flush,close}
> >>interface. Internally it creates a FIFO pair that we can use to associate
> >>handles to the stdin/stdout/stderr of a guest-exec spawned process. We can also
> >>also use them to redirect output into other processes, giving us the basic
> >>tools to build a basic shell (or a full-blown one if we add TTY support) using
> >>a single qemu-ga.
> >>
> >>Theoretically we can even deploy other agents, including session-level agents,
> >>and communicate with them via these same handles. Thus, ovirt could deploy and
> >>run an agent via qemu-ga, Spice could deploy vdagent, etc. Since the interface
> >>is somewhat tedious, I'm working on a wrapper script to try out some of
> >>these scenarios, but a basic use case using the raw QMP interface is included
> >>below.
> >>
> >>Any thoughts/comments on this approach are appreciated.

Is there a seperate transport (another virtio channel) for the launched
guest process? I'm thinking as usual on the copy/paste functionallity,
when passing a large buffer, I would not want to block any other future
qemu-ga command. Also I would like to avoid having to uuencode/decode
the traffic.

> >>
> >>EXAMPLE USAGE (execute `top -b -n1`):
> >>
> >>{'execute': 'guest-file-open-pipe'}
> >>{'return': 6}
> >>
> >>{'execute': 'guest-exec',                    \
> >>  'arguments': {'detach': True,               \
> >>                'handle_stdout': 6,           \
> >>                'params': [{'param': '-b'},   \
> >>                           {'param': '-n1'}], \
> >>                'path': 'top'}}
> >
> >This feels like a rather verbose way of specifying
> >the ARGV. Why not just allow
> >
> >   {'execute': 'guest-exec',                    \
> >    'arguments': {'detach': True,               \
> >                  'handle_stdout': 6,           \
> >                  'params': ['-b', '-n1'],      \
> >                  'path': 'top'}}
> >
> >Or even
> >
> >
> >   {'execute': 'guest-exec',                    \
> >    'arguments': {'detach': True,               \
> >                  'handle_stdout': 6,           \
> >                  'argv': ['top', '-b', '-n1']}} \
> >
> 
> Agreed, this would look way nicer and is what I was shooting for
> initially. Unfortunately, the QAPI-generated QMP marshalling code,
> and the visitor interfaces it uses, expects lists to be of
> structured types. We might be able to special-case it for int and
> char* arrays though and open-code it though...
> 
> The problem I ran into when I looked at it though is that the QMP
> request is a QObject at that point that's contained inside a
> Visitor. So to manipulate it we'd have to extract the QList and
> manipulate it outside the visitor, which would be difficult since
> the Visitor is using a stack to manage it's traversal of the QObject
> based on the visitor calls we make, so it's difficult to pull it out
> of there...
> 
> Although, maybe we could just add a qmp_input_vistor_stack_top()
> that gives us the top of the stack, then call it at the appropriate
> point in the marshalling code and key into it to get qobj{'argv'}
> and whatnot. It's kinda hacky, but it's QMP-specific, and useful, so
> it might be worth it.
> 
> Alternatively, we could introduce another Visitor interface for
> handling lists of int64_t and char*.
> 
> So maybe it's not so bad, I'll look into it more.
> 
> >and just use the first element of argv as the binary to
> >execute. Also you might need to set env variables for
> >some tools, so we'd want
> >
> >   {'execute': 'guest-exec',                    \
> >    'arguments': {'detach': True,               \
> >                  'handle_stdout': 6,           \
> >                  'argv': ['top', '-b', '-n1'], \
> >                  'env' : ['TMPDIR=/wibble']}}
> >
> >and perhaps also you might want to run as a non-root
> >user, so allow a username/groupname ?
> >
> 
> Good idea, this would open up a lot of potential use cases (X
> session/display management, copy/paste, etc).
> 
> >Regards,
> >Daniel
> 
>