[libvirt] [PATCH 11/26] Introduce an object for managing firewall rulesets

Daniel P. Berrange berrange at redhat.com
Tue Apr 8 15:38:03 UTC 2014


The network and nwfilter drivers both have a need to update
firewall rules. The currently share no code for interacting
with iptables / firewalld. The nwfilter driver is fairly
tied to the concept of creating shell scripts to execute
which makes it very hard to port to talk to firewalld via
DBus APIs.

This patch introduces a virFirewallPtr object which is able
to represent a complete sequence of rule changes, with the
ability to have multiple transactional checkpoints with
rollbacks. By formally separating the definition of the rules
to be applied from the mechanism used to apply them, it is
also possible to write a firewall engine that uses firewalld
DBus APIs natively instead of via the slow firewalld-cmd.

Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
---
 include/libvirt/virterror.h |    1 +
 po/POTFILES.in              |    1 +
 src/Makefile.am             |    2 +
 src/libvirt_private.syms    |   17 +
 src/util/virerror.c         |    1 +
 src/util/virfirewall.c      |  922 +++++++++++++++++++++++++++++++++
 src/util/virfirewall.h      |  109 ++++
 src/util/virfirewallpriv.h  |   45 ++
 tests/Makefile.am           |    7 +
 tests/testutils.c           |   18 +-
 tests/virfirewalltest.c     | 1186 +++++++++++++++++++++++++++++++++++++++++++
 11 files changed, 2305 insertions(+), 4 deletions(-)
 create mode 100644 src/util/virfirewall.c
 create mode 100644 src/util/virfirewall.h
 create mode 100644 src/util/virfirewallpriv.h
 create mode 100644 tests/virfirewalltest.c

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 495c121..be90797 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -122,6 +122,7 @@ typedef enum {
     VIR_FROM_SYSTEMD = 56,      /* Error from systemd code */
     VIR_FROM_BHYVE = 57,        /* Error from bhyve driver */
     VIR_FROM_CRYPTO = 58,       /* Error from crypto code */
+    VIR_FROM_FIREWALL = 59,     /* Error from firewall */
 
 # ifdef VIR_ENUM_SENTINELS
     VIR_ERR_DOMAIN_LAST
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 122b853..e35eb82 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -161,6 +161,7 @@ src/util/virdbus.c
 src/util/virdnsmasq.c
 src/util/vireventpoll.c
 src/util/virfile.c
+src/util/virfirewall.c
 src/util/virhash.c
 src/util/virhook.c
 src/util/virhostdev.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 7e9a702..7615294 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -106,6 +106,8 @@ UTIL_SOURCES =							\
 		util/virevent.c util/virevent.h			\
 		util/vireventpoll.c util/vireventpoll.h		\
 		util/virfile.c util/virfile.h			\
+		util/virfirewall.c util/virfirewall.h		\
+		util/virfirewallpriv.h				\
 		util/virhash.c util/virhash.h			\
 		util/virhashcode.c util/virhashcode.h		\
 		util/virhook.c util/virhook.h			\
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0c2cf75..18be0e1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1277,6 +1277,23 @@ virFileWriteStr;
 virFindFileInPath;
 
 
+# util/virfirewall.h
+virFirewallAddRule;
+virFirewallAddRuleFull;
+virFirewallApply;
+virFirewallFree;
+virFirewallNew;
+virFirewallRemoveRule;
+virFirewallRuleAddArg;
+virFirewallRuleAddArgFormat;
+virFirewallRuleAddArgList;
+virFirewallRuleAddArgSet;
+virFirewallRuleGetArgCount;
+virFirewallSetBackend;
+virFirewallStartRollback;
+virFirewallStartTransaction;
+
+
 # util/virhash.h
 virHashAddEntry;
 virHashCreate;
diff --git a/src/util/virerror.c b/src/util/virerror.c
index cbbaa83..e0bc970 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -129,6 +129,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
               "Systemd",
               "Bhyve",
               "Crypto",
+              "Firewall",
     )
 
 
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
new file mode 100644
index 0000000..b558d2f
--- /dev/null
+++ b/src/util/virfirewall.c
@@ -0,0 +1,922 @@
+/*
+ * virfirewall.c: integration with firewalls
+ *
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *    Daniel P. Berrange <berrange at redhat.com>
+ */
+
+#include <config.h>
+
+#define __VIR_FIREWALL_PRIV_H_ALLOW__
+
+#include <stdarg.h>
+
+#include "viralloc.h"
+#include "virfirewallpriv.h"
+#include "virerror.h"
+#include "virutil.h"
+#include "virstring.h"
+#include "vircommand.h"
+#include "virlog.h"
+#include "virdbus.h"
+#include "virfile.h"
+#include "virthread.h"
+
+#define VIR_FROM_THIS VIR_FROM_FIREWALL
+
+VIR_LOG_INIT("util.firewall");
+
+typedef struct _virFirewallGroup virFirewallGroup;
+typedef virFirewallGroup *virFirewallGroupPtr;
+
+VIR_ENUM_DECL(virFirewallLayerCommand)
+VIR_ENUM_IMPL(virFirewallLayerCommand, VIR_FIREWALL_LAYER_LAST,
+              EBTABLES_PATH,
+              IPTABLES_PATH,
+              IP6TABLES_PATH);
+
+VIR_ENUM_DECL(virFirewallLayerFirewallD)
+VIR_ENUM_IMPL(virFirewallLayerFirewallD, VIR_FIREWALL_LAYER_LAST,
+              "eb", "ipv4", "ipv6")
+
+
+struct _virFirewallRule {
+    virFirewallLayer layer;
+
+    virFirewallQueryCallback queryCB;
+    void *queryOpaque;
+    bool ignoreErrors;
+
+    size_t argsAlloc;
+    size_t argsLen;
+    char **args;
+};
+
+struct _virFirewallGroup {
+    unsigned int actionFlags;
+    unsigned int rollbackFlags;
+
+    size_t naction;
+    virFirewallRulePtr *action;
+
+    size_t nrollback;
+    virFirewallRulePtr *rollback;
+
+    bool addingRollback;
+};
+
+
+struct _virFirewall {
+    int err;
+
+    size_t ngroups;
+    virFirewallGroupPtr *groups;
+    size_t currentGroup;
+};
+
+static virFirewallBackend currentBackend = VIR_FIREWALL_BACKEND_AUTOMATIC;
+static virMutex ruleLock = VIR_MUTEX_INITIALIZER;
+
+static int
+virFirewallValidateBackend(virFirewallBackend backend);
+
+static int
+virFirewallOnceInit(void)
+{
+    return virFirewallValidateBackend(currentBackend);
+}
+
+VIR_ONCE_GLOBAL_INIT(virFirewall)
+
+static int
+virFirewallValidateBackend(virFirewallBackend backend)
+{
+    VIR_DEBUG("Validating backend %d", backend);
+#if WITH_DBUS
+    if (backend == VIR_FIREWALL_BACKEND_AUTOMATIC ||
+        backend == VIR_FIREWALL_BACKEND_FIREWALLD) {
+        int rv = virDBusIsServiceRegistered(VIR_FIREWALL_FIREWALLD_SERVICE);
+        VIR_DEBUG("Firewalled is registered ? %d", rv);
+        if (rv < 0) {
+            if (rv == -2) {
+                if (backend == VIR_FIREWALL_BACKEND_FIREWALLD) {
+                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                   _("firewalld firewall backend requested, but service is not running"));
+                    return -1;
+                } else {
+                    VIR_DEBUG("firewalld service not running, trying direct backend");
+                    backend = VIR_FIREWALL_BACKEND_DIRECT;
+                }
+            } else {
+                return -1;
+            }
+        } else {
+            VIR_DEBUG("firewalld service running, using firewalld backend");
+            backend = VIR_FIREWALL_BACKEND_FIREWALLD;
+        }
+    }
+#else
+    if (backend == VIR_FIREWALL_BACKEND_AUTOMATIC) {
+        VIR_DEBUG("DBus support disabled, trying direct backend");
+        backend = VIR_FIREWALL_BACKEND_DIRECT;
+    } else if (backend == VIR_FIREWALL_BACKEND_FIREWALLD) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("firewalld firewall backend requested, but DBus support disabled"));
+        return -1;
+    }
+#endif
+
+    if (backend == VIR_FIREWALL_BACKEND_DIRECT) {
+        const char *commands[] = {
+            IPTABLES_PATH, IP6TABLES_PATH, EBTABLES_PATH
+        };
+        size_t i;
+        for (i = 0; i < ARRAY_CARDINALITY(commands); i++) {
+            if (!virFileIsExecutable(commands[i])) {
+                virReportSystemError(errno,
+                                     _("direct firewall backend requested, but %s is not available"),
+                                     commands[i]);
+                return -1;
+            }
+        }
+        VIR_DEBUG("found iptables/ip6tables/ebtables, using direct backend");
+    }
+
+    currentBackend = backend;
+    return 0;
+}
+
+int
+virFirewallSetBackend(virFirewallBackend backend)
+{
+    currentBackend = backend;
+
+    if (virFirewallInitialize() < 0)
+        return -1;
+
+    return virFirewallValidateBackend(backend);
+}
+
+static virFirewallGroupPtr
+virFirewallGroupNew(void)
+{
+    virFirewallGroupPtr group;
+
+    if (VIR_ALLOC(group) < 0)
+        return NULL;
+
+    return group;
+}
+
+
+/**
+ * virFirewallNew:
+ *
+ * Creates a new firewall ruleset for changing rules
+ * of @layer. This should be followed by a call to
+ * virFirewallStartTransaction before adding
+ * any rules
+ *
+ * Returns the new firewall ruleset
+ */
+virFirewallPtr virFirewallNew(void)
+{
+    virFirewallPtr firewall;
+
+    if (VIR_ALLOC(firewall) < 0)
+        return NULL;
+
+    return firewall;
+}
+
+
+static void
+virFirewallRuleFree(virFirewallRulePtr rule)
+{
+    size_t i;
+
+    if (!rule)
+        return;
+
+    for (i = 0; i < rule->argsLen; i++)
+        VIR_FREE(rule->args[i]);
+    VIR_FREE(rule->args);
+    VIR_FREE(rule);
+}
+
+
+static void
+virFirewallGroupFree(virFirewallGroupPtr group)
+{
+    size_t i;
+
+    if (!group)
+        return;
+
+    for (i = 0; i < group->naction; i++)
+        virFirewallRuleFree(group->action[i]);
+    VIR_FREE(group->action);
+
+    for (i = 0; i < group->nrollback; i++)
+        virFirewallRuleFree(group->rollback[i]);
+    VIR_FREE(group->rollback);
+
+    VIR_FREE(group);
+}
+
+
+/**
+ * virFirewallFree:
+ *
+ * Release all memory associated with the firewall
+ * ruleset
+ */
+void virFirewallFree(virFirewallPtr firewall)
+{
+    size_t i;
+
+    if (!firewall)
+        return;
+
+    for (i = 0; i < firewall->ngroups; i++)
+        virFirewallGroupFree(firewall->groups[i]);
+    VIR_FREE(firewall->groups);
+
+    VIR_FREE(firewall);
+}
+
+#define VIR_FIREWALL_RETURN_IF_ERROR(firewall)          \
+    if (!firewall || firewall->err)                     \
+        return;
+
+#define VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, ruel)\
+    if (!firewall || firewall->err || !rule)            \
+        return;
+
+#define VIR_FIREWALL_RETURN_NULL_IF_ERROR(firewall)     \
+    if (!firewall || firewall->err)                     \
+        return NULL;
+
+#define ADD_ARG(rule, str)                              \
+    if (VIR_RESIZE_N(rule->args,                        \
+                     rule->argsAlloc,                   \
+                     rule->argsLen, 1) < 0)             \
+        goto no_memory;                                 \
+                                                        \
+    if (VIR_STRDUP(rule->args[rule->argsLen++], str) < 0)\
+        goto no_memory;
+
+static virFirewallRulePtr
+virFirewallAddRuleFullV(virFirewallPtr firewall,
+                        virFirewallLayer layer,
+                        bool ignoreErrors,
+                        virFirewallQueryCallback cb,
+                        void *opaque,
+                        va_list args)
+{
+    virFirewallGroupPtr group;
+    virFirewallRulePtr rule;
+    char *str;
+
+    VIR_FIREWALL_RETURN_NULL_IF_ERROR(firewall);
+
+    if (firewall->ngroups == 0) {
+        firewall->err = ENODATA;
+        return NULL;
+    }
+    group = firewall->groups[firewall->currentGroup];
+
+
+    if (VIR_ALLOC(rule) < 0)
+        goto no_memory;
+
+    rule->layer = layer;
+    rule->queryCB = cb;
+    rule->queryOpaque = opaque;
+    rule->ignoreErrors = ignoreErrors;
+
+    while ((str = va_arg(args, char *)) != NULL) {
+        ADD_ARG(rule, str);
+    }
+
+    if (group->addingRollback) {
+        if (VIR_APPEND_ELEMENT_COPY(group->rollback,
+                                    group->nrollback,
+                                    rule) < 0)
+            goto no_memory;
+    } else {
+        if (VIR_APPEND_ELEMENT_COPY(group->action,
+                                    group->naction,
+                                    rule) < 0)
+            goto no_memory;
+    }
+
+
+    return rule;
+
+ no_memory:
+    firewall->err = ENOMEM;
+    virFirewallRuleFree(rule);
+    return NULL;
+}
+
+/**
+ * virFirewallAddRule:
+ * @firewall: firewall ruleset to add to
+ * @layer: the firewall layer to change
+ * @...: NULL terminated list of strings for the rule
+ *
+ * Add any type of rule to the firewall ruleset.
+ *
+ * Returns the new rule
+ */
+virFirewallRulePtr
+virFirewallAddRule(virFirewallPtr firewall,
+                   virFirewallLayer layer,
+                   ...)
+{
+    virFirewallRulePtr rule;
+    va_list args;
+    va_start(args, layer);
+    rule = virFirewallAddRuleFullV(firewall, layer, false, NULL, NULL, args);
+    va_end(args);
+    return rule;
+}
+
+
+/**
+ * virFirewallAddRuleFull:
+ * @firewall: firewall ruleset to add to
+ * @layer: the firewall layer to change
+ * @ignoreErrors: true to ignore failure of the command
+ * @cb: callback to invoke with result of query
+ * @opaque: data passed into @cb
+ * @...: NULL terminated list of strings for the rule
+ *
+ * Add any type of rule to the firewall ruleset. Any output
+ * generated by the addition will be fed into the query
+ * callback @cb. This callback is permitted to create new
+ * rules by invoking the virFirewallAddRule method, but
+ * is not permitted to start new transactions.
+ *
+ * If @ignoreErrors is set to TRUE, then any failure of
+ * the command is ignored. If it is set to FALSE, then
+ * the behaviour upon failure is determined by the flags
+ * set when the transaction was started.
+ *
+ * Returns the new rule
+ */
+virFirewallRulePtr virFirewallAddRuleFull(virFirewallPtr firewall,
+                                          virFirewallLayer layer,
+                                          bool ignoreErrors,
+                                          virFirewallQueryCallback cb,
+                                          void *opaque,
+                                          ...)
+{
+    virFirewallRulePtr rule;
+    va_list args;
+    va_start(args, opaque);
+    rule = virFirewallAddRuleFullV(firewall, layer, ignoreErrors, cb, opaque, args);
+    va_end(args);
+    return rule;
+}
+
+
+/**
+ * virFirewallRemoveRule:
+ * @firewall: firewall ruleset to remove from
+ * @rule: the rule to remove
+ *
+ * Remove a rule from the current transaction
+ */
+void virFirewallRemoveRule(virFirewallPtr firewall,
+                           virFirewallRulePtr rule)
+{
+    size_t i;
+    virFirewallGroupPtr group;
+
+    /* Explicitly not checking firewall->err too,
+     * because if rule was partially created
+     * before hitting error we must still remove
+     * it to avoid leaking 'rule'
+     */
+    if (!firewall)
+        return;
+
+    if (firewall->ngroups == 0)
+        return;
+    group = firewall->groups[firewall->currentGroup];
+
+    if (group->addingRollback) {
+        for (i = 0; i < group->nrollback; i++) {
+            if (group->rollback[i] == rule) {
+                VIR_DELETE_ELEMENT(group->rollback,
+                                   i,
+                                   group->nrollback);
+                virFirewallRuleFree(rule);
+                break;
+            }
+        }
+    } else {
+        for (i = 0; i < group->naction; i++) {
+            if (group->action[i] == rule) {
+                VIR_DELETE_ELEMENT(group->action,
+                                   i,
+                                   group->naction);
+                virFirewallRuleFree(rule);
+                return;
+            }
+        }
+    }
+}
+
+
+void virFirewallRuleAddArg(virFirewallPtr firewall,
+                           virFirewallRulePtr rule,
+                           const char *arg)
+{
+    VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
+
+    ADD_ARG(rule, arg);
+
+    return;
+
+ no_memory:
+    firewall->err = ENOMEM;
+}
+
+
+void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
+                                 virFirewallRulePtr rule,
+                                 const char *fmt, ...)
+{
+    char *arg;
+    va_list list;
+
+    VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
+
+    va_start(list, fmt);
+
+    if (virVasprintf(&arg, fmt, list) < 0)
+        goto no_memory;
+
+    ADD_ARG(rule, arg);
+
+    va_end(list);
+
+    VIR_FREE(arg);
+    return;
+
+ no_memory:
+    firewall->err = ENOMEM;
+    va_end(list);
+    VIR_FREE(arg);
+}
+
+
+void virFirewallRuleAddArgSet(virFirewallPtr firewall,
+                              virFirewallRulePtr rule,
+                              const char *const *args)
+{
+    VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
+
+    while (*args) {
+        ADD_ARG(rule, *args);
+        args++;
+    }
+
+    return;
+
+ no_memory:
+    firewall->err = ENOMEM;
+}
+
+
+void virFirewallRuleAddArgList(virFirewallPtr firewall,
+                               virFirewallRulePtr rule,
+                               ...)
+{
+    va_list list;
+    const char *str;
+
+    VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
+
+    va_start(list, rule);
+
+    while ((str = va_arg(list, char *)) != NULL) {
+        ADD_ARG(rule, str);
+    }
+
+    va_end(list);
+
+    return;
+
+ no_memory:
+    firewall->err = ENOMEM;
+    va_end(list);
+}
+
+
+size_t virFirewallRuleGetArgCount(virFirewallRulePtr rule)
+{
+    if (!rule)
+        return 0;
+    return rule->argsLen;
+}
+
+
+/**
+ * virFirewallStartTransaction:
+ * @firewall: the firewall ruleset
+ * @flags: bitset of virFirewallTransactionFlags
+ *
+ * Start a new transaction with associated rollback
+ * block.
+ *
+ * Should be followed by calls to add various rules to
+ * the transaction. Then virFirwallStartRollback should
+ * be used to provide rules to rollback upon transaction
+ * failure
+ */
+void virFirewallStartTransaction(virFirewallPtr firewall,
+                                 unsigned int flags)
+{
+    virFirewallGroupPtr group;
+
+    VIR_FIREWALL_RETURN_IF_ERROR(firewall);
+
+    if (!(group = virFirewallGroupNew())) {
+        firewall->err = ENOMEM;
+        return;
+    }
+    group->actionFlags = flags;
+
+    if (VIR_EXPAND_N(firewall->groups,
+                     firewall->ngroups, 1) < 0) {
+        firewall->err = ENOMEM;
+        virFirewallGroupFree(group);
+        return;
+    }
+    firewall->groups[firewall->ngroups - 1] = group;
+    firewall->currentGroup = firewall->ngroups - 1;
+}
+
+/**
+ * virFirewallBeginRollback:
+ * @firewall: the firewall ruleset
+ * @flags: bitset of virFirewallRollbackFlags
+ *
+ * Mark the beginning of a set of rules able to rollback
+ * changes in this and all earlier transactions.
+ *
+ * Should be followed by calls to add various rules needed
+ * to rollback state. Then virFirewallStartTransaction
+ * should be used to indicate the beginning of the next
+ * transactional ruleset.
+ */
+void virFirewallStartRollback(virFirewallPtr firewall,
+                              unsigned int flags)
+{
+    virFirewallGroupPtr group;
+
+    VIR_FIREWALL_RETURN_IF_ERROR(firewall);
+
+    if (firewall->ngroups == 0) {
+        firewall->err = ENODATA;
+        return;
+    }
+
+    group = firewall->groups[firewall->ngroups-1];
+    group->rollbackFlags = flags;
+    group->addingRollback = true;
+}
+
+
+static char *
+virFirewallRuleToString(virFirewallRulePtr rule)
+{
+    const char *bin = virFirewallLayerCommandTypeToString(rule->layer);
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    size_t i;
+
+    virBufferAdd(&buf, bin, -1);
+    for (i = 0; i < rule->argsLen; i++) {
+        virBufferAddLit(&buf, " ");
+        virBufferAdd(&buf, rule->args[i], -1);
+    }
+
+    return virBufferContentAndReset(&buf);
+}
+
+static int
+virFirewallApplyRuleDirect(virFirewallRulePtr rule,
+                           bool ignoreErrors,
+                           char **output)
+{
+    size_t i;
+    const char *bin = virFirewallLayerCommandTypeToString(rule->layer);
+    virCommandPtr cmd = NULL;
+    int status;
+    int ret = -1;
+    char *error = NULL;
+
+    if (!bin) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unknown firewall layer %d"),
+                       rule->layer);
+        goto cleanup;
+    }
+
+    cmd = virCommandNewArgList(bin, NULL);
+
+    for (i = 0; i < rule->argsLen; i++)
+        virCommandAddArg(cmd, rule->args[i]);
+
+    virCommandSetOutputBuffer(cmd, output);
+    virCommandSetErrorBuffer(cmd, &error);
+
+    if (virCommandRun(cmd, &status) < 0)
+        goto cleanup;
+
+    if (status != 0) {
+        if (ignoreErrors) {
+            VIR_DEBUG("Ignoring error running command");
+        } else {
+            char *args = virCommandToString(cmd);
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Failed to apply firewall rules %s: %s"),
+                           NULLSTR(args), NULLSTR(error));
+            VIR_FREE(args);
+            VIR_FREE(*output);
+            goto cleanup;
+        }
+    }
+
+    ret = 0;
+ cleanup:
+    VIR_FREE(error);
+    virCommandFree(cmd);
+    return ret;
+}
+
+
+#ifdef WITH_DBUS
+static int
+virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
+                              bool ignoreErrors,
+                              char **output)
+{
+    const char *ipv = virFirewallLayerFirewallDTypeToString(rule->layer);
+    DBusConnection *sysbus = virDBusGetSystemBus();
+    DBusMessage *reply = NULL;
+    DBusError error;
+    int ret = -1;
+
+    if (!sysbus)
+        return -1;
+
+    dbus_error_init(&error);
+
+    if (!ipv) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unknown firewall layer %d"),
+                       rule->layer);
+        goto cleanup;
+    }
+
+    if (virDBusCallMethod(sysbus,
+                          &reply,
+                          &error,
+                          VIR_FIREWALL_FIREWALLD_SERVICE,
+                          "/org/fedoraproject/FirewallD1",
+                          "org.fedoraproject.FirewallD1.direct",
+                          "passthrough",
+                          "sa&s",
+                          ipv,
+                          (int)rule->argsLen,
+                          rule->args) < 0)
+        goto cleanup;
+
+    if (dbus_error_is_set(&error)) {
+        /*
+         * As of firewalld-0.3.9.3-1.fc20.noarch the name and
+         * message fields in the error look like
+         *
+         *    name="org.freedesktop.DBus.Python.dbus.exceptions.DBusException"
+         * message="COMMAND_FAILED: '/sbin/iptables --table filter --delete
+         *          INPUT --in-interface virbr0 --protocol udp --destination-port 53
+         *          --jump ACCEPT' failed: iptables: Bad rule (does a matching rule
+         *          exist in that chain?)."
+         *
+         * We'd like to only ignore DBus errors precisely related to the failure
+         * of iptables/ebtables commands. A well designed DBus interface would
+         * return specific named exceptions not the top level generic python dbus
+         * exception name. With this current scheme our only option is todo a
+         * sub-string match for 'COMMAND_FAILED' on the message. eg like
+         *
+         * if (ignoreErrors &&
+         *     STREQ(error.name,
+         *           "org.freedesktop.DBus.Python.dbus.exceptions.DBusException") &&
+         *     STRPREFIX(error.message, "COMMAND_FAILED"))
+         *    ...
+         *
+         * But this risks our error detecting code being broken if firewalld changes
+         * ever alter the message string, so we're avoiding doing that.
+         */
+        if (ignoreErrors) {
+            VIR_DEBUG("Ignoring error '%s': '%s'",
+                      error.name, error.message);
+        } else {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unable to apply rule '%s'"),
+                           error.message);
+            goto cleanup;
+        }
+    } else {
+        if (virDBusMessageRead(reply, "s", output) < 0)
+            goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    dbus_error_free(&error);
+    if (reply)
+        dbus_message_unref(reply);
+    return ret;
+}
+#endif
+
+static int
+virFirewallApplyRule(virFirewallPtr firewall,
+                     virFirewallRulePtr rule,
+                     bool ignoreErrors)
+{
+    char *output = NULL;
+    char **lines = NULL;
+    int ret = -1;
+    char *str = virFirewallRuleToString(rule);
+    VIR_INFO("Applying rule '%s'", NULLSTR(str));
+    VIR_FREE(str);
+
+    if (rule->ignoreErrors)
+        ignoreErrors = rule->ignoreErrors;
+
+    switch (currentBackend) {
+    case VIR_FIREWALL_BACKEND_DIRECT:
+        if (virFirewallApplyRuleDirect(rule, ignoreErrors, &output) < 0)
+            return -1;
+        break;
+#if WITH_DBUS
+    case VIR_FIREWALL_BACKEND_FIREWALLD:
+        if (virFirewallApplyRuleFirewallD(rule, ignoreErrors, &output) < 0)
+            return -1;
+        break;
+#endif
+    default:
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Unexpected firewall engine backend"));
+        return -1;
+    }
+
+    if (rule->queryCB && output) {
+        if (!(lines = virStringSplit(output, "\n", -1)))
+            goto cleanup;
+
+        VIR_DEBUG("Invoking query %p with '%s'", rule->queryCB, output);
+        if (rule->queryCB(firewall, (const char *const *)lines, rule->queryOpaque) < 0)
+            goto cleanup;
+
+        if (firewall->err == ENOMEM) {
+            virReportOOMError();
+            goto cleanup;
+        }
+        if (firewall->err) {
+            virReportSystemError(firewall->err, "%s",
+                                 _("Unable to create rule"));
+            goto cleanup;
+        }
+
+    }
+
+    ret = 0;
+ cleanup:
+    virStringFreeList(lines);
+    VIR_FREE(output);
+    return ret;
+}
+
+static int
+virFirewallApplyGroup(virFirewallPtr firewall,
+                      size_t idx)
+{
+    virFirewallGroupPtr group = firewall->groups[idx];
+    bool ignoreErrors = (group->actionFlags & VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
+    size_t i;
+
+    VIR_INFO("Starting transaction for %p flags=%x",
+             group, group->actionFlags);
+    firewall->currentGroup = idx;
+    group->addingRollback = false;
+    for (i = 0; i < group->naction; i++) {
+        if (virFirewallApplyRule(firewall,
+                                 group->action[i],
+                                 ignoreErrors) < 0)
+            return -1;
+    }
+    return 0;
+}
+
+
+static void
+virFirewallRollbackGroup(virFirewallPtr firewall,
+                         size_t idx)
+{
+    virFirewallGroupPtr group = firewall->groups[idx];
+    size_t i;
+
+    VIR_INFO("Starting rollback for group %p", group);
+    firewall->currentGroup = idx;
+    group->addingRollback = true;
+    for (i = 0; i < group->nrollback; i++) {
+        ignore_value(virFirewallApplyRule(firewall,
+                                          group->rollback[i],
+                                          true));
+    }
+}
+
+
+int
+virFirewallApply(virFirewallPtr firewall)
+{
+    size_t i, j;
+    int ret = -1;
+
+    virMutexLock(&ruleLock);
+    if (virFirewallInitialize() < 0)
+        goto cleanup;
+
+    if (!firewall || firewall->err == ENOMEM) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    if (firewall->err) {
+        virReportSystemError(firewall->err, "%s",
+                             _("Unable to create rule"));
+        goto cleanup;
+    }
+
+    VIR_DEBUG("Applying groups for %p", firewall);
+    for (i = 0; i < firewall->ngroups; i++) {
+        if (virFirewallApplyGroup(firewall, i) < 0) {
+            VIR_DEBUG("Rolling back groups upto %zu for %p", i, firewall);
+            size_t first = i;
+            virErrorPtr saved_error = virSaveLastError();
+
+            /*
+             * Look at any inheritance markers to figure out
+             * what the first rollback group we need to apply is
+             */
+            for (j = 0; j <= i; j++) {
+                VIR_DEBUG("Checking inheritance of group %zu", i - j);
+                if (firewall->groups[i - j]->rollbackFlags &
+                    VIR_FIREWALL_ROLLBACK_INHERIT_PREVIOUS)
+                    first = (i - j) - 1;
+            }
+            /*
+             * Now apply all rollback groups in order
+             */
+            for (j = first; j <= i; j++) {
+                VIR_DEBUG("Rolling back group %zu", j);
+                virFirewallRollbackGroup(firewall, j);
+            }
+
+            virSetError(saved_error);
+            virFreeError(saved_error);
+            VIR_DEBUG("Done rolling back groups for %p", firewall);
+            goto cleanup;
+        }
+    }
+    VIR_DEBUG("Done applying groups for %p", firewall);
+
+    ret = 0;
+ cleanup:
+    virMutexUnlock(&ruleLock);
+    return ret;
+}
diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h
new file mode 100644
index 0000000..5ca66fa
--- /dev/null
+++ b/src/util/virfirewall.h
@@ -0,0 +1,109 @@
+ /*
+ * virfirewall.h: integration with firewalls
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *    Daniel P. Berrange <berrange at redhat.com>
+ */
+
+#ifndef __VIR_FIREWALL_H__
+# define __VIR_FIREWALL_H__
+
+# include "internal.h"
+
+typedef struct _virFirewall virFirewall;
+typedef virFirewall *virFirewallPtr;
+
+typedef struct _virFirewallRule virFirewallRule;
+typedef virFirewallRule *virFirewallRulePtr;
+
+typedef enum {
+    VIR_FIREWALL_LAYER_ETHERNET,
+    VIR_FIREWALL_LAYER_IPV4,
+    VIR_FIREWALL_LAYER_IPV6,
+
+    VIR_FIREWALL_LAYER_LAST,
+} virFirewallLayer;
+
+virFirewallPtr virFirewallNew(void);
+
+void virFirewallFree(virFirewallPtr firewall);
+
+virFirewallRulePtr virFirewallAddRule(virFirewallPtr firewall,
+                                      virFirewallLayer layer,
+                                      ...)
+    ATTRIBUTE_SENTINEL;
+
+typedef int (*virFirewallQueryCallback)(virFirewallPtr firewall,
+                                        const char *const *lines,
+                                        void *opaque);
+
+virFirewallRulePtr virFirewallAddRuleFull(virFirewallPtr firewall,
+                                          virFirewallLayer layer,
+                                          bool ignoreErrors,
+                                          virFirewallQueryCallback cb,
+                                          void *opaque,
+                                          ...)
+    ATTRIBUTE_NONNULL(3) ATTRIBUTE_SENTINEL;
+
+void virFirewallRemoveRule(virFirewallPtr firewall,
+                           virFirewallRulePtr rule);
+
+void virFirewallRuleAddArg(virFirewallPtr firewall,
+                           virFirewallRulePtr rule,
+                           const char *arg)
+    ATTRIBUTE_NONNULL(3);
+
+void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
+                                 virFirewallRulePtr rule,
+                                 const char *fmt, ...)
+    ATTRIBUTE_NONNULL(3) ATTRIBUTE_FMT_PRINTF(3, 4);
+
+void virFirewallRuleAddArgSet(virFirewallPtr firewall,
+                              virFirewallRulePtr rule,
+                              const char *const *args)
+    ATTRIBUTE_NONNULL(3);
+
+void virFirewallRuleAddArgList(virFirewallPtr firewall,
+                               virFirewallRulePtr rule,
+                               ...)
+    ATTRIBUTE_SENTINEL;
+
+size_t virFirewallRuleGetArgCount(virFirewallRulePtr rule);
+
+typedef enum {
+    /* Ignore all errors when applying rules, so no
+     * rollback block will be required */
+    VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS = (1 << 0),
+} virFirewallTransactionFlags;
+
+void virFirewallStartTransaction(virFirewallPtr firewall,
+                                 unsigned int flags);
+
+typedef enum {
+    /* Execute previous rollback block before this
+     * one, to chain cleanup */
+    VIR_FIREWALL_ROLLBACK_INHERIT_PREVIOUS = (1 << 0),
+} virFirewallRollbackFlags;
+
+void virFirewallStartRollback(virFirewallPtr firewall,
+                              unsigned int flags);
+
+int virFirewallApply(virFirewallPtr firewall);
+
+#endif /* __VIR_FIREWALL_H__ */
diff --git a/src/util/virfirewallpriv.h b/src/util/virfirewallpriv.h
new file mode 100644
index 0000000..130aaa1
--- /dev/null
+++ b/src/util/virfirewallpriv.h
@@ -0,0 +1,45 @@
+/*
+ * virfirewallpriv.h: integration with firewalls private APIs
+ *
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *    Daniel P. Berrange <berrange at redhat.com>
+ */
+
+#ifndef __VIR_FIREWALL_PRIV_H_ALLOW__
+# error "virfirewallpriv.h may only be included by virfirewall.c or test suites"
+#endif
+
+#ifndef __VIR_FIREWALL_PRIV_H__
+# define __VIR_FIREWALL_PRIV_H__
+
+# include "virfirewall.h"
+
+# define VIR_FIREWALL_FIREWALLD_SERVICE "org.fedoraproject.FirewallD1"
+
+typedef enum {
+    VIR_FIREWALL_BACKEND_AUTOMATIC,
+    VIR_FIREWALL_BACKEND_DIRECT,
+    VIR_FIREWALL_BACKEND_FIREWALLD,
+
+    VIR_FIREWALL_BACKEND_LAST,
+} virFirewallBackend;
+
+int virFirewallSetBackend(virFirewallBackend backend);
+
+#endif /* __VIR_FIREWALL_PRIV_H__ */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index c13cc15..a10919d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -148,6 +148,7 @@ test_programs = virshtest sockettest \
 	virpcitest \
 	virendiantest \
 	virfiletest \
