GNOME Bugzilla – Bug 629179
Banshee 1.7.5 no longer recognizes mass storage device
Last modified: 2010-09-29 00:20:42 UTC
Since the upgrade to 1.7.5, Banshee no longer recognizes any connected USB mass storage devices. Steps to reproduce: Open Banshee and connect a USB mass storage mp3 device. Gnome pops up a window asking if you'd like open it with Banshee or some other application. Choose Banshee. Or use Nautilus to navigate to your device and then choose "Open in Banshee" (if Banshee is set as your default player, that option will appear in the upper-right of Nautilus). What happens: Nothing. If Banshee is on, it doesn't recognize the device. If Banshee is off and you choose the Gnome option to open with Banshee, Banshee starts, but the device isn't recognized. If you choose "Open in Banshee" from Nautilus, Banshee comes up, but the device isn't recognized. What's expected: It should just recognize a standard USB device, like Banshee and every other media player have for years.
I'm also seeing this behavior, using Ubuntu 10.10 and banshee 1.7.6. Running banshee in debug mode doesn't deliver any useful information - the Banshee.Dap.MassStorage extension is loaded, but there's no debug prints when I connect my device (nokia n900 in mass storage mode). In versions prior to 1.7.5 the device was detected and functioned perfectly in banshee.
By any chance can you guys do a binary search of the exact commit that caused this?
I am currently using Ubuntu 10.04, with Banshee 1.7.6 installed from the PPA. I updated from 1.6.x. My Sansa Fuze (MSC mode), and the internal micro-SD card are both mounted by the system, and appear as msdos volumes in the /media directory. Banshee recognizes the micro-SD card, but not the Fuze itself.
Using Ubuntu 10.04 and just was upgraded to 1.7.6 from Update Manager. Banshee now will not recognize my Sansa Clip+, although it worked flawlessly in previous versions. When the Clip+ is attached to my computer with the USB cable it is seen in Nautilus without a problem. Rhythmbox does recognize it. Please fix, I love Banshee.
Andrés - I'm trying to do some bisecting to work out when this broke, but I'm having trouble building banshee. I keep getting the error: Making all in Banshee.Gio MCS ../../../bin/Banshee.Gio.dll cp: will not overwrite just-created `../../../bin/gio-sharp.dll' with `/usr/lib/pkgconfig/../../lib/gtk-sharp-beans/gio-sharp.dll' cp: will not overwrite just-created `../../../bin/gio-sharp.dll.config' with `/usr/lib/pkgconfig/../../lib/gtk-sharp-beans/gio-sharp.dll.config' make[4]: *** [../../../bin/Banshee.Gio.dll] Error 1 Any idea what's wrong here?
Could you attach the logfile generated by banshee when you plug your device in? I need that to help diagnose the issue. The problem is probably that both filesystems have the same UUID under udev and that's causing banshee to ignore one. You should see the output in the terminal aswell which prints out the device name and uuid for every detected mountpoint.
In my case, banshee-1 --debug doesn't output anything when the device is plugged in.
If you don't post the log there's nothing I can do to help diagnose the issue ;)
Just to be clear, I need the entire log from the point where banshee is initially started. As extra debug info, could you also give me: 1) gvfs-mount --list after your device is plugged in 2) gvfs-mount --monitor while you are physically plugging in your device 3) udevadm info --export-db after your device has been plugged in (may need to be run as root). Those four things should give me everything I need.
Created attachment 171085 [details] Banshee log on attaching n900
Created attachment 171086 [details] gvfs-mount --list log
Created attachment 171087 [details] gvfs-mount --monitor log
Created attachment 171088 [details] udevadm info --export-db
I think that's everything -- let me know if there's anything more I can provide! :)
Created attachment 171090 [details] [review] patch to add more logging Hrm. Everything looks perfect in those logs except for banshee completely ignoring the device. Very strange. From your previous comments it looks like you're able to build banshee from git. Could you attach the patch here and then give me the new log when banshee is run with the patch applied.
Created attachment 171091 [details] log with extra debug patch
Here you go, with the extra debug patch applied.
Created attachment 171092 [details] [review] Better patch for logging Thanks. Unfortunately an exception was thrown while generating the debug output, I'm not sure why. I added some more debug logging and also adding handling for the exception which happened to you when you ran with the last patch. Hopefully this will work better now. Revert the old patch (git reset --hard would do it) and then apply this one. Thanks!
Created attachment 171094 [details] log from latest debug patch
Perfect! Between the last log and all the gvfs/udev logs I have everything I need now I think. I'll need a little while to come up with a solution as I have to head out now. I'll update the bug report with a patch later on today or tomorrow. Thanks for taking the time to run through all that debugging for me.
Gabriel, The bug is that the device only has one udev entry and that entry does not contain the BUSNUM or DEVNUM properties which are how we detect a USB device. How should this be fixed? I could add an additional check for "ID_BUS=usb" and accept that as the marker as well as the existance of BUSNUM and DEVNUM properties. Does that sound good?
Created attachment 171095 [details] [review] Allow usb devices without bus or device numbers to be detected nyall, Can you try the attached fix. I had a few minutes spare ;)
Created attachment 171112 [details] Log file with attempted fix patch Unfortunately no luck with that patch. I left in the debugging code from earlier, and here's the console output with this new patch.
Created attachment 171128 [details] [review] Ensure USB devices are detected as such There were two places which had the same detection code for usb devices and I only changed one. Unfortunately you were hitting the one I didn't change ;) I've fixed the code so the usb detection logic is only in one method so if you could try with this and let me know if it works, that'd be great! Note that this patch may break MTP devices. I just want to make sure that this approach will work with your device. Thanks.
Created attachment 171149 [details] Log from v2 of patch. Still no luck sorry! Here's the output log with this latest patch.
Are you sure you're applying this patch against the latest version of banshee in git? The line numbers in the exceptions being thrown don't match up with what I'm seeing in my own checkout. As one more test, could you open up MassStorageSource.cs and put in a load of Console.WriteLine ("XXX") calls so that each Console.WriteLine prints out a unique string to try and figure out exactly which line is failing. The important places would be before each instance of "throw new InvalidDeviceException". Thanks
Created attachment 171159 [details] Proper log from v2 of patch Sorry, I had some extra debug statements in there. I've reset to master, and applied the latest debug patch and your latest UsbDevice.cs patch. Here's the resultant log. I'll add the extra debug statements to MassStorageSource.cs and post the results of that too.
Hey, just to let you know that banshee 1.8.0 is supposed to be released tomorrow and I'd love to get a fix in for your issue, but without the extra logging statements I can't tell which InvalidOperationException is being hit so I can't debug the issue. If you're free any time to drop into IRC I should be able to get a solution to you relatively quickly, otherwise just attach the log and I'll try get something into 1.8.0. Thanks again for al the logs you've been attaching, they've been great so far!
Created attachment 171240 [details] [review] disable DiskDevices for now So to fix this bug we need the patch in this comment and also the previous one (171128). The attached patch disables detection of DiskDevices in the GIO backend as it is incorrectly (or maybe correctly) detecting the above device as a DiskDevice (internal harddrive?). Once the DiskDevice checking is disabled, it is (correctly?) recognised as a regular Volume and handled properly by the UMS dap. I think this is the correct approach as each DAP plugin can only really handle a single volume. The current mass storage addin would require a considerable amount of effort to make it work with multiple volumes in one device. It's probably better to allow them to show up as two separate devices like they would if a usb stick was partitioned in two. DiskDevices are essentially an IEnumerable<Volume> and so don't really fit into the DAP metaphor in banshee. Nothing in banshee deals with DiskDevices currently, so we'd probably be safe enough disabling this check for now, so are these two patches ok to commit? Is there a better approach?
Review of attachment 171128 [details] [review]: A couple nitpicks, please correct and push. ::: src/Backends/Banshee.Gio/Banshee.Hardware.Gio/UsbDevice.cs @@ +50,3 @@ + while (metadata != null) { + if (IsUsbDevice (metadata)) { + Console.WriteLine ("IT was usb"); remove WriteLine @@ +114,3 @@ + static bool IsUsbDevice (UdevMetadataSource metadata) + { +// if (metadata.PropertyExists (UdevUsbBusNumber) && metadata.PropertyExists (UdevUsbDeviceNumber)) Why did you have to comment these out and use the new checks? Assuming there's a good reason, any reason not to just remove them?
Review of attachment 171240 [details] [review]: Might as well comment out this entire class, no? Or just git rm it? Go ahead and push whatever approach you think is best.
171128: Whoops, I apparently had stuff already cached in git when I started creating those patches and didn't realise. I'll back out the stupid stuff which you picked out there. Good call! I'll also change the approach slightly so that we first scan the entire tree for usb port/bus numbers and then scan a second time for the usb type. This way we definitely won't break 'good' devices which do expose usb bus/port info at some level in their hierarchy and will be able to handle 'bad' devices which don't. 171240: I'll probably just nuke the file for now. If we need it later it's pretty trivial to recover or reimplement it.
Ok, the fixes for this have been pushed to git master. If you don't mind, please check out the latest from git and double-check everything works so that I know for sure I didn't blow up anything in the refactor of the patch. If things have broken, please reopen the bug and i'll fix it asap.
Just checked out git master and everything looks good. Thanks for your work on this one!