settings: enable "ibft" plugin by default together with "ifcfg-rh"
authorThomas Haller <thaller@redhat.com>
Tue, 9 Jun 2015 17:27:55 +0000 (19:27 +0200)
committerThomas Haller <thaller@redhat.com>
Thu, 2 Jul 2015 14:01:20 +0000 (16:01 +0200)
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.

configure.ac
contrib/fedora/rpm/NetworkManager.spec
contrib/fedora/rpm/build.sh
man/NetworkManager.conf.xml.in
src/settings/nm-settings.c

index b197ef7..e9b39d0 100644 (file)
@@ -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
index aa091e0..0786b30 100644 (file)
@@ -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/*
index c1ededb..8e6539d 100755 (executable)
@@ -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
index 0156a65..9557d7a 100644 (file)
@@ -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>
index 9f7b47d..befebc1 100644 (file)
@@ -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 */