Re: [libvirt] [PATCH 01/10] nwfilter: don't ignore child process failures

On 02/20/2014 07:13 AM, Eric Blake wrote: > While auditing all callers of virCommandRun, I noticed that nwfilter > code never paid attention to commands with a non-zero status. > In the cases where status was captured, either the callers required > that the status was 0, or they discarded any failures from > virCommandRun. Collecting status manually means that a non-zero > child exit status is not logged, but I could not see the benefit > in avoiding the logging in any command issued in the code. > Therefore, it was simpler to just nuke the wasted effort of > manually checking or ignoring non-zero status. You need to be careful with this - for some of the external execs in nwfilter (same with viriptables.c), a non-0 status *should* be ignored and not reported. In particular, if a command that is attempting to remove an iptables or ebtables rule fails, that is often because libvirt is attempting to remove a rule that actually isn't there. As a matter of fact, in all the cases where the 2nd argument to ebiptablesExecCLI is non-NULL, that is exactly what's happening - the code was written that way to avoid putting a bogus and misleading error message in the logs; viriptables.c *does* log these errors, and that has led to many bug reports that incorrectly list the error message about failure to remove a rule as evidence that there is a bug. (I think there may even be a BZ filed requesting that these error logs be removed because they are misleading.) Because of the experience with viriptables.c, I don't think we should change the code to add back in the logging of these messages.