Re: [libvirt] [PATCHv2] nwfilter: don't log error if firewalld is disabled, improve diagnostics

On Fri, Aug 24, 2012 at 02:39:48AM -0400, Laine Stump wrote: > The original patch to support firewalld in nwfilter wasn't personally > checking the exit status of firewall-cmd, but was instead sending NULL > in the *exitstatus arg, which meant that virCommandWait would log an > error just for the exit status being non-0 (and a "more scary than > useful" error at that). > > We don't want to treat this as an error, though, just as a reason to > use standard (ip|eb)tables commands instead of firewall-cmd. > > This patch modifies the virCommandRun in the nwfilter code to request > status back from the caller. This avoids virCommandWait logging an > error message, and allows the caller to do as it likes after examining > the status. > > The VIR_DEBUG() logged when firewalld is enabled has also been > reworded and changed to a VIR_INFO, and a similar VIR_INFO has been > added in the case that firewalld is *not* found+enabled. > --- > src/nwfilter/nwfilter_ebiptables_driver.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c > index b008879..11fd04c 100644 > --- a/src/nwfilter/nwfilter_ebiptables_driver.c > +++ b/src/nwfilter/nwfilter_ebiptables_driver.c > @@ -4140,6 +4140,7 @@ ebiptablesDriverInitWithFirewallD(void) > virBuffer buf = VIR_BUFFER_INITIALIZER; > char *firewall_cmd_path; > char *output = NULL; > + int status; > int ret = -1; > > if (!virNWFilterDriverIsWatchingFirewallD()) > @@ -4155,9 +4156,11 @@ ebiptablesDriverInitWithFirewallD(void) > "%s", > CMD_STOPONERR(1)); > > - if (ebiptablesExecCLI(&buf, NULL, &output) == 0 && > - strlen(output) == 0) { > - VIR_DEBUG("Using firewall-cmd in nwfilter_ebiptables_driver."); > + if (ebiptablesExecCLI(&buf, &status, &output) < 0 || > + status != 0) { > + VIR_INFO("firewalld support disabled for nwfilter"); > + } else { > + VIR_INFO("firewalld support enabled for nwfilter"); > > ignore_value(virAsprintf(&ebtables_cmd_path, > "%s --direct --passthrough eb", ACK, need fixing befor 0.10.0, thanks !