11:02 p.m.
On 08/08/2012 12:00 PM, Stefan Berger wrote:
This patch is from Thomas Woerner's previous submission.
I have split off the nwfilter part from this patch and fixed one compilation
error caused by an unused variable.
* bridge_driver, nwfilter_driver: new dbus filters to get FirewallD1.Reloaded
signal and DBus.NameOwnerChanged on org.fedoraproject.FirewallD1
* iptables, ebtables, nwfilter_ebiptables_driver: use firewall-cmd direct
passthrough interface
* spec file changed as requested
---
configure.ac | 8 +++++++
libvirt.spec.in | 11 ++++++++++
src/Makefile.am | 4 +--
src/network/bridge_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
src/util/ebtables.c | 35 +++++++++++++++++++++++++++++++++
src/util/iptables.c | 19 +++++++++++++++++-
6 files changed, 120 insertions(+), 3 deletions(-)
Index: libvirt-firewalld/configure.ac
===================================================================
--- libvirt-firewalld.orig/configure.ac
+++ libvirt-firewalld/configure.ac
@@ -1282,6 +1282,14 @@ AM_CONDITIONAL([HAVE_POLKIT1], [test "x$
AC_SUBST([POLKIT_CFLAGS])
AC_SUBST([POLKIT_LIBS])
+dnl firewalld
+AC_ARG_WITH([firewalld],
+ AC_HELP_STRING([--with-firewalld], [enable firewalld support]))
+if test "x$with_firewalld" = "xyes" ; then
+ AC_DEFINE_UNQUOTED([HAVE_FIREWALLD], [1], [whether firewalld support is enabled])
+fi
+AM_CONDITIONAL([HAVE_FIREWALLD], [test "x$with_firewalld" =
"xyes"])
+
dnl Avahi library
AC_ARG_WITH([avahi],
AC_HELP_STRING([--with-avahi], [use avahi to advertise remote daemon
@<:@default=check@:>@]),
Index: libvirt-firewalld/libvirt.spec.in
===================================================================
--- libvirt-firewalld.orig/libvirt.spec.in
+++ libvirt-firewalld/libvirt.spec.in
@@ -106,6 +106,7 @@
%define with_sanlock 0%{!?_without_sanlock:0}
%define with_systemd 0%{!?_without_systemd:0}
%define with_numad 0%{!?_without_numad:0}
+%define with_firewalld 0%{!?_without_firewalld:0}
# Non-server/HV driver defaults which are always enabled
%define with_python 0%{!?_without_python:1}
@@ -146,6 +147,11 @@
%define with_systemd 1
%endif
+# Fedora 18 / RHEL-7 are first where firewalld support is enabled
+%if 0%{?fedora} >= 17 || 0%{?rhel} >= 7
The comment and if statement don't match. Since firewalld is available
on F17, and official Fedora 17 builds will never get this specfile, we
could leave it at 17 in the if statement. I don't really have an
opinion, but the two lines should match.
Also, there is no Requires: line. But I guess that's intentional (and
rightly so) because we check for firewalld at run time and use it if
it's available, but go back to iptables/ebtables if it's not.
+%define with_firewalld 1
+%endif
+
# RHEL-5 has restricted QEMU to x86_64 only and is too old for LXC
%if 0%{?rhel} == 5
%define with_qemu_tcg 0
@@ -1182,6 +1188,10 @@ of recent versions of Linux (and other O
%define _without_driver_modules --without-driver-modules
%endif
+%if %{with_firewalld}
+%define _with_firewalld --with-firewalld
+%endif
+
%define when %(date +"%%F-%%T")
%define where %(hostname)
%define who %{?packager}%{!?packager:Unknown}
@@ -1240,6 +1250,7 @@ autoreconf -if
%{?_without_audit} \
%{?_without_dtrace} \
%{?_without_driver_modules} \
+ %{?_with_firewalld} \
The default of every other option (except rhel5-api) is "with". Is there
a reasonable way to make that the default as well, so that its logic
matches with the logic of all the other options? Or does that complicate
configure.ac too much?
%{with_packager} \
%{with_packager_version} \
--with-qemu-user=%{qemu_user} \
Index: libvirt-firewalld/src/Makefile.am
===================================================================
--- libvirt-firewalld.orig/src/Makefile.am
+++ libvirt-firewalld/src/Makefile.am
@@ -989,7 +989,7 @@ libvirt_driver_network_la_SOURCES =
libvirt_driver_network_la_LIBADD = libvirt_driver_network_impl.la
if WITH_DRIVER_MODULES
mod_LTLIBRARIES += libvirt_driver_network.la
-libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la $(LIBNL_LIBS)
+libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la $(LIBNL_LIBS) $(DBUS_LIBS)
libvirt_driver_network_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
else
noinst_LTLIBRARIES += libvirt_driver_network.la
@@ -999,7 +999,7 @@ endif
libvirt_driver_network_impl_la_CFLAGS = \
$(LIBNL_CFLAGS) \
- -I$(top_srcdir)/src/conf $(AM_CFLAGS)
+ -I$(top_srcdir)/src/conf $(AM_CFLAGS) $(DBUS_CFLAGS)
libvirt_driver_network_impl_la_SOURCES = $(NETWORK_DRIVER_SOURCES)
endif
EXTRA_DIST += network/default.xml
Index: libvirt-firewalld/src/network/bridge_driver.c
===================================================================
--- libvirt-firewalld.orig/src/network/bridge_driver.c
+++ libvirt-firewalld/src/network/bridge_driver.c
@@ -61,6 +61,7 @@
#include "virnetdev.h"
#include "virnetdevbridge.h"
#include "virnetdevtap.h"
+#include "virdbus.h"
#define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
#define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -248,6 +249,24 @@ networkAutostartConfigs(struct network_d
}
}
+#if HAVE_FIREWALLD
+static DBusHandlerResult
+firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage
*message, void *user_data) {
+ struct network_driver *_driverState = user_data;
+
+ if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS,
+ "NameOwnerChanged") ||
+ dbus_message_is_signal(message, "org.fedoraproject.FirewallD1",
+ "Reloaded"))
+ {
The presence of this code means that if firewalld is enable, the
specfile MUST require dbus to be enabled as well (either that or the
firewalld support must be able to work without dbus, and this code must
depend on HAVE_DBUS in addition to HAVE_FIREWALLD).
+ VIR_DEBUG("Reload in bridge_driver because of
firewalld.");
+ networkReloadIptablesRules(_driverState);
+ }
+
+ return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+}
+#endif
+
/**
* networkStartup:
*
@@ -256,6 +275,9 @@ networkAutostartConfigs(struct network_d
static int
networkStartup(int privileged) {
char *base = NULL;
+#ifdef HAVE_FIREWALLD
+ DBusConnection *sysbus = NULL;
+#endif
if (VIR_ALLOC(driverState) < 0)
goto error;
@@ -322,6 +344,30 @@ networkStartup(int privileged) {
networkDriverUnlock(driverState);
+#ifdef HAVE_FIREWALLD
+ if (!(sysbus = virDBusGetSystemBus())) {
+ virErrorPtr err = virGetLastError();
+ VIR_WARN("DBus not available, disabling firewalld support in bridge_driver:
%s", err->message);
+ } else {
+ /* add matches for
+ * NameOwnerChanged on org.freedesktop.DBus for firewalld start/stop
+ * Reloaded on org.fedoraproject.FirewallD1 for firewalld reload
+ */
+ dbus_bus_add_match(sysbus,
+ "type='signal'"
+
",interface='"DBUS_INTERFACE_DBUS"'"
+ ",member='NameOwnerChanged'"
+ ",arg0='org.fedoraproject.FirewallD1'",
+ NULL);
+ dbus_bus_add_match(sysbus,
+ "type='signal'"
+
",interface='org.fedoraproject.FirewallD1'"
+ ",member='Reloaded'",
+ NULL);
+ dbus_connection_add_filter(sysbus, firewalld_dbus_filter_bridge, driverState,
NULL);
+ }
+#endif
+
Same command about HAVE_DBUS, etc.
return 0;
out_of_memory:
Index: libvirt-firewalld/src/util/ebtables.c
===================================================================
--- libvirt-firewalld.orig/src/util/ebtables.c
+++ libvirt-firewalld/src/util/ebtables.c
@@ -176,11 +176,34 @@ ebtablesAddRemoveRule(ebtRules *rules, i
const char *s;
int n, command_idx;
+#if HAVE_FIREWALLD
+ int ret;
+ char *firewall_cmd_path = NULL;
+ virCommandPtr cmd = NULL;
+
+ firewall_cmd_path = virFindFileInPath("firewall-cmd");
+ if (firewall_cmd_path) {
+ cmd = virCommandNew(firewall_cmd_path);
+ virCommandAddArgList(cmd, "--state", NULL);
+ ret = virCommandRun(cmd, NULL);
+ if (ret != 0) {
+ VIR_FREE(firewall_cmd_path);
+ firewall_cmd_path = NULL;
+ }
+ virCommandFree(cmd);
+ }
+#endif
+
The piece above shouldn't be run each time an ebtable command is needed.
It should instead happen once when libvirtd is loaded, and cached.
The rest of this function really should be converted to use
virCommandRun instead of virRun. It could be done after this patch, but
I'd prefer it be done prior. (that's just a personal preference though -
shouldn't keep the patch from going in).
n = 1 + /* /sbin/ebtables */
2 + /* --table foo */
2 + /* --insert bar */
1; /* arg */
+#if HAVE_FIREWALLD
+ if (firewall_cmd_path)
+ n += 3; /* --direct --passthrough eb */
+#endif
+
va_start(args, arg);
while (va_arg(args, const char *))
n++;
@@ -192,6 +215,18 @@ ebtablesAddRemoveRule(ebtRules *rules, i
n = 0;
+#if HAVE_FIREWALLD
+ if (firewall_cmd_path) {
+ if (!(argv[n++] = strdup(firewall_cmd_path)))
+ goto error;
+ if (!(argv[n++] = strdup("--direct")))
+ goto error;
+ if (!(argv[n++] = strdup("--passthrough")))
+ goto error;
+ if (!(argv[n++] = strdup("eb")))
+ goto error;
+ } else
+#endif
if (!(argv[n++] = strdup(EBTABLES_PATH)))
goto error;
Index: libvirt-firewalld/src/util/iptables.c
===================================================================
--- libvirt-firewalld.orig/src/util/iptables.c
+++ libvirt-firewalld/src/util/iptables.c
@@ -101,9 +101,26 @@ iptablesAddRemoveRule(iptRules *rules, i
{
va_list args;
int ret;
- virCommandPtr cmd;
+ virCommandPtr cmd = NULL;
const char *s;
+#if HAVE_FIREWALLD
+ char *firewall_cmd_path = NULL;
+ firewall_cmd_path = virFindFileInPath("firewall-cmd");
+ if (firewall_cmd_path) {
+ cmd = virCommandNew(firewall_cmd_path);
+ virCommandAddArgList(cmd, "--state", NULL);
+ ret = virCommandRun(cmd, NULL);
+ if (ret == 0) {
+ cmd = virCommandNew(firewall_cmd_path);
+ virCommandAddArgList(cmd, "--direct", "--passthrough",
+ (family == AF_INET6) ? "ipv6" :
"ipv4", NULL);
+ } else
+ cmd = NULL;
+ VIR_FREE(firewall_cmd_path);
+ }
+ if (!cmd)
+#endif
Same comment here - this should only happen once. (Maybe using the
fabled "virOnceInit() API that danpb mentioned in the comments of his
virObject series :-)
cmd = virCommandNew((family == AF_INET6)
? IP6TABLES_PATH : IPTABLES_PATH);