hal: Branch 'master'

David Zeuthen david at kemper.freedesktop.org
Tue Sep 12 15:12:10 PDT 2006


 hald/hald_dbus.c      |   39 ++++++++++++++++++++++-----
 hald/hald_dbus.h      |    2 -
 hald/linux/blockdev.c |   71 ++++++++++++++++----------------------------------
 hald/linux/osspec.c   |    5 +++
 hald/osspec.h         |    3 ++
 5 files changed, 64 insertions(+), 56 deletions(-)

New commits:
diff-tree f1d889a41a96ef01991a3b1dacda14c234998d4b (from 10a19455b5e3cdb930a51235df8c0c58d052ae96)
Author: David Zeuthen <davidz at redhat.com>
Date:   Tue Sep 12 18:11:29 2006 -0400

    don't update mount state until Unmount() returns
    
    This makes it a bit nicer if there is a lot of outstanding IO as e.g.
    on GNOME the desktop icon won't disappear while Unmount() is blocking
    due to umount(8) blocking.

diff --git a/hald/hald_dbus.c b/hald/hald_dbus.c
index 9a865d0..7a51bfb 100644
--- a/hald/hald_dbus.c
+++ b/hald/hald_dbus.c
@@ -2830,6 +2830,7 @@ typedef struct {
 	char **extra_env;
 	char *mstdin;
 	char *member;
+	char *interface;
 	DBusMessage *message;
 	DBusConnection *connection;
 } MethodInvocation;
@@ -2849,6 +2850,7 @@ hald_exec_method_free_mi (MethodInvocati
 	g_strfreev (mi->extra_env);
 	g_free (mi->mstdin);
 	g_free (mi->member);
+	g_free (mi->interface);
 	g_free (mi);
 }
 
@@ -2892,7 +2894,7 @@ static GHashTable *udi_to_method_queue =
 
 
 gboolean 
