hal: Branch 'origin' - 8 commits

David Zeuthen david at kemper.freedesktop.org
Wed Sep 13 15:42:28 PDT 2006


 NEWS                      |   19 ++++++++++
 configure.in              |    2 -
 hald/hald_dbus.c          |   81 +++++++++++++++++++++++++++++++++++--------
 hald/hald_dbus.h          |    2 +
 hald/linux/blockdev.c     |   85 +++++++++++++++++++++++-----------------------
 hald/linux/classdev.c     |    3 +
 hald/linux/osspec.c       |    5 ++
 hald/osspec.h             |    3 +
 hald/util.c               |   27 --------------
 privileges/Makefile.am    |    2 +
 tools/hal-storage-mount.c |    4 +-
 11 files changed, 145 insertions(+), 88 deletions(-)

New commits:
diff-tree 025d3781e2c8fd0133934fd4c94e55d0178940c7 (from 11dbbc69c4ebc31e1113fe617f349315df5bfa1d)
Author: S.Çağlar Onur <caglar at pardus.org.tr>
Date:   Wed Sep 13 17:25:31 2006 -0400

    get ARPHRD_IEEE80211_RADIOTAP and similar from kernel, not glibc headers

diff --git a/hald/linux/classdev.c b/hald/linux/classdev.c
index 4e6d2c2..e29a61b 100644
--- a/hald/linux/classdev.c
+++ b/hald/linux/classdev.c
@@ -31,8 +31,9 @@
 
 #include <ctype.h>
 #include <limits.h>
+#include <sys/socket.h>   /* for ifru_* has incomplete type */
 #include <linux/types.h>
-#include <net/if_arp.h> /* for ARPHRD_... */
+#include <linux/if_arp.h> /* for ARPHRD_... */
 #include <stdlib.h>
 #include <string.h>
 #include <stdint.h>
diff-tree 11dbbc69c4ebc31e1113fe617f349315df5bfa1d (from 4efec60712d17f2fcf27cc1bc33b80072e9c3778)
Author: David Zeuthen <davidz at redhat.com>
Date:   Wed Sep 13 14:18:21 2006 -0400

    write new requirements for 0.5.9 including requiring udev >= 089

diff --git a/NEWS b/NEWS
index 9622500..062928e 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,23 @@
 ==========
