Re: Bug: NetworkManagerDispatcher, device removal events

[Alan Jenkins <alan-jenkins tuffmail co uk>]

Dan Williams wrote:
> On Thu, 2008-04-10 at 08:16 +0100, Alan Jenkins wrote:
>   
> > NetworkManagerDispatcher dispatches dbus signals to more traditional
> > if-up.d style scripts.  Unfortunately it doesn't work when the device is
> > removed.  It needs to look up the real interface name e.g. eth0, by
> > calling a method on the dbus device object but the object has already
> > been deleted by the time NMDispatcher processes the signal.
> >     
>
> Yep; that's fixed in head, I guess it just needs to be backported to the
> 0.6.x branch.
>
> Dan
>   
Great.  I guess I should have checked - at least before writing my own 
fix :-).  I've attached it on the offchance it proves useful.

I checked out head now, though I didn't have the dependancies 
(lib-nl-1.0pre8) to build it and play with it.  It looked like the 
dispatcher had changed quite a bit - in particular, it uses a new 
"nm-client" library.  Is a backport really feasible?  If I could build 
it, would the dispatcher from svn head be compatible with NM 0.6.x?

Thanks,

Alan

Index: network-manager-0.6.5/dispatcher-daemon/NetworkManagerDispatcher.c
===================================================================
--- network-manager-0.6.5.orig/dispatcher-daemon/NetworkManagerDispatcher.c	2008-04-10 14:08:31.000000000 +0100
+++ network-manager-0.6.5/dispatcher-daemon/NetworkManagerDispatcher.c	2008-04-10 15:15:47.000000000 +0100
@@ -54,6 +54,14 @@
 
 #define NMD_DEFAULT_PID_FILE	LOCALSTATEDIR"/run/NetworkManagerDispatcher.pid"
 
+/*
+ * Saved device path -> interface name mappings.
+ * (char* -> char*)
+ *
+ * Used in case the network device is removed before we can query the interface name.
+ */
+static GHashTable *dev_name_table = NULL;
+
 static DBusConnection *nmd_dbus_init (void);
 
 /*
@@ -182,6 +190,40 @@
 }
 
 /*
+ * nmd_dispatch
+ *
+ * Determine the device interface name and call execute_scripts
+ */
+static void nmd_dispatch(DBusConnection *connection, char *dev_object_path, NMDAction action)
+{
+	char *	dev_iface_name;
+
+	dev_iface_name = g_hash_table_lookup (dev_name_table, dev_object_path);
+	
+	if (!dev_iface_name) {
+		dev_iface_name = nmd_get_device_name (connection, dev_object_path);
+		
+		if (!dev_iface_name)
+			return;
+
+		g_hash_table_insert (dev_name_table, g_strdup (dev_object_path), dev_iface_name);
+	}
+			
+	nm_info ("Device %s (%s) is now %s.", dev_object_path, dev_iface_name,
+			(action == NMD_DEVICE_NOW_INACTIVE ? "going down" :
+			(action == NMD_DEVICE_NOW_ACTIVE ? "up" :
+			(action == NMD_DEVICE_GOING_INACTIVE ? "going down" :
+			(action == NMD_DEVICE_GOING_ACTIVE ? "going up" : "error")))));
+
+	nmd_execute_scripts (action, dev_iface_name);
+
+	if (action == NMD_DEVICE_NOW_INACTIVE) {
+		g_hash_table_remove (dev_name_table, dev_object_path);
+	}
+}
+
+
+/*
  * nmd reinit_dbus
  *
  * Reconnect to the system message bus if the connection was dropped.
@@ -235,26 +277,11 @@
 	{
 		if (dbus_message_get_args (message, &error, DBUS_TYPE_OBJECT_PATH, &dev_object_path, DBUS_TYPE_INVALID))
 		{
-			char *	dev_iface_name = NULL;
-
 			dev_object_path = nm_dbus_unescape_object_path (dev_object_path);
 			if (dev_object_path)
-				dev_iface_name = nmd_get_device_name (connection, dev_object_path);
-
-			if (dev_object_path && dev_iface_name)
-			{
-				nm_info ("Device %s (%s) is now %s.", dev_object_path, dev_iface_name,
-						(action == NMD_DEVICE_NOW_INACTIVE ? "going down" :
-						(action == NMD_DEVICE_NOW_ACTIVE ? "up" :
-						(action == NMD_DEVICE_GOING_INACTIVE ? "going down" :
-						(action == NMD_DEVICE_GOING_ACTIVE ? "going up" : "error")))));
-
-				nmd_execute_scripts (action, dev_iface_name);
-			}
+				nmd_dispatch(connection, dev_object_path, action);
 
 			g_free (dev_object_path);
-			g_free (dev_iface_name);
-
 			handled = TRUE;
 		}
 	}
@@ -414,6 +441,8 @@
 	if (!g_thread_supported ())
 		g_thread_init (NULL);
 
+	dev_name_table = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+
 	/* Connect to the NetworkManager dbus service and run the main loop */
 	if ((connection = nmd_dbus_init ()))
 	{