Bug 317485 - Solid::Device::listFromQuery( query ) returns fewer devices than `solid-hardware query $query`
Summary: Solid::Device::listFromQuery( query ) returns fewer devices than `solid-hardw...
Status: RESOLVED FIXED
Alias: None
Product: solid
Classification: Unclassified
Component: libsolid-udisks2 (show other bugs)
Version: 4.10.1
Platform: Other Linux
: NOR normal (vote)
Target Milestone: 4.11
Assignee: Lukáš Tinkl
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2013-03-28 12:06 UTC by Matěj Laitl
Modified: 2013-08-10 14:10 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.11


Attachments
amarok-debugging-for-afiestas.patch (2.74 KB, patch)
2013-07-22 11:20 UTC, Matěj Laitl
Details
kdelibs-debugging-for-afiestas.patch (4.44 KB, patch)
2013-07-22 11:20 UTC, Matěj Laitl
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matěj Laitl 2013-03-28 12:06:53 UTC
Hi Lukáš,
in Amarok, I do the following:
> QString query( "[IS StorageAccess OR IS PortableMediaPlayer]" );
> QList<Solid::Device> ipodDevices = Solid::Device::listFromQuery( query );
> foreach( const Solid::Device &device, ipodDevices ) { debug() << device.udi(); }
with the result
"/org/kde/solid/udev/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.2/1-1.2:1.0/host6/target6:0:0/6:0:0:0/block/sdc/sdc1" 
"/org/kde/solid/udev/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.2/1-1.2:1.0/host6/target6:0:0/6:0:0:0/block/sdc/sdc2" 
"/org/kde/solid/udev/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.2/1-1.2:1.0/host6/target6:0:0/6:0:0:0/block/sdc" 
"/org/kde/fstab/esprimo.local:/" 
"/org/kde/fstab/roundy.local:/"

But when I issue `solid-hardware query "[IS StorageAccess OR IS PortableMediaPlayer]"`, I get:
udi = '/org/kde/solid/udev/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.2/1-1.2:1.0/host6/target6:0:0/6:0:0:0/block/sdc/sdc1'
udi = '/org/kde/solid/udev/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.2/1-1.2:1.0/host6/target6:0:0/6:0:0:0/block/sdc/sdc2'
udi = '/org/kde/solid/udev/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.2/1-1.2:1.0/host6/target6:0:0/6:0:0:0/block/sdc'
udi = '/org/freedesktop/UDisks2/block_devices/sdb1'
udi = '/org/freedesktop/UDisks2/block_devices/sda1'
udi = '/org/freedesktop/UDisks2/block_devices/sdb2'
udi = '/org/freedesktop/UDisks2/block_devices/sda2'
udi = '/org/freedesktop/UDisks2/block_devices/sdb3'
udi = '/org/freedesktop/UDisks2/block_devices/sda3'
udi = '/org/freedesktop/UDisks2/block_devices/sdc2'
udi = '/org/kde/fstab/esprimo.local:/'
udi = '/org/kde/fstab/roundy.local:/'

Notice how UDisks2 devices are not returned in Amarok code. This causes that iPod devices that are connected at the time Amarok is started aren't recognized any more (when iPod is plugged in *while* Amarok is running, Solid emits deviceAdded() and it is recognized correctly); the exact code worked well with KDE 4.9 & udisks1. At this point I don't know what I'm doing wrong in Amarok code.
Comment 1 Alex Fiestas 2013-04-02 20:14:55 UTC
Uh, this is really weird, is that  bug reproducible all the times? if so, can you point us tot he file where this code comes from?

If you list all devices, are udisk2 devices included?
Comment 2 Matěj Laitl 2013-04-02 20:46:34 UTC
(In reply to comment #1)
> Uh, this is really weird, is that  bug reproducible all the times?

Yes.

> if so, can you point us tot he file where this code comes from?

http://quickgit.kde.org/?p=amarok.git&a=blob&f=src%2Fcore-impl%2Fcollections%2Fipodcollection%2FIpodCollectionFactory.cpp#45 - for debugging I apply patch below.

> If you list all devices, are udisk2 devices included?

Yes. Solid::Device::allDevices() includes UDisks2 devices; I get following ouput using the debugging patch below:

BEGIN: virtual void IpodCollectionFactory::init() 
  [IpodCollectionFactory] ::allDevices(): 
  [IpodCollectionFactory] "/org/kde/solid/udev/sys/devices/LNXSYSTM:00/LNXCPU:00" 
(...)
  [IpodCollectionFactory] "/org/freedesktop/UDisks2/block_devices/sdc1" 
  [IpodCollectionFactory] "/org/freedesktop/UDisks2/block_devices/sdc2" 
(...)
  [IpodCollectionFactory] ::listFromQuery(): 
  [IpodCollectionFactory] "/org/kde/solid/udev/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.1/1-1.1.1/1-1.1.1:1.0/host6/target6:0:0/6:0:0:0/block/sdc/sdc1" 
  [IpodCollectionFactory] "/org/kde/solid/udev/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.1/1-1.1.1/1-1.1.1:1.0/host6/target6:0:0/6:0:0:0/block/sdc/sdc2" 
  [IpodCollectionFactory] "/org/kde/solid/udev/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.1/1-1.1.1/1-1.1.1:1.0/host6/target6:0:0/6:0:0:0/block/sdc" 
  [IpodCollectionFactory] "/org/kde/fstab/esprimo.local:/" 
  [IpodCollectionFactory] "/org/kde/fstab/roundy.local:/" 
END__: virtual void IpodCollectionFactory::init() [Took: 0.33s]

The same thing also happens USB Mass Storage Collection which uses nearly identical code with simpler query: "IS StorageAccess"

diff --git a/src/core-impl/collections/ipodcollection/IpodCollectionFactory.cpp b/src/core-impl/collections/ipodcollection/IpodCollectionFactory.cpp
index 7ecdcfa..bb6f12b 100644
--- a/src/core-impl/collections/ipodcollection/IpodCollectionFactory.cpp
+++ b/src/core-impl/collections/ipodcollection/IpodCollectionFactory.cpp
@@ -16,6 +16,8 @@
 
 #include "IpodCollectionFactory.h"
 
+#define DEBUG_PREFIX "IpodCollectionFactory"
+
 #include "IpodCollection.h"
 #include "core/support/Debug.h"
 
@@ -42,14 +44,20 @@ IpodCollectionFactory::~IpodCollectionFactory()
 void
 IpodCollectionFactory::init()
 {
+    DEBUG_BLOCK
     connect( Solid::DeviceNotifier::instance(), SIGNAL(deviceAdded(QString)),
              SLOT(slotAddSolidDevice(QString)) );
     connect( Solid::DeviceNotifier::instance(), SIGNAL(deviceRemoved(QString)),
              SLOT(slotRemoveSolidDevice(QString)) );
 
+    debug() << "::allDevices():";
+    foreach( const Solid::Device &d, Solid::Device::allDevices() ) { debug() << d.udi(); }
+
     // detect iPods that were already connected on startup
     QString query( "[IS StorageAccess OR IS PortableMediaPlayer]" );
     QList<Solid::Device> ipodDevices = Solid::Device::listFromQuery( query );
+    debug() << "::listFromQuery():";
+    foreach( const Solid::Device &d, ipodDevices ) { debug() << d.udi(); }
     foreach( Solid::Device device, ipodDevices )
     {
         if( identifySolidDevice( device.udi() ) )
Comment 3 Alex Fiestas 2013-04-02 22:55:38 UTC
I'm quite certain that the bug is in udisk2 backend, method:

devicemanager.cpp
QStringList Manager::devicesFromQuery

Can you do some debugging around there?

Since parenetUdi is empty, I'm thinking that either:
deviceCache() is returning an empty stringlist

or we are failing to detect the type properly for some weird reason :/
Comment 4 Mattia 2013-05-14 19:03:47 UTC
Udisks2 backend do weird things also when detecting hard disks: see bug 311408 , could be related?
Comment 5 Matěj Laitl 2013-07-22 11:20:19 UTC
Created attachment 81261 [details]
amarok-debugging-for-afiestas.patch
Comment 6 Matěj Laitl 2013-07-22 11:20:46 UTC
Created attachment 81262 [details]
kdelibs-debugging-for-afiestas.patch
Comment 7 Matěj Laitl 2013-07-22 11:30:38 UTC
Hi Àlex,
please find the 2 debugging patches to identify the problem we've worked on at 
Akademy on bug https://bugs.kde.org/show_bug.cgi?id=317485

After applying both amarok and kdelibs patch, reproducing is as easy as 
plugging in and mounting a USB drive, starting Amarok from console as
amarok --debug --nofork
and watching for UmsCollection string.

Interesting observations:
 * The exact solid device is instantiated earlier and in that case it prints 
out product just fine.
 * Messages like 
`"/org/freedesktop/UDisks2/drives/Corsair_Flash_Voyager_A400000000025639" : 
property "Drive" does not exist` seem to be root of the cause
 * my udisks version is 2.1.0

	Matěj
Comment 8 Matěj Laitl 2013-07-30 12:57:11 UTC
*** Bug 322980 has been marked as a duplicate of this bug. ***
Comment 9 Matěj Laitl 2013-07-30 17:56:06 UTC
Git commit cfaa48278274f70fab3f6e1b610660edafbf7bbc by Matěj Laitl.
Committed on 30/07/2013 at 17:51.
Pushed by laitl into branch 'master'.

MediaDeviceCache: work-around Solid UDisks2 bug by not calling allDevices()

Kudos go to Alex Fiestas for extensive debugging and finding out the
root of the problem.

BUGFIXES:
 * Work-around Solid UDisks2 backend bug that caused USB Mass Storage
   devices and iPods not being recognized when connected before Amarok
   was started. Also fixes a bug where blank devices would appear in
   Play Media dialog. (BR 322980)

We don't need to call Solid::Device::allDevices() at all because that
was only needed for Apple iPhone support for the *old* iPod collection
that was replaced in Amarok 2.6.
Related: bug 322980
FIXED-IN: 2.8

M  +3    -0    ChangeLog
M  +0    -23   src/MediaDeviceCache.cpp

http://commits.kde.org/amarok/cfaa48278274f70fab3f6e1b610660edafbf7bbc
Comment 10 Alex Fiestas 2013-08-01 19:46:47 UTC
Git commit b92df2769477f48102ccc215f5d5eae5c4a6e5a5 by Àlex Fiestas.
Committed on 01/08/2013 at 19:24.
Pushed by afiestas into branch 'master'.

Do not clean the cache in UDisks2 backend

By deleting DeviceBackend we are invalidating all UDisks2::Devices that
are in libsolid frontend, since their m_backend will be 0
Related: bug 314544

REVIEW:111802

M  +0    -6    solid/solid/backends/udisks2/udisksmanager.cpp

http://commits.kde.org/kdelibs/b92df2769477f48102ccc215f5d5eae5c4a6e5a5
Comment 11 Alex Fiestas 2013-08-05 13:32:16 UTC
Git commit 9d7a02f3af96551eaed0bd97e9fb9ccd03d54650 by Àlex Fiestas.
Committed on 01/08/2013 at 19:24.
Pushed by afiestas into branch 'KDE/4.11'.

Do not clean the cache in UDisks2 backend

By deleting DeviceBackend we are invalidating all UDisks2::Devices that
are in libsolid frontend, since their m_backend will be 0
Related: bug 314544

REVIEW:111802
(cherry picked from commit b92df2769477f48102ccc215f5d5eae5c4a6e5a5)

M  +0    -6    solid/solid/backends/udisks2/udisksmanager.cpp

http://commits.kde.org/kdelibs/9d7a02f3af96551eaed0bd97e9fb9ccd03d54650