+HAL 0.5.9 ""
+==========
+
+Requirements for HAL 0.5.9 ""
+
+ - Linux kernel     >= 2.6.17
+ - util-linux       >= 2.12r1
+ - bash             >= 2.0
+ - udev             >= 089
+ - dbus             >= 0.60      (with glib bindings)
+ - glib             >= 2.6.0
+ - expat            >= 1.95.8
+ - libusb           >= 0 0.1.10a (optional)
+ - pciutils         >= 2.2.3     (optional
+ - dmidecode        >= 2.7       (optional)
+ - parted           == 1.7.1     (optional)
+ - cryptsetup-luks  >= 1.0.1     (optional, needs LUKS patches)
+
+==========
 HAL 0.5.8 "The Skynet Funding Bill is passed."
 ==========
 
diff-tree 4efec60712d17f2fcf27cc1bc33b80072e9c3778 (from 76c241fdd5ffb05ff40beee1207d1c74f076b99c)
Author: David Zeuthen <davidz at redhat.com>
Date:   Wed Sep 13 14:16:39 2006 -0400

    only require libvolume_id >= 061
    
    This is to avoid requiring a too new udev for people on old distributions.
    Note that libvolume_id >= 061 is provided only by udev >= 089.

diff --git a/configure.in b/configure.in
index 2b31e91..a2688f3 100644
--- a/configure.in
+++ b/configure.in
@@ -14,7 +14,7 @@ AM_MAINTAINER_MODE
 
 glib_module="glib-2.0 >= 2.6.0 dbus-glib-1 >= 0.61"
 dbus_module="dbus-1 >= 0.61"
-volume_id_module="libvolume_id >= 0.66"
+volume_id_module="libvolume_id >= 0.61"
 polkit_module="polkit >= 0.2"
 
 # libtool versioning - this applies to libhal and libhal-storage
diff-tree 76c241fdd5ffb05ff40beee1207d1c74f076b99c (from ffcbc8ac4a06fed7522ee4c2060fb9392d56a6dd)
Author: David Zeuthen <davidz at redhat.com>
Date:   Wed Sep 13 14:14:32 2006 -0400

    only install PolicyKit privilege files if we're using PolicyKit

diff --git a/privileges/Makefile.am b/privileges/Makefile.am
index d300a60..805dc67 100644
--- a/privileges/Makefile.am
+++ b/privileges/Makefile.am
@@ -1,4 +1,5 @@
 
+if HAVE_POLKIT
 polkit_privilegedir = $(sysconfdir)/PolicyKit/privilege.d
 
 dist_polkit_privilege_DATA =                              \
@@ -11,6 +12,7 @@ dist_polkit_privilege_DATA =            
 	hal-power-poweroff.privilege                      \
 	hal-power-reboot.privilege                        \
 	hal-power-cpufreq.privilege
+endif
 
 clean-local :
 	rm -f *~
diff-tree ffcbc8ac4a06fed7522ee4c2060fb9392d56a6dd (from f1d889a41a96ef01991a3b1dacda14c234998d4b)
Author: David Zeuthen <davidz at redhat.com>
Date:   Wed Sep 13 12:38:50 2006 -0400

    fix stupid typo in syslog reporting for Mount()

diff --git a/tools/hal-storage-mount.c b/tools/hal-storage-mount.c
index e1573b2..ef8d1a9 100644
--- a/tools/hal-storage-mount.c
+++ b/tools/hal-storage-mount.c
@@ -963,9 +963,9 @@ handle_mount (LibHalContext *hal_ctx, 
 
 	openlog ("hald", 0, LOG_DAEMON);
 	if (is_remount) {
-		syslog (LOG_INFO, "mounted %s at '%s' on behalf of uid %s", device, mount_dir, invoked_by_uid);
+		syslog (LOG_INFO, "remounted %s at '%s' on behalf of uid %s", device, mount_dir, invoked_by_uid);
 	} else {
-		syslog (LOG_INFO, "remounted %s on behalf of uid %s", device, invoked_by_uid);
+		syslog (LOG_INFO, "mounted %s on behalf of uid %s", device, invoked_by_uid);
 	}
 	closelog ();
 
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.
  *
diff-tree 10a19455b5e3cdb930a51235df8c0c58d052ae96 (from e35d886bf811baa0c15e9d71a6296f37ef403dde)
Author: David Zeuthen <davidz at redhat.com>
Date:   Tue Sep 12 03:55:55 2006 -0400

    don't lock the .hal-mtab file at all when checking mount status on unmount

diff --git a/hald/linux/blockdev.c b/hald/linux/blockdev.c
index cc21ddf..a5196ff 100644
--- a/hald/linux/blockdev.c
+++ b/hald/linux/blockdev.c
@@ -287,10 +287,23 @@ blockdev_refresh_mount_state (HalDevice 
 		 * 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
+		 * 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.
 		 */
 		if (!device_is_executing_method (dev, "Unmount")) {
 			if (mount_point != NULL && strlen (mount_point) > 0 && 
diff --git a/hald/util.c b/hald/util.c
index e2cf5aa..225142b 100644
--- a/hald/util.c
+++ b/hald/util.c
@@ -986,36 +986,11 @@ hal_util_is_mounted_by_hald (const char 
 	char *hal_mtab_buf;
 	char **lines;
 	gboolean found;
-	int lock_mtab_fd;
 
 	hal_mtab = NULL;
 	hal_mtab_buf = NULL;
 	found = FALSE;
 
-	/* take the lock on /media/.hal-mtab-lock so we don't race with the Mount() and Unmount() methods */
-
-	/* do not attempt to create the file; tools/hal-storage-shared.c will create it and
-	 * set the correct ownership so this unprivileged process (running as haldaemon) can
-	 * lock it too
-	 */
-	lock_mtab_fd = open ("/media/.hal-mtab-lock", 0);
-	if (lock_mtab_fd < 0) {
-		HAL_INFO (("Cannot open /media/.hal-mtab for locking"));
-		goto out;
-	}
-
-tryagain:
-#ifdef sun
-	if (lockf (lock_mtab_fd, F_LOCK, 0) != 0) {
-#else
-	if (flock (lock_mtab_fd, LOCK_EX) != 0) {
-#endif
-		if (errno == EINTR)
-			goto tryagain;
-		HAL_ERROR (("Cannot obtain lock on /media/.hal-mtab"));
-		goto out;
-	}
-
 	/*HAL_DEBUG (("examining /media/.hal-mtab for %s", mount_point));*/
 
 	hal_mtab = fopen ("/media/.hal-mtab", "r");
@@ -1081,8 +1056,6 @@ tryagain:
 	}
 
 out:
-	if (lock_mtab_fd >= 0)
-		close (lock_mtab_fd);
 	if (hal_mtab != NULL)
 		fclose (hal_mtab);
 	if (hal_mtab_buf != NULL)
diff-tree e35d886bf811baa0c15e9d71a6296f37ef403dde (from 5cd39bf7de4d3b6b4d317e7ea6d49fbac041aaa6)
Author: David Zeuthen <davidz at redhat.com>
Date:   Tue Sep 12 03:30:44 2006 -0400

    fix race condition when unmounting a volume takes a very long time
    
    Whilst trying to fix this bug
    
     https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=194296
    
    I discovered that there's a race condition when unmounting a volume
    that has lots of outstanding writes. Under Linux, at least, this
    results in umount(8) taking very long to complete. However, the
    kernel tells us, just after the umount syscall I believe, that
    /proc/mounts has changed. So we check that file to see if it was
    us, HAL, that unmounted the volume. Because if it wasn't, we want
    to clean up the mount point if we created it.
    
    To do this, we first lock /media/.hal-mtab to ensure noone else is
    modifying it. But this will block until Unmount() terminates (because
    Unmount() is holding the lock on that file) and that can take many
    many seconds, e.g. 30 secs for example, if you got a bunch of
    outstanding writes.
    
    The fix here is to check whether we're already executing Unmount()
    on the device in question. If we are, then, hey, we can infer
    that HAL is unmounting that volume, not someone else.

diff --git a/hald/hald_dbus.c b/hald/hald_dbus.c
index b45cf29..9a865d0 100644
--- a/hald/hald_dbus.c
+++ b/hald/hald_dbus.c
@@ -2829,6 +2829,7 @@ typedef struct {
 	char *execpath;
 	char **extra_env;
 	char *mstdin;
+	char *member;
 	DBusMessage *message;
 	DBusConnection *connection;
 } MethodInvocation;
@@ -2842,10 +2843,12 @@ hald_exec_method_cb (HalDevice *d, guint
 static void
 hald_exec_method_free_mi (MethodInvocation *mi)
 {
+	/* hald_runner_run_method() assumes ownership of mi->message.. so we don't free it here */
 	g_free (mi->udi);
 	g_free (mi->execpath);
 	g_strfreev (mi->extra_env);
 	g_free (mi->mstdin);
+	g_free (mi->member);
 	g_free (mi);
 }
 
@@ -2887,6 +2890,31 @@ hald_exec_method_do_invocation (MethodIn
 
 static GHashTable *udi_to_method_queue = NULL;
 
+
+gboolean 
+device_is_executing_method (HalDevice *d, const char *method_name)
+{
+	gpointer origkey;
+	gboolean ret;
+	GList *queue;
+
+	ret = FALSE;
+
+	if (g_hash_table_lookup_extended (udi_to_method_queue, d->udi, &origkey, (gpointer) &queue)) {
+
+		if (queue != NULL) {
+			MethodInvocation *mi;
+			mi = (MethodInvocation *) queue->data;
+			if (strcmp (mi->member, method_name) == 0) {
+				ret = TRUE;
+			}
+		}
+
+		ret = TRUE;
+	}
+	return ret;
+}
+
 static void 
 hald_exec_method_process_queue (const char *udi);
 
@@ -2911,7 +2939,6 @@ hald_exec_method_enqueue (MethodInvocati
 		g_hash_table_insert (udi_to_method_queue, g_strdup (mi->udi), queue);
 
 		hald_exec_method_do_invocation (mi);
-		hald_exec_method_free_mi (mi);
 	}
 }
 
@@ -2921,30 +2948,30 @@ hald_exec_method_process_queue (const ch
 {
 	gpointer origkey;
 	GList *queue;
+	MethodInvocation *mi;
 
 	if (g_hash_table_lookup_extended (udi_to_method_queue, udi, &origkey, (gpointer) &queue)) {
+
+		/* 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);
 		}
 
+		/* 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 {
-			MethodInvocation *mi;
-
 			HAL_INFO (("Execing next method in queue"));
 			g_hash_table_replace (udi_to_method_queue, g_strdup (udi), queue);
 
 			mi = (MethodInvocation *) queue->data;
-
 			if (!hald_exec_method_do_invocation (mi)) {
 				/* the device went away before we got to it... */
 				hald_exec_method_process_queue (mi->udi);
 			}
-
-			hald_exec_method_free_mi (mi);
-
 		}
 	}
 }
@@ -3173,6 +3200,7 @@ hald_exec_method (HalDevice *d, DBusConn
 	mi->mstdin = g_strdup (stdin_str->str);
 	mi->message = message;
 	mi->connection = connection;
+	mi->member = g_strdup (dbus_message_get_member (message));
 	hald_exec_method_enqueue (mi);
 
 	dbus_message_ref (message);
@@ -3566,14 +3594,12 @@ static DBusHandlerResult
 hald_dbus_filter_handle_methods (DBusConnection *connection, DBusMessage *message, 
 				 void *user_data, dbus_bool_t local_interface)
 {
-	/*
-	  HAL_INFO (("connection=0x%x obj_path=%s interface=%s method=%s local_interface=%d", 
+	/*HAL_INFO (("connection=0x%x obj_path=%s interface=%s method=%s local_interface=%d", 
 		   connection,
 		   dbus_message_get_path (message), 
 		   dbus_message_get_interface (message),
 		   dbus_message_get_member (message),
-		   local_interface));
-	*/
+		   local_interface));*/
 
 	if (dbus_message_is_method_call (message,
 					 "org.freedesktop.Hal.Manager",
diff --git a/hald/hald_dbus.h b/hald/hald_dbus.h
index 96fc987..5fe7307 100644
--- a/hald/hald_dbus.h
+++ b/hald/hald_dbus.h
@@ -95,4 +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);
+
 #endif /* HAL_DBUS_H */
diff --git a/hald/linux/blockdev.c b/hald/linux/blockdev.c
index 3061424..cc21ddf 100644
--- a/hald/linux/blockdev.c
+++ b/hald/linux/blockdev.c
@@ -281,24 +281,37 @@ blockdev_refresh_mount_state (HalDevice 
 			autofs_node = autofs_node->next;
 		}
 
-		/* look up in /media/.hal-mtab to see if we mounted this one */
-		if (mount_point != NULL && strlen (mount_point) > 0 && hal_util_is_mounted_by_hald (mount_point)) {
-			char *cleanup_stdin;
-			char *extra_env[2];
-
-			HAL_INFO (("Cleaning up directory '%s' since it was created by HAL's Mount()", mount_point));
-
-			extra_env[0] = g_strdup_printf ("HALD_CLEANUP=%s", mount_point);
-			extra_env[1] = NULL;
-			cleanup_stdin = "\n";
-
-			hald_runner_run_method (dev, 
-						"hal-storage-cleanup-mountpoint", 
-						extra_env, 
-						cleanup_stdin, TRUE,
-						0,
-						cleanup_mountpoint_cb,
-						g_strdup (mount_point), NULL);
+		/* 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.
+		 */
+		if (!device_is_executing_method (dev, "Unmount")) {
+			if (mount_point != NULL && strlen (mount_point) > 0 && 
+			    hal_util_is_mounted_by_hald (mount_point)) {
+				char *cleanup_stdin;
+				char *extra_env[2];
+				
+				HAL_INFO (("Cleaning up directory '%s' since it was created by HAL's Mount()", mount_point));
+				
+				extra_env[0] = g_strdup_printf ("HALD_CLEANUP=%s", mount_point);
+				extra_env[1] = NULL;
+				cleanup_stdin = "\n";
+				
+				hald_runner_run_method (dev, 
+							"hal-storage-cleanup-mountpoint", 
+							extra_env, 
+							cleanup_stdin, TRUE,
+							0,
+							cleanup_mountpoint_cb,
+							g_strdup (mount_point), NULL);
+			}
 		}
 
 		g_free (mount_point);


More information about the hal-commit mailing list