Hi Marc-André,
Apologies if it looks like I ignored your previous email. I forgot to
renew my domain registration so all my emails were getting blackholed
or bounced for ~17 hours.
But w.r.t. fallback with old versions of glib, I think it may be a
little uglier than that. It looks like the build system uses
preprocessor macros to warn on too-new APIs. So I suppose we could
#undef and redefine GLIB_VERSION_MIN_REQUIRED, but may not be very nice.
But if you feel strongly I can try that.
On Tue, Feb 28, 2023 at 11:26:09PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Tue, Feb 28, 2023 at 10:51 PM Daniel Xu <d...@dxuuu.xyz> wrote:
> >
> > 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 commit adds a new optional flag to the guest-exec qapi to merge the
> > output streams such that consumers can have a pristine view of the
> > original command output.
> >
> > Signed-off-by: Daniel Xu <d...@dxuuu.xyz>
> > ---
> > qga/commands.c | 14 ++++++++++++--
> > qga/qapi-schema.json | 6 +++++-
> > 2 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/qga/commands.c b/qga/commands.c
> > index 172826f8f8..e13a8e86df 100644
> > --- a/qga/commands.c
> > +++ b/qga/commands.c
> > @@ -270,12 +270,20 @@ 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) {
> > + if (dup2(STDOUT_FILENO, STDERR_FILENO) != 0) {
> > + slog("dup2() failed to merge stderr into stdout: %s",
> > + strerror(errno));
>
> I would leave a FIXME comment for glib 2.58 g_spawn_async_with_fds() usage
Ack.
>
> > + }
> > + }
> > +
> > + /* Reset ignored signals back to default. */
> > memset(&sigact, 0, sizeof(struct sigaction));
> > sigact.sa_handler = SIG_DFL;
> >
> > @@ -384,6 +392,7 @@ GuestExec *qmp_guest_exec(const char *path,
> > bool has_env, strList *env,
> > const char *input_data,
> > bool has_capture_output, bool capture_output,
> > + bool has_merge_output, bool merge_output,
> > Error **errp)
> > {
> > GPid pid;
> > @@ -397,6 +406,7 @@ GuestExec *qmp_guest_exec(const char *path,
> > GIOChannel *in_ch, *out_ch, *err_ch;
> > GSpawnFlags flags;
> > bool has_output = (has_capture_output && capture_output);
> > + bool has_merge = (has_merge_output && merge_output);
> > g_autofree uint8_t *input = NULL;
> > size_t ninput = 0;
> >
> > @@ -420,7 +430,7 @@ GuestExec *qmp_guest_exec(const char *path,
> > }
> >
> > 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 796434ed34..9c2367acdf 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -1211,6 +1211,9 @@
> > # @input-data: data to be passed to process stdin (base64 encoded)
> > # @capture-output: bool flag to enable capture of
> > # stdout/stderr of running process. defaults to false.
> > +# @merge-output: bool flag to merge stdout/stderr of running process
> > +# into stdout. only effective if used with @capture-output.
> > +# not effective on windows guests. defaults to false.
> > (since 8.0)
>
> I think you should return an error on Windows instead.
Ack.
[...]
Thanks,
Daniel
