hal: Branch 'master'

David Zeuthen david at kemper.freedesktop.org
Thu Mar 1 00:45:19 PST 2007


 hald/linux/blockdev.c                  |   78 +++++++++++++++++++++------------
 hald/linux/osspec.c                    |    2 
 tools/hal-storage-cleanup-mountpoint.c |    5 ++
 3 files changed, 56 insertions(+), 29 deletions(-)

New commits:
diff-tree 193b7bbb91cb56d5fa5e87e90382449a023b6915 (from 565868fe6c360abea4f9b8f855f8c86bc02ff44c)
Author: David Zeuthen <davidz at redhat.com>
Date:   Thu Mar 1 03:45:01 2007 -0500

    fix detection of mounted volumes from yanked storage devices
    
    This should fix some problems when mounted devices are yanked out. At
    least that's the problem I had with my new 4GB SanDisk U3 Cruzer
    Micro. I have two partitions on it - one cleartext vfat partition and
    one LUKS partition. When yanking the stick, HAL nicely reported that
    the cleartext partition was unmounted even though it was still
    mounted. Which is a lie.
    
    There's a race we don't handle. In particular, the device node for a
    device, such as /dev/sdb1, may have been unlinked by udev but the
    device is still mounted and we haven't processed the uevent for that
    deletion from udev. So when we process entries from /proc/mounts we
    fail to stat the device node /dev/sdb1 and this leads us to assume the
    device is not mounted. That's wrong.
    
    So in this case fall back to comparing on device names rather than
    pretending the device is not mounted as that's what will happen if we
    just skip this /proc/mounts entry.
    
    The reason it's nicer to compare on major:minor, and why we do that in
    general, is that /proc/mounts is *broken by design* - it contains the
    *device name* passed to mount(2) which in some cases may be a
    symlink. In fact, on many distros it's common to see /proc/mounts
    contain /dev/root as the device for /.