-device_is_executing_method (HalDevice *d, const char *method_name)
+device_is_executing_method (HalDevice *d, const char *interface_name, const char *method_name)
 {
 	gpointer origkey;
 	gboolean ret;
@@ -2905,7 +2907,8 @@ device_is_executing_method (HalDevice *d
 		if (queue != NULL) {
 			MethodInvocation *mi;
 			mi = (MethodInvocation *) queue->data;
-			if (strcmp (mi->member, method_name) == 0) {
+			if ((strcmp (mi->interface, interface_name) == 0) &&
+			    (strcmp (mi->member, method_name) == 0)) {
 				ret = TRUE;
 			}
 		}
@@ -2955,15 +2958,36 @@ hald_exec_method_process_queue (const ch
 		/* clean the top of the list */
 		if (queue != NULL) {
 			mi = (MethodInvocation *) queue->data;
-			hald_exec_method_free_mi (mi);
 			queue = g_list_delete_link (queue, queue);
+			if (queue == NULL) {
+				g_hash_table_remove (udi_to_method_queue, udi);
+				HAL_INFO (("No more methods in queue"));
+			}
+
+			/* if method was Volume.Unmount() then refresh mount state */
+			if (strcmp (mi->interface, "org.freedesktop.Hal.Device.Volume") == 0 &&
+			    strcmp (mi->member, "Unmount") == 0) {
+				HalDevice *d;
+
+				HAL_INFO (("Refreshing mount state for %s since Unmount() completed", mi->udi));
+
+				d = hal_device_store_find (hald_get_gdl (), mi->udi);
+				if (d == NULL) {
+					d = hal_device_store_find (hald_get_tdl (), mi->udi);
+				}
+
+				if (d != NULL) {
+					osspec_refresh_mount_state_for_block_device (d);
+				} else {
+					HAL_WARNING ((" Cannot find device object for %s", mi->udi));
+				}
+			}
+
+			hald_exec_method_free_mi (mi);
 		}
 
 		/* process the rest of the list */
-		if (queue == NULL) {
-			HAL_INFO (("No more methods in queue"));
-			g_hash_table_remove (udi_to_method_queue, udi);
-		} else {
+		if (queue != NULL) {
 			HAL_INFO (("Execing next method in queue"));
 			g_hash_table_replace (udi_to_method_queue, g_strdup (udi), queue);
 
@@ -3201,6 +3225,7 @@ hald_exec_method (HalDevice *d, DBusConn
 	mi->message = message;
 	mi->connection = connection;
 	mi->member = g_strdup (dbus_message_get_member (message));
+	mi->interface = g_strdup (dbus_message_get_interface (message));
 	hald_exec_method_enqueue (mi);
 
 	dbus_message_ref (message);
diff --git a/hald/hald_dbus.h b/hald/hald_dbus.h
index 5fe7307..d81cb47 100644
--- a/hald/hald_dbus.h
+++ b/hald/hald_dbus.h
@@ -95,6 +95,6 @@ DBusHandlerResult hald_dbus_filter_funct
 
 char *hald_dbus_local_server_addr (void);
 
-gboolean device_is_executing_method (HalDevice *d, const char *method_name);
+gboolean device_is_executing_method (HalDevice *d, const char *interface_name, const char *method_name);
 
 #endif /* HAL_DBUS_H */
diff --git a/hald/linux/blockdev.c b/hald/linux/blockdev.c
index a5196ff..d6b10c4 100644
--- a/hald/linux/blockdev.c
+++ b/hald/linux/blockdev.c
@@ -255,57 +255,33 @@ blockdev_refresh_mount_state (HalDevice 
 		}
 	}
 
+	g_slist_foreach (autofs_mounts, (GFunc) g_free, NULL);
+	g_slist_free (autofs_mounts);
+
 	/* all remaining volumes are not mounted */
 	for (volume = volumes; volume != NULL; volume = g_slist_next (volume)) {
 		HalDevice *dev;
-		char *mount_point;
-		GSList *autofs_node;
 
 		dev = HAL_DEVICE (volume->data);
-		mount_point = g_strdup (hal_device_property_get_string (dev, "volume.mount_point"));
-		device_property_atomic_update_begin ();
-		hal_device_property_set_bool (dev, "volume.is_mounted", FALSE);
-		hal_device_property_set_bool (dev, "volume.is_mounted_read_only", FALSE);
-		hal_device_property_set_string (dev, "volume.mount_point", "");
-		device_property_atomic_update_end ();
-		/*HAL_INFO (("set %s to unmounted", hal_device_get_udi (dev)));*/
-
-		/* check to see if mount point falls under autofs */
-		autofs_node = autofs_mounts;
-		while (autofs_node != NULL) {
-			char *am = (char *)autofs_node->data;
-
-			if (strncmp (am, mount_point, strlen (am)) == 0);
-				break;
-
-			autofs_node = autofs_node->next;
-		}
-
-		/* look up in /media/.hal-mtab to see if we mounted this one - unless there is a
-		 * Unmount() method on the device already being processed. Because, if there is,
-		 * the Unmount() method will hold the lock /media/.hal-mtab and then the function
-		 * hal_util_is_mounted_by_hald() will block until Unmount() returns. 
-		 *
-		 * And this is a problem because on Linux /proc/mounts is changed immediately, e.g.
-		 * we get to here... but umount(8) don't return until much later. This is normally
-		 * not a problem, it only surfaces under circumstances described in 
-		 * https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=194296, e.g. when there
-		 * is a lot of data waiting to be written.
-		 *
-		 * Actually, when we do the "is Unmount() executing on this device" check, there is
-		 * no need to lock the file to check to see if we're unmounted. Because either
-		 * a) we're executing Unmount => Unmount() unmounting this => Unmount() cleaning up
-		 * the /media mount point => hald needs to do nothing. Or b) we're not executing 
-		 * Unmount() on this device, so Unmount() already done this => Unmount() not 
-		 * currently modifying the .hal-mtab entry for this device => safe for hald to look 
-		 * at this entry.
-		 * 
-		 * This will also take care of nasty lock-ups in the event some long-running 
-		 * Unmount() method, executing on another device, is holding the lock. So since 
-		 * we do this check, no locking is necessary. Hence, we've now 
-		 * changed hal_util_is_mounted_by_hald() to not lock.
+		/* do nothing if we have a Unmount() method running on the object. This is
+		 * is because on Linux /proc/mounts is changed immediately while umount(8)
+		 * doesn't return until the block cache is flushed. Note that when Unmount()
+		 * terminates we'll be checking /proc/mounts again so this event is not
+		 * lost... it is merely delayed...
 		 */
-		if (!device_is_executing_method (dev, "Unmount")) {
+		if (device_is_executing_method (dev, "org.freedesktop.Hal.Device.Volume", "Unmount")) {
+			HAL_INFO (("/proc/mounts tells that %s is unmounted - waiting for Unmount() to complete to change mount state", dev->udi));
+		} else {
+			char *mount_point;
+
+			mount_point = g_strdup (hal_device_property_get_string (dev, "volume.mount_point"));
+			device_property_atomic_update_begin ();
+			hal_device_property_set_bool (dev, "volume.is_mounted", FALSE);
+			hal_device_property_set_bool (dev, "volume.is_mounted_read_only", FALSE);
+			hal_device_property_set_string (dev, "volume.mount_point", "");
+			device_property_atomic_update_end ();
+			/*HAL_INFO (("set %s to unmounted", hal_device_get_udi (dev)));*/
+			
 			if (mount_point != NULL && strlen (mount_point) > 0 && 
 			    hal_util_is_mounted_by_hald (mount_point)) {
 				char *cleanup_stdin;
@@ -325,13 +301,12 @@ blockdev_refresh_mount_state (HalDevice 
 							cleanup_mountpoint_cb,
 							g_strdup (mount_point), NULL);
 			}
+
+			g_free (mount_point);
 		}
 
-		g_free (mount_point);
 	}
 	g_slist_free (volumes);
-	g_slist_foreach (autofs_mounts, (GFunc) g_free, NULL);
-	g_slist_free (autofs_mounts);
 exit:
 	endmntent (f);
 }
diff --git a/hald/linux/osspec.c b/hald/linux/osspec.c
index 2d62f7a..ed762a8 100644
--- a/hald/linux/osspec.c
+++ b/hald/linux/osspec.c
@@ -639,3 +639,8 @@ hal_util_find_closest_ancestor (const gc
 	return parent;
 }
 
+void 
+osspec_refresh_mount_state_for_block_device (HalDevice *d)
+{
+	blockdev_refresh_mount_state (d);
+}
diff --git a/hald/osspec.h b/hald/osspec.h
index 83a4343..787a66c 100644
--- a/hald/osspec.h
+++ b/hald/osspec.h
@@ -45,6 +45,9 @@ gboolean osspec_device_rescan (HalDevice
 
 gboolean osspec_device_reprobe (HalDevice *d);
 
+/* Called to refresh mount state for a device object of capability volume */
+void osspec_refresh_mount_state_for_block_device (HalDevice *d);
+
 /** Called when the org.freedesktop.Hal service receives a messaged that the generic daemon 
  *  doesn't handle. Can be used for intercepting messages from kernel or core OS components.
  *


More information about the hal-commit mailing list