在 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@xxxxxxxxxxxxxx FNST linux kernel team -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list