Re: [Utopia] Re: [patch] improved hal support for gnome-vfs

[David Zeuthen <david fubar dk>]

Hi,

On Thu, 2005-06-30 at 11:16 +0200, Alexander Larsson wrote:
> On Wed, 2005-06-29 at 12:00 -0400, David Zeuthen wrote:
> 
> > There's a few things left to do but the patch is mostly done.
> > Specifically, one of the things I want to do is test with with a wider
> > range of hardware, specifically legacy floppy drives and IDE zip drives.
> > I'm submitting the patch now as it got a number of strings for i18n and
> > there's a deadline on July 1st.
> 
> I'm commiting this to get all the new strings in. There are some issues
> with the patch though. I didn't review all the code in the smallest
> detail (to much code!), but looking at the structure I have a few
> comments:

Thanks.

> * Does this work with any version of libhal, or should we update the
> libhal version requirement?

Should work with hal >= 0.5.0

> +static char *
> +_hal_size_as_string (gint64 size)
> +{
> ....
> +	char* sizes_str[] = {"K", "M", "G", "T", NULL};
> +	gint64 cur = 1000L;
> +	gint64 base = 10L;
> +	gint64 step = 10L*10L*10L;
> +	int cur_str = 0;
> 
> Gnome in general, and gnome-vfs in particular uses 1024 for disk sizes.
> Actually, this code should probably just use the function:
> gchar* gnome_vfs_format_file_size_for_display (GnomeVFSFileSize size)

I've changed this. The motivation for using the SI units were that it
fits much better with typical media people uses. For instance, with the
change it sortof looks like this

 http://freedesktop.org/~david/gnome-vfs-hacking-2.png

which is not too nice IMHO. The old code we display "64M" for the MMC
card and "129M" for the Memory Stick. I actually thought about going
even further: rounding values close to powers of two to powers of two so
the Memory Stick would also show 128M (e.g. map [126;130] to 128 and so
on).

I don't really know what the right thing to do is; Bryan suggested we
should show what the media says on the label but I see your point about
consistency. Dunno.

> * _hal_drive_policy_check calls gconf calls directly. This can be a
> problem in the daemon, because gconf calls are Corba calls. In this
> particular instance this might be a problem because this code can be
> reached by _gnome_vfs_hal_mounts_force_reprobe, which can be called in a
> corba method handler. This can lead to strange reentrancy problem.
> 

Yikes, I didn't think of that. Thanks for pointing it out.

> To solve this you need to make sure to always get the gconf value from a
> well known non-reentrant place in the mainloop. I'd recommend reading
> all the gconf values to static variables in a function, then calling
> this function in _init and _hal_settings_changed.

I've changed this.

> +	g_debug ("entering _hal_add_drive_without_volumes for\n  drive udi '%s'\n",
> +		 libhal_drive_get_udi (hal_drive));
> 
> I'm not sure we want all these debug calls compiled into the code.
> Whenever you enable debug logging you'll get them all, which might mess
> up things for an app developer using g_debug. You need to either delete
> these or wrap them in some #ifdef action.

Right, I've commented them out for now. I did talk to mclasen about this
and he told me to do something clever but I forget.

> + #define HAL_MOUNTS_INVALID_URI "/gnome/vfs2/internal/no/uri/available"
> 
> This is sort of ugly. Doesn't an empty uri work? maybe not...

I've removed this because it's not necessary anymore. The reason for
this is that the patch I sent actually changes gnome-vfs-volume-ops.c to
always use the device_file if hal_udi != NULL, e.g. the drive/volume
stems from the hal backend. This is sane as we guarantee that there are
no two GnomeVFSVolume objects with the same device file.

> 
> +#if defined(USE_HAL) && defined(HAL_MOUNT) && defined(HAL_UMOUNT)
> +				  /* do pass our environment when using hal mount progams */
> +				  ((strcmp (info->argv[0], HAL_MOUNT) == 0) ||
> +				   (strcmp (info->argv[0], HAL_UMOUNT) == 0)) ? NULL : envp,
> +#else
> 
> Not using envp means we're not adding "LC_ALL=C" to the environment. Is
> this really right? It means we'll get translated error message from the
> app, which we don't understand. Isn't this a problem?

It's intentional. If we build with hal-specific mount/umount/eject
programs these will display the error messages using their own dialogs
(and return nothing on stderr). And we do want to pass $LANG,
$GNOME_KEYRING_SOCKET and other goodies in the session environment to
these.

> +	/* if we had a drive from here but where we weren't mounted, just use that drive and update
> +	 * activation_uri as we now have a definite mount point...
> +	 */
> +	drive = _gnome_vfs_volume_monitor_find_drive_by_hal_udi (volume_monitor, libhal_volume_get_udi (hal_volume));
> +	if (drive != NULL) {
> +		if (libhal_volume_disc_has_audio (hal_volume)) {
> +			drive->priv->activation_uri = g_strdup_printf ("cdda://%s", 
> +								       libhal_volume_get_device_file (hal_volume));
> +		} else if (libhal_volume_disc_is_blank (hal_volume)) {
> +			drive->priv->activation_uri = g_strdup ("burn:///");
> +		} else if (libhal_volume_is_mounted (hal_volume)) {
> +			drive->priv->activation_uri = gnome_vfs_get_uri_from_local_path (
> +				libhal_volume_get_mount_point (hal_volume));
> +		} else {
> +			/* This sucks but it doesn't make sense to talk about the activation_uri if we're not mounted!
> +			 * So fake it...
> +			 */
> +			drive->priv->activation_uri = gnome_vfs_get_uri_from_local_path (HAL_MOUNTS_INVALID_URI); 
> +		}
> +
> 
> This doesn't work. First of all, drives and volumes, once initialized
> are generally read-only (except the volume-drive relationships). This is
> partly a threading issue, as all thread protection on these datatypes is
> based on them being read-only. It also lacks any code to push such
> changes from the daemon to the clients, so nobody will actually see this
> change.

Right, sorry, I should have deleted this code. Now we just delete the
drive if it was already there and then add a new one.

> It would be nice to eventually change this and support changes to
> drives/volumes, as that would allow nice things like renaming of
> connected servers. However, right now there is no such support.

Right, this would be nice. I know Kay Sievers wants to add code to
volume_id (the library used by hal to detect file systems types, labels
and UUID's) so we can write new labels. This would enable us to also
rename media.

I've attached a patch with all the fixes.

Cheers,
David

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/gnome-vfs/ChangeLog,v
retrieving revision 1.2213
diff -u -p -r1.2213 ChangeLog
--- ChangeLog	1 Jul 2005 21:56:38 -0000	1.2213
+++ ChangeLog	2 Jul 2005 02:44:14 -0000
@@ -1,3 +1,27 @@
+2005-07-01  David Zeuthen  <davidz redhat com>
+
+	Fixes pointed out by Alexander Larsson.
+
+	* libgnomevfs/gnome-vfs-hal-mounts.h: Don't define 
+	HAL_MOUNTS_INVALID_URI as it's actually not used
+
+	* libgnomevfs/gnome-vfs-hal-mounts.c: Uncomment a lot of g_debug calls
+	(GnomeVFSHalUserData): Add display_* settings from gconf
+	(_hal_size_as_string): Delete this function
+	(_hal_drive_policy_get_display_name): Use
+	gnome_vfs_format_file_size_ for_display instead of
+	_hal_size_as_string
+	(_hal_drive_policy_check): Use settings from GnomeVFSHalUserData
+	instead of calling into gconf
+	(_hal_add_drive_without_volumes): Set activation_uri to empty
+	string instead of HAL_MOUNTS_INVALID_URI
+	(_hal_add_volume): Don't attempt to reuse a drive if it's already
+	there - instead just delete the drive and add a new one. Also set
+	activation_uri to empty string instead of HAL_MOUNTS_INVALID_URI
+	(_hal_get_settings_from_gconf): New function
+	(_hal_settings_changed): Call _hal_get_settings_from_gconf
+	(_gnome_vfs_hal_mounts_init): Call _hal_get_settings_from_gconf
+
 2005-07-01  Christian Kellner  <gicmo gnome org>
 
 	* programs/gnomevfs-info.c: Add the
Index: libgnomevfs/gnome-vfs-hal-mounts.c
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-hal-mounts.c,v
retrieving revision 1.12
diff -u -p -r1.12 gnome-vfs-hal-mounts.c
--- libgnomevfs/gnome-vfs-hal-mounts.c	30 Jun 2005 09:17:20 -0000	1.12
+++ libgnomevfs/gnome-vfs-hal-mounts.c	2 Jul 2005 02:44:15 -0000
@@ -51,6 +51,11 @@
 
 typedef struct {
 	GnomeVFSVolumeMonitorDaemon *volume_monitor_daemon;
+	gboolean display_internal_hard_drives;
+	gboolean display_scsi_drives;
+	gboolean display_scsi_optical_drives;
+	gboolean display_external_drives;
+	gboolean display_drives_removable;
 	guint gconf_client_connection_id;
 } GnomeVFSHalUserData;
 
@@ -301,39 +306,6 @@ out:
 }
 
 /*------------------------------------------------------------------------*/
-static char *
-_hal_size_as_string (gint64 size)
-{
-	char *result;
-	char* sizes_str[] = {"K", "M", "G", "T", NULL};
-	gint64 cur = 1000L;
-	gint64 base = 10L;
-	gint64 step = 10L*10L*10L;
-	int cur_str = 0;
-
-	result = NULL;
-
-	do {
-		if (sizes_str[cur_str+1] == NULL || size < cur*step) {
-			/* found the unit, display a comma number if result is a single digit */
-			if (size < cur*base) {
-				result = g_strdup_printf ("%.01f%s", ((double)size)/((double)cur), sizes_str[cur_str]);
-			} else {
-				result = g_strdup_printf ("%lld%s", size / cur, sizes_str[cur_str]);
-			}
-			goto out;
-		}
-
-		cur *= step;
-		cur_str++;
-
-	} while (TRUE);
-
-out:
-	return result;
-}
-
-/*------------------------------------------------------------------------*/
 /* hal_volume may be NULL */
 static char *
 _hal_drive_policy_get_display_name (GnomeVFSVolumeMonitorDaemon *volume_monitor_daemon,
@@ -368,7 +340,7 @@ _hal_drive_policy_get_display_name (Gnom
 
 		/* Otherwise, just use volume size */
 
-		size_str = _hal_size_as_string (libhal_volume_get_size (hal_volume));
+		size_str = gnome_vfs_format_file_size_for_display (libhal_volume_get_size (hal_volume));
 		if (size_str != NULL) {
 			name = g_strdup_printf (_("%s Volume"), size_str);
 		}
@@ -581,7 +553,7 @@ _hal_volume_policy_get_display_name (Gno
 
 	/* Fallback: size of media */
 
-	size_str = _hal_size_as_string (libhal_volume_get_size (hal_volume));
+	size_str = gnome_vfs_format_file_size_for_display (libhal_volume_get_size (hal_volume));
 	if (size_str != NULL) {
 		if (libhal_drive_uses_removable_media (hal_drive)) {
 			name = g_strdup_printf (_("%s Removable Volume"), size_str);
@@ -608,55 +580,35 @@ _hal_drive_policy_check (GnomeVFSVolumeM
 			 LibHalDrive *hal_drive, LibHalVolume *hal_volume)
 {
 	gboolean ret;
-	gboolean display_internal_hard_drives;
-	gboolean display_scsi_drives;
-	gboolean display_scsi_optical_drives;
-	gboolean display_external_drives;
-	gboolean display_drives_removable;
+	GnomeVFSHalUserData *hal_userdata;
 
 	ret = FALSE;
 
-	/* respect the gconf settings! */
-	display_internal_hard_drives = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
-							      PATH_GCONF_GNOME_VFS_STORAGE 
-							      "/display_internal_hard_drives",
-							      NULL);
-	display_scsi_drives = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
-						     PATH_GCONF_GNOME_VFS_STORAGE 
-						     "/display_scsi_drives",
-						     NULL);
-	display_scsi_optical_drives = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
-							     PATH_GCONF_GNOME_VFS_STORAGE 
-							     "/display_scsi_optical_drives",
-							     NULL);
-	display_external_drives = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
-							 PATH_GCONF_GNOME_VFS_STORAGE
-							 "/display_external_drives",
-							 NULL);
-	display_drives_removable = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
-							  PATH_GCONF_GNOME_VFS_STORAGE
-							  "/display_drives_with_removable_media",
-							  NULL);
+	g_return_val_if_fail (volume_monitor_daemon != NULL, ret);
+
+	hal_userdata = (GnomeVFSHalUserData *) libhal_ctx_get_user_data (volume_monitor_daemon->hal_ctx);
 
-	if (!display_external_drives) {
+	g_return_val_if_fail (hal_userdata != NULL, ret);
+
+	if (!hal_userdata->display_external_drives) {
 		if (libhal_drive_is_hotpluggable (hal_drive))
 			goto out;
 	}
 
-	if (!display_drives_removable) {
+	if (!hal_userdata->display_drives_removable) {
 		if (libhal_drive_uses_removable_media (hal_drive))
 			goto out;
 	}
 
-	if (!display_internal_hard_drives) {
+	if (!hal_userdata->display_internal_hard_drives) {
 		if ((libhal_drive_uses_removable_media (hal_drive) == FALSE) && 
 		    (libhal_drive_is_hotpluggable (hal_drive) == FALSE))
 			goto out;
 	}
 
-	if (!display_scsi_drives) {
+	if (!hal_userdata->display_scsi_drives) {
 		if (libhal_drive_get_bus (hal_drive) == LIBHAL_DRIVE_BUS_SCSI) {
-			if (display_scsi_optical_drives) {
+			if (hal_userdata->display_scsi_optical_drives) {
 				if (libhal_drive_get_type (hal_drive) == LIBHAL_DRIVE_TYPE_CDROM) {
 					/* we're safe */
 				} else {
@@ -830,8 +782,8 @@ _hal_add_drive_without_volumes (GnomeVFS
 
 	g_return_if_fail (hal_drive != NULL);
 
-	g_debug ("entering _hal_add_drive_without_volumes for\n  drive udi '%s'\n",
-		 libhal_drive_get_udi (hal_drive));
+	/* g_debug ("entering _hal_add_drive_without_volumes for\n  drive udi '%s'\n", 
+	   libhal_drive_get_udi (hal_drive)); */
 
 	volume_monitor = GNOME_VFS_VOLUME_MONITOR (volume_monitor_daemon);
 	hal_userdata = (GnomeVFSHalUserData *) libhal_ctx_get_user_data (volume_monitor_daemon->hal_ctx);
@@ -856,7 +808,7 @@ _hal_add_drive_without_volumes (GnomeVFS
 		goto out;
 	
 	drive = g_object_new (GNOME_VFS_TYPE_DRIVE, NULL);
-	drive->priv->activation_uri = gnome_vfs_get_uri_from_local_path (HAL_MOUNTS_INVALID_URI); /* (!) */
+	drive->priv->activation_uri = g_strdup ("");
 	drive->priv->is_connected = 1;
 	drive->priv->device_path = g_strdup (libhal_drive_get_device_file (hal_drive));
 	drive->priv->device_type = _hal_get_gnome_vfs_device_type (hal_drive);
@@ -909,8 +861,8 @@ _hal_add_volume (GnomeVFSVolumeMonitorDa
 		goto out;
 	}
 
-	//g_debug ("entering _hal_add_volume for\n  drive udi '%s'\n  volume udi '%s'\n",
-	//	 libhal_drive_get_udi (drive), libhal_volume_get_udi (volume));
+	/* g_debug ("entering _hal_add_volume for\n  drive udi '%s'\n  volume udi '%s'\n",
+	   libhal_drive_get_udi (drive), libhal_volume_get_udi (volume)); */
 
 	/* OK, check if we got a drive_without_volumes drive and delete that since we're going to add a
 	 * drive for added partitions */
@@ -924,57 +876,42 @@ _hal_add_volume (GnomeVFSVolumeMonitorDa
 	 */
 	drive = _gnome_vfs_volume_monitor_find_drive_by_hal_udi (volume_monitor, libhal_volume_get_udi (hal_volume));
 	if (drive != NULL) {
-		if (libhal_volume_disc_has_audio (hal_volume)) {
-			drive->priv->activation_uri = g_strdup_printf ("cdda://%s", 
-								       libhal_volume_get_device_file (hal_volume));
-		} else if (libhal_volume_disc_is_blank (hal_volume)) {
-			drive->priv->activation_uri = g_strdup ("burn:///");
-		} else if (libhal_volume_is_mounted (hal_volume)) {
-			drive->priv->activation_uri = gnome_vfs_get_uri_from_local_path (
-				libhal_volume_get_mount_point (hal_volume));
-		} else {
-			/* This sucks but it doesn't make sense to talk about the activation_uri if we're not mounted!
-			 * So fake it...
-			 */
-			drive->priv->activation_uri = gnome_vfs_get_uri_from_local_path (HAL_MOUNTS_INVALID_URI); 
-		}
+		_gnome_vfs_volume_monitor_disconnected (volume_monitor, drive);
+	}
 
+	drive = g_object_new (GNOME_VFS_TYPE_DRIVE, NULL);
+	if (libhal_volume_disc_has_audio (hal_volume)) {
+		drive->priv->activation_uri = g_strdup_printf ("cdda://%s", 
+							       libhal_volume_get_device_file (hal_volume));
+	} else if (libhal_volume_disc_is_blank (hal_volume)) {
+		drive->priv->activation_uri = g_strdup ("burn:///");
+	} else if (libhal_volume_is_mounted (hal_volume)) {
+		drive->priv->activation_uri = gnome_vfs_get_uri_from_local_path (
+			libhal_volume_get_mount_point (hal_volume));
 	} else {
-		drive = g_object_new (GNOME_VFS_TYPE_DRIVE, NULL);
-		if (libhal_volume_disc_has_audio (hal_volume)) {
-			drive->priv->activation_uri = g_strdup_printf ("cdda://%s", 
-								       libhal_volume_get_device_file (hal_volume));
-		} else if (libhal_volume_disc_is_blank (hal_volume)) {
-			drive->priv->activation_uri = g_strdup ("burn:///");
-		} else if (libhal_volume_is_mounted (hal_volume)) {
-			drive->priv->activation_uri = gnome_vfs_get_uri_from_local_path (
-				libhal_volume_get_mount_point (hal_volume));
-		} else {
-			/* This sucks but it doesn't make sense to talk about the activation_uri if we're not mounted!
-			 * So fake it...
-			 */
-			drive->priv->activation_uri = gnome_vfs_get_uri_from_local_path (HAL_MOUNTS_INVALID_URI); 
-		}
-		drive->priv->is_connected = TRUE;
-		drive->priv->device_path = g_strdup (libhal_volume_get_device_file (hal_volume));
-		drive->priv->device_type = _hal_get_gnome_vfs_device_type (hal_drive);
-		
-		/* TODO: could add an icon of a drive with media in it since this codepath only
-		 * handles drives with media in them
+		/* This sucks but it doesn't make sense to talk about the activation_uri if we're not mounted!
+		 * So just set it to the empty string
 		 */
-		drive->priv->icon = _hal_drive_policy_get_icon (volume_monitor_daemon, hal_drive, NULL);
-		name = _hal_drive_policy_get_display_name (volume_monitor_daemon, hal_drive, hal_volume);
-		drive->priv->display_name = _gnome_vfs_volume_monitor_uniquify_drive_name (volume_monitor, name);
-		g_free (name);
-		drive->priv->is_user_visible = TRUE;
-		drive->priv->volumes = NULL;
-		drive->priv->hal_udi = g_strdup (libhal_volume_get_udi (hal_volume));
-		drive->priv->hal_drive_udi = g_strdup (libhal_drive_get_udi (hal_drive));
-		
-		_gnome_vfs_volume_monitor_connected (volume_monitor, drive);
-		gnome_vfs_drive_unref (drive);
+		drive->priv->activation_uri = g_strdup ("");
 	}
-
+	drive->priv->is_connected = TRUE;
+	drive->priv->device_path = g_strdup (libhal_volume_get_device_file (hal_volume));
+	drive->priv->device_type = _hal_get_gnome_vfs_device_type (hal_drive);
+	
+	/* TODO: could add an icon of a drive with media in it since this codepath only
+	 * handles drives with media in them
+	 */
+	drive->priv->icon = _hal_drive_policy_get_icon (volume_monitor_daemon, hal_drive, NULL);
+	name = _hal_drive_policy_get_display_name (volume_monitor_daemon, hal_drive, hal_volume);
+	drive->priv->display_name = _gnome_vfs_volume_monitor_uniquify_drive_name (volume_monitor, name);
+	g_free (name);
+	drive->priv->is_user_visible = TRUE;
+	drive->priv->volumes = NULL;
+	drive->priv->hal_udi = g_strdup (libhal_volume_get_udi (hal_volume));
+	drive->priv->hal_drive_udi = g_strdup (libhal_drive_get_udi (hal_drive));
+	
+	_gnome_vfs_volume_monitor_connected (volume_monitor, drive);
+	gnome_vfs_drive_unref (drive);
 
 	vol = _gnome_vfs_volume_monitor_find_volume_by_hal_udi (volume_monitor, libhal_volume_get_udi (hal_volume));
 	if (vol == NULL && 
@@ -1034,7 +971,7 @@ _hal_update_all (GnomeVFSVolumeMonitorDa
 	int num_drives;
 	DBusError error;
 
-	g_debug ("entering _hal_update_all");
+	/* g_debug ("entering _hal_update_all"); */
 
 	dbus_error_init (&error);
 	drives = libhal_find_device_by_capability (volume_monitor_daemon->hal_ctx,
@@ -1045,7 +982,7 @@ _hal_update_all (GnomeVFSVolumeMonitorDa
 		for (i = 0; i < num_drives; i++) {
 			LibHalDrive *drive;
 
-			g_debug ("drive = '%s'", drives[i]);
+			/* g_debug ("drive = '%s'", drives[i]); */
 			drive = libhal_drive_from_udi (volume_monitor_daemon->hal_ctx, drives[i]);
 			if (drive != NULL) {
 				char **volumes;
@@ -1062,7 +999,7 @@ _hal_update_all (GnomeVFSVolumeMonitorDa
 					for (j = 0; j < num_volumes; j++) {
 						LibHalVolume *volume;
 
-						g_debug ("  volume = '%s'", volumes[j]);
+						/* g_debug ("  volume = '%s'", volumes[j]); */
 						volume = libhal_volume_from_udi (volume_monitor_daemon->hal_ctx, 
 										 volumes[j]);
 
@@ -1099,7 +1036,7 @@ _hal_device_added (LibHalContext *hal_ct
 	GnomeVFSHalUserData *hal_userdata;
 	GnomeVFSVolumeMonitorDaemon *volume_monitor_daemon;
 
-	g_debug ("Entering %s", __FUNCTION__);
+	/* g_debug ("Entering %s", __FUNCTION__); */
 	
 	hal_userdata = (GnomeVFSHalUserData *) libhal_ctx_get_user_data (hal_ctx);
 	volume_monitor_daemon = hal_userdata->volume_monitor_daemon;
@@ -1184,7 +1121,7 @@ _hal_device_removed (LibHalContext *hal_
 	GnomeVFSVolumeMonitorDaemon *volume_monitor_daemon;
 	char *hal_drive_udi;
 
-	g_debug ("Entering %s", __FUNCTION__);
+	/* g_debug ("Entering %s", __FUNCTION__); */
 	
 	hal_userdata = (GnomeVFSHalUserData *) libhal_ctx_get_user_data (hal_ctx);
 	volume_monitor_daemon = hal_userdata->volume_monitor_daemon;
@@ -1207,7 +1144,7 @@ _hal_device_removed (LibHalContext *hal_
 		_gnome_vfs_volume_monitor_disconnected (GNOME_VFS_VOLUME_MONITOR (volume_monitor_daemon), drive);
 	}
 
-	g_debug ("hal_drive_udi = %s", hal_drive_udi);
+	/* g_debug ("hal_drive_udi = %s", hal_drive_udi); */
 
 	/* if there are no other drives with the same hal_drive_udi as us, add a drive_without_volumes object  */
 	if (hal_drive_udi != NULL && 
@@ -1215,7 +1152,7 @@ _hal_device_removed (LibHalContext *hal_
 		    GNOME_VFS_VOLUME_MONITOR (volume_monitor_daemon), hal_drive_udi) == NULL) {
 		LibHalDrive *drive;
 
-		g_debug ("going to add drive_without_volumes for %s", hal_drive_udi);
+		/* g_debug ("going to add drive_without_volumes for %s", hal_drive_udi); */
 
 		drive = libhal_drive_from_udi (volume_monitor_daemon->hal_ctx, hal_drive_udi);
 		if (drive != NULL) {
@@ -1308,6 +1245,39 @@ out:
 }
 	
 
+static void 
+_hal_get_settings_from_gconf (GnomeVFSVolumeMonitorDaemon *volume_monitor_daemon)
+{
+	GnomeVFSHalUserData *hal_userdata;
+
+	g_return_if_fail (volume_monitor_daemon != NULL);
+
+	hal_userdata = (GnomeVFSHalUserData *) libhal_ctx_get_user_data (volume_monitor_daemon->hal_ctx);
+
+	g_return_if_fail (hal_userdata != NULL);
+
+	/* respect the gconf settings! */
+	hal_userdata->display_internal_hard_drives = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
+									    PATH_GCONF_GNOME_VFS_STORAGE 
+									    "/display_internal_hard_drives",
+									    NULL);
+	hal_userdata->display_scsi_drives = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
+								   PATH_GCONF_GNOME_VFS_STORAGE 
+								   "/display_scsi_drives",
+								   NULL);
+	hal_userdata->display_scsi_optical_drives = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
+									   PATH_GCONF_GNOME_VFS_STORAGE 
+									   "/display_scsi_optical_drives",
+									   NULL);
+	hal_userdata->display_external_drives = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
+								       PATH_GCONF_GNOME_VFS_STORAGE
+								       "/display_external_drives",
+								       NULL);
+	hal_userdata->display_drives_removable = gconf_client_get_bool (volume_monitor_daemon->gconf_client,
+									PATH_GCONF_GNOME_VFS_STORAGE
+									"/display_drives_with_removable_media",
+									NULL);
+}
 
 static void
 _hal_settings_changed (GConfClient* client,
@@ -1319,10 +1289,15 @@ _hal_settings_changed (GConfClient* clie
 
 	volume_monitor_daemon = data;
 
-	g_debug ("in _hal_settings_changed");
+	g_return_if_fail (volume_monitor_daemon != NULL);
+
+	/* g_debug ("in _hal_settings_changed"); */
+
+	_hal_get_settings_from_gconf (volume_monitor_daemon);
 	_gnome_vfs_hal_mounts_force_reprobe (volume_monitor_daemon);
 }
 
+
 gboolean
 _gnome_vfs_hal_mounts_init (GnomeVFSVolumeMonitorDaemon *volume_monitor_daemon)
 {
@@ -1330,7 +1305,7 @@ _gnome_vfs_hal_mounts_init (GnomeVFSVolu
 	DBusConnection *dbus_connection;
 	GnomeVFSHalUserData *hal_userdata;
 
-	g_debug ("Entering %s", __FUNCTION__);
+	/* g_debug ("Entering %s", __FUNCTION__); */
 
 	/* Initialise the connection to the hal daemon */
 	if ((volume_monitor_daemon->hal_ctx = 
@@ -1384,13 +1359,14 @@ _gnome_vfs_hal_mounts_init (GnomeVFSVolu
 					 NULL,
 					 NULL);
 
-
 	/* Simply watch all property changes instead of dynamically
 	 * adding/removing match rules bus-side to only match certain
 	 * objects...
 	 */
 	libhal_device_property_watch_all (volume_monitor_daemon->hal_ctx, &error);
 
+	_hal_get_settings_from_gconf (volume_monitor_daemon);
+
 	/* add drives/volumes from HAL */
 	_hal_update_all (volume_monitor_daemon);
 
@@ -1424,7 +1400,7 @@ _gnome_vfs_hal_mounts_shutdown (GnomeVFS
 void
 _gnome_vfs_hal_mounts_force_reprobe (GnomeVFSVolumeMonitorDaemon *volume_monitor_daemon)
 {
-	g_debug ("entering _gnome_vfs_hal_mounts_force_reprobe");
+	/* g_debug ("entering _gnome_vfs_hal_mounts_force_reprobe"); */
 	_hal_update_all (volume_monitor_daemon);
 }
 
Index: libgnomevfs/gnome-vfs-hal-mounts.h
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-hal-mounts.h,v
retrieving revision 1.5
diff -u -p -r1.5 gnome-vfs-hal-mounts.h
--- libgnomevfs/gnome-vfs-hal-mounts.h	30 Jun 2005 09:17:20 -0000	1.5
+++ libgnomevfs/gnome-vfs-hal-mounts.h	2 Jul 2005 02:44:15 -0000
@@ -26,8 +26,6 @@
 
 #include "gnome-vfs-volume-monitor-daemon.h"
 
-#define HAL_MOUNTS_INVALID_URI "/gnome/vfs2/internal/no/uri/available"
-
 gboolean _gnome_vfs_hal_mounts_init (GnomeVFSVolumeMonitorDaemon *volume_monitor_daemon);
 
 void _gnome_vfs_hal_mounts_force_reprobe (GnomeVFSVolumeMonitorDaemon *volume_monitor_daemon);