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

Eric Blake eblake at redhat.com
Fri Feb 21 17:19:38 UTC 2014


On 02/21/2014 04:20 AM, Laine Stump wrote:
> 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.

Then how about I rewrite this patch to instead pass a bool to
ebiptablesExecCLI that says whether to ignore non-zero status.  That
way, it doesn't look as crazy to have a status parameter passed through
a lot of the call stack that is otherwise ignored.  v2 coming up.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140221/cc794106/attachment-0001.sig>


More information about the libvir-list mailing list