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.
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.
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); -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org