All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/3] qga: Support merging output streams in guest-exec
@ 2023-03-23  0:19 Daniel Xu
  2023-03-23  0:19 ` [PATCH v6 1/3] qga: Refactor guest-exec capture-output to take enum Daniel Xu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Daniel Xu @ 2023-03-23  0:19 UTC (permalink / raw)
  To: qemu-devel, marcandre.lureau, berrange

Currently, the captured output (via `capture-output`) is segregated into
separate GuestExecStatus fields (`out-data` and `err-data`). This means
that downstream consumers have no way to reassemble the captured data
back into the original stream.

This is relevant for chatty and semi-interactive (ie. read only) CLI
tools.  Such tools may deliberately interleave stdout and stderr for
visual effect. If segregated, the output becomes harder to visually
understand.

This patchset adds support for merging stderr and stdout output streams
via a backwards compatibile refactor and a new enum variant,
`merged`.

---

Changes from v5:
* Add qapi conditional for `merged` flag
* Remove error check for windows as above conditional enforces it

Changes from v4:
* Rename `all` -> `separated`
* Rename `all-merge` -> `merged`

Changes from v3:
* Split out ASAN fixes into separate patch series
* Refactor `capture-output` flag into an enum
* Avoid using /bin/bash on windows

Changes from v2:
* Error out if `merge-output` on windows guests
* Add FIXMEs for when glib is updated
* Fix memory leaks in qemu-keymap

Changes from v1:
* Drop invalid test fix
* Do not support `merge-output` on windows guests
* Fix a UAF in tests

Daniel Xu (3):
  qga: Refactor guest-exec capture-output to take enum
  qga: Add `merged` variant to GuestExecCaptureOutputMode
  qga: test: Add tests for `merged` flag

 qga/commands.c        |  62 +++++++++++++++--
 qga/qapi-schema.json  |  36 +++++++++-
 tests/unit/test-qga.c | 158 +++++++++++++++++++++++++++++++++++++-----
 3 files changed, 233 insertions(+), 23 deletions(-)

-- 
2.39.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v6 1/3] qga: Refactor guest-exec capture-output to take enum
  2023-03-23  0:19 [PATCH v6 0/3] qga: Support merging output streams in guest-exec Daniel Xu
@ 2023-03-23  0:19 ` Daniel Xu
  2023-03-23  0:19 ` [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode Daniel Xu
  2023-03-23  0:19 ` [PATCH v6 3/3] qga: test: Add tests for `merged` flag Daniel Xu
  2 siblings, 0 replies; 8+ messages in thread
From: Daniel Xu @ 2023-03-23  0:19 UTC (permalink / raw)
  To: kkostiuk, michael.roth, marcandre.lureau, berrange; +Cc: qemu-devel

Previously capture-output was an optional boolean flag that either
captured all output or captured none. While this is OK in most cases, it
lacks flexibility for more advanced capture cases, such as wanting to
only capture stdout.

This commits refactors guest-exec qapi to take an enum for capture mode
instead while preserving backwards compatibility.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 qga/commands.c       | 37 ++++++++++++++++++++++++++++++++++---
 qga/qapi-schema.json | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/qga/commands.c b/qga/commands.c
index 172826f8f8..01f68b45ab 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -379,11 +379,23 @@ close:
     return false;
 }
 
