diff mbox

configure: Remove build time checks for (ip|ip6|eb)tables

Message ID 311bc674d484cfc6cf545903ab92e31e85f21078.1461272770.git.crobinso@redhat.com
State Superseded
Headers show

Commit Message

Cole Robinson April 21, 2016, 9:06 p.m. UTC 
And the 'ip' tool. There isn't much benefit to checking this at
configure time when we have infrastructure nowadays for looking up
binaries in the PATH

https://bugzilla.redhat.com/show_bug.cgi?id=661262
---
 configure.ac            | 12 ------
 src/util/virfirewall.c  | 18 +++++----
 src/util/virnetdev.c    |  6 +--
 tests/virfirewalltest.c | 98 ++++++++++++++++++++++++-------------------------
 4 files changed, 62 insertions(+), 72 deletions(-)

-- 
2.7.3

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

Comments

Cole Robinson April 22, 2016, 1:42 p.m. UTC | #1 
On 04/22/2016 09:18 AM, Andrea Bolognani wrote:
> On Thu, 2016-04-21 at 17:06 -0400, Cole Robinson wrote:

>> And the 'ip' tool. There isn't much benefit to checking this at

>> configure time when we have infrastructure nowadays for looking up

>> binaries in the PATH

>>  

>> https://bugzilla.redhat.com/show_bug.cgi?id=661262

>> ---

>>   configure.ac            | 12 ------

>>   src/util/virfirewall.c  | 18 +++++----

>>   src/util/virnetdev.c    |  6 +--

>>   tests/virfirewalltest.c | 98 ++++++++++++++++++++++++-------------------------

>>   4 files changed, 62 insertions(+), 72 deletions(-)

> 

> I haven't tried running this so I'm probably missing

> something, but...

> 

>> @@ -182,17 +182,19 @@ virFirewallValidateBackend(virFirewallBackend backend)

>>   

