Re: [libvirt] [PATCH v2 06/10] virCommandWait: Propagate dryRunCallback return value properly
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(a)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;
}