2008-05-13 Dan Williams <dcbw@redhat.com>

* marshallers/nm-marshal.list - Add VOID:POINTER,STRING marshaller for ifcfg-fedora plugin * system-settings/plugins/ifcfg-fedora/Makefile.am system-settings/plugins/ifcfg-fedora/nm-inotify-helper.c system-settings/plugins/ifcfg-fedora/nm-inotify-helper.h - Implement a minimal inotify helper for watch paths for IN_CLOSE_WRITE events. Solely for use watching ifcfg files to pick up changes to their hardlinks, since GIO doesn't support this yet (bgo #532815) * system-settings/plugins/ifcfg-fedora/nm-ifcfg-connection.c - (nm_ifcfg_connection_class_init): new 'ifcfg-changed' signal when the file contents change - (finalize): clean up inotify watches - (nm_ifcfg_connection_new): store keyfile; inotify watch the keyfile and the connection ifcfg for changes on their hardlinks - (files_changed_cb): proxy the changed signal back out to listeners * system-settings/plugins/ifcfg-fedora/plugin.c - (dir_changed): - (connection_ifcfg_changed): re-read the connection when the ifcfg changes - (read_one_connection): connect to change signals on the new connection - (dir_changed, connection_changed_handler, handle_connection_remove_or_new): break out connection change handling and connection new/remove handling so it can be used from both the GFileMonitor callback and the NMIfcfgConnection changed signals * system-settings/plugins/ifcfg-fedora/reader.c system-settings/plugins/ifcfg-fedora/reader.h - (connection_from_file): return the keyfile path the connection would use git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@3663 4912f4e0-d625-0410-9fb7-b9a5a253dbdc
2024-07-22 02:35:25 +00:00 · 2008-05-13 16:53:50 +00:00 · 2008-05-13 16:53:50 +00:00 · 4ce69835f5
parent b8df4e8217
commit 4ce69835f5
9 changed files with 581 additions and 108 deletions
--- a/35
+++ b/35
@ -1,3 +1,38 @@
+2008-05-13  Dan Williams  <dcbw@redhat.com>
+
+	* marshallers/nm-marshal.list
+		- Add VOID:POINTER,STRING marshaller for ifcfg-fedora plugin
+
+	* system-settings/plugins/ifcfg-fedora/Makefile.am
+	  system-settings/plugins/ifcfg-fedora/nm-inotify-helper.c
+	  system-settings/plugins/ifcfg-fedora/nm-inotify-helper.h
+		- Implement a minimal inotify helper for watch paths for IN_CLOSE_WRITE
+			events.  Solely for use watching ifcfg files to pick up changes
+			to their hardlinks, since GIO doesn't support this yet (bgo #532815)
+
+	* system-settings/plugins/ifcfg-fedora/nm-ifcfg-connection.c
+		- (nm_ifcfg_connection_class_init): new 'ifcfg-changed' signal when the
+			file contents change
+		- (finalize): clean up inotify watches
+		- (nm_ifcfg_connection_new): store keyfile; inotify watch the keyfile
+			and the connection ifcfg for changes on their hardlinks
+		- (files_changed_cb): proxy the changed signal back out to listeners
+
+	* system-settings/plugins/ifcfg-fedora/plugin.c
+		- (dir_changed): 
+		- (connection_ifcfg_changed): re-read the connection when the ifcfg
+			changes
+		- (read_one_connection): connect to change signals on the new connection
+		- (dir_changed, connection_changed_handler,
+		   handle_connection_remove_or_new): break out connection change
+			handling and connection new/remove handling so it can be used from
+			both the GFileMonitor callback and the NMIfcfgConnection changed
+			signals
+
+	* system-settings/plugins/ifcfg-fedora/reader.c
+	  system-settings/plugins/ifcfg-fedora/reader.h
+		- (connection_from_file): return the keyfile path the connection would use
+
 2008-05-13  Tambet Ingo  <tambet@gmail.com>

 	* system-settings/src/nm-polkit-helpers.c (create_polkit_context): Use a 
--- a/marshallers/nm-marshal.list
+++ b/marshallers/nm-marshal.list
@ -13,4 +13,4 @@ VOID:OBJECT,UINT,UINT
 VOID:STRING,INT
 VOID:STRING,UINT
 VOID:OBJECT,OBJECT,ENUM
-
+VOID:POINTER,STRING
--- a/system-settings/plugins/ifcfg-fedora/Makefile.am
+++ b/system-settings/plugins/ifcfg-fedora/Makefile.am
@ -10,7 +10,9 @@ libnm_settings_plugin_ifcfg_fedora_la_SOURCES = \
 	nm-ifcfg-connection.h \
 	reader.c \
 	reader.h \
