* [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers
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-07 10:32 ` [Qemu-devel] [PATCH 2/5] qga: guest exec functionality Denis V. Lunev
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ 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
This just makes code shorter and better.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
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] 10+ messages in thread* [Qemu-devel] [PATCH 2/5] qga: guest exec functionality
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 1/5] qga: drop guest_file_init helper and replace it with static initializers Denis V. Lunev
@ 2015-10-07 10:32 ` Denis V. Lunev
2015-10-07 10:32 ` [Qemu-devel] [PATCH 3/5] qga: handle possible SIGPIPE in guest-file-write Denis V. Lunev
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ 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>
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 | 188 +++++++++++++++++++++++++++++++++++++++++++++++++++
qga/qapi-schema.json | 62 +++++++++++++++++
2 files changed, 250 insertions(+)
diff --git a/qga/commands.c b/qga/commands.c
index 7834967..1673941 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -70,3 +70,191 @@ 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_new0(GuestExecInfo, 1);
+ 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_new0(GuestExecStatus, 1);
+ ges->exited = gei->finished;
+
+ if (gei->finished) {
+ /* Glib has no portable way to parse exit status.
+ * On UNIX, we can get either exit code from normal termination
+ * or signal number.
+ * On Windows, it is either the same exit code or the exception
+ * value for an unhandled exception that caused the process
+ * to terminate.
+ * See MSDN for GetExitCodeProcess() and ntstatus.h for possible
+ * well-known codes, e.g. C0000005 ACCESS_DENIED - analog of SIGSEGV
+ * References:
+ * https://msdn.microsoft.com/en-us/library/windows/desktop/ms683189(v=vs.85).aspx
+ * https://msdn.microsoft.com/en-us/library/aa260331(v=vs.60).aspx
+ */
+#ifdef G_OS_WIN32
+ /* Additionally WIN32 does not provide any additional information
+ * on whetherthe child exited or terminated via signal.
+ * We use this simple range check to distingish application exit code
+ * (usually value less then 256) and unhandled exception code with
+ * ntstatus (always value greater then 0xC0000005). */
+ if ((uint32_t)gei->status < 0xC0000000U) {
+ ges->has_exitcode = true;
+ ges->exitcode = gei->status;
+ } else {
+ ges->has_signal = true;
+ ges->signal = gei->status;
+ }
+#else
+ if (WIFEXITED(gei->status)) {
+ ges->has_exitcode = true;
+ ges->exitcode = WEXITSTATUS(gei->status);
+ } else if (WIFSIGNALED(gei->status)) {
+ ges->has_signal = true;
+ ges->signal = WTERMSIG(gei->status);
+ }
+#endif
+ QTAILQ_REMOVE(&guest_exec_state.processes, gei, next);
+ g_free(gei);
+ }
+
+ return ges;
+}
+
+/* Get environment variables or arguments array for execve(). */
+static char **guest_exec_get_args(const strList *entry, bool log)
+{
+ const strList *it;
+ int count = 1, i = 0; /* reserve for NULL terminator */
+ char **args;
+ char *str; /* for logging array of arguments */
+ size_t str_size = 1;
+
+ for (it = entry; it != NULL; it = it->next) {
+ count++;
+ str_size += 1 + strlen(it->value);
+ }
+
+ str = g_malloc(str_size);
+ *str = 0;
+ args = g_malloc(count * sizeof(char *));
+ for (it = entry; it != NULL; it = it->next) {
+ args[i++] = it->value;
+ pstrcat(str, str_size, it->value);
+ if (it->next) {
+ pstrcat(str, str_size, " ");
+ }
+ }
+ args[i] = NULL;
+
+ if (log) {
+ slog("guest-exec called: \"%s\"", str);
+ }
+ g_free(str);
+
+ return args;
+}
+
+static void guest_exec_child_watch(GPid pid, gint status, gpointer data)
+{
+ GuestExecInfo *gei = (GuestExecInfo *)data;
+
+ g_debug("guest_exec_child_watch called, pid: %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..da6b64f 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -930,3 +930,65 @@
##
{ 'command': 'guest-get-memory-block-info',
'returns': 'GuestMemoryBlockInfo' }
+
+# @GuestExecStatus:
+#
+# @exited: true if process has already terminated.
+# @exitcode: #optional process exit code if it was normally terminated.
+# @signal: #optional signal number (linux) or unhandled exception code
+# (windows) if the process was abnormally terminated.
+# @out-data: #optional base64-encoded stdout of the process
+# @err-data: #optional base64-encoded stderr of the process
+# Note: @out-data and @err-data are present only
+# if 'capture-output' was specified for 'guest-exec'
+#
+# Since: 2.5
+##
+{ 'struct': 'GuestExecStatus',
+ 'data': { 'exited': 'bool', '*exitcode': 'int', '*signal': 'int',
+ '*out-data': 'str', '*err-data': 'str' }}
+##
+# @guest-exec-status
+#
+# Check status of process associated with PID retrieved via guest-exec.
+# Reap the process and associated metadata if it has exited.
+#
+# @pid: pid returned from guest-exec
+#
+# Returns: @GuestExecStatus on success.
+#
+# Since 2.3
+##
+{ '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] 10+ messages in thread* [Qemu-devel] [PATCH 3/5] qga: handle possible SIGPIPE in guest-file-write
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 1/5] qga: drop guest_file_init helper and replace it with static initializers Denis V. Lunev
2015-10-07 10:32 ` [Qemu-devel] [PATCH 2/5] qga: guest exec functionality Denis V. Lunev
@ 2015-10-07 10:32 ` Denis V. Lunev
2015-10-07 10:32 ` [Qemu-devel] [PATCH 4/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all() Denis V. Lunev
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ 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
qemu-ga should not exit on guest-file-write to pipe without read end
but proper error code should be returned. The behavior of the
spawned process should be default thus SIGPIPE processing should be
reset to default after fork() but before exec().
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
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 1673941..b487324 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -214,6 +214,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,
@@ -239,7 +255,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] 10+ messages in thread* [Qemu-devel] [PATCH 4/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()
2015-10-07 10:32 [Qemu-devel] [PATCH v3 0/5] simplified QEMU guest exec Denis V. Lunev
` (2 preceding siblings ...)
2015-10-07 10:32 ` [Qemu-devel] [PATCH 3/5] qga: handle possible SIGPIPE in guest-file-write Denis V. Lunev
@ 2015-10-07 10:32 ` 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-12 6:35 ` [Qemu-devel] [PATCH v3 0/5] simplified QEMU guest exec Denis V. Lunev
5 siblings, 0 replies; 10+ 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>
glib may return G_IO_STATUS_AGAIN which is actually not an error.
Also fixed a bug when on incomplete write buf pointer was not adjusted.
Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
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] 10+ 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
` (3 preceding siblings ...)
2015-10-07 10:32 ` [Qemu-devel] [PATCH 4/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all() Denis V. Lunev
@ 2015-10-07 10:32 ` Denis V. Lunev
2015-10-13 4:05 ` Michael Roth
2015-10-12 6:35 ` [Qemu-devel] [PATCH v3 0/5] simplified QEMU guest exec Denis V. Lunev
5 siblings, 1 reply; 10+ 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] 10+ 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; 10+ 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] 10+ 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
2015-10-13 15:28 ` [Qemu-devel] [PATCH v4 " Denis V. Lunev
0 siblings, 1 reply; 10+ 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] 10+ messages in thread* [Qemu-devel] [PATCH v4 5/5] qga: guest-exec simple stdin/stdout/stderr redirection
2015-10-13 14:09 ` Denis V. Lunev
@ 2015-10-13 15:28 ` Denis V. Lunev
0 siblings, 0 replies; 10+ messages in thread
From: Denis V. Lunev @ 2015-10-13 15:28 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>
Signed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Changes from v3:
- fixed bug in WIN32 EOF processing
- fixed typo with in_ch/in_fd for WIN32 code
- refactored output watcher function, merged similar branches
qga/commands.c | 188 ++++++++++++++++++++++++++++++++++++++++++++++++---
qga/qapi-schema.json | 11 ++-
2 files changed, 188 insertions(+), 11 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 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] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/5] simplified QEMU guest exec
2015-10-07 10:32 [Qemu-devel] [PATCH v3 0/5] simplified QEMU guest exec Denis V. Lunev
` (4 preceding siblings ...)
2015-10-07 10:32 ` [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection Denis V. Lunev
@ 2015-10-12 6:35 ` Denis V. Lunev
5 siblings, 0 replies; 10+ messages in thread
From: Denis V. Lunev @ 2015-10-12 6:35 UTC (permalink / raw)
Cc: Yuri Pudgorodskiy, mdroth, v.tolstov, qemu-devel
On 10/07/2015 01:32 PM, Denis V. Lunev wrote:
> 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.
>
> Changed from v2:
> - fixed last minute typo in Win32 code in patch 2 (s/exiticode/exitcode/)
>
> Changes from v1:
> - use g_new0() instead of g_malloc0
> - added explicit 'exited' bool to GuestExecStatus
> - reworked documentation for GuestExecStatus
> - added comment about platform-specific signals and exception codes
> - replaces 'pid' with 'handle' in guest-exec api
>
> 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: guest exec functionality
> qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()
> 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 | 363 +++++++++++++++++++++++++++++++++++++++++++++++++++
> qga/main.c | 6 +
> qga/qapi-schema.json | 67 ++++++++++
> 6 files changed, 453 insertions(+), 26 deletions(-)
>
ping
^ permalink raw reply [flat|nested] 10+ messages in thread