Re: [PATCH 2/3] qga: Add optional stream-output argument to guest-exec

[Markus Armbruster <armbru@redhat.com>]

Daniel Xu <dxu@dxuuu.xyz> writes:

> Currently, commands run through guest-exec are "silent" until they
> finish running. This is fine for short lived commands. But for commands
> that take a while, this is a bad user experience.
>
> Usually long running programs know that they will run for a while. To
> improve user experience, they will typically print some kind of status
> to output at a regular interval. So that the user knows that their
> command isn't just hanging.
>
> This commit adds support for an optional stream-output parameter to
> guest-exec. This causes subsequent calls to guest-exec-status to return
> all buffered output. This allows downstream applications to be able to
> relay "status" to the end user.
>
> If stream-output is requested, it is up to the guest-exec-status caller
> to keep track of the last seen output position and slice the returned
> output appropriately. This is fairly trivial for a client to do. And it
> is a more reliable design than having QGA internally keep track of
> position -- for the cases that the caller "loses" a response.
>
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ---

[...]

> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index b720dd4379..0a76e35082 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1315,13 +1315,18 @@
>  # @capture-output: bool flag to enable capture of stdout/stderr of
>  #     running process.  defaults to false.
>  #
> +# @stream-output: causes future guest-exec-status calls to always
> +#     return current captured output rather than waiting to return
> +#     it all when the command exits. defaults to false. (since: 8.2)

So guest-exec-status normally returns no captured output until the
process terminates, right?  Its documentation (shown below for
convenience) did not make me expect this!

> +#
>  # Returns: PID on success.
>  #
>  # Since: 2.5
>  ##
>  { 'command': 'guest-exec',
>    'data':    { 'path': 'str', '*arg': ['str'], '*env': ['str'],
> -               '*input-data': 'str', '*capture-output': 'GuestExecCaptureOutput' },
> +               '*input-data': 'str', '*capture-output': 'GuestExecCaptureOutput',
> +               '*stream-output': 'bool' },
>    'returns': 'GuestExec' }

   ##
   # @GuestExecStatus:
   #
   # @exited: true if process has already terminated.
   #
   # @exitcode: process exit code if it was normally terminated.
   #
   # @signal: signal number (linux) or unhandled exception code (windows)
   #     if the process was abnormally terminated.
   #
   # @out-data: base64-encoded stdout of the process
   #
   # @err-data: 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: true if stdout was not fully captured due to size
   #     limitation.
   #
   # @err-truncated: 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-truncated': 'bool', '*err-truncated': 'bool' }}
   ##
   # @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.5
   ##
   { 'command': 'guest-exec-status',
     'data':    { 'pid': 'int' },
     'returns': 'GuestExecStatus' }

Could you throw in a patch to clarify what exactly is returned while the
process is still running, and what only after it terminated?  It should
go first.