settings: enable "ibft" plugin by default together with "ifcfg-rh"

Originally, ibft settings were handled by "ifcfg-rh" plugin. Later, we added
a separate "ibft" plugin and moved the functionality there.

The problem was that users quite possibly had a configuration like
  [main]
  plugins=ifcfg-rh
in their "NetworkManager.conf". That meant, after upgrade users would
no longer have ibft support.

We fixed that by installing "/etc/NetworkManager/conf.d/10-ibft-plugin.conf"
which was read after the main file and contained:
  [main]
  plugins+=ibft

We no longer want to install configuration snippets with our core packages to
/etc. Avoid the regression by changing the meaning of "ifcfg-rh". By enabling
"ifcfg-rh" you now implicitly enable "ibft" plugin as well. This can be
turned off via "no-ibft". And you can continue to enable "ibft" plugin
alone.

5 files changed, 98 insertions, 67 deletions

diff --git a/configure.ac b/configure.ac
index b197ef7926..e9b39d01f7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -126,6 +126,11 @@ test "$enable_ifnet" = "yes"              && distro_plugins="$distro_plugins,ifn
 distro_plugins="${distro_plugins#,}"
 
 AC_DEFINE_UNQUOTED(CONFIG_PLUGINS_DEFAULT, "$config_plugins_default", [Default configuration option for main.plugins setting])
+if test "${enable_config_plugin_ibft}" = yes; then
+	AC_DEFINE(WITH_SETTINGS_PLUGIN_IBFT, 1, [Whether compilation of ibft setting plugin is enabled])
+else
+	AC_DEFINE(WITH_SETTINGS_PLUGIN_IBFT, 0, [Whether compilation of ibft setting plugin is enabled])
+fi
 
 if test "$enable_ifcfg_rh" = "yes"; then
     DISTRO_NETWORK_SERVICE=network.service
diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec
index aa091e04f1..0786b305a8 100644
--- a/contrib/fedora/rpm/NetworkManager.spec
+++ b/contrib/fedora/rpm/NetworkManager.spec
@@ -84,8 +84,7 @@ URL: http://www.gnome.org/projects/NetworkManager/
 Source: __SOURCE1__
 Source1: NetworkManager.conf
 Source2: 00-server.conf
-Source3: 10-ibft-plugin.conf
-Source4: 20-connectivity-fedora.conf
+Source3: 20-connectivity-fedora.conf
 
 #Patch1: 0001-some.patch
 
@@ -446,8 +445,7 @@ make install DESTDIR=$RPM_BUILD_ROOT
 mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d
 mkdir -p $RPM_BUILD_ROOT%{nmlibdir}/conf.d
 %{__cp} %{SOURCE2} $RPM_BUILD_ROOT%{nmlibdir}/conf.d/
-%{__cp} %{SOURCE3} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d
-%{__cp} %{SOURCE4} $RPM_BUILD_ROOT%{nmlibdir}/conf.d/
+%{__cp} %{SOURCE3} $RPM_BUILD_ROOT%{nmlibdir}/conf.d/
 
 # create a VPN directory
 %{__mkdir_p} $RPM_BUILD_ROOT%{_sysconfdir}/NetworkManager/VPN
@@ -547,7 +545,6 @@ fi
 %dir %{_sysconfdir}/%{name}/conf.d
 %dir %{nmlibdir}
 %dir %{nmlibdir}/conf.d
