Re: [libvirt] [PATCH 2/5] util: move all firewalld-specific stuff into its own file

Since I'm going to be adding at least one more firewalld-specific function, this seems like a good time to separate the code that's unique to firewalld from the more-generic "firewall" file. Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- include/libvirt/virterror.h | 1 + src/libvirt_private.syms | 3 + src/util/Makefile.inc.am | 2 + src/util/virerror.c | 1 + src/util/virfirewall.c | 86 ++---------------------- src/util/virfirewalld.c | 128 ++++++++++++++++++++++++++++++++++++ src/util/virfirewalld.h | 33 ++++++++++ src/util/virfirewallpriv.h | 2 - tests/virfirewalltest.c | 1 + 9 files changed, 173 insertions(+), 84 deletions(-) create mode 100644 src/util/virfirewalld.c create mode 100644 src/util/virfirewalld.h

+ if (error.level == VIR_ERR_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.str1, error.message); + } else { + virReportErrorObject(&error); + goto cleanup; + } + } else { + if (virDBusMessageRead(reply, "s", output) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + virResetError(&error); + virDBusMessageUnref(reply); + return ret; +}

diff --git a/src/util/virfirewalld.h b/src/util/virfirewalld.h new file mode 100644 index 0000000000..c1c929399a --- /dev/null +++ b/src/util/virfirewalld.h @@ -0,0 +1,33 @@

+#ifndef LIBVIRT_VIRFIREWALLD_H +# define LIBVIRT_VIRFIREWALLD_H + +# define VIR_FIREWALL_FIREWALLD_SERVICE "org.fedoraproject.FirewallD1"

+ +int virFirewallDStatus(void);

+ +int virFirewallDApplyRule(virFirewallLayer layer, + char **args, size_t argsLen, + bool ignoreErrors, + char **output);

+ +#endif /* LIBVIRT_VIRFIREWALLD_H */ diff --git a/src/util/virfirewallpriv.h b/src/util/virfirewallpriv.h index efa94a7da4..7c31d0680d 100644 --- a/src/util/virfirewallpriv.h +++ b/src/util/virfirewallpriv.h @@ -27,8 +27,6 @@ # include "virfirewall.h" -# define VIR_FIREWALL_FIREWALLD_SERVICE "org.fedoraproject.FirewallD1" - typedef enum { VIR_FIREWALL_BACKEND_AUTOMATIC, VIR_FIREWALL_BACKEND_DIRECT, diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c index 63b9ced820..573ab1f9cd 100644 --- a/tests/virfirewalltest.c +++ b/tests/virfirewalltest.c @@ -27,6 +27,7 @@ # include "vircommandpriv.h" # define LIBVIRT_VIRFIREWALLPRIV_H_ALLOW # include "virfirewallpriv.h" +# include "virfirewalld.h"