[libvirt] [re-send][PATCH 2/3] use WIFEXITED macro to see exit status of child

[li guang]

在 2012-10-08一的 21:05 -0600，Eric Blake写道：
> On 10/08/2012 08:51 PM, li guang wrote:
> > 在 2012-10-08一的 20:05 -0600，Eric Blake写道：
> >> On 10/08/2012 07:51 PM, liguang wrote:
> >>> this usage was suggested by man-page of waitpid,
> >>> returns  true  if  the  child terminated normally
> >>
> >> NACK.  virCommandRun already did this for you.
> > 
> > right, but not exactly, 
> > virCommandRun will leave raw exit-status out of there,
> 
> Ah, after re-reading the code, so it does (I had thought we changed it
> to guarantee a return of -1 on any !WIFEXITED() exit, and a sanitized
> WEXITSTATUS() value, rather than making all the callers do that, but I
> guess not).
> 
> > so if the waited-command exit with a code '1',
> > then the caller of virCommandRun will see exit-status
> > value 0x100, and report an odd '256' exit-status. 
> 
> That depends on your OS.  There are some systems out there where normal
> exit is in the low-order rather than high-order byte.  But that's why we
> have virProcessTranslateStatus(), to do the work correctly.

virProcessTranslateStatus seems just a int to string translation


> 
> >>>      ret = virCommandRun(cmd, &exitstatus);
> >>> -    if (ret == 0 && exitstatus != 0) {
> >>> +    if (ret == 0 && WIFEXITED(exitstatus) == 0) {
> 
> You have a logic bug - the old code was checking for non-zero status,
> and you switched it to check for zero status.
> 

'WIFEXITED(exitstatus) == 0' means waited-command exit unexpectedly, so,
report error, my thought is try to ignore the normal 'exit(x)'s x value'
returned by waited-command.


> >>>          virReportError(VIR_ERR_HOOK_SCRIPT_FAILED,
> >>>                         _("Hook script %s %s failed with error code %d"),
> >>>                         path, drvstr, exitstatus);
> 
> This is not a correct error message - if you ever use WIFEXITED, you
> must also use WEXITSTATUS, otherwise you have a mismatch.  And for this
> particular error message, I think we are losing useful information; I
> argue that we'd probably get a much better error message if we changed
> this to let virCommandRun do ALL the work:
> 
> ret = virCommandRun(cmd, NULL);
> 

-- 
liguang    lig.fnst at cn.fujitsu.com
FNST linux kernel team