ifcfg-rh: merge branch 'th/ifcfg-rh-fixes-bgo750231'

Some fixes for loading ifcfg-rh files, related to
alias files.

8 files changed, 102 insertions, 92 deletions

diff --git a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
index 06a7f21039..a6324469cc 100644
--- a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
+++ b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
@@ -201,7 +201,8 @@ files_changed_cb (NMInotifyHelper *ih,
 NMIfcfgConnection *
 nm_ifcfg_connection_new (NMConnection *source,
                          const char *full_path,
-                         GError **error)
+                         GError **error,
+                         gboolean *out_ignore_error)
 {
 	GObject *object;
 	NMConnection *tmp;
@@ -211,13 +212,17 @@ nm_ifcfg_connection_new (NMConnection *source,
 
 	g_assert (source || full_path);
 
+	if (out_ignore_error)
+		*out_ignore_error = FALSE;
+
 	/* If we're given a connection already, prefer that instead of re-reading */
 	if (source)
 		tmp = g_object_ref (source);
 	else {
 		tmp = connection_from_file (full_path,
 		                            &unhandled_spec,
-		                            error);
+		                            error,
+		                            out_ignore_error);
 		if (!tmp)
 			return NULL;
 
@@ -376,7 +381,7 @@ commit_changes (NMSettingsConnection *connection,
 	 */
 	filename = nm_settings_connection_get_filename (connection);
 	if (filename) {
-		reread = connection_from_file (filename, NULL, NULL);
+		reread = connection_from_file (filename, NULL, NULL, NULL);
 		if (reread) {
 			same = nm_connection_compare (NM_CONNECTION (connection),
 			                              reread,
diff --git a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.h b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.h
index 328e58f5f2..44e0298772 100644
--- a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.h
+++ b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.h
@@ -48,7 +48,8 @@ GType nm_ifcfg_connection_get_type (void);
 
 NMIfcfgConnection *nm_ifcfg_connection_new (NMConnection *source,
                                             const char *full_path,
-                                            GError **error);
+                                            GError **error,
+                                            gboolean *out_ignore_error);
 
 const char *nm_ifcfg_connection_get_unmanaged_spec (NMIfcfgConnection *self);
 const char *nm_ifcfg_connection_get_unrecognized_spec (NMIfcfgConnection *self);
diff --git a/src/settings/plugins/ifcfg-rh/plugin.c b/src/settings/plugins/ifcfg-rh/plugin.c
index 15e702ef8b..5dd32af83a 100644
--- a/src/settings/plugins/ifcfg-rh/plugin.c
+++ b/src/settings/plugins/ifcfg-rh/plugin.c
@@ -213,6 +213,7 @@ update_connection (SCPluginIfcfg *self,
 	const char *new_unrecognized = NULL, *old_unrecognized = NULL;
 	gboolean unmanaged_changed = FALSE, unrecognized_changed = FALSE;
 	const char *uuid;
+	gboolean ignore_error = FALSE;
 
 	g_return_val_if_fail (!source || NM_IS_CONNECTION (source), NULL);
 	g_return_val_if_fail (full_path || source, NULL);
@@ -222,13 +223,16 @@ update_connection (SCPluginIfcfg *self,
 
 	/* Create a NMIfcfgConnection instance, either by reading from @full_path or
 	 * based on @source. */
-	connection_new = nm_ifcfg_connection_new (source, full_path, error);
+	connection_new = nm_ifcfg_connection_new (source, full_path, &local, &ignore_error);
 	if (!connection_new) {
 		/* Unexpected failure. Probably the file is invalid? */
 		if (   connection
 		    && !protect_existing_connection
 		    && (!protected_connections || !g_hash_table_contains (protected_connections, connection)))
 			remove_connection (self, connection);
+		if (!source && !ignore_error)
+			_LOGW ("loading \"%s\" fails: %s", full_path, local ? local->message : "(unknown reason)");
+		g_propagate_error (error, local);
 		return NULL;
 	}
 
@@ -421,25 +425,13 @@ ifcfg_dir_changed (GFileMonitor *monitor,
                    gpointer user_data)
 {
 	SCPluginIfcfg *plugin = SC_PLUGIN_IFCFG (user_data);
-	char *path, *base, *ifcfg_path;
+	char *path, *ifcfg_path;
 	NMIfcfgConnection *connection;
 
 	path = g_file_get_path (file);
-	if (utils_should_ignore_file (path, FALSE)) {
-		g_free (path);
-		return;
-	}
-
-	_LOGD ("ifcfg_dir_changed(%s) = %d", path, event_type);
 
-	base = g_file_get_basename (file);
-	if (utils_is_ifcfg_alias_file (base, NULL)) {
-		/* Alias file changed. Get the base ifcfg file from it */
-		ifcfg_path = utils_get_ifcfg_from_alias (path);
-	} else {
-		/* Given any ifcfg, keys, or routes file, get the ifcfg file path */
-		ifcfg_path = utils_get_ifcfg_path (path);
-	}
+	ifcfg_path = utils_detect_ifcfg_path (path, FALSE);
+	_LOGD ("ifcfg_dir_changed(%s) = %d // %s", path, event_type, ifcfg_path ? ifcfg_path : "(none)");
 	if (ifcfg_path) {
 		connection = find_by_path (plugin, ifcfg_path);
 		switch (event_type) {
@@ -458,7 +450,6 @@ ifcfg_dir_changed (GFileMonitor *monitor,
 		g_free (ifcfg_path);
 	}
 	g_free (path);
-	g_free (base);
 }
 
 static void
@@ -542,18 +533,14 @@ read_connections (SCPluginIfcfg *plugin)
 
 	filenames = g_ptr_array_new_with_free_func (g_free);
 	while ((item = g_dir_read_name (dir))) {
-		char *full_path;
-
-		if (utils_should_ignore_file (item, TRUE))
-			continue;
-		if (utils_is_ifcfg_alias_file (item, NULL))
-			continue;
+		char *full_path, *real_path;
 
 		full_path = g_build_filename (IFCFG_DIR, item, NULL);
-		if (!utils_get_ifcfg_name (full_path, TRUE))
-			g_free (full_path);
-		else
-			g_ptr_array_add (filenames, full_path);
+		real_path = utils_detect_ifcfg_path (full_path, TRUE);
+
+		if (real_path)
+			g_ptr_array_add (filenames, real_path);
+		g_free (full_path);
 	}
 	g_dir_close (dir);
 
@@ -625,20 +612,25 @@ load_connection (NMSystemConfigInterface *config,
 	SCPluginIfcfg *plugin = SC_PLUGIN_IFCFG (config);
 	NMIfcfgConnection *connection;
 	int dir_len = strlen (IFCFG_DIR);
+	char *ifcfg_path;
 
 	if (   strncmp (filename, IFCFG_DIR, dir_len) != 0
 	    || filename[dir_len] != '/'
 	    || strchr (filename + dir_len + 1, '/') != NULL)
 		return FALSE;
 
-	if (utils_should_ignore_file (filename + dir_len + 1, TRUE))
+	/* get the real ifcfg-path. This allows us to properly
+	 * handle load command using a route-* file etc. */
+	ifcfg_path = utils_detect_ifcfg_path (filename, FALSE);
+	if (!ifcfg_path)
 		return FALSE;
 
-	connection = find_by_path (plugin, filename);
-	update_connection (plugin, NULL, filename, connection, TRUE, NULL, NULL);
+	connection = find_by_path (plugin, ifcfg_path);
+	update_connection (plugin, NULL, ifcfg_path, connection, TRUE, NULL, NULL);
 	if (!connection)
-		connection = find_by_path (plugin, filename);
+		connection = find_by_path (plugin, ifcfg_path);
 
+	g_free (ifcfg_path);
 	return (connection != NULL);
 }
 
diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
index ed49f40e66..ccbcdb07ab 100644
--- a/src/settings/plugins/ifcfg-rh/reader.c
+++ b/src/settings/plugins/ifcfg-rh/reader.c
@@ -1180,16 +1180,14 @@ read_aliases (NMSettingIPConfig *s_ip4, const char *filename, const char *networ
 	GDir *dir;
 	char *dirname, *base;
 	shvarFile *parsed;
-	NMIPAddress *base_addr;
+	NMIPAddress *base_addr = NULL;
 	GError *err = NULL;
 
 	g_return_if_fail (s_ip4 != NULL);
 	g_return_if_fail (filename != NULL);
 
-	if (nm_setting_ip_config_get_num_addresses (s_ip4) == 0)
-		return;
-
-	base_addr = nm_setting_ip_config_get_address (s_ip4, 0);
+	if (nm_setting_ip_config_get_num_addresses (s_ip4) > 0)
+		base_addr = nm_setting_ip_config_get_address (s_ip4, 0);
 
 	dirname = g_path_get_dirname (filename);
 	g_return_if_fail (dirname != NULL);
@@ -4889,18 +4887,13 @@ done:
 NMConnection *
 connection_from_file (const char *filename,
                       char **out_unhandled,
-                      GError **error)
+                      GError **error,
+                      gboolean *out_ignore_error)
 {
-	gboolean ignore_error = FALSE;
-	NMConnection *conn;
-
-	conn = connection_from_file_full (filename, NULL, NULL,
+	return connection_from_file_full (filename, NULL, NULL,
 	                                  out_unhandled,
 	                                  error,
-	                                  &ignore_error);
-	if (error && *error && !ignore_error)
-		PARSE_WARNING ("%s", (*error)->message);
-	return conn;
+	                                  out_ignore_error);
 }
 
 NMConnection *
diff --git a/src/settings/plugins/ifcfg-rh/reader.h b/src/settings/plugins/ifcfg-rh/reader.h
index 70e9ce4e24..2096ffc40e 100644
--- a/src/settings/plugins/ifcfg-rh/reader.h
+++ b/src/settings/plugins/ifcfg-rh/reader.h
@@ -28,7 +28,8 @@
 
 NMConnection *connection_from_file (const char *filename,
                                     char **out_unhandled,
-                                    GError **error);
+                                    GError **error,
+                                    gboolean *out_ignore_error);
 
 char *uuid_from_file (const char *filename);
 
diff --git a/src/settings/plugins/ifcfg-rh/utils.c b/src/settings/plugins/ifcfg-rh/utils.c
index 87f4aba3e5..a793288d91 100644
--- a/src/settings/plugins/ifcfg-rh/utils.c
+++ b/src/settings/plugins/ifcfg-rh/utils.c
@@ -27,6 +27,7 @@
 #include "nm-core-internal.h"
 #include "nm-macros-internal.h"
 #include "NetworkManagerUtils.h"
+#include "gsystem-local-alloc.h"
 
 #include "utils.h"
 #include "shvar.h"
@@ -151,41 +152,34 @@ check_suffix (const char *base, const char *tag)
 gboolean
 utils_should_ignore_file (const char *filename, gboolean only_ifcfg)
 {
-	char *base;
-	gboolean ignore = TRUE;
-	gboolean is_ifcfg = FALSE;
-	gboolean is_other = FALSE;
+	gs_free char *base = NULL;
 
 	g_return_val_if_fail (filename != NULL, TRUE);
 
 	base = g_path_get_basename (filename);
-	g_return_val_if_fail (base != NULL, TRUE);
 
 	/* Only handle ifcfg, keys, and routes files */
-	if (!strncmp (base, IFCFG_TAG, strlen (IFCFG_TAG)))
-		is_ifcfg = TRUE;
-
-	if (only_ifcfg == FALSE) {
-		if (   !strncmp (base, KEYS_TAG, strlen (KEYS_TAG))
-		    || !strncmp (base, ROUTE_TAG, strlen (ROUTE_TAG))
-		    || !strncmp (base, ROUTE6_TAG, strlen (ROUTE6_TAG)))
-				is_other = TRUE;
+	if (strncmp (base, IFCFG_TAG, strlen (IFCFG_TAG)) != 0) {
+		if (only_ifcfg)
+			return TRUE;
+		else if (   strncmp (base, KEYS_TAG, strlen (KEYS_TAG)) != 0
+		         && strncmp (base, ROUTE_TAG, strlen (ROUTE_TAG)) != 0
+		         && strncmp (base, ROUTE6_TAG, strlen (ROUTE6_TAG)) != 0)
+			return TRUE;
 	}
 
 	/* But not those that have certain suffixes */
-	if (   (is_ifcfg || is_other)
-	    && !check_suffix (base, BAK_TAG)
-	    && !check_suffix (base, TILDE_TAG)
-	    && !check_suffix (base, ORIG_TAG)
-	    && !check_suffix (base, REJ_TAG)
-	    && !check_suffix (base, RPMNEW_TAG)
-	    && !check_suffix (base, AUGNEW_TAG)
-	    && !check_suffix (base, AUGTMP_TAG)
-	    && !check_rpm_temp_suffix (base))
-		ignore = FALSE;
-
-	g_free (base);
-	return ignore;
+	if (   check_suffix (base, BAK_TAG)
+	    || check_suffix (base, TILDE_TAG)
+	    || check_suffix (base, ORIG_TAG)
+	    || check_suffix (base, REJ_TAG)
+	    || check_suffix (base, RPMNEW_TAG)
+	    || check_suffix (base, AUGNEW_TAG)
+	    || check_suffix (base, AUGTMP_TAG)
+	    || check_rpm_temp_suffix (base))
+		return TRUE;
+
+	return FALSE;
 }
 
 char *
@@ -230,6 +224,12 @@ utils_get_ifcfg_name (const char *file, gboolean only_ifcfg)
 		} \
 	} G_STMT_END
 
+	/* Do not detect alias files and return 'eth0:0' instead of 'eth0'.
+	 * Unfortunately, we cannot be sure that our files don't contain colons,
+	 * so we cannot reject files with colons.
+	 *
+	 * Instead, you must not call utils_get_ifcfg_name() with an alias file
+	 * or files that are ignored. */
 	MATCH_TAG_AND_RETURN (name, IFCFG_TAG);
 	if (!only_ifcfg) {
 		MATCH_TAG_AND_RETURN (name, KEYS_TAG);
@@ -425,26 +425,43 @@ utils_is_ifcfg_alias_file (const char *alias, const char *ifcfg)
 }
 
 char *
-utils_get_ifcfg_from_alias (const char *alias)
+utils_detect_ifcfg_path (const char *path, gboolean only_ifcfg)
 {
-	char *base, *ptr, *ifcfg = NULL;
+	gs_free char *base = NULL;
+	char *ptr, *ifcfg = NULL;
 
-	g_return_val_if_fail (alias != NULL, NULL);
+	g_return_val_if_fail (path != NULL, NULL);
 
-	base = g_path_get_basename (alias);
-	g_return_val_if_fail (base != NULL, NULL);
+	if (utils_should_ignore_file (path, only_ifcfg))
+		return NULL;
 
-	if (utils_is_ifcfg_alias_file (base, NULL)) {
-		ifcfg = g_strdup (alias);
-		ptr = strrchr (ifcfg, ':');
-		if (ptr)
-			*ptr = '\0';
-		else {
+	base = g_path_get_basename (path);
+
+	if (strncmp (base, IFCFG_TAG, STRLEN (IFCFG_TAG)) == 0) {
+		if (base[STRLEN (IFCFG_TAG)] == '\0')
+			return NULL;
+		if (utils_is_ifcfg_alias_file (base, NULL)) {
+			ifcfg = g_strdup (path);
+			ptr = strrchr (ifcfg, ':');
+			if (ptr && ptr > ifcfg) {
+				*ptr = '\0';
+				if (g_file_test (ifcfg, G_FILE_TEST_EXISTS)) {
+					/* the file has a colon, so it is probably an alias.
+					 * To be ~more~ certain that this is an alias file,
+					 * check whether a corresponding base file exists. */
+					if (only_ifcfg) {
+						g_free (ifcfg);
+						return NULL;
+					}
+					return ifcfg;
+				}
+			}
 			g_free (ifcfg);
-			ifcfg = NULL;
 		}
+		return g_strdup (path);
 	}
 
-	g_free (base);
-	return ifcfg;
+	if (only_ifcfg)
+		return NULL;
+	return utils_get_ifcfg_path (path);
 }
diff --git a/src/settings/plugins/ifcfg-rh/utils.h b/src/settings/plugins/ifcfg-rh/utils.h
index 445437c48b..547bfcb2d7 100644
--- a/src/settings/plugins/ifcfg-rh/utils.h
+++ b/src/settings/plugins/ifcfg-rh/utils.h
@@ -59,7 +59,8 @@ gboolean utils_has_complex_routes (const char *filename);
 gboolean utils_ignore_ip_config (NMConnection *connection);
 
 gboolean utils_is_ifcfg_alias_file (const char *alias, const char *ifcfg);
-char *utils_get_ifcfg_from_alias (const char *alias);
+
+char *utils_detect_ifcfg_path (const char *path, gboolean only_ifcfg);
 
 #endif  /* _UTILS_H_ */
 
diff --git a/src/settings/plugins/ifcfg-rh/writer.c b/src/settings/plugins/ifcfg-rh/writer.c
index d5f26a14b0..27f1c70599 100644
--- a/src/settings/plugins/ifcfg-rh/writer.c
+++ b/src/settings/plugins/ifcfg-rh/writer.c
@@ -2530,7 +2530,7 @@ escape_id (const char *id)
 	while (*p) {
 		if (*p == ' ')
 			*p = '_';
-		else if (strchr ("\\][|/=()!", *p))
+		else if (strchr ("\\][|/=()!:", *p))
 			*p = '-';
 		p++;
 	}