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

[Daniel Veillard]

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 !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/