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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]