On Tue, Nov 15, 2022 at 11:21:43AM +0100, Michal Prívozník wrote:
On 11/10/22 17:31, Eric Garver wrote:
> This factors out the firewalld pieces of the iptables + firewalld
> backend.
>
> Signed-off-by: Eric Garver <eric(a)garver.life>
> ---
> src/network/bridge_driver_linux.c | 117 ++++++++++++++++--------------
> 1 file changed, 61 insertions(+), 56 deletions(-)
>
> diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
> index d9597d91beed..88a8e9c5fa27 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -801,6 +801,58 @@ networkRemoveIPSpecificFirewallRules(virFirewall *fw,
> }
>
>
> +static int
> +networkAddHybridFirewallDRules(virNetworkDef *def)
> +{
> + /* if firewalld is active, try to set the "libvirt" zone. This is
> + * desirable (for consistency) if firewalld is using the iptables
> + * backend, but is necessary (for basic network connectivity) if
> + * firewalld is using the nftables backend
> + */
> +
> + /* if the "libvirt" zone exists, then set it. If not, and
> + * if firewalld is using the nftables backend, then we
> + * need to log an error because the combination of
> + * nftables + default zone means that traffic cannot be
> + * forwarded (and even DHCP and DNS from guest to host
> + * will probably no be permitted by the default zone
> + */
> + if (virFirewallDZoneExists("libvirt")) {
> + if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") <
0)
> + return -1;
> + } else {
> + unsigned long version;
> + int vresult = virFirewallDGetVersion(&version);
> +
> + if (vresult < 0)
> + return -1;
> +
> + /* Support for nftables backend was added in firewalld
> + * 0.6.0. Support for rule priorities (required by the
> + * 'libvirt' zone, which should be installed by a
> + * libvirt package, *not* by firewalld) was not added
> + * until firewalld 0.7.0 (unless it was backported).
> + */
> + if (version >= 6000 &&
> + virFirewallDGetBackend() == VIR_FIREWALLD_BACKEND_NFTABLES) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("firewalld is set to use the nftables "
> + "backend, but the required firewalld "
> + "'libvirt' zone is missing. Either set
"
> + "the firewalld backend to 'iptables', or
"
> + "ensure that firewalld has a 'libvirt'
"
> + "zone by upgrading firewalld to a "
> + "version supporting rule priorities "
> + "(0.7.0+) and/or rebuilding "
> + "libvirt with --with-firewalld-zone"));
I know you wanted this to be plain code movement, but since you're
touching this error message you can apply our coding style rule [1] and
unbreak the message onto one, long line. All error messages are exempt
from the 80 chars rule, because it's easier to git grep them.
And on a similar note, per out deprecation policy [2], firewalld-0.6.0
or older is ancient history, thus this version check can be dropped (in
a separate commit of course).
ACK. I'll drop the whole else block in the next version.