+	virfirewalltest \
 	viriscsitest \
 	virkeycodetest \
 	virlockspacetest \
@@ -998,6 +999,12 @@ virfiletest_SOURCES = \
 	virfiletest.c testutils.h testutils.c
 virfiletest_LDADD = $(LDADDS)
 
+virfirewalltest_SOURCES = \
+	virfirewalltest.c testutils.h testutils.c
+virfirewalltest_LDADD = $(LDADDS)
+virfirewalltest_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS)
+virfirewalltest_LDFLAGS = $(DRIVER_MODULE_LDFLAGS)
+
 jsontest_SOURCES = \
 	jsontest.c testutils.h testutils.c
 jsontest_LDADD = $(LDADDS)
diff --git a/tests/testutils.c b/tests/testutils.c
index 9767a78..022d543 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -459,10 +459,20 @@ int virtTestDifference(FILE *stream,
                        const char *expect,
                        const char *actual)
 {
-    const char *expectStart = expect;
-    const char *expectEnd = expect + (strlen(expect)-1);
-    const char *actualStart = actual;
-    const char *actualEnd = actual + (strlen(actual)-1);
+    const char *expectStart;
+    const char *expectEnd;
+    const char *actualStart;
+    const char *actualEnd;
+
+    if (!expect)
+        expect = "";
+    if (!actual)
+        actual = "";
+
+    expectStart = expect;
+    expectEnd = expect + (strlen(expect)-1);
+    actualStart = actual;
+    actualEnd = actual + (strlen(actual)-1);
 
     if (!virTestGetDebug())
         return 0;
diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
new file mode 100644
index 0000000..805fa44
--- /dev/null
+++ b/tests/virfirewalltest.c
@@ -0,0 +1,1186 @@
+/*
+ * Copyright (C) 2013-2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Daniel P. Berrange <berrange at redhat.com>
+ */
+
+#include <config.h>
+
+#define __VIR_FIREWALL_PRIV_H_ALLOW__
+#define __VIR_COMMAND_PRIV_H_ALLOW__
+
+#include "testutils.h"
+#include "virbuffer.h"
+#include "vircommandpriv.h"
+#include "virfirewallpriv.h"
+#include "virmock.h"
+#include "virdbuspriv.h"
+
+#define VIR_FROM_THIS VIR_FROM_FIREWALL
+
+#if WITH_DBUS
+# include <dbus/dbus.h>
+#endif
+
+static bool fwDisabled = true;
+static virBufferPtr fwBuf;
+static bool fwError;
+
+#define TEST_FILTER_TABLE_LIST                                  \
+    "Chain INPUT (policy ACCEPT)\n"                             \
+    "target     prot opt source               destination\n"    \
+    "\n"                                                        \
+    "Chain FORWARD (policy ACCEPT)\n"                           \
+    "target     prot opt source               destination\n"    \
+    "\n"                                                        \
+    "Chain OUTPUT (policy ACCEPT)\n"                            \
+    "target     prot opt source               destination\n"
+
+#define TEST_NAT_TABLE_LIST                                             \
+    "Chain PREROUTING (policy ACCEPT)\n"                                \
+    "target     prot opt source               destination\n"            \
+    "\n"                                                                \
+    "Chain INPUT (policy ACCEPT)\n"                                     \
+    "target     prot opt source               destination\n"            \
+    "\n"                                                                \
+    "Chain OUTPUT (policy ACCEPT)\n"                                    \
+    "target     prot opt source               destination\n"            \
+    "\n"                                                                \
+    "Chain POSTROUTING (policy ACCEPT)\n"                               \
+    "target     prot opt source               destination\n"
+
+#if WITH_DBUS
+VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
+                       DBusMessage *,
+                       DBusConnection *, connection,
+                       DBusMessage *, message,
+                       int, timeout_milliseconds,
+                       DBusError *, error)
+{
+    DBusMessage *reply = NULL;
+    const char *service = dbus_message_get_destination(message);
+    const char *member = dbus_message_get_member(message);
+    size_t i;
+    size_t nargs = 0;
+    char **args = NULL;
+    char *type = NULL;
+
+    VIR_MOCK_IMPL_INIT_REAL(dbus_connection_send_with_reply_and_block);
+
+    if (STREQ(service, "org.freedesktop.DBus") &&
+        STREQ(member, "ListNames")) {
+        const char *svc1 = "org.foo.bar.wizz";
+        const char *svc2 = VIR_FIREWALL_FIREWALLD_SERVICE;
+        DBusMessageIter iter;
+        DBusMessageIter sub;
+        reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
+        dbus_message_iter_init_append(reply, &iter);
+        dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+                                         "s", &sub);
+
+        if (!dbus_message_iter_append_basic(&sub,
+                                            DBUS_TYPE_STRING,
+                                            &svc1))
+            goto error;
+        if (!fwDisabled &&
+            !dbus_message_iter_append_basic(&sub,
+                                            DBUS_TYPE_STRING,
+                                            &svc2))
+            goto error;
+        dbus_message_iter_close_container(&iter, &sub);
+    } else if (STREQ(service, VIR_FIREWALL_FIREWALLD_SERVICE) &&
+               STREQ(member, "passthrough")) {
+        bool isAdd = false;
+        bool doError = false;
+
+        if (virDBusMessageDecode(message,
+                                 "sa&s",
+                                 &type,
+                                 &nargs,
+                                 &args) < 0)
+            goto error;
+
+        for (i = 0; i < nargs; i++) {
+            /* Fake failure on the command with this IP addr */
+            if (STREQ(args[i], "-A")) {
+                isAdd = true;
+            } else if (isAdd && STREQ(args[i], "192.168.122.255")) {
+                doError = true;
+            }
+        }
+
+        if (fwBuf) {
+            if (STREQ(type, "ipv4"))
+                virBufferAddLit(fwBuf, IPTABLES_PATH);
+            else if (STREQ(type, "ipv4"))
+                virBufferAddLit(fwBuf, IP6TABLES_PATH);
+            else
+                virBufferAddLit(fwBuf, EBTABLES_PATH);
+        }
+        for (i = 0; i < nargs; i++) {
+            if (fwBuf) {
+                virBufferAddLit(fwBuf, " ");
+                virBufferEscapeShell(fwBuf, args[i]);
+            }
+        }
+        if (fwBuf)
+            virBufferAddLit(fwBuf, "\n");
+        if (doError) {
+            dbus_set_error_const(error,
+                                 "org.firewalld.error",
+                                 "something bad happened");
+        } else {
+            if (nargs == 1 &&
+                STREQ(type, "ipv4") &&
+                STREQ(args[0], "-L")) {
+                if (virDBusCreateReply(&reply,
+                                       "s", TEST_FILTER_TABLE_LIST) < 0)
+                    goto error;
+            } else if (nargs == 3 &&
+                       STREQ(type, "ipv4") &&
+                       STREQ(args[0], "-t") &&
+                       STREQ(args[1], "nat") &&
+                       STREQ(args[2], "-L")) {
+                if (virDBusCreateReply(&reply,
+                                       "s", TEST_NAT_TABLE_LIST) < 0)
+                    goto error;
+            } else {
+                if (virDBusCreateReply(&reply,
+                                       "s", "success") < 0)
+                    goto error;
+            }
+        }
+    } else {
+        reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
+    }
+
+ cleanup:
+    VIR_FREE(type);
+    for (i = 0; i < nargs; i++)
+        VIR_FREE(args[i]);
+    VIR_FREE(args);
+    return reply;
+
+ error:
+    if (reply)
+        dbus_message_unref(reply);
+    reply = NULL;
+    if (error && !dbus_error_is_set(error))
+        dbus_set_error_const(error,
+                             "org.firewalld.error",
+                             "something unexpected happened");
+
+    goto cleanup;
+}
+#endif
+
+struct testFirewallData {
+    virFirewallBackend tryBackend;
+    virFirewallBackend expectBackend;
+    bool fwDisabled;
+};
+
+static int
+testFirewallSingleGroup(const void *opaque)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT)
+        virCommandSetDryRun(&cmdbuf, NULL, NULL);
+    else
+        fwBuf = &cmdbuf;
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    if (virFirewallApply(fw) < 0)
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+
+static int
+testFirewallRemoveRule(const void *opaque)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+    virFirewallRulePtr fwrule;
+
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT)
+        virCommandSetDryRun(&cmdbuf, NULL, NULL);
+    else
+        fwBuf = &cmdbuf;
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    fwrule = virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                                "-A", "INPUT", NULL);
+    virFirewallRuleAddArg(fw, fwrule, "--source-host");
+    virFirewallRemoveRule(fw, fwrule);
+
+    fwrule = virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                                "-A", "INPUT", NULL);
+    virFirewallRuleAddArg(fw, fwrule, "--source-host");
+    virFirewallRuleAddArgFormat(fw, fwrule, "%s", "!192.168.122.1");
+    virFirewallRuleAddArgList(fw, fwrule, "--jump", "REJECT", NULL);
+
+    if (virFirewallApply(fw) < 0)
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+
+static int
+testFirewallManyGroups(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT)
+        virCommandSetDryRun(&cmdbuf, NULL, NULL);
+    else
+        fwBuf = &cmdbuf;
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "OUTPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "OUTPUT",
+                       "--jump", "DROP", NULL);
+
+
+    if (virFirewallApply(fw) < 0)
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+static void
+testFirewallRollbackHook(const char *const*args,
+                         const char *const*env ATTRIBUTE_UNUSED,
+                         const char *input ATTRIBUTE_UNUSED,
+                         char **output ATTRIBUTE_UNUSED,
+                         char **error ATTRIBUTE_UNUSED,
+                         int *status,
+                         void *opaque ATTRIBUTE_UNUSED)
+{
+    bool isAdd = false;
+    while (*args) {
+        /* Fake failure on the command with this IP addr */
+        if (STREQ(*args, "-A")) {
+            isAdd = true;
+        } else if (isAdd && STREQ(*args, "192.168.122.255")) {
+            *status = 127;
+            break;
+        }
+        args++;
+    }
+}
+
+static int
+testFirewallIgnoreFailGroup(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+        virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
+    } else {
+        fwBuf = &cmdbuf;
+        fwError = true;
+    }
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.255",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "OUTPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "OUTPUT",
+                       "--jump", "DROP", NULL);
+
+
+    if (virFirewallApply(fw) < 0)
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+
+static int
+testFirewallIgnoreFailRule(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+        virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
+    } else {
+        fwBuf = &cmdbuf;
+        fwError = true;
+    }
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_IPV4,
+                           true, NULL, NULL,
+                           "-A", "INPUT",
+                           "--source-host", "192.168.122.255",
+                           "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "OUTPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "OUTPUT",
+                       "--jump", "DROP", NULL);
+
+
+    if (virFirewallApply(fw) < 0)
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+
+static int
+testFirewallNoRollback(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+        virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
+    } else {
+        fwBuf = &cmdbuf;
+        fwError = true;
+    }
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.255",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    if (virFirewallApply(fw) == 0) {
+        fprintf(stderr, "Firewall apply unexpectedly worked\n");
+        goto cleanup;
+    }
+
+    if (virtTestOOMActive())
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+static int
+testFirewallSingleRollback(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+        virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
+    } else {
+        fwError = true;
+        fwBuf = &cmdbuf;
+    }
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.255",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallStartRollback(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "192.168.122.255",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    if (virFirewallApply(fw) == 0) {
+        fprintf(stderr, "Firewall apply unexpectedly worked\n");
+        goto cleanup;
+    }
+
+    if (virtTestOOMActive())
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+static int
+testFirewallManyRollback(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+        virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
+    } else {
+        fwBuf = &cmdbuf;
+        fwError = true;
+    }
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallStartRollback(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.255",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallStartRollback(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "192.168.122.255",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    if (virFirewallApply(fw) == 0) {
+        fprintf(stderr, "Firewall apply unexpectedly worked\n");
+        goto cleanup;
+    }
+
+    if (virtTestOOMActive())
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+static int
+testFirewallChainedRollback(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+        virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
+    } else {
+        fwBuf = &cmdbuf;
+        fwError = true;
+    }
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallStartRollback(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.127",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallStartRollback(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "192.168.122.127",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.255",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallStartRollback(fw, VIR_FIREWALL_ROLLBACK_INHERIT_PREVIOUS);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "192.168.122.255",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-D", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    if (virFirewallApply(fw) == 0) {
+        fprintf(stderr, "Firewall apply unexpectedly worked\n");
+        goto cleanup;
+    }
+
+    if (virtTestOOMActive())
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+
+static const char *expectedLines[] = {
+    "Chain INPUT (policy ACCEPT)",
+    "target     prot opt source               destination",
+    "",
+    "Chain FORWARD (policy ACCEPT)",
+    "target     prot opt source               destination",
+    "",
+    "Chain OUTPUT (policy ACCEPT)",
+    "target     prot opt source               destination",
+    "",
+    "Chain PREROUTING (policy ACCEPT)",
+    "target     prot opt source               destination",
+    "",
+    "Chain INPUT (policy ACCEPT)",
+    "target     prot opt source               destination",
+    "",
+    "Chain OUTPUT (policy ACCEPT)",
+    "target     prot opt source               destination",
+    "",
+    "Chain POSTROUTING (policy ACCEPT)",
+    "target     prot opt source               destination",
+    "",
+};
+static size_t expectedLineNum;
+static bool expectedLineError;
+
+static void
+testFirewallQueryHook(const char *const*args,
+                      const char *const*env ATTRIBUTE_UNUSED,
+                      const char *input ATTRIBUTE_UNUSED,
+                      char **output,
+                      char **error ATTRIBUTE_UNUSED,
+                      int *status,
+                      void *opaque ATTRIBUTE_UNUSED)
+{
+    if (STREQ(args[0], IPTABLES_PATH) &&
+        STREQ(args[1], "-L")) {
+        if (VIR_STRDUP(*output, TEST_FILTER_TABLE_LIST) < 0)
+            *status = 127;
+    } else if (STREQ(args[0], IPTABLES_PATH) &&
+               STREQ(args[1], "-t") &&
+               STREQ(args[2], "nat") &&
+               STREQ(args[3], "-L")) {
+        if (VIR_STRDUP(*output, TEST_NAT_TABLE_LIST) < 0)
+            *status = 127;
+    }
+}
+
+
+static int
+testFirewallQueryCallback(virFirewallPtr fw,
+                          const char *const *lines,
+                          void *opaque ATTRIBUTE_UNUSED)
+{
+    size_t i;
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "!192.168.122.129",
+                       "--jump", "REJECT", NULL);
+
+    for (i = 0; lines[i] != NULL; i++) {
+        if (expectedLineNum >= ARRAY_CARDINALITY(expectedLines)) {
+            expectedLineError = true;
+            break;
+        }
+        if (STRNEQ(expectedLines[expectedLineNum], lines[i])) {
+            fprintf(stderr, "Mismatch '%s' vs '%s' at %zu, %zu\n",
+                    expectedLines[expectedLineNum], lines[i],
+                    expectedLineNum, i);
+            expectedLineError = true;
+            break;
+        }
+        expectedLineNum++;
+    }
+    return 0;
+}
+
+static int
+testFirewallQuery(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer cmdbuf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = NULL;
+    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";
+    const struct testFirewallData *data = opaque;
+
+    expectedLineNum = 0;
+    expectedLineError = false;
+    fwDisabled = data->fwDisabled;
+    if (virFirewallSetBackend(data->tryBackend) < 0)
+        goto cleanup;
+
+    if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+        virCommandSetDryRun(&cmdbuf, testFirewallQueryHook, NULL);
+    } else {
+        fwBuf = &cmdbuf;
+        fwError = true;
+    }
+
+    fw = virFirewallNew();
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.1",
+                       "--jump", "ACCEPT", NULL);
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.127",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_IPV4,
+                           false,
+                           testFirewallQueryCallback,
+                           NULL,
+                           "-L", NULL);
+    virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_IPV4,
+                           false,
+                           testFirewallQueryCallback,
+                           NULL,
+                           "-t", "nat", "-L", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.130",
+                       "--jump", "REJECT", NULL);
+
+
+    virFirewallStartTransaction(fw, 0);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "192.168.122.128",
+                       "--jump", "REJECT", NULL);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
+                       "-A", "INPUT",
+                       "--source-host", "!192.168.122.1",
+                       "--jump", "REJECT", NULL);
+
+    if (virFirewallApply(fw) < 0)
+        goto cleanup;
+
+    if (virBufferError(&cmdbuf))
+        goto cleanup;
+
+    actual = virBufferCurrentContent(&cmdbuf);
+
+    if (expectedLineError) {
+        fprintf(stderr, "Got some unexpected query data\n");
+        goto cleanup;
+    }
+
+    if (STRNEQ_NULLABLE(expected, actual)) {
+        fprintf(stderr, "Unexected command execution\n");
+        virtTestDifference(stderr, expected, actual);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virBufferFreeAndReset(&cmdbuf);
+    fwBuf = NULL;
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virFirewallFree(fw);
+    return ret;
+}
+
+static int
+mymain(void)
+{
+    int ret = 0;
+
+#define RUN_TEST_DIRECT(name, method)                                   \
+    do {                                                                \
+        struct testFirewallData data;                                   \
+        data.tryBackend = VIR_FIREWALL_BACKEND_AUTOMATIC;               \
+        data.expectBackend = VIR_FIREWALL_BACKEND_DIRECT;               \
+        data.fwDisabled = true;                                         \
+        if (virtTestRun(name " auto direct", method, &data) < 0)        \
+            ret = -1;                                                   \
+        data.tryBackend = VIR_FIREWALL_BACKEND_DIRECT;                  \
+        data.expectBackend = VIR_FIREWALL_BACKEND_DIRECT;               \
+        data.fwDisabled = true;                                         \
+        if (virtTestRun(name " manual direct", method, &data) < 0)      \
+            ret = -1;                                                   \
+    } while (0)
+
+#if WITH_DBUS
+# define RUN_TEST_FIREWALLD(name, method)                               \
+    do {                                                                \
+        struct testFirewallData data;                                   \
+        data.tryBackend = VIR_FIREWALL_BACKEND_AUTOMATIC;               \
+        data.expectBackend = VIR_FIREWALL_BACKEND_FIREWALLD;            \
+        data.fwDisabled = false;                                        \
+        if (virtTestRun(name " auto firewalld", method, &data) < 0)     \
+            ret = -1;                                                   \
+        data.tryBackend = VIR_FIREWALL_BACKEND_FIREWALLD;               \
+        data.expectBackend = VIR_FIREWALL_BACKEND_FIREWALLD;            \
+        data.fwDisabled = false;                                        \
+        if (virtTestRun(name " manual firewalld", method, &data) < 0)   \
+            ret = -1;                                                   \
+    } while (0)
+
+# define RUN_TEST(name, method)                 \
+    RUN_TEST_DIRECT(name, method);              \
+    RUN_TEST_FIREWALLD(name, method)
+#else /* ! WITH_DBUS */
+# define RUN_TEST(name, method)                 \
+    RUN_TEST_DIRECT(name, method)
+#endif /* ! WITH_DBUS */
+
+    RUN_TEST("single group", testFirewallSingleGroup);
+    RUN_TEST("remove rule", testFirewallRemoveRule);
+    RUN_TEST("many groups", testFirewallManyGroups);
+    RUN_TEST("ignore fail group", testFirewallIgnoreFailGroup);
+    RUN_TEST("ignore fail rule", testFirewallIgnoreFailRule);
+    RUN_TEST("no rollback", testFirewallNoRollback);
+    RUN_TEST("single rollback", testFirewallSingleRollback);
+    RUN_TEST("many rollback", testFirewallManyRollback);
+    RUN_TEST("chained rollback", testFirewallChainedRollback);
+    RUN_TEST("query transaction", testFirewallQuery);
+
+    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+#if WITH_DBUS
+VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmockdbus.so")
+#else
+VIRT_TEST_MAIN(mymain)
+#endif
-- 
1.9.0




More information about the libvir-list mailing list