diff --git a/hald/linux/blockdev.c b/hald/linux/blockdev.c
index 6c43ba2..b76baa7 100644
--- a/hald/linux/blockdev.c
+++ b/hald/linux/blockdev.c
@@ -189,6 +189,7 @@ blockdev_refresh_mount_state (HalDevice 
 	/* loop over /proc/mounts */
 	while ((mnte = getmntent_r (f, &mnt, buf, sizeof(buf))) != NULL) {
 		struct stat statbuf;
+		gboolean use_device_name_for_match;
 
 		/*HAL_INFO ((" * /proc/mounts contain dev %s - type %s", mnt.mnt_fsname, mnt.mnt_type));*/
 
@@ -200,27 +201,60 @@ blockdev_refresh_mount_state (HalDevice 
 		if (strcmp(mnt.mnt_type, "nfs") == 0)
 			continue;
 
+		use_device_name_for_match = FALSE;
 		/* get major:minor of special device file */
-		if (stat (mnt.mnt_fsname, &statbuf) != 0)
-			continue;
-
-		if (major (statbuf.st_rdev) == 0)
-			continue;
+		if (stat (mnt.mnt_fsname, &statbuf) != 0) {
+			/* DING DING DING... device node may have been deleted by udev 
+			 * but device is still mounted and we haven't processed the uevent
+			 * for that deletion from udev.. 
+			 * 
+			 * So in this case... fall back to comparing on device names 
+			 * rather than pretending the device is not mounted as that's
+			 * what will happen if we just skip this /proc/mounts entry.
+			 *
+			 * The reason it's nicer to compare on major:minor is that
+			 * /proc/mounts is broken - it contains the *device name*
+			 * passed to mount(2) which in some cases may be a symlink
+			 * (on many distros it's common to see /proc/mounts contain
+			 * /dev/root as the device for /). Sigh...
+			 */
+			use_device_name_for_match = TRUE;
+		} else {
+			if (major (statbuf.st_rdev) == 0)
+				continue;
+		}
 
 		/*HAL_INFO (("* found mounts dev %s (%i:%i)", mnt.mnt_fsname, major (statbuf.st_rdev), minor (statbuf.st_rdev)));*/
 		/* match against all hal volumes */
 		for (volume = volumes; volume != NULL; volume = g_slist_next (volume)) {
 			HalDevice *dev;
+			gboolean is_match;
 
+			is_match = FALSE;
 			dev = HAL_DEVICE (volume->data);
-			major = hal_device_property_get_int (dev, "block.major");
-			if (major == 0)
-				continue;
-			minor = hal_device_property_get_int (dev, "block.minor");
-			devt = makedev (major, minor);
-			/*HAL_INFO (("  match %s (%i:%i)", hal_device_get_udi (dev), major, minor));*/
 
-			if (statbuf.st_rdev == devt) {
+			if (use_device_name_for_match) {
+				const char *device_name;
+
+				device_name = hal_device_property_get_string (dev, "block.device");
+				if (device_name == NULL)
+					continue;
+
+				if (strcmp (device_name, mnt.mnt_fsname) == 0)
+					is_match = TRUE;
+			} else {
+				major = hal_device_property_get_int (dev, "block.major");
+				if (major == 0)
+					continue;
+				minor = hal_device_property_get_int (dev, "block.minor");
+				devt = makedev (major, minor);
+				/*HAL_INFO (("  match %s (%i:%i)", hal_device_get_udi (dev), major, minor));*/
+
+				if (statbuf.st_rdev == devt)
+					is_match = TRUE;
+			}
+
+			if (is_match) {
 				/* found entry for this device in /proc/mounts */
 				device_property_atomic_update_begin ();
 				hal_device_property_set_bool (dev, "volume.is_mounted", TRUE);
@@ -229,7 +263,7 @@ blockdev_refresh_mount_state (HalDevice 
 				hal_device_property_set_string (dev, "volume.mount_point", mnt.mnt_dir);
 				device_property_atomic_update_end ();
 				/*HAL_INFO (("  set %s to be mounted at %s (%s)",
-					   hal_device_get_udi (dev), mnt.mnt_dir,
+				  hal_device_get_udi (dev), mnt.mnt_dir,
 					   hasmntopt (&mnt, MNTOPT_RO) ? "ro" : "rw"));*/
 				volumes = g_slist_delete_link (volumes, volume);
 				break;
@@ -639,23 +673,8 @@ hotplug_event_begin_add_blockdev (const 
 	if (parent == NULL && !is_partition && !is_fakevolume) {
 		DIR * dir;
 		struct dirent *dp;
-		
 		char path[HAL_PATH_MAX];
 
-
-		/* sleep one second since device mapper needs additional
-		 * time before the device file is ready
-		 *
-		 * this is a hack and will only affect device mapper block
-		 * devices. It can go away once the kernel emits a "changed"
-		 * event for the device file (this is about to go upstream)
-		 * and we can depend on a released kernel with this feature.
-		 */
-		if (strncmp (hal_util_get_last_element (sysfs_path), "dm-", 3) == 0) {
-			HAL_INFO (("Waiting 1000ms to wait for device mapper to be ready", path));
-			usleep (1000 * 1000);
-		}
-
 		g_snprintf (path, HAL_PATH_MAX, "%s/slaves", sysfs_path);
 		HAL_INFO (("Looking in %s", path));
 
@@ -1171,6 +1190,8 @@ force_unmount (HalDevice *d, void *end_t
 	device_file = hal_device_property_get_string (d, "block.device");
 	mount_point = hal_device_property_get_string (d, "volume.mount_point");
 
+	HAL_INFO (("in force_unmount for %s %s", device_file, mount_point));
+
 	/* 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 *unmount_stdin;
@@ -1259,6 +1280,7 @@ hotplug_event_begin_remove_blockdev (con
 		if (hal_device_property_get_bool (d, "volume.is_mounted")) {
 			force_unmount (d, end_token);
 		} else {
+			HAL_INFO (("device at sysfs_path %s is not mounted", sysfs_path));
 			hal_util_callout_device_remove (d, blockdev_callouts_remove_done, end_token, NULL);
 		}
 	}
diff --git a/hald/linux/osspec.c b/hald/linux/osspec.c
index 9da684d..5c2bed4 100644
--- a/hald/linux/osspec.c
+++ b/hald/linux/osspec.c
@@ -226,7 +226,7 @@ hald_udev_data (GIOChannel *source, GIOC
 		   hotplug_event->sysfs.seqnum, action, hotplug_event->sysfs.subsystem, hotplug_event->sysfs.sysfs_path,
 		   hotplug_event->sysfs.device_file, hotplug_event->sysfs.net_ifindex));
 
-	if (strcmp (action, "add") == 0) {
+	if (strcmp (action, "add") == 0 || strcmp (action, "change") == 0) {
 		hotplug_event->action = HOTPLUG_ACTION_ADD;
 		hotplug_event_enqueue (hotplug_event);
 		hotplug_event_process_queue ();
diff --git a/tools/hal-storage-cleanup-mountpoint.c b/tools/hal-storage-cleanup-mountpoint.c
index f4973a8..4c3b317 100644
--- a/tools/hal-storage-cleanup-mountpoint.c
+++ b/tools/hal-storage-cleanup-mountpoint.c
@@ -154,18 +154,23 @@ do_cleanup (const char *mount_point)
 
 	g_strfreev (lines);
 
+	printf ("removing directory", mount_point);
+
 	/* remove directory */
 	if (g_rmdir (mount_point) != 0) {
 		unlink ("/media/.hal-mtab~");
 		unknown_error ("Cannot remove directory");
 	}
 
+	printf ("atomically creating new /media/.hal-mtab file");
+
 	/* set new .hal-mtab file */
 	if (rename ("/media/.hal-mtab~", "/media/.hal-mtab") != 0) {
 		unlink ("/media/.hal-mtab~");
 		unknown_error ("Cannot rename /media/.hal-mtab~ to /media/.hal-mtab");
 	}
 
+	printf ("hal-storage-cleanup-mountpoint done for %s", mount_point);
 }
 
 int


More information about the hal-commit mailing list