On 08/13/2012 01:23 PM, Thomas Woerner wrote:
Subject line was a mess. You forgot a one-line summary followed by a
blank line before the actual listing of changes you made. Also, by
replying in a thread, it's not clear whether this is a new patch
unrelated to the rest of the thread, or a resubmission. Using 'git
send-email --subject-prefix=PATCHv2' will help on that front. Can you
please submit a new version with fixed subject, and perhaps as a
standalone thread to make it easier to spot? Also, these findings need
fixing:
> ---
> configure.ac | 11 +++++++++++
> libvirt.spec.in | 11 +++++++++++
> src/Makefile.am | 4 ++--
> src/network/bridge_driver.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
> src/util/ebtables.c | 35 ++++++++++++++++++++++++++++++++++
> src/util/iptables.c | 21 +++++++++++++++++++--
> 6 files changed, 124 insertions(+), 4 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 8a04d91..8fd3e34 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1282,6 +1282,17 @@ AM_CONDITIONAL([HAVE_POLKIT1], [test "x$with_polkit1" = "xyes"])
> AC_SUBST([POLKIT_CFLAGS])
> AC_SUBST([POLKIT_LIBS])
>
> +dnl firewalld
> +AC_ARG_WITH([firewalld],
> + AC_HELP_STRING([--with-firewalld], [enable firewalld support]))
Needs a fourth argument to AC_ARG_WITH to set the default when neither
'--with-firewalld' nor '--without-firewalld' is given.
> +if test "x$with_firewalld" != "xno" ; then
> + AC_DEFINE_UNQUOTED([HAVE_FIREWALLD], [1], [whether firewalld support is enabled])
> + if test "x$with_dbus" != "xyes" ; then
> + AC_MSG_ERROR([You must have dbus enabled for firewalld support])
> + fi
> +fi
> +AM_CONDITIONAL([HAVE_FIREWALLD], [test "x$with_firewalld" != "xno"])
We tend to prefer tri-state checking (yes, no, and 'check', with the
default being check, and where check changes to yes or no depending on
finding prerequisites in place).
Also, at the end of configure.ac, we have a series of outputs that
summarize what was selected; firewalld should be listed in that summary.
> +++ b/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_driver *driver) {
> }
> }
>
> +#if HAVE_FIREWALLD
> +static DBusHandlerResult
> +firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *user_data) {
Long line. I'd wrap at the comma between 'connection ATTRIBUTE_UNUSED,
DBusMessage'.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list