-	common.h
+	common.h \
+	nm-inotify-helper.c \
+	nm-inotify-helper.h

 libnm_settings_plugin_ifcfg_fedora_la_CPPFLAGS = \
 	$(GLIB_CFLAGS) \
@ -21,6 +23,7 @@ libnm_settings_plugin_ifcfg_fedora_la_CPPFLAGS = \
 	-I$(top_srcdir)/include \
 	-I$(top_srcdir)/libnm-glib \
 	-I$(top_srcdir)/libnm-util \
+	-I$(top_builddir)/marshallers \
 	-DSYSCONFDIR=\"$(sysconfdir)\"

 libnm_settings_plugin_ifcfg_fedora_la_LDFLAGS = -module -avoid-version
@ -28,7 +31,8 @@ libnm_settings_plugin_ifcfg_fedora_la_LIBADD = \
 	$(GLIB_LIBS) \
 	$(GMODULE_LIBS) \
 	$(top_builddir)/libnm-util/libnm-util.la \
-	$(top_builddir)/libnm-glib/libnm_glib.la
+	$(top_builddir)/libnm-glib/libnm_glib.la \
+	$(top_builddir)/marshallers/libmarshallers.la

 if NO_GIO
 libnm_settings_plugin_ifcfg_fedora_la_LIBADD += \
--- a/system-settings/plugins/ifcfg-fedora/nm-ifcfg-connection.c
+++ b/system-settings/plugins/ifcfg-fedora/nm-ifcfg-connection.c
@ -38,14 +38,20 @@
 #include "nm-ifcfg-connection.h"
 #include "nm-system-config-hal-manager.h"
 #include "reader.h"
