[libvirt] [PATCH v2 06/10] virCommandWait: Propagate dryRunCallback return value properly

[John Ferlan]

On 07/04/2018 05:23 AM, Michal Privoznik wrote:
> The documentation to virCommandWait() function states that if
> @exitstatus is NULL and command finished with error -1 is
> returned. In other words, if @dryRunCallback is set and returns
> an error (by setting its @status argument to a nonzero value) we
> must propagate this error properly honouring the documentation
> (and also regular run).
> 

That's not how I read virCommandWait:

 * Wait for the command previously started with virCommandRunAsync()
 * to complete. Return -1 on any error waiting for
 * completion. Returns 0 if the command
 * finished with the exit status set.  If @exitstatus is NULL, then the
 * child must exit with status 0 for this to succeed.  By default,
 * a non-NULL @exitstatus contains the normal exit status of the child
 * (death from a signal is treated as execution error); but if
 * virCommandRawStatus() was used, it instead contains the raw exit
 * status that the caller must then decipher using WIFEXITED() and friends.

perhaps the author (danpb) of commit id 7b3f1f8c3 would be able to say
for sure...

I only see -1 being returned "on any error waiting for completion".
Filling @exitstatus with @dryRunStatus is reasonable since it is
initialized to 0 in virCommandRunAsync and is what is passed to
@dryRunCallback and thus only changed as a result of running
@dryRunCallback.

It has nothing to do with virCommandWait AFAICT.

John


> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/util/vircommand.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index 6dab105f56..13f75967fa 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -2553,6 +2553,8 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
>                    dryRunStatus);
>          if (exitstatus)
>              *exitstatus = dryRunStatus;
> +        else if (dryRunStatus)
> +            return -1;
>          return 0;
>      }
>  
>