* [Qemu-devel] [PATCH 0/5] simplified QEMU guest exec
@ 2015-10-01 7:37 Denis V. Lunev
2015-10-01 7:37 ` [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers Denis V. Lunev
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-01 7:37 UTC (permalink / raw)
Cc: Denis V. Lunev, Yuri Pudgorodskiy, qemu-devel, Michael Roth
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers
2015-10-01 7:37 [Qemu-devel] [PATCH 0/5] simplified QEMU guest exec Denis V. Lunev
@ 2015-10-01 7:37 ` Denis V. Lunev
2015-10-01 21:46 ` Michael Roth
2015-10-01 7:38 ` [Qemu-devel] [PATCH 2/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all() Denis V. Lunev
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-01 7:37 UTC (permalink / raw)
Cc: Denis V. Lunev, Yuri Pudgorodskiy, qemu-devel, Michael Roth
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()
2015-10-01 7:37 [Qemu-devel] [PATCH 0/5] simplified QEMU guest exec Denis V. Lunev
2015-10-01 7:37 ` [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers Denis V. Lunev
@ 2015-10-01 7:38 ` Denis V. Lunev
2015-10-01 21:54 ` Michael Roth
2015-10-01 7:38 ` [Qemu-devel] [PATCH 3/5] qga: guest exec functionality Denis V. Lunev
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-01 7:38 UTC (permalink / raw)
Cc: Denis V. Lunev, Yuri Pudgorodskiy, qemu-devel, Michael Roth
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 3/5] qga: guest exec functionality
2015-10-01 7:37 [Qemu-devel] [PATCH 0/5] simplified QEMU guest exec Denis V. Lunev
2015-10-01 7:37 ` [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers Denis V. Lunev
2015-10-01 7:38 ` [Qemu-devel] [PATCH 2/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all() Denis V. Lunev
@ 2015-10-01 7:38 ` Denis V. Lunev
2015-10-01 22:59 ` Michael Roth
2015-10-01 7:38 ` [Qemu-devel] [PATCH 4/5] qga: handle possible SIGPIPE in guest-file-write Denis V. Lunev
2015-10-01 7:38 ` [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection Denis V. Lunev
4 siblings, 1 reply; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-01 7:38 UTC (permalink / raw)
Cc: Denis V. Lunev, Yuri Pudgorodskiy, qemu-devel, Michael Roth
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 4/5] qga: handle possible SIGPIPE in guest-file-write
2015-10-01 7:37 [Qemu-devel] [PATCH 0/5] simplified QEMU guest exec Denis V. Lunev
` (2 preceding siblings ...)
2015-10-01 7:38 ` [Qemu-devel] [PATCH 3/5] qga: guest exec functionality Denis V. Lunev
@ 2015-10-01 7:38 ` Denis V. Lunev
2015-10-01 23:03 ` Michael Roth
2015-10-01 7:38 ` [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection Denis V. Lunev
4 siblings, 1 reply; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-01 7:38 UTC (permalink / raw)
Cc: Denis V. Lunev, Yuri Pudgorodskiy, qemu-devel, Michael Roth
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection
2015-10-01 7:37 [Qemu-devel] [PATCH 0/5] simplified QEMU guest exec Denis V. Lunev
` (3 preceding siblings ...)
2015-10-01 7:38 ` [Qemu-devel] [PATCH 4/5] qga: handle possible SIGPIPE in guest-file-write Denis V. Lunev
@ 2015-10-01 7:38 ` Denis V. Lunev
4 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-01 7:38 UTC (permalink / raw)
Cc: Denis V. Lunev, Yuri Pudgorodskiy, qemu-devel, Michael Roth
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers
2015-10-01 7:37 ` [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers Denis V. Lunev
@ 2015-10-01 21:46 ` Michael Roth
0 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-01 21:46 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Yuri Pudgorodskiy, qemu-devel
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()
2015-10-01 7:38 ` [Qemu-devel] [PATCH 2/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all() Denis V. Lunev
@ 2015-10-01 21:54 ` Michael Roth
0 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-01 21:54 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Yuri Pudgorodskiy, qemu-devel
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] qga: guest exec functionality
2015-10-01 7:38 ` [Qemu-devel] [PATCH 3/5] qga: guest exec functionality Denis V. Lunev
@ 2015-10-01 22:59 ` Michael Roth
2015-10-05 14:18 ` Yuri Pudgorodskiy
0 siblings, 1 reply; 16+ messages in thread
From: Michael Roth @ 2015-10-01 22:59 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Yuri Pudgorodskiy, qemu-devel
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] qga: handle possible SIGPIPE in guest-file-write
2015-10-01 7:38 ` [Qemu-devel] [PATCH 4/5] qga: handle possible SIGPIPE in guest-file-write Denis V. Lunev
@ 2015-10-01 23:03 ` Michael Roth
0 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2015-10-01 23:03 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Yuri Pudgorodskiy, qemu-devel
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] qga: guest exec functionality
2015-10-01 22:59 ` Michael Roth
@ 2015-10-05 14:18 ` Yuri Pudgorodskiy
0 siblings, 0 replies; 16+ messages in thread
From: Yuri Pudgorodskiy @ 2015-10-05 14:18 UTC (permalink / raw)
To: Michael Roth, Denis V. Lunev; +Cc: qemu-devel
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection
2015-10-05 14:57 [Qemu-devel] [PATCH v2 0/5] simplified QEMU guest exec Denis V. Lunev
@ 2015-10-05 14:57 ` Denis V. Lunev
0 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-05 14:57 UTC (permalink / raw)
Cc: Denis V. Lunev, Yuri Pudgorodskiy, mdroth, qemu-devel
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 | 175 ++++++++++++++++++++++++++++++++++++++++++++++++---
qga/qapi-schema.json | 11 +++-
2 files changed, 175 insertions(+), 11 deletions(-)
diff --git a/qga/commands.c b/qga/commands.c
index 9fb9271..df2f320 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;
@@ -123,9 +141,18 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
}
ges = g_new0(GuestExecStatus, 1);
- ges->exited = gei->finished;
- if (gei->finished) {
+ bool finished = g_atomic_int_get(&gei->finished);
+
+ /* need to wait till output channels are closed
+ * to be sure we captured all output at this point */
+ if (gei->has_output) {
+ finished = finished && g_atomic_int_get(&gei->out.closed);
+ finished = finished && g_atomic_int_get(&gei->err.closed);
+ }
+
+ ges->exited = finished;
+ if (finished) {
/* Glib has no portable way to parse exit status.
* On UNIX, we can get either exit code from normal termination
* or signal number.
@@ -160,6 +187,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);
}
@@ -230,6 +271,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,
@@ -244,6 +364,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;
@@ -251,11 +375,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);
@@ -266,8 +393,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 da6b64f..6f5ea50 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -941,12 +941,17 @@
# @err-data: #optional base64-encoded stderr of the process
# Note: @out-data and @err-data are present only
# if 'capture-output' was specified for 'guest-exec'
+# @out-truncated: #optional true if stdout was not fully captured
+# due to size limitation.
+# @err-truncated: #optional true if stderr was not fully captured
+# due to size limitation.
#
# Since: 2.5
##
{ 'struct': 'GuestExecStatus',
'data': { 'exited': 'bool', '*exitcode': 'int', '*signal': 'int',
- '*out-data': 'str', '*err-data': 'str' }}
+ '*out-data': 'str', '*err-data': 'str',
+ '*out-truncated': 'bool', '*err-truncated': 'bool' }}
##
# @guest-exec-status
#
@@ -955,9 +960,9 @@
#
# @pid: pid returned from guest-exec
#
-# Returns: @GuestExecStatus on success.
+# Returns: GuestExecStatus on success.
#
-# Since 2.3
+# Since 2.5
##
{ 'command': 'guest-exec-status',
'data': { 'pid': 'int' },
--
2.1.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection
2015-10-07 10:32 [Qemu-devel] [PATCH v3 0/5] simplified QEMU guest exec Denis V. Lunev
@ 2015-10-07 10:32 ` Denis V. Lunev
2015-10-13 4:05 ` Michael Roth
0 siblings, 1 reply; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-07 10:32 UTC (permalink / raw)
Cc: Denis V. Lunev, Yuri Pudgorodskiy, mdroth, v.tolstov, qemu-devel
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 | 175 ++++++++++++++++++++++++++++++++++++++++++++++++---
qga/qapi-schema.json | 11 +++-
2 files changed, 175 insertions(+), 11 deletions(-)
diff --git a/qga/commands.c b/qga/commands.c
index b487324..740423f 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;
@@ -123,9 +141,18 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
}
ges = g_new0(GuestExecStatus, 1);
- ges->exited = gei->finished;
- if (gei->finished) {
+ bool finished = g_atomic_int_get(&gei->finished);
+
+ /* need to wait till output channels are closed
+ * to be sure we captured all output at this point */
+ if (gei->has_output) {
+ finished = finished && g_atomic_int_get(&gei->out.closed);
+ finished = finished && g_atomic_int_get(&gei->err.closed);
+ }
+
+ ges->exited = finished;
+ if (finished) {
/* Glib has no portable way to parse exit status.
* On UNIX, we can get either exit code from normal termination
* or signal number.
@@ -160,6 +187,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);
}
@@ -230,6 +271,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,
@@ -244,6 +364,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;
@@ -251,11 +375,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);
@@ -266,8 +393,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 da6b64f..6f5ea50 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -941,12 +941,17 @@
# @err-data: #optional base64-encoded stderr of the process
# Note: @out-data and @err-data are present only
# if 'capture-output' was specified for 'guest-exec'
+# @out-truncated: #optional true if stdout was not fully captured
+# due to size limitation.
+# @err-truncated: #optional true if stderr was not fully captured
+# due to size limitation.
#
# Since: 2.5
##
{ 'struct': 'GuestExecStatus',
'data': { 'exited': 'bool', '*exitcode': 'int', '*signal': 'int',
- '*out-data': 'str', '*err-data': 'str' }}
+ '*out-data': 'str', '*err-data': 'str',
+ '*out-truncated': 'bool', '*err-truncated': 'bool' }}
##
# @guest-exec-status
#
@@ -955,9 +960,9 @@
#
# @pid: pid returned from guest-exec
#
-# Returns: @GuestExecStatus on success.
+# Returns: GuestExecStatus on success.
#
-# Since 2.3
+# Since 2.5
##
{ 'command': 'guest-exec-status',
'data': { 'pid': 'int' },
--
2.1.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection
2015-10-07 10:32 ` [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection Denis V. Lunev
@ 2015-10-13 4:05 ` Michael Roth
2015-10-13 14:09 ` Denis V. Lunev
0 siblings, 1 reply; 16+ messages in thread
From: Michael Roth @ 2015-10-13 4:05 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Yuri Pudgorodskiy, qemu-devel, v.tolstov
Quoting Denis V. Lunev (2015-10-07 05:32:21)
> 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>
For capture-output mode, win32 guests appear to spin forever on EOF and
the exec'd process never gets reaped as a result. The below patch
seems to fix this issue. If this seems reasonable I can squash it
into the patch directly, but you might want to check I didn't break one
of your !capture-output use-cases (I assume this is still mainly focused
on redirecting to virtio-serial for stdio).
Another issue I noticed is that glib relies on
gspawn-win{32,64}-helper[-console].exe for g_spawn*() interface, so guest
exec won't work for .msi distributables unless those are added. This can
probably be addressed during soft-freeze though.
diff --git a/qga/commands.c b/qga/commands.c
index 27c06c5..fbf8abd 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -318,7 +318,7 @@ static gboolean guest_exec_output_watch(GIOChannel *ch,
struct GuestExecIOData *p = (struct GuestExecIOData *)p_;
gsize bytes_read = 0;
- if (cond == G_IO_HUP) {
+ if (cond == G_IO_HUP || cond == G_IO_ERR) {
g_io_channel_unref(ch);
g_atomic_int_set(&p->closed, 1);
return FALSE;
@@ -330,10 +330,18 @@ static gboolean guest_exec_output_watch(GIOChannel *ch,
t = g_try_realloc(p->data, p->size + GUEST_EXEC_IO_SIZE);
}
if (t == NULL) {
+ GIOStatus gstatus = 0;
p->truncated = true;
/* ignore truncated output */
gchar buf[GUEST_EXEC_IO_SIZE];
- g_io_channel_read_chars(ch, buf, sizeof(buf), &bytes_read, NULL);
+ gstatus = g_io_channel_read_chars(ch, buf, sizeof(buf),
+ &bytes_read, NULL);
+ if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
+ g_io_channel_unref(ch);
+ g_atomic_int_set(&p->closed, 1);
+ return false;
+ }
+
return TRUE;
}
p->size += GUEST_EXEC_IO_SIZE;
@@ -342,8 +350,14 @@ static gboolean guest_exec_output_watch(GIOChannel *ch,
/* 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,
+ GIOStatus gstatus = 0;
+ gstatus = g_io_channel_read_chars(ch, (gchar *)p->data + p->length,
p->size - p->length, &bytes_read, NULL);
+ if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
+ g_io_channel_unref(ch);
+ g_atomic_int_set(&p->closed, 1);
+ return false;
+ }
> ---
> qga/commands.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++---
> qga/qapi-schema.json | 11 +++-
> 2 files changed, 175 insertions(+), 11 deletions(-)
>
> diff --git a/qga/commands.c b/qga/commands.c
> index b487324..740423f 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;
> @@ -123,9 +141,18 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
> }
>
> ges = g_new0(GuestExecStatus, 1);
> - ges->exited = gei->finished;
>
> - if (gei->finished) {
> + bool finished = g_atomic_int_get(&gei->finished);
> +
> + /* need to wait till output channels are closed
> + * to be sure we captured all output at this point */
> + if (gei->has_output) {
> + finished = finished && g_atomic_int_get(&gei->out.closed);
> + finished = finished && g_atomic_int_get(&gei->err.closed);
> + }
> +
> + ges->exited = finished;
> + if (finished) {
> /* Glib has no portable way to parse exit status.
> * On UNIX, we can get either exit code from normal termination
> * or signal number.
> @@ -160,6 +187,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);
> }
> @@ -230,6 +271,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,
> @@ -244,6 +364,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;
> @@ -251,11 +375,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);
> @@ -266,8 +393,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 da6b64f..6f5ea50 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -941,12 +941,17 @@
> # @err-data: #optional base64-encoded stderr of the process
> # Note: @out-data and @err-data are present only
> # if 'capture-output' was specified for 'guest-exec'
> +# @out-truncated: #optional true if stdout was not fully captured
> +# due to size limitation.
> +# @err-truncated: #optional true if stderr was not fully captured
> +# due to size limitation.
> #
> # Since: 2.5
> ##
> { 'struct': 'GuestExecStatus',
> 'data': { 'exited': 'bool', '*exitcode': 'int', '*signal': 'int',
> - '*out-data': 'str', '*err-data': 'str' }}
> + '*out-data': 'str', '*err-data': 'str',
> + '*out-truncated': 'bool', '*err-truncated': 'bool' }}
> ##
> # @guest-exec-status
> #
> @@ -955,9 +960,9 @@
> #
> # @pid: pid returned from guest-exec
> #
> -# Returns: @GuestExecStatus on success.
> +# Returns: GuestExecStatus on success.
> #
> -# Since 2.3
> +# Since 2.5
> ##
> { 'command': 'guest-exec-status',
> 'data': { 'pid': 'int' },
> --
> 2.1.4
>
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection
2015-10-13 4:05 ` Michael Roth
@ 2015-10-13 14:09 ` Denis V. Lunev
0 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-13 14:09 UTC (permalink / raw)
To: Michael Roth; +Cc: Yuri Pudgorodskiy, qemu-devel, v.tolstov
On 10/13/2015 07:05 AM, Michael Roth wrote:
> Quoting Denis V. Lunev (2015-10-07 05:32:21)
>> 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>
> For capture-output mode, win32 guests appear to spin forever on EOF and
> the exec'd process never gets reaped as a result. The below patch
> seems to fix this issue. If this seems reasonable I can squash it
> into the patch directly, but you might want to check I didn't break one
> of your !capture-output use-cases (I assume this is still mainly focused
> on redirecting to virtio-serial for stdio).
>
> Another issue I noticed is that glib relies on
> gspawn-win{32,64}-helper[-console].exe for g_spawn*() interface, so guest
> exec won't work for .msi distributables unless those are added. This can
> probably be addressed during soft-freeze though.
>
> diff --git a/qga/commands.c b/qga/commands.c
> index 27c06c5..fbf8abd 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -318,7 +318,7 @@ static gboolean guest_exec_output_watch(GIOChannel *ch,
> struct GuestExecIOData *p = (struct GuestExecIOData *)p_;
> gsize bytes_read = 0;
>
> - if (cond == G_IO_HUP) {
> + if (cond == G_IO_HUP || cond == G_IO_ERR) {
> g_io_channel_unref(ch);
> g_atomic_int_set(&p->closed, 1);
> return FALSE;
> @@ -330,10 +330,18 @@ static gboolean guest_exec_output_watch(GIOChannel *ch,
> t = g_try_realloc(p->data, p->size + GUEST_EXEC_IO_SIZE);
> }
> if (t == NULL) {
> + GIOStatus gstatus = 0;
> p->truncated = true;
> /* ignore truncated output */
> gchar buf[GUEST_EXEC_IO_SIZE];
> - g_io_channel_read_chars(ch, buf, sizeof(buf), &bytes_read, NULL);
> + gstatus = g_io_channel_read_chars(ch, buf, sizeof(buf),
> + &bytes_read, NULL);
> + if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
> + g_io_channel_unref(ch);
> + g_atomic_int_set(&p->closed, 1);
> + return false;
> + }
> +
> return TRUE;
> }
> p->size += GUEST_EXEC_IO_SIZE;
> @@ -342,8 +350,14 @@ static gboolean guest_exec_output_watch(GIOChannel *ch,
>
> /* 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,
> + GIOStatus gstatus = 0;
> + gstatus = g_io_channel_read_chars(ch, (gchar *)p->data + p->length,
> p->size - p->length, &bytes_read, NULL);
> + if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
> + g_io_channel_unref(ch);
> + g_atomic_int_set(&p->closed, 1);
> + return false;
> + }
not completely. we have tested your code and found that minor
change is required
Signed-off-by: Yuri Pudgorodskiy<yur@virtuozzo.com>
---
qga/commands.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qga/commands.c b/qga/commands.c
index 1811ce6..deb041e 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -413,7 +413,7 @@ GuestExec *qmp_guest_exec(const char *path,
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);
+ in_ch = g_io_channel_win32_new_fd(in_fd);
#else
in_ch = g_io_channel_unix_new(in_fd);
#endif
I'll send a patch with this change and minor cosmetic
improvements soon (to make code shorter), but you can
take your version with this fix applied at your taste.
Den
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection
2015-10-13 15:41 [Qemu-devel] [PATCH v5 0/5] simplified QEMU guest exec Denis V. Lunev
@ 2015-10-13 15:41 ` Denis V. Lunev
0 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-10-13 15:41 UTC (permalink / raw)
Cc: Denis V. Lunev, Yuri Pudgorodskiy, mdroth, v.tolstov, qemu-devel
From: Yuri Pudgorodskiy <yur@virtuozzo.com>
Implemented with base64-encoded strings in qga json protocol.
Glib portable GIOChannel is used for data I/O.
Optinal stdin parameter of guest-exec command is now used as
stdin content for spawned subprocess.
If capture-output bool flag is specified, guest-exec redirects out/err
file descriptiors internally to pipes and collects subprocess
output.
Guest-exe-status is modified to return this collected data to requestor
in base64 encoding.
Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands.c | 188 ++++++++++++++++++++++++++++++++++++++++++++++++---
qga/qapi-schema.json | 7 +-
2 files changed, 186 insertions(+), 9 deletions(-)
diff --git a/qga/commands.c b/qga/commands.c
index b487324..2f88026 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;
@@ -123,9 +141,18 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
}
ges = g_new0(GuestExecStatus, 1);
- ges->exited = gei->finished;
- if (gei->finished) {
+ bool finished = g_atomic_int_get(&gei->finished);
+
+ /* need to wait till output channels are closed
+ * to be sure we captured all output at this point */
+ if (gei->has_output) {
+ finished = finished && g_atomic_int_get(&gei->out.closed);
+ finished = finished && g_atomic_int_get(&gei->err.closed);
+ }
+
+ ges->exited = finished;
+ if (finished) {
/* Glib has no portable way to parse exit status.
* On UNIX, we can get either exit code from normal termination
* or signal number.
@@ -160,6 +187,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);
}
@@ -230,6 +271,98 @@ 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;
+ GIOStatus gstatus;
+
+ if (cond == G_IO_HUP || cond == G_IO_ERR) {
+ goto close;
+ }
+
+ if (p->size == p->length) {
+ gpointer t = NULL;
+ if (!p->truncated && p->size < GUEST_EXEC_MAX_OUTPUT) {
+ t = g_try_realloc(p->data, p->size + GUEST_EXEC_IO_SIZE);
+ }
+ if (t == NULL) {
+ /* ignore truncated output */
+ gchar buf[GUEST_EXEC_IO_SIZE];
+
+ p->truncated = true;
+ gstatus = g_io_channel_read_chars(ch, buf, sizeof(buf),
+ &bytes_read, NULL);
+ if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
+ goto close;
+ }
+
+ return TRUE;
+ }
+ p->size += GUEST_EXEC_IO_SIZE;
+ p->data = t;
+ }
+
+ /* Calling read API once.
+ * On next available data our callback will be called again */
+ gstatus = g_io_channel_read_chars(ch, (gchar *)p->data + p->length,
+ p->size - p->length, &bytes_read, NULL);
+ if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
+ goto close;
+ }
+
+ p->length += bytes_read;
+
+ return TRUE;
+
+close:
+ g_io_channel_unref(ch);
+ g_atomic_int_set(&p->closed, 1);
+ return FALSE;
+}
+
GuestExec *qmp_guest_exec(const char *path,
bool has_arg, strList *arg,
bool has_env, strList *env,
@@ -244,6 +377,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;
@@ -251,11 +388,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);
@@ -266,8 +406,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_fd);
+#else
+ in_ch = g_io_channel_unix_new(in_fd);
+#endif
+ g_io_channel_set_encoding(in_ch, NULL, NULL);
+ g_io_channel_set_buffered(in_ch, false);
+ g_io_channel_set_flags(in_ch, G_IO_FLAG_NONBLOCK, NULL);
+ g_io_add_watch(in_ch, G_IO_OUT, guest_exec_input_watch, &gei->in);
+ }
+
+ if (has_output) {
+#ifdef G_OS_WIN32
+ out_ch = g_io_channel_win32_new_fd(out_fd);
+ err_ch = g_io_channel_win32_new_fd(err_fd);
+#else
+ out_ch = g_io_channel_unix_new(out_fd);
+ err_ch = g_io_channel_unix_new(err_fd);
+#endif
+ g_io_channel_set_encoding(out_ch, NULL, NULL);
+ g_io_channel_set_encoding(err_ch, NULL, NULL);
+ g_io_channel_set_buffered(out_ch, false);
+ g_io_channel_set_buffered(err_ch, false);
+ g_io_add_watch(out_ch, G_IO_IN | G_IO_HUP,
+ guest_exec_output_watch, &gei->out);
+ g_io_add_watch(err_ch, G_IO_IN | G_IO_HUP,
+ guest_exec_output_watch, &gei->err);
+ }
+
done:
g_free(argv);
g_free(envp);
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 6143632..6f5ea50 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -941,12 +941,17 @@
# @err-data: #optional base64-encoded stderr of the process
# Note: @out-data and @err-data are present only
# if 'capture-output' was specified for 'guest-exec'
+# @out-truncated: #optional true if stdout was not fully captured
+# due to size limitation.
+# @err-truncated: #optional true if stderr was not fully captured
+# due to size limitation.
#
# Since: 2.5
##
{ 'struct': 'GuestExecStatus',
'data': { 'exited': 'bool', '*exitcode': 'int', '*signal': 'int',
- '*out-data': 'str', '*err-data': 'str' }}
+ '*out-data': 'str', '*err-data': 'str',
+ '*out-truncated': 'bool', '*err-truncated': 'bool' }}
##
# @guest-exec-status
#
--
2.1.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-10-13 15:41 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-01 7:37 [Qemu-devel] [PATCH 0/5] simplified QEMU guest exec Denis V. Lunev
2015-10-01 7:37 ` [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers Denis V. Lunev
2015-10-01 21:46 ` Michael Roth
2015-10-01 7:38 ` [Qemu-devel] [PATCH 2/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all() Denis V. Lunev
2015-10-01 21:54 ` Michael Roth
2015-10-01 7:38 ` [Qemu-devel] [PATCH 3/5] qga: guest exec functionality Denis V. Lunev
2015-10-01 22:59 ` Michael Roth
2015-10-05 14:18 ` Yuri Pudgorodskiy
2015-10-01 7:38 ` [Qemu-devel] [PATCH 4/5] qga: handle possible SIGPIPE in guest-file-write Denis V. Lunev
2015-10-01 23:03 ` Michael Roth
2015-10-01 7:38 ` [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection Denis V. Lunev
-- strict thread matches above, loose matches on Subject: below --
2015-10-05 14:57 [Qemu-devel] [PATCH v2 0/5] simplified QEMU guest exec Denis V. Lunev
2015-10-05 14:57 ` [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection Denis V. Lunev
2015-10-07 10:32 [Qemu-devel] [PATCH v3 0/5] simplified QEMU guest exec Denis V. Lunev
2015-10-07 10:32 ` [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection Denis V. Lunev
2015-10-13 4:05 ` Michael Roth
2015-10-13 14:09 ` Denis V. Lunev
2015-10-13 15:41 [Qemu-devel] [PATCH v5 0/5] simplified QEMU guest exec Denis V. Lunev
2015-10-13 15:41 ` [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection Denis V. Lunev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).