[libvirt] [PATCH] nwfilter: don't log error if firewalld is disabled
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. --- src/nwfilter/nwfilter_ebiptables_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index b008879..8f7a453 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,8 +4156,8 @@ ebiptablesDriverInitWithFirewallD(void) "%s", CMD_STOPONERR(1)); - if (ebiptablesExecCLI(&buf, NULL, &output) == 0 && - strlen(output) == 0) { + if (ebiptablesExecCLI(&buf, &status, &output) == 0 && + status == 0) { VIR_DEBUG("Using firewall-cmd in nwfilter_ebiptables_driver."); ignore_value(virAsprintf(&ebtables_cmd_path, -- 1.7.11.4
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", -- 1.7.11.4
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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
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!
participants (2)
-
Daniel Veillard -
Laine Stump