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@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; }