-%config %{_sysconfdir}/%{name}/conf.d/10-ibft-plugin.conf
 %{_mandir}/man1/*
 %{_mandir}/man5/*
 %{_mandir}/man8/*
diff --git a/contrib/fedora/rpm/build.sh b/contrib/fedora/rpm/build.sh
index c1ededb554..8e6539dcb1 100755
--- a/contrib/fedora/rpm/build.sh
+++ b/contrib/fedora/rpm/build.sh
@@ -73,7 +73,6 @@ SOURCE="$(abs_path "$SOURCE" "$(ls -1 "$GITDIR/NetworkManager-$VERSION"*.tar* 2>
 [[ -f "$SOURCE" ]] || die "could not find source ${_SOURCE:-$GITDIR/NetworkManager-$VERSION*.tar*} . Did you execute \`make dist\`? Otherwise set \$SOURCE variable"
 SOURCE_NETWORKMANAGER_CONF="$(abs_path "$SOURCE_NETWORKMANAGER_CONF" "$SCRIPTDIR/NetworkManager.conf")"
 SOURCE_CONFIG_SERVER="$(abs_path "$SOURCE_CONFIG_SERVER" "$SCRIPTDIR/00-server.conf")"
-SOURCE_CONFIG_IBFT_PLUGIN="$(abs_path "$SOURCE_CONFIG_IBFT_PLUGIN" "$SCRIPTDIR/10-ibft-plugin.conf")"
 SOURCE_CONFIG_CONNECTIVITY_FEDORA="$(abs_path "$SOURCE_CONFIG_CONNECTIVITY_FEDORA" "$SCRIPTDIR/20-connectivity-fedora.conf")"
 
 TEMP="$(mktemp -d "$SCRIPTDIR/NetworkManager.$DATE.XXXXXX")"
@@ -88,7 +87,6 @@ LOG "SPECFILE=$SPECFILE"
 LOG "SOURCE=$SOURCE"
 LOG "SOURCE_NETWORKMANAGER_CONF=$SOURCE_NETWORKMANAGER_CONF"
 LOG "SOURCE_CONFIG_SERVER=$SOURCE_CONFIG_SERVER"
-LOG "SOURCE_CONFIG_IBFT_PLUGIN=$SOURCE_CONFIG_IBFT_PLUGIN"
 LOG "SOURCE_CONFIG_CONNECTIVITY_FEDORA=$SOURCE_CONFIG_CONNECTIVITY_FEDORA"
 LOG "BASEDIR=$TEMP"
 
@@ -102,7 +100,6 @@ mkdir -p "$TEMP/SOURCES/" "$TEMP/SPECS/" || die "error creating SPECS directoy"
 cp "$SOURCE" "$TEMP/SOURCES/" || die "Could not copy source $SOURCE to $TEMP/SOURCES"
 cp "$SOURCE_NETWORKMANAGER_CONF" "$TEMP/SOURCES/NetworkManager.conf" || die "Could not copy source $SOURCE_NETWORKMANAGER_CONF to $TEMP/SOURCES"
 cp "$SOURCE_CONFIG_SERVER" "$TEMP/SOURCES/00-server.conf" || die "Could not copy source $SOURCE_CONFIG_SERVER to $TEMP/SOURCES"
-cp "$SOURCE_CONFIG_IBFT_PLUGIN" "$TEMP/SOURCES/10-ibft-plugin.conf" || die "Could not copy source $SOURCE_CONFIG_IBFT_PLUGIN to $TEMP/SOURCES"
 cp "$SOURCE_CONFIG_CONNECTIVITY_FEDORA" "$TEMP/SOURCES/20-connectivity-fedora.conf" || die "Could not copy source $SOURCE_CONFIG_CONNECTIVITY_FEDORA to $TEMP/SOURCES"
 
 write_changelog
diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in
index 0156a655fb..9557d7a9be 100644
--- a/man/NetworkManager.conf.xml.in
+++ b/man/NetworkManager.conf.xml.in
@@ -632,6 +632,9 @@ ipv6.ip6-privacy=1
 	    <filename>/etc/sysconfig/network-scripts/ifcfg-*</filename>
 	    files. It currently supports reading Ethernet, Wi-Fi,
 	    InfiniBand, VLAN, Bond, Bridge, and Team connections.
+	    Enabling <literal>ifcfg-rh</literal> implicitly enables
+	    <literal>ibft</literal> plugin, if it is available.
+	    This can be disabled by adding <literal>no-ibft</literal>.
 	  </para>
 	</listitem>
       </varlistentry>
@@ -665,12 +668,16 @@ ipv6.ip6-privacy=1
       </varlistentry>
 
       <varlistentry>
-	<term><varname>ibft</varname></term>
+	<term><varname>ibft</varname>, <varname>no-ibft</varname></term>
 	<listitem>
 	  <para>
 	    This plugin allows to read iBFT configuration (iSCSI Boot Firmware Table).
 	    The configuration is read using /sbin/iscsiadm. Users are expected to
 	    configure iBFT connections via the firmware interfaces.
+	    If ibft support is available, it is automatically enabled after
+	    <literal>ifcfg-rh</literal>. This can be disabled by <literal>no-ibft</literal>.
+	    You can also explicitly specify <literal>ibft</literal> to load the
+	    plugin without <literal>ifcfg-rh</literal> or to change the plugin order.
 	  </para>
 	</listitem>
       </varlistentry>
diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index 9f7b47d9bc..befebc1591 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -764,18 +764,20 @@ load_plugins (NMSettings *self, const char **plugins, GError **error)
 	const char **iter;
 	gboolean keyfile_added = FALSE;
 	gboolean success = TRUE;
+	gboolean add_ibft = FALSE;
+	gboolean has_no_ibft;
+	gssize idx_no_ibft, idx_ibft;
+
+	idx_ibft    = _nm_utils_strv_find_first ((char **) plugins, -1, "ibft");
+	idx_no_ibft = _nm_utils_strv_find_first ((char **) plugins, -1, "no-ibft");
+	has_no_ibft = idx_no_ibft >= 0 && idx_no_ibft > idx_ibft;
+#if WITH_SETTINGS_PLUGIN_IBFT
+	add_ibft = idx_no_ibft < 0 && idx_ibft < 0;
+#endif
 
 	for (iter = plugins; iter && *iter; iter++) {
-		GModule *plugin;
-		gs_free char *full_name = NULL;
-		gs_free char *path = NULL;
-		const char *pname;
+		const char *pname = *iter;
 		GObject *obj;
-		GObject * (*factory_func) (void);
-		struct stat st;
-		int errsv;
-
-		pname = *iter;
 
 		if (!*pname || strchr (pname, '/')) {
 			LOG (LOGL_WARN, "ignore invalid plugin \"%s\"", pname);
@@ -787,6 +789,11 @@ load_plugins (NMSettings *self, const char **plugins, GError **error)
 			continue;
 		}
 
+		if (!strcmp (pname, "no-ibft"))
+			continue;
+		if (has_no_ibft && !strcmp (pname, "ibft"))
+			continue;
+
 		obj = find_plugin (list, pname);
 		if (obj)
 			continue;
@@ -800,61 +807,79 @@ load_plugins (NMSettings *self, const char **plugins, GError **error)
 			continue;
 		}
 
-		full_name = g_strdup_printf ("nm-settings-plugin-%s", pname);
-		path = g_module_build_path (NMPLUGINDIR, full_name);
+load_plugin:
+		{
+			GModule *plugin;
+			gs_free char *full_name = NULL;
+			gs_free char *path = NULL;
+			GObject * (*factory_func) (void);
+			struct stat st;
+			int errsv;
+
+			full_name = g_strdup_printf ("nm-settings-plugin-%s", pname);
+			path = g_module_build_path (NMPLUGINDIR, full_name);
+
+			if (stat (path, &st) != 0) {
+				errsv = errno;
+				LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': %s", pname, path, strerror (errsv));
+				goto next;
+			}
+			if (!S_ISREG (st.st_mode)) {
+				LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': not a file", pname, path);
+				goto next;
+			}
+			if (st.st_uid != 0) {
+				LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': file must be owned by root", pname, path);
+				goto next;
+			}
+			if (st.st_mode & (S_IWGRP | S_IWOTH | S_ISUID)) {
+				LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': invalid file permissions", pname, path);
+				goto next;
+			}
 
-		if (stat (path, &st) != 0) {
-			errsv = errno;
-			LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': %s", pname, path, strerror (errsv));
-			continue;
-		}
-		if (!S_ISREG (st.st_mode)) {
-			LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': not a file", pname, path);
-			continue;
-		}
-		if (st.st_uid != 0) {
-			LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': file must be owned by root", pname, path);
-			continue;
-		}
-		if (st.st_mode & (S_IWGRP | S_IWOTH | S_ISUID)) {
-			LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': invalid file permissions", pname, path);
-			continue;
-		}
+			plugin = g_module_open (path, G_MODULE_BIND_LOCAL);
+			if (!plugin) {
+				LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': %s",
+				     pname, path, g_module_error ());
+				goto next;
+			}
 
-		plugin = g_module_open (path, G_MODULE_BIND_LOCAL);
-		if (!plugin) {
-			LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': %s",
-			     pname, path, g_module_error ());
-			continue;
-		}
+			/* errors after this point are fatal, because we loaded the shared library already. */
+
+			if (!g_module_symbol (plugin, "nm_system_config_factory", (gpointer) (&factory_func))) {
+				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
+				             "Could not find plugin '%s' factory function.",
+				             pname);
+				success = FALSE;
+				g_module_close (plugin);
+				break;
+			}
 