+#include "nm-inotify-helper.h"

 G_DEFINE_TYPE (NMIfcfgConnection, nm_ifcfg_connection, NM_TYPE_EXPORTED_CONNECTION)

 #define NM_IFCFG_CONNECTION_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_IFCFG_CONNECTION, NMIfcfgConnectionPrivate))

 typedef struct {
+	gulong ih_event_id;
+
 	char *filename;
+	int file_wd;
+
 	char *keyfile;
+	int keyfile_wd;

 	char *udi;
 	gboolean unmanaged;
@ -64,6 +70,14 @@ enum {
 	LAST_PROP
 };

+/* Signals */
+enum {
+	IFCFG_CHANGED,
+	LAST_SIGNAL
+};
+
+static guint signals[LAST_SIGNAL] = { 0 };
+
 static char *
 get_ether_device_udi (DBusGConnection *g_connection, GByteArray *mac, GSList *devices)
 {
@ -205,6 +219,22 @@ device_added_cb (NMSystemConfigHalManager *hal_mgr,
 	priv->daid = 0;
 }

+static void
+files_changed_cb (NMInotifyHelper *ih,
+                  struct inotify_event *evt,
+                  const char *path,
+                  gpointer user_data)
+{
+	NMIfcfgConnection *self = NM_IFCFG_CONNECTION (user_data);
+	NMIfcfgConnectionPrivate *priv = NM_IFCFG_CONNECTION_GET_PRIVATE (self);
+
+	if ((evt->wd != priv->file_wd) && (evt->wd != priv->keyfile_wd))
+		return;
+
+	/* push the event up to the plugin */
+	g_signal_emit (self, signals[IFCFG_CHANGED], 0);
+}
+
 NMIfcfgConnection *
 nm_ifcfg_connection_new (const char *filename,
                         DBusGConnection *g_connection,
@ -212,13 +242,16 @@ nm_ifcfg_connection_new (const char *filename,
                         GError **error)
 {
 	GObject *object;
+	NMIfcfgConnectionPrivate *priv;
 	NMConnection *wrapped;
 	gboolean unmanaged = FALSE;
 	char *udi;
+	char *keyfile = NULL;
+	NMInotifyHelper *ih;

 	g_return_val_if_fail (filename != NULL, NULL);

-	wrapped = connection_from_file (filename, &unmanaged, error);
+	wrapped = connection_from_file (filename, &unmanaged, &keyfile, error);
 	if (!wrapped)
 		return NULL;

@ -230,14 +263,26 @@ nm_ifcfg_connection_new (const char *filename,
 	                                   NM_IFCFG_CONNECTION_UDI, udi,
 	                                   NM_EXPORTED_CONNECTION_CONNECTION, wrapped,
 	                                   NULL);
-	if (object && !udi) {
-		NMIfcfgConnectionPrivate *priv = NM_IFCFG_CONNECTION_GET_PRIVATE (object);
+	if (!object)
+		goto out;

+	priv = NM_IFCFG_CONNECTION_GET_PRIVATE (object);
+
+	if (!udi) {
 		priv->hal_mgr = g_object_ref (hal_mgr);
 		priv->g_connection = dbus_g_connection_ref (g_connection);
 		priv->daid = g_signal_connect (priv->hal_mgr, "device-added", G_CALLBACK (device_added_cb), object);
 	}

+	ih = nm_inotify_helper_get ();
+	priv->ih_event_id = g_signal_connect (ih, "event", G_CALLBACK (files_changed_cb), object);
+
+	priv->file_wd = nm_inotify_helper_add_watch (ih, filename);
+
+	priv->keyfile = keyfile;
+	priv->keyfile_wd = nm_inotify_helper_add_watch (ih, keyfile);
+
+out:
 	g_object_unref (wrapped);
 	g_free (udi);
 	return (NMIfcfgConnection *) object;
@ -310,13 +355,25 @@ finalize (GObject *object)
 {
 	NMIfcfgConnectionPrivate *priv = NM_IFCFG_CONNECTION_GET_PRIVATE (object);
 	NMConnection *wrapped;
+	NMInotifyHelper *ih;
+
+	g_free (priv->udi);

 	wrapped = nm_exported_connection_get_connection (NM_EXPORTED_CONNECTION (object));
 	if (wrapped)
 		nm_connection_clear_secrets (wrapped);

+	ih = nm_inotify_helper_get ();
+
+	g_signal_handler_disconnect (ih, priv->ih_event_id);
+
 	g_free (priv->filename);
-	g_free (priv->udi);
+	if (priv->file_wd >= 0)
+		nm_inotify_helper_remove_watch (ih, priv->file_wd);
+
+	g_free (priv->keyfile);
+	if (priv->keyfile_wd >= 0)
+		nm_inotify_helper_remove_watch (ih, priv->keyfile_wd);

 	if (priv->hal_mgr) {
 		if (priv->daid)
@ -419,4 +476,13 @@ nm_ifcfg_connection_class_init (NMIfcfgConnectionClass *ifcfg_connection_class)
 						  "UDI",
 						  NULL,
 						  G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
+
+	signals[IFCFG_CHANGED] =
+		g_signal_new ("ifcfg-changed",
+		              G_OBJECT_CLASS_TYPE (object_class),
+		              G_SIGNAL_RUN_LAST,
+		              0, NULL, NULL,
+		              g_cclosure_marshal_VOID__VOID,
+		              G_TYPE_NONE, 0);
 }
+
--- a/system-settings/plugins/ifcfg-fedora/nm-inotify-helper.c
+++ b/system-settings/plugins/ifcfg-fedora/nm-inotify-helper.c
@ -0,0 +1,211 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
+/* NetworkManager system settings service
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * (C) Copyright 2008 Red Hat, Inc.
+ */
+
+#include <unistd.h>
+#include <string.h>
+#include <sys/inotify.h>
+#include <glib.h>
+
+#include "nm-marshal.h"
+#include "nm-inotify-helper.h"
+
+G_DEFINE_TYPE (NMInotifyHelper, nm_inotify_helper, G_TYPE_OBJECT)
+
+#define NM_INOTIFY_HELPER_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_INOTIFY_HELPER, NMInotifyHelperPrivate))
+
+typedef struct {
+	int ifd;
+
+	GHashTable *wd_refs;
+} NMInotifyHelperPrivate;
+
+/* Signals */
+enum {
+	EVENT,
+	LAST_SIGNAL
+};
+
+static guint signals[LAST_SIGNAL] = { 0 };
+
+int
+nm_inotify_helper_add_watch (NMInotifyHelper *self, const char *path)
+{
+	NMInotifyHelperPrivate *priv = NM_INOTIFY_HELPER_GET_PRIVATE (self);
+	int wd;
+	guint32 refcount;
+
+	g_return_val_if_fail (priv->ifd >= 0, -1);
+
+	/* We only care about modifications since we're just trying to get change
+	 * notifications on hardlinks.
+	 */
+
+	wd = inotify_add_watch (priv->ifd, path, IN_CLOSE_WRITE);
+	if (wd < 0)
+		return -1;
+
+	refcount = GPOINTER_TO_UINT (g_hash_table_lookup (priv->wd_refs, GINT_TO_POINTER (wd)));
+	refcount++;
+	g_hash_table_replace (priv->wd_refs, GINT_TO_POINTER (wd), GUINT_TO_POINTER (refcount));
+
+	return wd;
+}
+
+void
+nm_inotify_helper_remove_watch (NMInotifyHelper *self, int wd)
+{
+	NMInotifyHelperPrivate *priv = NM_INOTIFY_HELPER_GET_PRIVATE (self);
+	guint32 refcount;
+
+	g_return_if_fail (priv->ifd >= 0);
+
+	refcount = GPOINTER_TO_UINT (g_hash_table_lookup (priv->wd_refs, GINT_TO_POINTER (wd)));
+	if (!refcount)
+		return;
+
+	refcount--;
+	if (!refcount) {
+		g_hash_table_remove (priv->wd_refs, GINT_TO_POINTER (wd));
+		inotify_rm_watch (priv->ifd, wd);
+	} else
+		g_hash_table_replace (priv->wd_refs, GINT_TO_POINTER (wd), GUINT_TO_POINTER (refcount));
+}
+
+static gboolean
+inotify_event_handler (GIOChannel *channel, GIOCondition cond, gpointer user_data)
+{
+	NMInotifyHelper *self = NM_INOTIFY_HELPER (user_data);
+	struct inotify_event evt;
+
+	/* read the notifications from the watch descriptor */
+	while (g_io_channel_read_chars (channel, (gchar *) &evt, sizeof (struct inotify_event), NULL, NULL) == G_IO_STATUS_NORMAL) {
+		gchar filename[PATH_MAX + 1];
+
+		filename[0] = '\0';
+		if (evt.len > 0) {
+			g_io_channel_read_chars (channel,
+			                        filename,
+			                        evt.len > PATH_MAX ? PATH_MAX : evt.len,
+			                        NULL, NULL);
+		}
+
+		if (!(evt.mask & IN_IGNORED))
+			g_signal_emit (self, signals[EVENT], 0, &evt, &filename[0]);
+	}
+
+	return TRUE;
+}
+
+static gboolean
+init_inotify (NMInotifyHelper *self)
+{
+	NMInotifyHelperPrivate *priv = NM_INOTIFY_HELPER_GET_PRIVATE (self);
+	GIOChannel *channel;
+	guint source_id;
+
+	priv->ifd = inotify_init ();
+	if (priv->ifd == -1) {
+		g_warning ("%s: couldn't initialize inotify", __func__);
+		return FALSE;
+	}
+
+	/* Watch the inotify descriptor for file/directory change events */
+	channel = g_io_channel_unix_new (priv->ifd);
+	if (!channel) {
+		g_warning ("%s: couldn't create new GIOChannel", __func__);
+		close (priv->ifd);
+		priv->ifd = -1;
+		return FALSE;
+	}
+
+	g_io_channel_set_flags (channel, G_IO_FLAG_NONBLOCK, NULL);
+	g_io_channel_set_encoding (channel, NULL, NULL); 
+
+	source_id = g_io_add_watch (channel,
+	                            G_IO_IN | G_IO_ERR,
+	                            (GIOFunc) inotify_event_handler,
+	                            (gpointer) self);
+	g_io_channel_unref (channel);
+	return TRUE;
+}
+
+NMInotifyHelper *
+nm_inotify_helper_get (void)
+{
+	static NMInotifyHelper *singleton = NULL;
+
+	if (!singleton) {
+		singleton = (NMInotifyHelper *) g_object_new (NM_TYPE_INOTIFY_HELPER, NULL);
+		if (!singleton)
+			return NULL;
+
+		if (!init_inotify (singleton)) {
+			g_object_unref (singleton);
+			return NULL;
+		}
+	} else
+		g_object_ref (singleton);
+
+	g_assert (singleton);
+	return singleton;
+}
+
+static void
+nm_inotify_helper_init (NMInotifyHelper *self)
+{
+	NMInotifyHelperPrivate *priv = NM_INOTIFY_HELPER_GET_PRIVATE (self);
+
+	priv->wd_refs = g_hash_table_new (g_direct_hash, g_direct_equal);
+}
+
+static void
+finalize (GObject *object)
+{
+	NMInotifyHelperPrivate *priv = NM_INOTIFY_HELPER_GET_PRIVATE (object);
+
+	if (priv->ifd >= 0)
+		close (priv->ifd);
+
+	g_hash_table_destroy (priv->wd_refs);
+
+	G_OBJECT_CLASS (nm_inotify_helper_parent_class)->finalize (object);
+}
+
+static void
+nm_inotify_helper_class_init (NMInotifyHelperClass *klass)
+{
+	GObjectClass *object_class = G_OBJECT_CLASS (klass);
+
+	g_type_class_add_private (klass, sizeof (NMInotifyHelperPrivate));
+
+	/* Virtual methods */
+	object_class->finalize = finalize;
+
+	/* Signals */
+	signals[EVENT] =
+		g_signal_new ("event",
+		              G_OBJECT_CLASS_TYPE (object_class),
+		              G_SIGNAL_RUN_LAST,
+		              G_STRUCT_OFFSET (NMInotifyHelperClass, event),
+		              NULL, NULL,
+		              nm_marshal_VOID__POINTER_STRING,
+		              G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_STRING);
+}
+
--- a/system-settings/plugins/ifcfg-fedora/nm-inotify-helper.h
+++ b/system-settings/plugins/ifcfg-fedora/nm-inotify-helper.h
@ -0,0 +1,54 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
+/* NetworkManager system settings service
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * (C) Copyright 2008 Red Hat, Inc.
+ */
+
+#ifndef __INOTIFY_HELPER_H__
+#define __INOTIFY_HELPER_H__
+
+#include <glib.h>
+#include <glib-object.h>
+#include <sys/inotify.h>
+
+#define NM_TYPE_INOTIFY_HELPER            (nm_inotify_helper_get_type ())
+#define NM_INOTIFY_HELPER(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_INOTIFY_HELPER, NMInotifyHelper))
+#define NM_INOTIFY_HELPER_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_INOTIFY_HELPER, NMInotifyHelperClass))
+#define NM_IS_INOTIFY_HELPER(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_INOTIFY_HELPER))
+#define NM_IS_INOTIFY_HELPER_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((obj), NM_TYPE_INOTIFY_HELPER))
+#define NM_INOTIFY_HELPER_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_INOTIFY_HELPER, NMInotifyHelperClass))
+
+typedef struct {
+	GObject parent;
+} NMInotifyHelper;
+
+typedef struct {
+	GObjectClass parent;
+
+	/* signals */
+	void (* event) (NMInotifyHelper *helper, struct inotify_event *evt, const char *filename);
+} NMInotifyHelperClass;
+
+GType nm_inotify_helper_get_type (void);
+
+NMInotifyHelper * nm_inotify_helper_get (void);
+
+int nm_inotify_helper_add_watch (NMInotifyHelper *helper, const char *path);
+
+void nm_inotify_helper_remove_watch (NMInotifyHelper *helper, int wd);
+
+#endif  /* __INOTIFY_HELPER_H__ */
--- a/system-settings/plugins/ifcfg-fedora/plugin.c
+++ b/system-settings/plugins/ifcfg-fedora/plugin.c
@ -52,6 +52,18 @@

 static void system_config_interface_init (NMSystemConfigInterface *system_config_interface_class);

+static void connection_changed_handler (SCPluginIfcfg *plugin,
+                                        const char *path,
+                                        NMIfcfgConnection *connection,
+                                        gboolean *do_remove,
+                                        gboolean *do_new);
+
+static void handle_connection_remove_or_new (SCPluginIfcfg *plugin,
+                                             const char *path,
+                                             NMIfcfgConnection *connection,
+                                             gboolean do_remove,
+                                             gboolean do_new);
+
 G_DEFINE_TYPE_EXTENDED (SCPluginIfcfg, sc_plugin_ifcfg, G_TYPE_OBJECT, 0,
 						G_IMPLEMENT_INTERFACE (NM_TYPE_SYSTEM_CONFIG_INTERFACE,
 											   system_config_interface_init))
@ -123,6 +135,20 @@ connection_unmanaged_changed (NMIfcfgConnection *connection,
 	g_signal_emit_by_name (SC_PLUGIN_IFCFG (user_data), "unmanaged-devices-changed");
 }

+static void
+connection_ifcfg_changed (NMIfcfgConnection *connection, gpointer user_data)
+{
+	SCPluginIfcfg *plugin = SC_PLUGIN_IFCFG (user_data);
+	gboolean do_remove = FALSE, do_new = FALSE;
+	const char *path;
+
+	path = nm_ifcfg_connection_get_filename (connection);
+	g_return_if_fail (path != NULL);
+
+	connection_changed_handler (plugin, path, connection, &do_remove, &do_new);
+	handle_connection_remove_or_new (plugin, path, connection, do_remove, do_new);
+}
+
 static NMIfcfgConnection *
 read_one_connection (SCPluginIfcfg *plugin, const char *filename)
 {
@ -159,6 +185,10 @@ read_one_connection (SCPluginIfcfg *plugin, const char *filename)
 			g_signal_connect (G_OBJECT (connection), "notify::unmanaged",
 			                  G_CALLBACK (connection_unmanaged_changed), plugin);
 		}
+
+		/* watch changes of ifcfg hardlinks */
+		g_signal_connect (G_OBJECT (connection), "ifcfg-changed",
+		                  G_CALLBACK (connection_ifcfg_changed), plugin);
 	} else {
 		PLUGIN_PRINT (IFCFG_PLUGIN_NAME, "    error: %s",
 		              error->message ? error->message : "(unknown)");
@ -169,7 +199,7 @@ read_one_connection (SCPluginIfcfg *plugin, const char *filename)
 }

 static gboolean
-should_ignore_file (const char *basename, const char *tag)
+check_suffix (const char *basename, const char *tag)
 {
 	int len, tag_len;

@ -183,6 +213,28 @@ should_ignore_file (const char *basename, const char *tag)
 	return FALSE;
 }

+static gboolean
+should_ignore_file (const char *filename)
+{
+	char *basename;
+	gboolean ignore = TRUE;
+
+	g_return_val_if_fail (filename != NULL, TRUE);
+
+	basename = g_path_get_basename (filename);
+	g_return_val_if_fail (basename != NULL, TRUE);
+
+	if (   !strncmp (basename, IFCFG_TAG, strlen (IFCFG_TAG))
+		&& !check_suffix (basename, BAK_TAG)
+		&& !check_suffix (basename, TILDE_TAG)
+		&& !check_suffix (basename, ORIG_TAG)
+		&& !check_suffix (basename, REJ_TAG))
+		ignore = FALSE;
+
+	g_free (basename);
+	return ignore;
+}
+
 static void
 read_connections (SCPluginIfcfg *plugin)
 {
@ -196,14 +248,7 @@ read_connections (SCPluginIfcfg *plugin)
 		while ((item = g_dir_read_name (dir))) {
 			char *full_path;

-			if (strncmp (item, IFCFG_TAG, strlen (IFCFG_TAG)))
-				continue;
-
-			/* ignore some files */
-			if (   should_ignore_file (item, BAK_TAG)
-			    || should_ignore_file (item, TILDE_TAG)
-			    || should_ignore_file (item, ORIG_TAG)
-			    || should_ignore_file (item, REJ_TAG))
+			if (should_ignore_file (item))
 				continue;

 			full_path = g_build_filename (IFCFG_DIR, item, NULL);
@ -220,6 +265,108 @@ read_connections (SCPluginIfcfg *plugin)

 /* Monitoring */

+static void
+connection_changed_handler (SCPluginIfcfg *plugin,
+                            const char *path,
+                            NMIfcfgConnection *connection,
+                            gboolean *do_remove,
+                            gboolean *do_new)
+{
+	SCPluginIfcfgPrivate *priv = SC_PLUGIN_IFCFG_GET_PRIVATE (plugin);
+	NMIfcfgConnection *tmp;
+	GError *error = NULL;
+	GHashTable *settings;
+	gboolean new_unmanaged, old_unmanaged;
+
+	g_return_if_fail (plugin != NULL);
+	g_return_if_fail (path != NULL);
+	g_return_if_fail (connection != NULL);
+	g_return_if_fail (do_remove != NULL);
+	g_return_if_fail (do_new != NULL);
+
+	PLUGIN_PRINT (IFCFG_PLUGIN_NAME, "updating %s", path);
+
+	tmp = (NMIfcfgConnection *) nm_ifcfg_connection_new (path, priv->g_connection, priv->hal_mgr, &error);
+	if (!tmp) {
+		/* couldn't read connection; remove it */
+
+		PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    error: %s",
+		             error->message ? error->message : "(unknown)");
+		g_error_free (error);
+
+		PLUGIN_PRINT (IFCFG_PLUGIN_NAME, "removed %s.", path);
+		*do_remove = TRUE;
+		return;
+	}
+
+	/* Successfully read connection changes */
+
+	old_unmanaged = nm_ifcfg_connection_get_unmanaged (NM_IFCFG_CONNECTION (connection));
+	new_unmanaged = nm_ifcfg_connection_get_unmanaged (NM_IFCFG_CONNECTION (tmp));
+
+	if (new_unmanaged) {
+		if (!old_unmanaged) {
+			/* Unexport the connection by destroying it, then re-creating it as unmanaged */
+			*do_remove = *do_new = TRUE;
+		}
+	} else {
+		NMConnection *old_wrapped, *new_wrapped;
+
+		if (old_unmanaged)  /* no longer unmanaged */
+			g_signal_emit_by_name (plugin, "connection-added", connection);
+
+		new_wrapped = nm_exported_connection_get_connection (NM_EXPORTED_CONNECTION (tmp));
+		old_wrapped = nm_exported_connection_get_connection (NM_EXPORTED_CONNECTION (connection));
+
+		/* Only update if different */
+		if (!nm_connection_compare (new_wrapped, old_wrapped, COMPARE_FLAGS_EXACT)) {
+			settings = nm_connection_to_hash (new_wrapped);
+			nm_exported_connection_update (NM_EXPORTED_CONNECTION (connection), settings, NULL);
+			g_hash_table_destroy (settings);
+		}
+
+		/* Update unmanaged status */
+		g_object_set (connection, "unmanaged", new_unmanaged, NULL);
+		g_signal_emit_by_name (plugin, "unmanaged-devices-changed");
+	}
+	g_object_unref (tmp);
+}
+
+static void
+handle_connection_remove_or_new (SCPluginIfcfg *plugin,
+                                 const char *path,
+                                 NMIfcfgConnection *connection,
+                                 gboolean do_remove,
+                                 gboolean do_new)
+{
+	SCPluginIfcfgPrivate *priv = SC_PLUGIN_IFCFG_GET_PRIVATE (plugin);
+
+	g_return_if_fail (plugin != NULL);
+	g_return_if_fail (path != NULL);
+
+	if (do_remove) {
+		gboolean unmanaged;
+
+		g_return_if_fail (connection != NULL);
+
+		unmanaged = nm_ifcfg_connection_get_unmanaged (connection);
+		g_hash_table_remove (priv->connections, path);
+		nm_exported_connection_signal_removed (NM_EXPORTED_CONNECTION (connection));
+
+		/* Emit unmanaged changes _after_ removing the connection */
+		if (unmanaged)
+			g_signal_emit_by_name (plugin, "unmanaged-devices-changed");
+	}
+
+	if (do_new) {
+		connection = read_one_connection (plugin, path);
+		if (connection) {
+			if (!nm_ifcfg_connection_get_unmanaged (NM_IFCFG_CONNECTION (connection)))
+				g_signal_emit_by_name (plugin, "connection-added", connection);
+		}
+	}
+}
+
 static void
 dir_changed (GFileMonitor *monitor,
 		   GFile *file,
@ -234,95 +381,32 @@ dir_changed (GFileMonitor *monitor,
 	gboolean do_remove = FALSE, do_new = FALSE;

 	name = g_file_get_path (file);
-	connection = g_hash_table_lookup (priv->connections, name);
+	if (should_ignore_file (name)) {
+		g_free (name);
+		return;
+	}

-	switch (event_type) {
-	case G_FILE_MONITOR_EVENT_DELETED:
-		if (connection) {
+	connection = g_hash_table_lookup (priv->connections, name);
+	if (!connection) {
+		do_new = TRUE;
+	} else {
+		switch (event_type) {
+		case G_FILE_MONITOR_EVENT_DELETED:
 			PLUGIN_PRINT (IFCFG_PLUGIN_NAME, "removed %s.", name);
 			do_remove = TRUE;
-		}
-		break;
-	case G_FILE_MONITOR_EVENT_CREATED:
-	case G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT:
-		if (connection) {
+			break;
+		case G_FILE_MONITOR_EVENT_CREATED:
+		case G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT:
 			/* Update */
-			NMIfcfgConnection *tmp;
-			GError *error = NULL;
-
-			PLUGIN_PRINT (IFCFG_PLUGIN_NAME, "updating %s", name);
-
-			tmp = (NMIfcfgConnection *) nm_ifcfg_connection_new (name, priv->g_connection, priv->hal_mgr, &error);
-			if (tmp) {
-				GHashTable *settings;
-				gboolean new_unmanaged, old_unmanaged;
-
-				old_unmanaged = nm_ifcfg_connection_get_unmanaged (NM_IFCFG_CONNECTION (connection));
-				new_unmanaged = nm_ifcfg_connection_get_unmanaged (NM_IFCFG_CONNECTION (tmp));
-
-				if (new_unmanaged) {
-					if (!old_unmanaged) {
-						/* Unexport the connection by destroying it, then re-creating it as unmanaged */
-						do_remove = do_new = TRUE;
-					}
-				} else {
-					NMConnection *old_wrapped, *new_wrapped;
-
-					if (old_unmanaged)  /* no longer unmanaged */
-						g_signal_emit_by_name (plugin, "connection-added", connection);
-
-					new_wrapped = nm_exported_connection_get_connection (NM_EXPORTED_CONNECTION (tmp));
-					old_wrapped = nm_exported_connection_get_connection (NM_EXPORTED_CONNECTION (connection));
-
-					/* Only update if different */
-					if (!nm_connection_compare (new_wrapped, old_wrapped, COMPARE_FLAGS_EXACT)) {
-						settings = nm_connection_to_hash (new_wrapped);
-						nm_exported_connection_update (NM_EXPORTED_CONNECTION (connection), settings, NULL);
-						g_hash_table_destroy (settings);
-					}
-
-					/* Update unmanaged status */
-					g_object_set (connection, "unmanaged", new_unmanaged, NULL);
-					g_signal_emit_by_name (plugin, "unmanaged-devices-changed");
-				}
-				g_object_unref (tmp);
-			} else {
-				/* couldn't read connection; remove it */
-
-				PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    error: %s",
-				             error->message ? error->message : "(unknown)");
-				g_error_free (error);
-
-				PLUGIN_PRINT (IFCFG_PLUGIN_NAME, "removed %s.", name);
-				do_remove = TRUE;
-			}
-		} else {
-			do_new = TRUE;
-		}
-		break;
-	default:
-		break;
-	}
-
-	if (do_remove) {
-		gboolean unmanaged = nm_ifcfg_connection_get_unmanaged (connection);
-
-		g_hash_table_remove (priv->connections, name);
-		nm_exported_connection_signal_removed (NM_EXPORTED_CONNECTION (connection));
-
-		/* Emit unmanaged changes _after_ removing the connection */
-		if (unmanaged)
-			g_signal_emit_by_name (plugin, "unmanaged-devices-changed");
-	}
-
-	if (do_new) {
-		connection = read_one_connection (plugin, name);
-		if (connection) {
-			if (!nm_ifcfg_connection_get_unmanaged (NM_IFCFG_CONNECTION (connection)))
-				g_signal_emit_by_name (plugin, "connection-added", connection);
+			connection_changed_handler (plugin, name, connection, &do_remove, &do_new);
+			break;
+		default:
+			break;
 		}
 	}

+	handle_connection_remove_or_new (plugin, name, connection, do_remove, do_new);
+
 	g_free (name);
 }

--- a/system-settings/plugins/ifcfg-fedora/reader.c
+++ b/system-settings/plugins/ifcfg-fedora/reader.c
@ -379,13 +379,12 @@ out:
 	return success;
 }

-static shvarFile *
-get_keys_ifcfg (const char *parent)
+static char *
+get_keys_file_path (const char *parent)
 {
 	char *ifcfg_name;
 	char *keys_file = NULL;
 	char *tmp = NULL;
-	shvarFile *ifcfg = NULL;

 	ifcfg_name = get_ifcfg_name (parent);
 	if (!ifcfg_name)
@ -396,15 +395,25 @@ get_keys_ifcfg (const char *parent)
 		goto out;

 	keys_file = g_strdup_printf ("%s/" KEYS_TAG "%s", tmp, ifcfg_name);
-	if (!keys_file)
-		goto out;
-
-	ifcfg = svNewFile (keys_file);

 out:
-	g_free (keys_file);
 	g_free (tmp);
 	g_free (ifcfg_name);
+	return keys_file;
+}
+
+static shvarFile *
+get_keys_ifcfg (const char *parent)
+{
+	shvarFile *ifcfg = NULL;
+	char *keys_file;
+
+	keys_file = get_keys_file_path (parent);
+	if (!keys_file)
+		return NULL;
+
+	ifcfg = svNewFile (keys_file);
+	g_free (keys_file);
 	return ifcfg;
 }

@ -789,7 +798,10 @@ out:
 }

 NMConnection *
-connection_from_file (const char *filename, gboolean *ignored, GError **error)
+connection_from_file (const char *filename,
+                      gboolean *ignored,
+                      char **keyfile,
+                      GError **error)
 {
 	NMConnection *connection = NULL;
 	shvarFile *parsed;
@ -800,6 +812,8 @@ connection_from_file (const char *filename, gboolean *ignored, GError **error)

 	g_return_val_if_fail (filename != NULL, NULL);
 	g_return_val_if_fail (ignored != NULL, NULL);
+	g_return_val_if_fail (keyfile != NULL, NULL);
+	g_return_val_if_fail (*keyfile == NULL, NULL);

 	ifcfg_name = get_ifcfg_name (filename);
 	if (!ifcfg_name) {
@ -897,6 +911,8 @@ connection_from_file (const char *filename, gboolean *ignored, GError **error)
 		             "Connection was invalid");
 	}

+	*keyfile = get_keys_file_path (filename);
+
 done:
 	svCloseFile (parsed);
 	return connection;
--- a/system-settings/plugins/ifcfg-fedora/reader.h
+++ b/system-settings/plugins/ifcfg-fedora/reader.h
@ -24,6 +24,9 @@
 #include <glib.h>
 #include <nm-connection.h>

-NMConnection *connection_from_file (const char *filename, gboolean *ignored, GError **error);
+NMConnection *connection_from_file (const char *filename,
+                                    gboolean *ignored,
+                                    char **keyfile,
+                                    GError **error);

 #endif  /* __READER_H__ */