ifcfg-rh: log warning when loading of connection fails

connection_from_file() used to log a warning about failure,
but only when an @error argument was given.

update_connection() didn't ensure that in several cases,
so we would not log any failure reason when an ifcfg file
failed to read.

This behavior of controlling logging by passing @error (or not)
is unexpected. Instead, refactor the code so that the caller
can do appropriate logging.
Another reason for this refactoring is that PARSE_WARNING() does
not mention the file for which the failure is and uses some extra
indention that looks wrong. IOW, connection_from_file() doesn't
have the context to give the logging line a proper formatting.

5 files changed, 21 insertions, 15 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..0d4ad1be35 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;
 	}
 
diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
index ed49f40e66..6dcb1a3641 100644
--- a/src/settings/plugins/ifcfg-rh/reader.c
+++ b/src/settings/plugins/ifcfg-rh/reader.c
@@ -4889,18 +4889,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);