GNOME Bugzilla – Bug 640845
USB disk not indexed
Last modified: 2011-02-17 11:43:40 UTC
Created attachment 179533 [details] tracker-miner-fs traces when inserting a USB hard drive Hello, I'm using Tracker to index my USB devices. Unfortunately any of my USB disk manages to be indexed but my USB keys (flash nand on USB connector) all are. See traces from tracker-miner-fs when connecting my usb hard drive. This problems comes from the way Tracker considers a device as removable. Currently it relies on the g_drive_is_media_removable() method from GIO which is not a good choice because this method does not tell us whether a device can be disconnected from the system but rather whether a device contains a media that might be removed from it. It maps the removable flag from the kernel block device subsystem. Attached is a patch that proposes to use the g_unix_mount_guess_should_display() method instead, which is also used by the gnome volume manager to decide whether or not the device must be display in the UI.
Created attachment 179534 [details] [review] Patch
Comment on attachment 179534 [details] [review] Patch The patch looks fine to me at first sight, but I don't think we need the GDrive anymore in that code section, so there's no need to get/unref it at all
g_unix_mount_guess_should_display() doesn't seem the correct solution here. But g_drive_is_media_removable() isn't either. See these 3 cases I just tested: g_drive_is_media_removable(): * 2GB USB dongle: TRUE * 300GB USB HD: FALSE (wrong for our purposes) * sshfs mount: FALSE g_unix_mount_guess_should_display(): * 2GB USB dongle: TRUE * 300GB USB HD: TRUE * sshfs mount: TRUE (wrong for our purposes) g_unix_mount_guess_should_display() returns TRUE for a sshfs mount, so we cannot assume that every 'non-removable media' (as we consider it) will have should_display() return FALSE. I wonder now how we could effectively do this. Maybe also playing with g_unix_mount_get_fs_type()? So if should_display() and get_fs_type() is FAT or any other non-remote fs type, then it can be considered as removable... something like that? Maybe its a good idea to check how the disk mounter applet in gnome shows different icons for the different mounts. Does it use g_unix_mount_guess_icon()? That may give us some hints, as it always seems to do it properly.
Created attachment 180912 [details] [review] New patch version
Created attachment 180915 [details] [review] New patch version Add some comment to explain why we keep checking the driver exists.
Review of attachment 180915 [details] [review]: Seems to be a better approach than our previous g_drive_is_media_removable(), even if I still think that g_unix_mount_guess_should_display() is not a good choice... Anyway, probably with the changes below is enough for now. ::: src/libtracker-miner/tracker-storage.c @@ +597,3 @@ drive = g_volume_get_drive (volume); if (drive) { + GUnixMountEntry *mount_entry = g_unix_mount_at (mount_path, NULL); To follow with the coding style, it would be: GUnixMountEntry *mount_entry; <emptyline> mount_entry = g_unix_mount_at (mount_path, NULL); @@ +601,3 @@ + /* We can't mount/unmount system volumes, so tag + * them as non removable. */ + is_removable = g_volume_can_mount (volume); I would merge this in the previous if(), something like: if (drive && g_volume_can_mount (volume)) So that you only try to get the mount_entry if that already succeeded. @@ +607,3 @@ + * mount points out of /media/ or + * $HOME/. */ + is_removable &= g_unix_mount_guess_should_display (mount_entry); If you do the previous fix of merging the can_mount() in the if(), you don't need to "&=", just "=" which is clearer. @@ +610,3 @@ + + g_unix_mount_free (mount_entry); + } And if !mount_entry, please g_debug() the condition so that if any happens to have a case where that branch is accessed, we know about it.
Created attachment 181114 [details] [review] Updated patch after review
Tried with all disks I have here and worked ok, so pushed to git master. Thanks for the patch! This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.