>>       if (backend == VIR_FIREWALL_BACKEND_DIRECT) {

>>           const char *commands[] = {

>> -            IPTABLES_PATH, IP6TABLES_PATH, EBTABLES_PATH

>> +            "iptables", "ip6tables", "ebtables"

>>           };

>>           size_t i;

>>   

>>           for (i = 0; i < ARRAY_CARDINALITY(commands); i++) {

>> -            if (!virFileIsExecutable(commands[i])) {

>> +            char *path = virFindFileInPath(commands[i]);

>> +            if (!path) {

>>                   virReportSystemError(errno,

>>                                        _("direct firewall backend requested, but %s is not available"),

>>                                        commands[i]);

>>                   return -1;

>>               }

>> +            VIR_FREE(path);

>>           }

>>           VIR_DEBUG("found iptables/ip6tables/ebtables, using direct backend");

>>       }

> 

> ... how is this fixing the issue reported above?

> 


Oh, hmm, maybe it doesn't, sorry. I was misreading; I thought the report was
'build libvirtd without iptables, install it later, libvirt won't work'.

> AFAICT you just changed it to perform a filesystem lookup instead

> of relying on the information obtained at configure time. And you

> removed the check on the file being executable, which is probably

> not a good idea?


Judging from the error message it seems like virFileIsExecutable was just a
surrogate for access(path, F_OK), but I can re add it. As long as someone at
least thinks this is a worthwhile patch otherwise

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Cole Robinson April 22, 2016, 2:15 p.m. UTC | #2 
On 04/22/2016 10:14 AM, Laine Stump wrote:
> On 04/22/2016 09:42 AM, Cole Robinson wrote:

>> On 04/22/2016 09:18 AM, Andrea Bolognani wrote:

>>> On Thu, 2016-04-21 at 17:06 -0400, Cole Robinson wrote:

>>>> And the 'ip' tool. There isn't much benefit to checking this at

>>>> configure time when we have infrastructure nowadays for looking up

>>>> binaries in the PATH

>>>>   https://bugzilla.redhat.com/show_bug.cgi?id=661262

>>>> ---

>>>>    configure.ac            | 12 ------

>>>>    src/util/virfirewall.c  | 18 +++++----

>>>>    src/util/virnetdev.c    |  6 +--

>>>>    tests/virfirewalltest.c | 98

>>>> ++++++++++++++++++++++++-------------------------

>>>>    4 files changed, 62 insertions(+), 72 deletions(-)

>>> I haven't tried running this so I'm probably missing

>>> something, but...

>>>

>>>> @@ -182,17 +182,19 @@ virFirewallValidateBackend(virFirewallBackend backend)

>>>>           if (backend == VIR_FIREWALL_BACKEND_DIRECT) {

>>>>            const char *commands[] = {

>>>> -            IPTABLES_PATH, IP6TABLES_PATH, EBTABLES_PATH

>>>> +            "iptables", "ip6tables", "ebtables"

>>>>            };

>>>>            size_t i;

>>>>               for (i = 0; i < ARRAY_CARDINALITY(commands); i++) {

>>>> -            if (!virFileIsExecutable(commands[i])) {

>>>> +            char *path = virFindFileInPath(commands[i]);

>>>> +            if (!path) {

>>>>                    virReportSystemError(errno,

>>>>                                         _("direct firewall backend

>>>> requested, but %s is not available"),

>>>>                                         commands[i]);

>>>>                    return -1;

>>>>                }

>>>> +            VIR_FREE(path);

>>>>            }

>>>>            VIR_DEBUG("found iptables/ip6tables/ebtables, using direct

>>>> backend");

>>>>        }

>>> ... how is this fixing the issue reported above?

>>>

>> Oh, hmm, maybe it doesn't, sorry. I was misreading; I thought the report was

>> 'build libvirtd without iptables, install it later, libvirt won't work'.

>>

>>> AFAICT you just changed it to perform a filesystem lookup instead

>>> of relying on the information obtained at configure time. And you

>>> removed the check on the file being executable, which is probably

>>> not a good idea?

>> Judging from the error message it seems like virFileIsExecutable was just a

>> surrogate for access(path, F_OK), but I can re add it. As long as someone at

>> least thinks this is a worthwhile patch otherwise

> 

> I think it's worthwhile; even though the number of self-builders is fairly

> low, they do take time sorting out on IRC. I agree with Andrea that the

> virFileIsExecutable call should remain in, since you can never count on one of

> these self-built systems to have *anything* setup sanely :-P

> 

> Once this change is made, I think we can remove all the "BuildRequires:

> (ebtables/iptables/iptables-ipv6)" from the specfile (as long as there is no

> other odd usage of them in configure.ac)

> 


Okay I'll look into it

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Cole Robinson April 23, 2016, 7:40 p.m. UTC | #3 
On 04/22/2016 10:14 AM, Laine Stump wrote:
> On 04/22/2016 09:42 AM, Cole Robinson wrote:

>> On 04/22/2016 09:18 AM, Andrea Bolognani wrote:

>>> On Thu, 2016-04-21 at 17:06 -0400, Cole Robinson wrote:

>>>> And the 'ip' tool. There isn't much benefit to checking this at

>>>> configure time when we have infrastructure nowadays for looking up

>>>> binaries in the PATH

>>>>   https://bugzilla.redhat.com/show_bug.cgi?id=661262

>>>> ---

>>>>    configure.ac            | 12 ------

>>>>    src/util/virfirewall.c  | 18 +++++----

>>>>    src/util/virnetdev.c    |  6 +--

>>>>    tests/virfirewalltest.c | 98

>>>> ++++++++++++++++++++++++-------------------------

>>>>    4 files changed, 62 insertions(+), 72 deletions(-)

>>> I haven't tried running this so I'm probably missing

>>> something, but...

>>>

>>>> @@ -182,17 +182,19 @@ virFirewallValidateBackend(virFirewallBackend backend)

>>>>           if (backend == VIR_FIREWALL_BACKEND_DIRECT) {

>>>>            const char *commands[] = {

>>>> -            IPTABLES_PATH, IP6TABLES_PATH, EBTABLES_PATH

>>>> +            "iptables", "ip6tables", "ebtables"

>>>>            };

>>>>            size_t i;

>>>>               for (i = 0; i < ARRAY_CARDINALITY(commands); i++) {

>>>> -            if (!virFileIsExecutable(commands[i])) {

>>>> +            char *path = virFindFileInPath(commands[i]);

>>>> +            if (!path) {

>>>>                    virReportSystemError(errno,

>>>>                                         _("direct firewall backend

>>>> requested, but %s is not available"),

>>>>                                         commands[i]);

>>>>                    return -1;

>>>>                }

>>>> +            VIR_FREE(path);

>>>>            }

>>>>            VIR_DEBUG("found iptables/ip6tables/ebtables, using direct

>>>> backend");

>>>>        }

>>> ... how is this fixing the issue reported above?

>>>

>> Oh, hmm, maybe it doesn't, sorry. I was misreading; I thought the report was

>> 'build libvirtd without iptables, install it later, libvirt won't work'.

>>

>>> AFAICT you just changed it to perform a filesystem lookup instead

>>> of relying on the information obtained at configure time. And you

>>> removed the check on the file being executable, which is probably

>>> not a good idea?

>> Judging from the error message it seems like virFileIsExecutable was just a

>> surrogate for access(path, F_OK), but I can re add it. As long as someone at

>> least thinks this is a worthwhile patch otherwise

> 

> I think it's worthwhile; even though the number of self-builders is fairly

> low, they do take time sorting out on IRC. I agree with Andrea that the

> virFileIsExecutable call should remain in, since you can never count on one of

> these self-built systems to have *anything* setup sanely :-P

> 

> Once this change is made, I think we can remove all the "BuildRequires:

> (ebtables/iptables/iptables-ipv6)" from the specfile (as long as there is no

> other odd usage of them in configure.ac)

> 


I didn't do the spec change... it looks like the test suite is indirectly
dependent on host iptables/ip6tables/ebtables being available, it's calling
into virFirewallValidateBackend somehow. I didn't dig into it though

- Cole

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

Patch

diff --git a/configure.ac b/configure.ac
index de5f430..35ae16e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -694,18 +694,6 @@  if test x"$with_rhel5_api" = x"yes"; then
    AC_DEFINE([WITH_RHEL5_API], [1], [whether building for the RHEL-5 API])
 fi
 
-AC_PATH_PROG([IP_PATH], [ip], /sbin/ip, [/usr/sbin:$PATH])
-AC_DEFINE_UNQUOTED([IP_PATH], "$IP_PATH", [path to ip binary])
-
-AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH])
-AC_DEFINE_UNQUOTED([IPTABLES_PATH], "$IPTABLES_PATH", [path to iptables binary])
-
-AC_PATH_PROG([IP6TABLES_PATH], [ip6tables], /sbin/ip6tables, [/usr/sbin:$PATH])
-AC_DEFINE_UNQUOTED([IP6TABLES_PATH], "$IP6TABLES_PATH", [path to ip6tables binary])
-
-AC_PATH_PROG([EBTABLES_PATH], [ebtables], /sbin/ebtables, [/usr/sbin:$PATH])
-AC_DEFINE_UNQUOTED([EBTABLES_PATH], "$EBTABLES_PATH", [path to ebtables binary])
-
 
 dnl
 dnl Checks for the OpenVZ driver
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index f26fd86..63f9709 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -47,9 +47,9 @@  typedef virFirewallGroup *virFirewallGroupPtr;
 
 VIR_ENUM_DECL(virFirewallLayerCommand)
 VIR_ENUM_IMPL(virFirewallLayerCommand, VIR_FIREWALL_LAYER_LAST,
-              EBTABLES_PATH,
-              IPTABLES_PATH,
-              IP6TABLES_PATH);
+              "ebtables",
+              "iptables",
+              "ip6tables");
 
 VIR_ENUM_DECL(virFirewallLayerFirewallD)
 VIR_ENUM_IMPL(virFirewallLayerFirewallD, VIR_FIREWALL_LAYER_LAST,
@@ -134,13 +134,13 @@  static void
 virFirewallCheckUpdateLocking(void)
 {
     const char *iptablesArgs[] = {
-        IPTABLES_PATH, "-w", "-L", "-n", NULL,
+        "iptables", "-w", "-L", "-n", NULL,
     };
     const char *ip6tablesArgs[] = {
-        IP6TABLES_PATH, "-w", "-L", "-n", NULL,
+        "ip6tables", "-w", "-L", "-n", NULL,
     };
     const char *ebtablesArgs[] = {
-        EBTABLES_PATH, "--concurrent", "-L", NULL,
+        "ebtables", "--concurrent", "-L", NULL,
     };
     if (lockOverride)
         return;
@@ -182,17 +182,19 @@  virFirewallValidateBackend(virFirewallBackend backend)
 
     if (backend == VIR_FIREWALL_BACKEND_DIRECT) {
         const char *commands[] = {
-            IPTABLES_PATH, IP6TABLES_PATH, EBTABLES_PATH
+            "iptables", "ip6tables", "ebtables"
         };
         size_t i;
 
         for (i = 0; i < ARRAY_CARDINALITY(commands); i++) {
-            if (!virFileIsExecutable(commands[i])) {
+            char *path = virFindFileInPath(commands[i]);
+            if (!path) {
                 virReportSystemError(errno,
                                      _("direct firewall backend requested, but %s is not available"),
                                      commands[i]);
                 return -1;
             }
+            VIR_FREE(path);
         }
         VIR_DEBUG("found iptables/ip6tables/ebtables, using direct backend");
     }
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index bb17b84..75e45fd 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1469,7 +1469,7 @@  int virNetDevSetIPAddress(const char *ifname,
         virCommandAddArgList(cmd, "broadcast", bcaststr, NULL);
     virCommandAddArg(cmd, "alias");
 # else
-    cmd = virCommandNew(IP_PATH);
+    cmd = virCommandNew("ip");
     virCommandAddArgList(cmd, "addr", "add", NULL);
     virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
     if (peerstr)
@@ -1506,7 +1506,7 @@  virNetDevAddRoute(const char *ifname,
         goto cleanup;
     if (!(gatewaystr = virSocketAddrFormat(gateway)))
         goto cleanup;
-    cmd = virCommandNew(IP_PATH);
+    cmd = virCommandNew("ip");
     virCommandAddArgList(cmd, "route", "add", NULL);
     virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
     virCommandAddArgList(cmd, "via", gatewaystr, "dev", ifname,
@@ -1544,7 +1544,7 @@  int virNetDevClearIPAddress(const char *ifname,
     virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
     virCommandAddArg(cmd, "-alias");
 # else
-    cmd = virCommandNew(IP_PATH);
+    cmd = virCommandNew("ip");
     virCommandAddArgList(cmd, "addr", "del", NULL);
     virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
     virCommandAddArgList(cmd, "dev", ifname, NULL);
diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
index f1f29c6..976e883 100644
--- a/tests/virfirewalltest.c
+++ b/tests/virfirewalltest.c
@@ -128,11 +128,11 @@  VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
 
         if (fwBuf) {
             if (STREQ(type, "ipv4"))
-                virBufferAddLit(fwBuf, IPTABLES_PATH);
+                virBufferAddLit(fwBuf, "iptables");
             else if (STREQ(type, "ipv4"))
-                virBufferAddLit(fwBuf, IP6TABLES_PATH);
+                virBufferAddLit(fwBuf, "ip6tables");
             else
-                virBufferAddLit(fwBuf, EBTABLES_PATH);
+                virBufferAddLit(fwBuf, "ebtables");
         }
         for (i = 0; i < nargs; i++) {
             if (fwBuf) {
@@ -204,8 +204,8 @@  testFirewallSingleGroup(const void *opaque)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
     const struct testFirewallData *data = opaque;
 
     fwDisabled = data->fwDisabled;
@@ -263,8 +263,8 @@  testFirewallRemoveRule(const void *opaque)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
     const struct testFirewallData *data = opaque;
     virFirewallRulePtr fwrule;
 
@@ -329,10 +329,10 @@  testFirewallManyGroups(const void *opaque ATTRIBUTE_UNUSED)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"
-        IPTABLES_PATH " -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A OUTPUT --jump DROP\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"
+        "iptables -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A OUTPUT --jump DROP\n";
     const struct testFirewallData *data = opaque;
 
     fwDisabled = data->fwDisabled;
@@ -423,10 +423,10 @@  testFirewallIgnoreFailGroup(const void *opaque ATTRIBUTE_UNUSED)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
-        IPTABLES_PATH " -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A OUTPUT --jump DROP\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+        "iptables -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A OUTPUT --jump DROP\n";
     const struct testFirewallData *data = opaque;
 
     fwDisabled = data->fwDisabled;
@@ -498,10 +498,10 @@  testFirewallIgnoreFailRule(const void *opaque ATTRIBUTE_UNUSED)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
-        IPTABLES_PATH " -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A OUTPUT --jump DROP\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+        "iptables -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A OUTPUT --jump DROP\n";
     const struct testFirewallData *data = opaque;
 
     fwDisabled = data->fwDisabled;
@@ -572,8 +572,8 @@  testFirewallNoRollback(const void *opaque ATTRIBUTE_UNUSED)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n";
     const struct testFirewallData *data = opaque;
 
     fwDisabled = data->fwDisabled;
@@ -642,11 +642,11 @@  testFirewallSingleRollback(const void *opaque ATTRIBUTE_UNUSED)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
-        IPTABLES_PATH " -D INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
-        IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+        "iptables -D INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
+        "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
     const struct testFirewallData *data = opaque;
 
     fwDisabled = data->fwDisabled;
@@ -732,10 +732,10 @@  testFirewallManyRollback(const void *opaque ATTRIBUTE_UNUSED)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
-        IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
-        IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+        "iptables -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
+        "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
     const struct testFirewallData *data = opaque;
 
     fwDisabled = data->fwDisabled;
@@ -825,14 +825,14 @@  testFirewallChainedRollback(const void *opaque ATTRIBUTE_UNUSED)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.127 --jump REJECT\n"
-        IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
-        IPTABLES_PATH " -D INPUT --source-host 192.168.122.127 --jump REJECT\n"
-        IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"
-        IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
-        IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host 192.168.122.127 --jump REJECT\n"
+        "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"
+        "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+        "iptables -D INPUT --source-host 192.168.122.127 --jump REJECT\n"
+        "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"
+        "iptables -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
+        "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
     const struct testFirewallData *data = opaque;
 
     fwDisabled = data->fwDisabled;
@@ -976,11 +976,11 @@  testFirewallQueryHook(const char *const*args,
                       int *status,
                       void *opaque ATTRIBUTE_UNUSED)
 {
-    if (STREQ(args[0], IPTABLES_PATH) &&
+    if (STREQ(args[0], "iptables") &&
         STREQ(args[1], "-L")) {
         if (VIR_STRDUP(*output, TEST_FILTER_TABLE_LIST) < 0)
             *status = 127;
-    } else if (STREQ(args[0], IPTABLES_PATH) &&
+    } else if (STREQ(args[0], "iptables") &&
                STREQ(args[1], "-t") &&
                STREQ(args[2], "nat") &&
                STREQ(args[3], "-L")) {
@@ -1026,15 +1026,15 @@  testFirewallQuery(const void *opaque ATTRIBUTE_UNUSED)
     int ret = -1;
     const char *actual = NULL;
     const char *expected =
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.127 --jump REJECT\n"
-        IPTABLES_PATH " -L\n"
-        IPTABLES_PATH " -t nat -L\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.130 --jump REJECT\n"
-        IPTABLES_PATH " -A INPUT --source-host '!192.168.122.129' --jump REJECT\n"
-        IPTABLES_PATH " -A INPUT --source-host '!192.168.122.129' --jump REJECT\n"
-        IPTABLES_PATH " -A INPUT --source-host 192.168.122.128 --jump REJECT\n"
-        IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+        "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+        "iptables -A INPUT --source-host 192.168.122.127 --jump REJECT\n"
+        "iptables -L\n"
+        "iptables -t nat -L\n"
+        "iptables -A INPUT --source-host 192.168.122.130 --jump REJECT\n"
+        "iptables -A INPUT --source-host '!192.168.122.129' --jump REJECT\n"
+        "iptables -A INPUT --source-host '!192.168.122.129' --jump REJECT\n"
+        "iptables -A INPUT --source-host 192.168.122.128 --jump REJECT\n"
+        "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
     const struct testFirewallData *data = opaque;
 
     expectedLineNum = 0;