meson: Improve default firewall backend configuration

The current implementation requires users to configure the
preference as such:

  -Dfirewall_backend_default_1=iptables
  -Dfirewall_backend_default_2=nftables

In addition to being more verbose than one would hope, there
are several things that could go wrong.

First of all, meson performs no validation on the provided
values, so mistakes will only be caught by the compiler.
Additionally, it's entirely possible to provide nonsensical
combinations, such as repeating the same value twice.

Change things so that the preference can now be configured
as such:

  -Dfirewall_backend_priority=iptables,nftables

Checks have been added to prevent invalid values from being
accepted.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

diff --git a/meson.build b/meson.build
index f67c3d272478992daacff35bc3aa66b5b493a8be..ed0e9686f8638c48948c4eec524a86e7548f5af2 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -1635,15 +1635,17 @@ endif
 
 if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD')
   conf.set('WITH_NETWORK', 1)
-  firewall_backend_default_1 = get_option('firewall_backend_default_1')
-  firewall_backend_default_conf = firewall_backend_default_1
-  firewall_backend_default_1 = 'VIR_FIREWALL_BACKEND_' + firewall_backend_default_1.to_upper()
-  conf.set('FIREWALL_BACKEND_DEFAULT_1', firewall_backend_default_1)
 
-  firewall_backend_default_2 = get_option('firewall_backend_default_2')
-  firewall_backend_default_2 = 'VIR_FIREWALL_BACKEND_' + firewall_backend_default_2.to_upper()
-  conf.set('FIREWALL_BACKEND_DEFAULT_2', firewall_backend_default_2)
+  firewall_backend_priority = get_option('firewall_backend_priority')
+  if (not firewall_backend_priority.contains('nftables') or
+      not firewall_backend_priority.contains('iptables') or
+      firewall_backend_priority.length() != 2)
+    error('invalid value for firewall_backend_priority option')
+  endif
 
+  conf.set('FIREWALL_BACKEND_PRIORITY_0', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[0].to_upper())
+  conf.set('FIREWALL_BACKEND_PRIORITY_1', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[1].to_upper())
+  conf.set('FIREWALL_BACKEND_PRIORITY_NUM', firewall_backend_priority.length())
 elif get_option('driver_network').enabled()
   error('libvirtd must be enabled to build the network driver')
 endif

diff --git a/meson_options.txt b/meson_options.txt
index ad354a8668fd3327a67686bfc55a0a35c21c1736..cdc86877952233067076e1727f0f7a4ba8b9f00f 100644 (file)
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -115,8 +115,7 @@ option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for st
 option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support')
 # dep:firewalld
 option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone')
-option('firewall_backend_default_1', type: 'string', value: 'nftables', description: 'first firewall backend to try  when none is specified')
-option('firewall_backend_default_2', type: 'string', value: 'iptables', description: 'second firewall backend to try when none is specified (and first is unavailable)')
+option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], description: 'order in which to try firewall backends')
 option('host_validate', type: 'feature', value: 'auto', description: 'build virt-host-validate')
 option('init_script', type: 'combo', choices: ['systemd', 'openrc', 'check', 'none'], value: 'check', description: 'Style of init script to install')
 option('loader_nvram', type: 'string', value: '', description: 'Pass list of pairs of <loader>:<nvram> paths. Both pairs and list items are separated by a colon.')

diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c
index 8f4956dacefd360a3b9a985a431345d1c0fdb825..e2f3613a4126425857205081e2f59376df176aa1 100644 (file)
--- a/src/network/bridge_driver_conf.c
+++ b/src/network/bridge_driver_conf.c
@@ -67,8 +67,12 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED,
     g_autofree char *fwBackendStr = NULL;
     bool fwBackendSelected = false;
     size_t i;
-    int fwBackends[] = { FIREWALL_BACKEND_DEFAULT_1, FIREWALL_BACKEND_DEFAULT_2 };
+    int fwBackends[] = {
+        FIREWALL_BACKEND_PRIORITY_0,
+        FIREWALL_BACKEND_PRIORITY_1,
+    };
     G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST);
+    G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == FIREWALL_BACKEND_PRIORITY_NUM);
     int nFwBackends = G_N_ELEMENTS(fwBackends);
 
     if (access(filename, R_OK) == 0) {

diff --git a/src/network/meson.build b/src/network/meson.build
index bf2893accc8c8a9bab3b484866018627abe53eb8..07cd5cda55b375690437468555255fa048a2f700 100644 (file)
--- a/src/network/meson.build
+++ b/src/network/meson.build
@@ -51,7 +51,8 @@ if conf.has('WITH_NETWORK')
   }
 
   network_options_conf = configuration_data({
-    'FIREWALL_BACKEND': firewall_backend_default_conf,
+    'FIREWALL_BACKEND_PRIORITY': ', '.join(firewall_backend_priority),
+    'FIREWALL_BACKEND': firewall_backend_priority[0],
   })
 
   network_conf = configure_file(
@@ -61,7 +62,8 @@ if conf.has('WITH_NETWORK')
   )
 
   network_options_hack_conf = configuration_data({
-    'FIREWALL_BACKEND': firewall_backend_default_conf,
+    'FIREWALL_BACKEND_PRIORITY': ', '.join(firewall_backend_priority),
+    'FIREWALL_BACKEND': firewall_backend_priority[0],
     # This hack is necessary because the output file is going to be
     # used as input for another configure_file() call later, which
     # will take care of substituting @CONFIG@ with useful data

diff --git a/src/network/network.conf.in b/src/network/network.conf.in
index f579f39fcd81962b7988af3c26c1e73fd083aa65..5ed64a04a5bcbc82dfa442af0ccd7ac49ee2c43e 100644 (file)
--- a/src/network/network.conf.in
+++ b/src/network/network.conf.in
@@ -12,12 +12,13 @@
 #     iptables - use iptables commands to construct the firewall
 #     nftables - use nft commands to construct the firewall
 #
-#   If firewall_backend isn't set in this file, libvirt will
-#   prefer the @FIREWALL_BACKEND@ backend *if the necessary package.
-#   binary is installed*, otherwise it will look for the package/binary
-#   needed for the other backend and use that if available. If neither
-#   is available on the host, then the network driver will fail to
-#   start, and an error will be logged.
+#   If firewall_backend isn't configured, libvirt will choose the
+#   first available backend from the following list:
+#
+#     [@FIREWALL_BACKEND_PRIORITY@]
+#
+#   If no backend is available on the host, then the network driver
+#   will fail to start, and an error will be logged.
 #
 #   (NB: switching from one backend to another while there are active
 #   virtual networks *is* supported. The change will take place the