-		/* errors after this point are fatal, because we loaded the shared library already. */
+			obj = (*factory_func) ();
+			if (!obj || !NM_IS_SYSTEM_CONFIG_INTERFACE (obj)) {
+				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
+				             "Plugin '%s' returned invalid system config object.",
+				             pname);
+				success = FALSE;
+				g_module_close (plugin);
+				break;
+			}
 
-		if (!g_module_symbol (plugin, "nm_system_config_factory", (gpointer) (&factory_func))) {
-			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-			             "Could not find plugin '%s' factory function.",
-			             pname);
-			success = FALSE;
-			g_module_close (plugin);
-			break;
+			g_module_make_resident (plugin);
+			g_object_weak_ref (obj, (GWeakNotify) g_module_close, plugin);
+			g_object_set_data_full (obj, PLUGIN_MODULE_PATH, path, g_free);
+			path = NULL;
+			add_plugin (self, NM_SYSTEM_CONFIG_INTERFACE (obj));
+			list = g_slist_append (list, obj);
 		}
-
-		obj = (*factory_func) ();
-		if (!obj || !NM_IS_SYSTEM_CONFIG_INTERFACE (obj)) {
-			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-			             "Plugin '%s' returned invalid system config object.",
-			             pname);
-			success = FALSE;
-			g_module_close (plugin);
-			break;
+next:
+		if (add_ibft && !strcmp (pname, "ifcfg-rh")) {
+			/* The plugin ibft is not explicitly mentioned but we just enabled "ifcfg-rh".
+			 * Enable "ibft" by default after "ifcfg-rh". */
+			pname = "ibft";
+			add_ibft = FALSE;
+			goto load_plugin;
 		}
-
-		g_module_make_resident (plugin);
-		g_object_weak_ref (obj, (GWeakNotify) g_module_close, plugin);
-		g_object_set_data_full (obj, PLUGIN_MODULE_PATH, path, g_free);
-		path = NULL;
-		add_plugin (self, NM_SYSTEM_CONFIG_INTERFACE (obj));
-		list = g_slist_append (list, obj);
 	}
 
 	/* If keyfile plugin was not among configured plugins, add it as the last one */