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

[Laine Stump]

On 08/24/2012 03:02 AM, Daniel Veillard wrote:
> 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 !
>

Pushed. Thanks!