GNOME Bugzilla – Bug 789491
mtp volume is not removed when unplugging
Last modified: 2018-04-03 15:03:01 UTC
Created attachment 362286 [details] udevadm monitor -u -p log Volume for my Nexus 5x is not removed correctly and thus the number of volumes increases when plugging... gio mount -l Volume(0): Nexus 5X Type: GProxyVolume (GProxyVolumeMonitorMTP) Volume(1): Nexus 5X Type: GProxyVolume (GProxyVolumeMonitorMTP) Volume(2): Nexus 5X Type: GProxyVolume (GProxyVolumeMonitorMTP) ... The problem is that remove uevent is discarded because ID_MTP_DEVICE is not set: https://git.gnome.org/browse/gvfs/tree/monitor/mtp/gmtpvolumemonitor.c#n212 $ G_MESSAGES_DEBUG=all /opt/gnome/libexec/gvfs-mtp-volume-monitor ... (process:8527): GVFS-MTP-DEBUG: on_uevent: action=remove, device=(null) (process:8527): GVFS-MTP-DEBUG: on_uevent: discarding, not ID_MTP (process:8527): GVFS-MTP-DEBUG: on_uevent: action=unbind, device=/dev/bus/usb/002/095 (process:8527): GVFS-MTP-DEBUG: on_uevent: discarding, not ID_MTP (process:8527): GVFS-MTP-DEBUG: on_uevent: action=remove, device=/dev/bus/usb/002/095 (process:8527): GVFS-MTP-DEBUG: on_uevent: discarding, not ID_MTP Not sure why this is happening and whether it applies also to other devices, or not... Philip, do you have any idea what is wrong?
Created attachment 362287 [details] [review] mtp: Do not check ID_MTP_DEVICE when removing UDev events for devices without ID_MTP_DEVICE property are ignored. Although ID_MTP_DEVICE seems don't have to be set for device when removing and thus the volume is not removed. Let's ignore ID_MTP_DEVICE when removing.
Review of attachment 362287 [details] [review]: Looks fine; it makes sense to consider a remove event on a known device to be valid, regardless of the device properties. I assume this is due to some recent udev or device database change.
Created attachment 362310 [details] [review] mtp: Fix volume removal with current udev behavior Thanks for review! Updated commit title...
Attachment 362310 [details] pushed as 0c7f803 - mtp: Fix volume removal with current udev behavior
And pushed to gnome-3-26 as well for sure...
Thanks. Note that the logic here was shamelessly copied from the gphoto2 volume monitor, and it continues to do a property check on events (looks for ID_GPHOTO2 property). You might want to verify what's going on there as well.
Thanks for the notice, the same happens also for gphoto2 volume monitor indeed...
So I have pushed the same fix for gphoto2 also as commit 9f8561d.
Review of attachment 362310 [details] [review]: ::: monitor/mtp/gmtpvolumemonitor.c @@ +211,3 @@ + if (g_strcmp0 (action, "add") == 0 && g_udev_device_has_property (device, "ID_MTP_DEVICE")) + gudev_add_device (monitor, device, TRUE); + else if (g_strcmp0 (action, "remove") == 0 && g_udev_device_get_device_file (device) != NULL) I've just realized that I used g_udev_device_get_device_file () check here, because it seems it can be NULL in some cases and I thought that it is used for comparison in gudev_remove_device, however, g_udev_device_get_sysfs_path is used there... should I remove it? Philip?
Honestly, I'd be fine with no check there - after all, if you get a remove event on a device you already added, you should remove it right? Regardless of anything else. But if you think some check is required, switching would make sense.
Created attachment 362647 [details] [review] monitor: Remove device file checks The device file checks were added by mistake and it is not clear whether it can't have some side-effect. Let's remove them and use g_strcmp0 for sysfs path comparison for sure...
Comment on attachment 362647 [details] [review] monitor: Remove device file checks Attachment 362647 [details] pushed as 5db1651 - monitor: Remove device file checks
*** Bug 790555 has been marked as a duplicate of this bug. ***
Hello! I'm also running into this issue in Debian Stretch (Gnome 3.22) and suspect there are many other users since Debian is so widely used. Would it be possible to get this fix pushed into 3.22 so that (hopefully soon), it would flow down through the Debian repository? Pretty please? If not, could you suggest a hack to get this fix without upgrading all of Gnome to a newer rev? Thanks much! Mark
The fix is already part of gnome-3-28 and gnome-3-26 releases. I am going to push into older branches also, but it is up to downstream packagers to use this commits... I am not about making another release in those old branches. So you should report this to Debian.
Thanks very much! I'll contact Debian to ask them to incorporate your fix.