+static GuestExecCaptureOutputMode ga_parse_capture_output(
+        GuestExecCaptureOutput *capture_output)
+{
+    if (!capture_output)
+        return GUEST_EXEC_CAPTURE_OUTPUT_MODE_NONE;
+    else if (capture_output->type == QTYPE_QBOOL)
+        return capture_output->u.flag ? GUEST_EXEC_CAPTURE_OUTPUT_MODE_SEPARATED
+                                      : GUEST_EXEC_CAPTURE_OUTPUT_MODE_NONE;
+    else
+        return capture_output->u.mode;
+}
+
 GuestExec *qmp_guest_exec(const char *path,
                        bool has_arg, strList *arg,
                        bool has_env, strList *env,
                        const char *input_data,
-                       bool has_capture_output, bool capture_output,
+                       GuestExecCaptureOutput *capture_output,
                        Error **errp)
 {
     GPid pid;
@@ -396,7 +408,8 @@ GuestExec *qmp_guest_exec(const char *path,
     gint in_fd, out_fd, err_fd;
     GIOChannel *in_ch, *out_ch, *err_ch;
     GSpawnFlags flags;
-    bool has_output = (has_capture_output && capture_output);
+    bool has_output = false;
+    GuestExecCaptureOutputMode output_mode;
     g_autofree uint8_t *input = NULL;
     size_t ninput = 0;
 
@@ -415,8 +428,26 @@ GuestExec *qmp_guest_exec(const char *path,
 
     flags = G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD |
         G_SPAWN_SEARCH_PATH_FROM_ENVP;
-    if (!has_output) {
+
+    output_mode = ga_parse_capture_output(capture_output);
+    switch (output_mode) {
+    case GUEST_EXEC_CAPTURE_OUTPUT_MODE_NONE:
         flags |= G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL;
+        break;
+    case GUEST_EXEC_CAPTURE_OUTPUT_MODE_STDOUT:
+        has_output = true;
+        flags |= G_SPAWN_STDERR_TO_DEV_NULL;
+        break;
+    case GUEST_EXEC_CAPTURE_OUTPUT_MODE_STDERR:
+        has_output = true;
+        flags |= G_SPAWN_STDOUT_TO_DEV_NULL;
+        break;
+    case GUEST_EXEC_CAPTURE_OUTPUT_MODE_SEPARATED:
+        has_output = true;
+        break;
+    case GUEST_EXEC_CAPTURE_OUTPUT_MODE__MAX:
+        /* Silence warning; impossible branch */
+        break;
     }
 
     ret = g_spawn_async_with_pipes(NULL, argv, envp, flags,
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 796434ed34..d1e00a4234 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1200,6 +1200,37 @@
 { 'struct': 'GuestExec',
   'data': { 'pid': 'int'} }
 
+##
+# @GuestExecCaptureOutputMode:
+#
+# An enumeration of guest-exec capture modes.
+#
+# @none: do not capture any output
+# @stdout: only capture stdout
+# @stderr: only capture stderr
+# @separated: capture both stdout and stderr, but separated into
+#             GuestExecStatus out-data and err-data, respectively
+#
+# Since: 8.0
+##
+ { 'enum': 'GuestExecCaptureOutputMode',
+   'data': [ 'none', 'stdout', 'stderr', 'separated' ] }
+
+##
+# @GuestExecCaptureOutput:
+#
+# Controls what guest-exec output gets captures.
+#
+# @flag: captures both stdout and stderr if true. Equivalent
+#        to GuestExecCaptureOutputMode::all. (since 2.5)
+# @mode: capture mode; preferred interface
+#
+# Since: 8.0
+##
+ { 'alternate': 'GuestExecCaptureOutput',
+   'data': { 'flag': 'bool',
+             'mode': 'GuestExecCaptureOutputMode'} }
+
 ##
 # @guest-exec:
 #
@@ -1218,7 +1249,7 @@
 ##
 { 'command': 'guest-exec',
   'data':    { 'path': 'str', '*arg': ['str'], '*env': ['str'],
-               '*input-data': 'str', '*capture-output': 'bool' },
+               '*input-data': 'str', '*capture-output': 'GuestExecCaptureOutput' },
   'returns': 'GuestExec' }
 
 
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode
  2023-03-23  0:19 [PATCH v6 0/3] qga: Support merging output streams in guest-exec Daniel Xu
  2023-03-23  0:19 ` [PATCH v6 1/3] qga: Refactor guest-exec capture-output to take enum Daniel Xu
@ 2023-03-23  0:19 ` Daniel Xu
  2023-03-23  9:26   ` Daniel P. Berrangé
  2023-03-23  0:19 ` [PATCH v6 3/3] qga: test: Add tests for `merged` flag Daniel Xu
  2 siblings, 1 reply; 8+ messages in thread
From: Daniel Xu @ 2023-03-23  0:19 UTC (permalink / raw)
  To: kkostiuk, michael.roth, marcandre.lureau, berrange; +Cc: qemu-devel

Currently, any captured output (via `capture-output`) is segregated into
separate GuestExecStatus fields (`out-data` and `err-data`). This means
that downstream consumers have no way to reassemble the captured data
back into the original stream.

This is relevant for chatty and semi-interactive (ie. read only) CLI
tools.  Such tools may deliberately interleave stdout and stderr for
visual effect. If segregated, the output becomes harder to visually
understand.

This commit adds a new enum variant to the GuestExecCaptureOutputMode
qapi to merge the output streams such that consumers can have a pristine
view of the original command output.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 qga/commands.c       | 25 +++++++++++++++++++++++--
 qga/qapi-schema.json |  5 ++++-
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/qga/commands.c b/qga/commands.c
index 01f68b45ab..09c683e263 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -270,12 +270,26 @@ 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)
+    bool has_merge = *(bool *)data;
     struct sigaction sigact;
 
+    if (has_merge) {
+        /*
+         * FIXME: When `GLIB_VERSION_MIN_REQUIRED` is bumped to 2.58+, use
+         * g_spawn_async_with_fds() to be portable on windows. The current
+         * logic does not work on windows b/c `GSpawnChildSetupFunc` is run
+         * inside the parent, not the child.
+         */
+        if (dup2(STDOUT_FILENO, STDERR_FILENO) != 0) {
+            slog("dup2() failed to merge stderr into stdout: %s",
+                 strerror(errno));
+        }
+    }
+
+    /* Reset ignored signals back to default. */
     memset(&sigact, 0, sizeof(struct sigaction));
     sigact.sa_handler = SIG_DFL;
 
@@ -409,6 +423,7 @@ GuestExec *qmp_guest_exec(const char *path,
     GIOChannel *in_ch, *out_ch, *err_ch;
     GSpawnFlags flags;
     bool has_output = false;
+    bool has_merge = false;
     GuestExecCaptureOutputMode output_mode;
     g_autofree uint8_t *input = NULL;
     size_t ninput = 0;
@@ -445,13 +460,19 @@ GuestExec *qmp_guest_exec(const char *path,
     case GUEST_EXEC_CAPTURE_OUTPUT_MODE_SEPARATED:
         has_output = true;
         break;
+#if !defined(G_OS_WIN32)
+    case GUEST_EXEC_CAPTURE_OUTPUT_MODE_MERGED:
+        has_output = true;
+        has_merge = true;
+        break;
+#endif
     case GUEST_EXEC_CAPTURE_OUTPUT_MODE__MAX:
         /* Silence warning; impossible branch */
         break;
     }
 
     ret = g_spawn_async_with_pipes(NULL, argv, envp, flags,
-            guest_exec_task_setup, NULL, &pid, input_data ? &in_fd : NULL,
+            guest_exec_task_setup, &has_merge, &pid, input_data ? &in_fd : NULL,
             has_output ? &out_fd : NULL, has_output ? &err_fd : NULL, &gerr);
     if (!ret) {
         error_setg(errp, QERR_QGA_COMMAND_FAILED, gerr->message);
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index d1e00a4234..39dd006d16 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1210,11 +1210,14 @@
 # @stderr: only capture stderr
 # @separated: capture both stdout and stderr, but separated into
 #             GuestExecStatus out-data and err-data, respectively
+# @merged: capture both stdout and stderr, but merge together
+#          into out-data. not effective on windows guests.
 #
 # Since: 8.0
 ##
  { 'enum': 'GuestExecCaptureOutputMode',
-   'data': [ 'none', 'stdout', 'stderr', 'separated' ] }
+   'data': [ 'none', 'stdout', 'stderr', 'separated',
+             { 'name': 'merged', 'if': { 'not': 'CONFIG_WIN32' } } ] }
 
 ##
 # @GuestExecCaptureOutput:
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 3/3] qga: test: Add tests for `merged` flag
  2023-03-23  0:19 [PATCH v6 0/3] qga: Support merging output streams in guest-exec Daniel Xu
  2023-03-23  0:19 ` [PATCH v6 1/3] qga: Refactor guest-exec capture-output to take enum Daniel Xu
  2023-03-23  0:19 ` [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode Daniel Xu
@ 2023-03-23  0:19 ` Daniel Xu
  2 siblings, 0 replies; 8+ messages in thread
From: Daniel Xu @ 2023-03-23  0:19 UTC (permalink / raw)
  To: kkostiuk, michael.roth, marcandre.lureau, berrange; +Cc: qemu-devel

This commit adds a test to ensure `merged` functions as expected.
We also add a negative test to ensure we haven't regressed previous
functionality.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 tests/unit/test-qga.c | 158 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 141 insertions(+), 17 deletions(-)

diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
index b4e0a14573..360b4cab23 100644
--- a/tests/unit/test-qga.c
+++ b/tests/unit/test-qga.c
@@ -755,6 +755,31 @@ static void test_qga_fsfreeze_status(gconstpointer fix)
     g_assert_cmpstr(status, ==, "thawed");
 }
 
+static QDict *wait_for_guest_exec_completion(int fd, int64_t pid)
+{
+    QDict *ret = NULL;
+    int64_t now;
+    bool exited;
+    QDict *val;
+
+    now = g_get_monotonic_time();
+    do {
+        ret = qmp_fd(fd,
+                     "{'execute': 'guest-exec-status',"
+                     " 'arguments': { 'pid': %" PRId64 " } }", pid);
+        g_assert_nonnull(ret);
+        val = qdict_get_qdict(ret, "return");
+        exited = qdict_get_bool(val, "exited");
+        if (!exited) {
+            qobject_unref(ret);
+        }
+    } while (!exited &&
+             g_get_monotonic_time() < now + 5 * G_TIME_SPAN_SECOND);
+    g_assert(exited);
+
+    return ret;
+}
+
 static void test_qga_guest_exec(gconstpointer fix)
 {
     const TestFixture *fixture = fix;
@@ -762,9 +787,8 @@ static void test_qga_guest_exec(gconstpointer fix)
     QDict *val;
     const gchar *out;
     g_autofree guchar *decoded = NULL;
-    int64_t pid, now, exitcode;
+    int64_t pid, exitcode;
     gsize len;
-    bool exited;
 
     /* exec 'echo foo bar' */
     ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
@@ -777,23 +801,10 @@ static void test_qga_guest_exec(gconstpointer fix)
     g_assert_cmpint(pid, >, 0);
     qobject_unref(ret);
 
-    /* wait for completion */
-    now = g_get_monotonic_time();
-    do {
-        ret = qmp_fd(fixture->fd,
-                     "{'execute': 'guest-exec-status',"
-                     " 'arguments': { 'pid': %" PRId64 " } }", pid);
-        g_assert_nonnull(ret);
-        val = qdict_get_qdict(ret, "return");
-        exited = qdict_get_bool(val, "exited");
-        if (!exited) {
-            qobject_unref(ret);
-        }
-    } while (!exited &&
-             g_get_monotonic_time() < now + 5 * G_TIME_SPAN_SECOND);
-    g_assert(exited);
+    ret = wait_for_guest_exec_completion(fixture->fd, pid);
 
     /* check stdout */
+    val = qdict_get_qdict(ret, "return");
     exitcode = qdict_get_int(val, "exitcode");
     g_assert_cmpint(exitcode, ==, 0);
     out = qdict_get_str(val, "out-data");
@@ -802,6 +813,115 @@ static void test_qga_guest_exec(gconstpointer fix)
     g_assert_cmpstr((char *)decoded, ==, "\" test_str \"");
 }
 
+#if defined(G_OS_WIN32)
+static void test_qga_guest_exec_separated(gconstpointer fix)
+{
+}
+static void test_qga_guest_exec_merged(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    g_autoptr(QDict) ret = NULL;
+    QDict *val;
+    const gchar *class, *desc;
+    g_autofree guchar *decoded = NULL;
+
+    /* exec 'echo foo bar' */
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
+                 " 'path': 'echo',"
+                 " 'arg': [ 'execution never reaches here' ],"
+                 " 'capture-output': 'merged' } }");
+
+    g_assert_nonnull(ret);
+    val = qdict_get_qdict(ret, "error");
+    g_assert_nonnull(val);
+    class = qdict_get_str(val, "class");
+    desc = qdict_get_str(val, "desc");
+    g_assert_cmpstr(class, ==, "GenericError");
+    g_assert_cmpint(strlen(desc), >, 0);
+}
+#else
+static void test_qga_guest_exec_separated(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    g_autoptr(QDict) ret = NULL;
+    QDict *val;
+    const gchar *out, *err;
+    g_autofree guchar *out_decoded = NULL;
+    g_autofree guchar *err_decoded = NULL;
+    int64_t pid, exitcode;
+    gsize len;
+
+    /* exec 'echo foo bar' */
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
+                 " 'path': '/bin/bash',"
+                 " 'arg': [ '-c', 'for i in $(seq 4); do if (( $i %% 2 )); then echo stdout; else echo stderr 1>&2; fi; done;' ],"
+                 " 'capture-output': 'separated' } }");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    val = qdict_get_qdict(ret, "return");
+    pid = qdict_get_int(val, "pid");
+    g_assert_cmpint(pid, >, 0);
+    qobject_unref(ret);
+
+    ret = wait_for_guest_exec_completion(fixture->fd, pid);
+
+    val = qdict_get_qdict(ret, "return");
+    exitcode = qdict_get_int(val, "exitcode");
+    g_assert_cmpint(exitcode, ==, 0);
+
+    /* check stdout */
+    out = qdict_get_str(val, "out-data");
+    out_decoded = g_base64_decode(out, &len);
+    g_assert_cmpint(len, ==, 14);
+    g_assert_cmpstr((char *)out_decoded, ==, "stdout\nstdout\n");
+
+    /* check stderr */
+    err = qdict_get_try_str(val, "err-data");
+    err_decoded = g_base64_decode(err, &len);
+    g_assert_cmpint(len, ==, 14);
+    g_assert_cmpstr((char *)err_decoded, ==, "stderr\nstderr\n");
+}
+
+static void test_qga_guest_exec_merged(gconstpointer fix)
+{
+    const TestFixture *fixture = fix;
+    g_autoptr(QDict) ret = NULL;
+    QDict *val;
+    const gchar *out, *err;
+    g_autofree guchar *decoded = NULL;
+    int64_t pid, exitcode;
+    gsize len;
+
+    /* exec 'echo foo bar' */
+    ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
+                 " 'path': '/bin/bash',"
+                 " 'arg': [ '-c', 'for i in $(seq 4); do if (( $i %% 2 )); then echo stdout; else echo stderr 1>&2; fi; done;' ],"
+                 " 'capture-output': 'merged' } }");
+    g_assert_nonnull(ret);
+    qmp_assert_no_error(ret);
+    val = qdict_get_qdict(ret, "return");
+    pid = qdict_get_int(val, "pid");
+    g_assert_cmpint(pid, >, 0);
+    qobject_unref(ret);
+
+    ret = wait_for_guest_exec_completion(fixture->fd, pid);
+
+    val = qdict_get_qdict(ret, "return");
+    exitcode = qdict_get_int(val, "exitcode");
+    g_assert_cmpint(exitcode, ==, 0);
+
+    /* check stdout */
+    out = qdict_get_str(val, "out-data");
+    decoded = g_base64_decode(out, &len);
+    g_assert_cmpint(len, ==, 28);
+    g_assert_cmpstr((char *)decoded, ==, "stdout\nstderr\nstdout\nstderr\n");
+
+    /* check stderr */
+    err = qdict_get_try_str(val, "err-data");
+    g_assert_null(err);
+}
+#endif
+
 static void test_qga_guest_exec_invalid(gconstpointer fix)
 {
     const TestFixture *fixture = fix;
@@ -972,6 +1092,10 @@ int main(int argc, char **argv)
     g_test_add_data_func("/qga/blockedrpcs", NULL, test_qga_blockedrpcs);
     g_test_add_data_func("/qga/config", NULL, test_qga_config);
     g_test_add_data_func("/qga/guest-exec", &fix, test_qga_guest_exec);
+    g_test_add_data_func("/qga/guest-exec-separated", &fix,
+                         test_qga_guest_exec_separated);
+    g_test_add_data_func("/qga/guest-exec-merged", &fix,
+                         test_qga_guest_exec_merged);
     g_test_add_data_func("/qga/guest-exec-invalid", &fix,
                          test_qga_guest_exec_invalid);
     g_test_add_data_func("/qga/guest-get-osinfo", &fix,
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode
  2023-03-23  0:19 ` [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode Daniel Xu
@ 2023-03-23  9:26   ` Daniel P. Berrangé
  2023-03-31 20:23     ` Daniel Xu
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2023-03-23  9:26 UTC (permalink / raw)
  To: Daniel Xu; +Cc: kkostiuk, michael.roth, marcandre.lureau, qemu-devel

On Wed, Mar 22, 2023 at 06:19:27PM -0600, Daniel Xu wrote:
> Currently, any captured output (via `capture-output`) is segregated into
> separate GuestExecStatus fields (`out-data` and `err-data`). This means
> that downstream consumers have no way to reassemble the captured data
> back into the original stream.
> 
> This is relevant for chatty and semi-interactive (ie. read only) CLI
> tools.  Such tools may deliberately interleave stdout and stderr for
> visual effect. If segregated, the output becomes harder to visually
> understand.
> 
> This commit adds a new enum variant to the GuestExecCaptureOutputMode
> qapi to merge the output streams such that consumers can have a pristine
> view of the original command output.
> 
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ---
>  qga/commands.c       | 25 +++++++++++++++++++++++--
>  qga/qapi-schema.json |  5 ++++-
>  2 files changed, 27 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode
  2023-03-23  9:26   ` Daniel P. Berrangé
@ 2023-03-31 20:23     ` Daniel Xu
  2023-04-03 14:56       ` Konstantin Kostiuk
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Xu @ 2023-03-31 20:23 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: kkostiuk, michael.roth, Marc-André Lureau, qemu-devel

Hi Daniel,

On Thu, Mar 23, 2023, at 3:26 AM, Daniel P. Berrangé wrote:
> On Wed, Mar 22, 2023 at 06:19:27PM -0600, Daniel Xu wrote:
>> Currently, any captured output (via `capture-output`) is segregated into
>> separate GuestExecStatus fields (`out-data` and `err-data`). This means
>> that downstream consumers have no way to reassemble the captured data
>> back into the original stream.
>> 
>> This is relevant for chatty and semi-interactive (ie. read only) CLI
>> tools.  Such tools may deliberately interleave stdout and stderr for
>> visual effect. If segregated, the output becomes harder to visually
>> understand.
>> 
>> This commit adds a new enum variant to the GuestExecCaptureOutputMode
>> qapi to merge the output streams such that consumers can have a pristine
>> view of the original command output.
>> 
>> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
>> ---
>>  qga/commands.c       | 25 +++++++++++++++++++++++--
>>  qga/qapi-schema.json |  5 ++++-
>>  2 files changed, 27 insertions(+), 3 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Is there anyone in particular I should CC to get this series merged?

Thanks,
Daniel


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode
  2023-03-31 20:23     ` Daniel Xu
@ 2023-04-03 14:56       ` Konstantin Kostiuk
  2023-05-01 17:54         ` Daniel Xu
  0 siblings, 1 reply; 8+ messages in thread
From: Konstantin Kostiuk @ 2023-04-03 14:56 UTC (permalink / raw)
  To: Daniel Xu
  Cc: Daniel P. Berrangé,
	michael.roth, Marc-André Lureau, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]

Hi Daniel,

I will merge this series after the 8.0 release.

Best Regards,
Konstantin Kostiuk.


On Fri, Mar 31, 2023 at 11:24 PM Daniel Xu <dxu@dxuuu.xyz> wrote:

> Hi Daniel,
>
> On Thu, Mar 23, 2023, at 3:26 AM, Daniel P. Berrangé wrote:
> > On Wed, Mar 22, 2023 at 06:19:27PM -0600, Daniel Xu wrote:
> >> Currently, any captured output (via `capture-output`) is segregated into
> >> separate GuestExecStatus fields (`out-data` and `err-data`). This means
> >> that downstream consumers have no way to reassemble the captured data
> >> back into the original stream.
> >>
> >> This is relevant for chatty and semi-interactive (ie. read only) CLI
> >> tools.  Such tools may deliberately interleave stdout and stderr for
> >> visual effect. If segregated, the output becomes harder to visually
> >> understand.
> >>
> >> This commit adds a new enum variant to the GuestExecCaptureOutputMode
> >> qapi to merge the output streams such that consumers can have a pristine
> >> view of the original command output.
> >>
> >> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> >> ---
> >>  qga/commands.c       | 25 +++++++++++++++++++++++--
> >>  qga/qapi-schema.json |  5 ++++-
> >>  2 files changed, 27 insertions(+), 3 deletions(-)
> >
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
> Is there anyone in particular I should CC to get this series merged?
>
> Thanks,
> Daniel
>
>

[-- Attachment #2: Type: text/html, Size: 2209 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode
  2023-04-03 14:56       ` Konstantin Kostiuk
@ 2023-05-01 17:54         ` Daniel Xu
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Xu @ 2023-05-01 17:54 UTC (permalink / raw)
  To: Konstantin Kostiuk
  Cc: Daniel P. Berrangé,
	michael.roth, Marc-André Lureau, qemu-devel

Hi Konstantin,

On Mon, Apr 3, 2023, at 8:56 AM, Konstantin Kostiuk wrote:
> Hi Daniel,
>
> I will merge this series after the 8.0 release.
>
> Best Regards,
> Konstantin Kostiuk.
>

Sorry to bug again, but 8.0 is out now right? Does this need a rebase
or is it good to go?

Thanks,
Daniel

[...]


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-05-01 17:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-23  0:19 [PATCH v6 0/3] qga: Support merging output streams in guest-exec Daniel Xu
2023-03-23  0:19 ` [PATCH v6 1/3] qga: Refactor guest-exec capture-output to take enum Daniel Xu
2023-03-23  0:19 ` [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode Daniel Xu
2023-03-23  9:26   ` Daniel P. Berrangé
2023-03-31 20:23     ` Daniel Xu
2023-04-03 14:56       ` Konstantin Kostiuk
2023-05-01 17:54         ` Daniel Xu
2023-03-23  0:19 ` [PATCH v6 3/3] qga: test: Add tests for `merged` flag Daniel Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.