[kdelibs] fix ghost devices in the udisks2 Solid backend

Lukas Tinkl ltinkl at fedoraproject.org
Mon Dec 3 16:31:39 UTC 2012


commit 3966b2d0bd7499e4257e40def6a39b65e257880b
Author: Lukáš Tinkl <lukas at kde.org>
Date:   Mon Dec 3 17:31:16 2012 +0100

    fix ghost devices in the udisks2 Solid backend

 kdelibs-udisks2-backend.patch |  295 +++++++++++++++++++++++------------------
 kdelibs.spec                  |    3 +-
 2 files changed, 170 insertions(+), 128 deletions(-)
---
diff --git a/kdelibs-udisks2-backend.patch b/kdelibs-udisks2-backend.patch
index ece03b2..2657a67 100644
--- a/kdelibs-udisks2-backend.patch
+++ b/kdelibs-udisks2-backend.patch
@@ -471,10 +471,10 @@ index 0000000..19cb70a
 +#endif // UDISKS2BLOCK_H
 diff --git a/solid/solid/backends/udisks2/udisksdevice.cpp b/solid/solid/backends/udisks2/udisksdevice.cpp
 new file mode 100644
-index 0000000..b888360
+index 0000000..2a4313a
 --- /dev/null
 +++ b/solid/solid/backends/udisks2/udisksdevice.cpp
-@@ -0,0 +1,834 @@
+@@ -0,0 +1,833 @@
 +/*
 +    Copyright 2010 Michael Zanetti <mzanetti at kde.org>
 +    Copyright 2010-2012 Lukáš Tinkl <ltinkl at redhat.com>
@@ -581,7 +581,60 @@ index 0000000..b888360
 +
 +Device::~Device()
 +{
++}
++
++QString Device::udi() const
++{
++    if (m_backend) {
++        return m_backend->udi();
++    }
++
++    return QString();
++}
++
++QVariant Device::prop(const QString &key) const
++{
++    if (m_backend) {
++        return m_backend->prop(key);
++    }
++
++    return QVariant();
++}
++
++bool Device::propertyExists(const QString &key) const
++{
++    if (m_backend) {
++        return m_backend->propertyExists(key);
++    }
++
++    return false;
++}
++
++QVariantMap Device::allProperties() const
++{
++    if (m_backend) {
++        return m_backend->allProperties();
++    }
++
++    return QVariantMap();
++}
++
++bool Device::hasInterface(const QString &name) const
++{
++    if (m_backend) {
++        return m_backend->interfaces().contains(name);
++    }
++
++    return false;
++}
 +
++QStringList Device::interfaces() const
++{
++    if (m_backend) {
++        return m_backend->interfaces();
++    }
++
++    return QStringList();
 +}
 +
 +QObject* Device::createDeviceInterface(const Solid::DeviceInterface::Type& type)
@@ -1105,15 +1158,6 @@ index 0000000..b888360
 +    return prop("Vendor").toString();
 +}
 +
-+QString Device::udi() const
-+{
-+    if (m_backend) {
-+        return m_backend->udi;
-+    }
-+
-+    return QString();
-+}
-+
 +QString Device::parentUdi() const
 +{
 +    QString parent;
@@ -1128,51 +1172,6 @@ index 0000000..b888360
 +    return parent;
 +}
 +
-+QVariant Device::prop(const QString &key) const
-+{
-+    if (m_backend) {
-+        return m_backend->prop(key);
-+    }
-+
-+    return QVariant();
-+}
-+
-+bool Device::propertyExists(const QString &key) const
-+{
-+    if (m_backend) {
-+        return m_backend->propertyExists(key);
-+    }
-+
-+    return false;
-+}
-+
-+QVariantMap Device::allProperties() const
-+{
-+    if (m_backend) {
-+        return m_backend->allProperties();
-+    }
-+
-+    return QVariantMap();
-+}
-+
-+bool Device::hasInterface(const QString &name) const
-+{
-+    if (m_backend) {
-+        return m_backend->interfaces().contains(name);
-+    }
-+
-+    return false;
-+}
-+
-+QStringList Device::interfaces() const
-+{
-+    if (m_backend) {
-+        return m_backend->interfaces();
-+    }
-+
-+    return QStringList();
-+}
-+
 +QString Device::errorToString(const QString & error) const
 +{
 +    if (error == UD2_ERROR_UNAUTHORIZED || error == UD2_ERROR_NOT_AUTHORIZED)
@@ -1311,10 +1310,10 @@ index 0000000..b888360
 +}
 diff --git a/solid/solid/backends/udisks2/udisksdevice.h b/solid/solid/backends/udisks2/udisksdevice.h
 new file mode 100644
-index 0000000..90bb042
+index 0000000..6038178
 --- /dev/null
 +++ b/solid/solid/backends/udisks2/udisksdevice.h
-@@ -0,0 +1,103 @@
+@@ -0,0 +1,104 @@
 +/*
 +    Copyright 2010 Michael Zanetti <mzanetti at kde.org>
 +    Copyright 2010-2012 Lukáš Tinkl <ltinkl at redhat.com>
@@ -1406,11 +1405,12 @@ index 0000000..90bb042
 +    void changed();
 +    void propertyChanged(const QMap<QString,int> &changes);
 +
++protected:
++    QPointer<DeviceBackend> m_backend;
++
 +private:
 +    QString storageDescription() const;
 +    QString volumeDescription() const;
-+
-+    DeviceBackend *m_backend;
 +};
 +
 +}
@@ -1420,10 +1420,10 @@ index 0000000..90bb042
 +#endif // UDISKS2DEVICE_H
 diff --git a/solid/solid/backends/udisks2/udisksdevicebackend.cpp b/solid/solid/backends/udisks2/udisksdevicebackend.cpp
 new file mode 100644
-index 0000000..ffa98db
+index 0000000..8131cb5
 --- /dev/null
 +++ b/solid/solid/backends/udisks2/udisksdevicebackend.cpp
-@@ -0,0 +1,205 @@
+@@ -0,0 +1,239 @@
 +/*
 +    Copyright 2010 Michael Zanetti <mzanetti at kde.org>
 +    Copyright 2010-2012 Lukáš Tinkl <ltinkl at redhat.com>
@@ -1477,16 +1477,25 @@ index 0000000..ffa98db
 +    return backend;
 +}
 +
++void DeviceBackend::destroyBackend(const QString& udi)
++{
++    if (s_backends.contains(udi)) {
++        DeviceBackend *backend = s_backends.value(udi);
++        s_backends.remove(udi);
++        delete backend;
++    }
++}
++
 +DeviceBackend::DeviceBackend(const QString& udi)
-+    : udi(udi)
++    : m_udi(udi)
 +{
-+    qDebug() << "Creating backend for device" << udi;
-+    m_device = new QDBusInterface(UD2_DBUS_SERVICE, udi,
++    qDebug() << "Creating backend for device" << m_udi;
++    m_device = new QDBusInterface(UD2_DBUS_SERVICE, m_udi,
 +                                  QString(), // no interface, we aggregate them
 +                                  QDBusConnection::systemBus(), this);
 +
 +    if (m_device->isValid()) {
-+        QDBusConnection::systemBus().connect(UD2_DBUS_SERVICE, udi, DBUS_INTERFACE_PROPS, "PropertiesChanged", this,
++        QDBusConnection::systemBus().connect(UD2_DBUS_SERVICE, m_udi, DBUS_INTERFACE_PROPS, "PropertiesChanged", this,
 +                                            SLOT(slotPropertiesChanged(QString,QVariantMap,QStringList)));
 +        QDBusConnection::systemBus().connect(UD2_DBUS_SERVICE, UD2_DBUS_PATH, DBUS_INTERFACE_MANAGER, "InterfacesAdded",
 +                                            this, SLOT(slotInterfacesAdded(QDBusObjectPath,QVariantMapMap)));
@@ -1499,20 +1508,33 @@ index 0000000..ffa98db
 +
 +DeviceBackend::~DeviceBackend()
 +{
++    qDebug() << "Destroying backend for device" << m_udi;
 +}
 +
 +void DeviceBackend::initInterfaces()
 +{
 +    m_interfaces.clear();
++
 +    const QString xmlData = introspect();
++    if (xmlData.isEmpty()) {
++        qDebug() << m_udi << "has no interfaces!";
++        return;
++    }
++
 +    QDomDocument dom;
 +    dom.setContent(xmlData);
++
 +    QDomNodeList ifaceNodeList = dom.elementsByTagName("interface");
 +    for (int i = 0; i < ifaceNodeList.count(); i++) {
 +        QDomElement ifaceElem = ifaceNodeList.item(i).toElement();
-+        if (!ifaceElem.isNull())
++        /* Accept only org.freedesktop.UDisks2.* interfaces so that when the device is unplugged,
++         * m_interfaces goes empty and we can easily verify that the device is gone. */
++        if (!ifaceElem.isNull() && ifaceElem.attribute("name").startsWith(UD2_DBUS_SERVICE)) {
 +            m_interfaces.append(ifaceElem.attribute("name"));
++        }
 +    }
++
++    qDebug() << m_udi << "has interfaces:" << m_interfaces;
 +}
 +
 +QStringList DeviceBackend::interfaces() const
@@ -1520,42 +1542,30 @@ index 0000000..ffa98db
 +    return m_interfaces;
 +}
 +
-+
-+QString DeviceBackend::introspect() const
++const QString& DeviceBackend::udi() const
 +{
-+    QDBusMessage call = QDBusMessage::createMethodCall(UD2_DBUS_SERVICE, udi,
-+                                                    DBUS_INTERFACE_INTROSPECT, "Introspect");
-+    QDBusPendingReply<QString> reply = QDBusConnection::systemBus().call(call);
-+
-+    if (reply.isValid())
-+        return reply.value();
-+    else {
-+        return QString();
-+    }
++    return m_udi;
 +}
 +
-+QVariant DeviceBackend::prop(const QString& key)
++QVariant DeviceBackend::prop(const QString& key) const
 +{
 +    checkCache(key);
 +    return m_propertyCache.value(key);
 +}
 +
-+bool DeviceBackend::propertyExists(const QString& key)
++bool DeviceBackend::propertyExists(const QString& key) const
 +{
 +    checkCache(key);
-+    return m_propertyCache.contains(key);
++    /* checkCache() will put an invalid QVariant in cache when the property
++     * does not exist, so check for validity, not for an actual presence. */
++    return m_propertyCache.value(key).isValid();
 +}
 +
-+
 +QVariantMap DeviceBackend::allProperties() const
 +{
-+    QDBusMessage call = QDBusMessage::createMethodCall(UD2_DBUS_SERVICE, udi, DBUS_INTERFACE_PROPS, "GetAll");
++    QDBusMessage call = QDBusMessage::createMethodCall(UD2_DBUS_SERVICE, m_udi, DBUS_INTERFACE_PROPS, "GetAll");
 +
 +    Q_FOREACH (const QString & iface, m_interfaces) {
-+        if (iface.startsWith("org.freedesktop.DBus")) {
-+            continue;
-+        }
-+
 +        call.setArguments(QVariantList() << iface);
 +        QDBusPendingReply<QVariantMap> reply = QDBusConnection::systemBus().call(call);
 +
@@ -1570,6 +1580,19 @@ index 0000000..ffa98db
 +    return m_propertyCache;
 +}
 +
++QString DeviceBackend::introspect() const
++{
++    QDBusMessage call = QDBusMessage::createMethodCall(UD2_DBUS_SERVICE, m_udi,
++                                                    DBUS_INTERFACE_INTROSPECT, "Introspect");
++    QDBusPendingReply<QString> reply = QDBusConnection::systemBus().call(call);
++
++    if (reply.isValid())
++        return reply.value();
++    else {
++        return QString();
++    }
++}
++
 +void DeviceBackend::checkCache(const QString& key) const
 +{
 +    if (m_propertyCache.isEmpty()) { // recreate the cache
@@ -1581,17 +1604,19 @@ index 0000000..ffa98db
 +    }
 +
 +    QVariant reply = m_device->property(key.toUtf8());
++    m_propertyCache.insert(key, reply);
 +
-+    if (reply.isValid()) {
-+        m_propertyCache.insert(key, reply);
-+    } else {
-+        qWarning() << "got invalid reply for cache:" << key;
++    if (!reply.isValid()) {
++        /* Store the item in the cache anyway so next time we don't have to
++         * do the DBus call to find out it does not exist but just check whether
++         * prop(key).isValid() */
++        qDebug() << m_udi << ": property" << key << "does not exist";
 +    }
 +}
 +
 +void DeviceBackend::slotPropertiesChanged(const QString& ifaceName, const QVariantMap& changedProps, const QStringList& invalidatedProps)
 +{
-+    qDebug() << udi << "'s interface" << ifaceName << "changed props:";
++    qDebug() << m_udi << "'s interface" << ifaceName << "changed props:";
 +
 +    QMap<QString, int> changeMap;
 +
@@ -1616,25 +1641,34 @@ index 0000000..ffa98db
 +
 +void DeviceBackend::slotInterfacesAdded(const QDBusObjectPath& object_path, const QVariantMapMap& interfaces_and_properties)
 +{
-+    if (object_path.path() == udi) {
-+        m_interfaces.append(interfaces_and_properties.keys());
++    if (object_path.path() != m_udi) {
++        return;
++    }
++
++    Q_FOREACH(const QString & iface, interfaces_and_properties.keys()) {
++        /* Don't store generic DBus interfaces */
++        if (iface.startsWith(UD2_DBUS_SERVICE)) {
++            m_interfaces.append(interfaces_and_properties.keys());
++        }
 +    }
 +}
 +
 +void DeviceBackend::slotInterfacesRemoved(const QDBusObjectPath& object_path, const QStringList& interfaces)
 +{
-+    if (object_path.path() == udi) {
-+        Q_FOREACH(const QString & iface, interfaces) {
-+            m_interfaces.removeAll(iface);
-+        }
++    if (object_path.path() != m_udi) {
++        return;
++    }
++
++    Q_FOREACH(const QString & iface, interfaces) {
++        m_interfaces.removeAll(iface);
 +    }
 +}
 diff --git a/solid/solid/backends/udisks2/udisksdevicebackend.h b/solid/solid/backends/udisks2/udisksdevicebackend.h
 new file mode 100644
-index 0000000..4953ad5
+index 0000000..829fa41
 --- /dev/null
 +++ b/solid/solid/backends/udisks2/udisksdevicebackend.h
-@@ -0,0 +1,83 @@
+@@ -0,0 +1,84 @@
 +/*
 +    Copyright 2010 Michael Zanetti <mzanetti at kde.org>
 +    Copyright 2010-2012 Lukáš Tinkl <ltinkl at redhat.com>
@@ -1678,21 +1712,17 @@ index 0000000..4953ad5
 +
 +  public:
 +    static DeviceBackend* backendForUDI(const QString &udi);
++    static void destroyBackend(const QString &udi);
 +
 +    DeviceBackend(const QString &udi);
 +    ~DeviceBackend();
 +
-+    void initInterfaces();
-+    QString introspect() const;
-+
-+    QVariant prop(const QString &key);
-+    bool propertyExists(const QString &key);
++    QVariant prop(const QString &key) const;
++    bool propertyExists(const QString &key) const;
 +    QVariantMap allProperties() const;
-+    void checkCache(const QString &key) const;
 +
 +    QStringList interfaces() const;
-+
-+    QString udi;
++    const QString & udi() const;
 +
 +  Q_SIGNALS:
 +    void propertyChanged(const QMap<QString, int> &changeMap);
@@ -1704,10 +1734,15 @@ index 0000000..4953ad5
 +    void slotPropertiesChanged(const QString &ifaceName, const QVariantMap &changedProps, const QStringList &invalidatedProps);
 +
 +  private:
++    void initInterfaces();
++    QString introspect() const;
++    void checkCache(const QString &key) const;
++
 +    QDBusInterface *m_device;
 +
 +    mutable QVariantMap m_propertyCache;
 +    QStringList m_interfaces;
++    QString m_udi;
 +
 +    static QMap<QString, DeviceBackend*> s_backends;
 +
@@ -2036,10 +2071,10 @@ index 0000000..d225f32
 +#endif // SOLID_BACKENDS_UDISKS2_GENERICINTERFACE_H
 diff --git a/solid/solid/backends/udisks2/udisksmanager.cpp b/solid/solid/backends/udisks2/udisksmanager.cpp
 new file mode 100644
-index 0000000..69e053f
+index 0000000..35b0d23
 --- /dev/null
 +++ b/solid/solid/backends/udisks2/udisksmanager.cpp
-@@ -0,0 +1,238 @@
+@@ -0,0 +1,248 @@
 +/*
 +    Copyright 2012 Lukáš Tinkl <ltinkl at redhat.com>
 +
@@ -2061,6 +2096,7 @@ index 0000000..69e053f
 +*/
 +
 +#include "udisksmanager.h"
++#include "udisksdevicebackend.h"
 +
 +#include <QtCore/QCoreApplication>
 +#include <QtCore/QDebug>
@@ -2118,7 +2154,10 @@ index 0000000..69e053f
 +
 +Manager::~Manager()
 +{
-+    QDBusConnection::systemBus().disconnectFromBus("Solid::UDisks2");
++    while (!m_deviceCache.isEmpty()) {
++        QString udi = m_deviceCache.takeFirst();
++        DeviceBackend::destroyBackend(udi);
++    }
 +}
 +
 +QObject* Manager::createDevice(const QString& udi)
@@ -2170,7 +2209,11 @@ index 0000000..69e053f
 +
 +QStringList Manager::allDevices()
 +{
-+    m_deviceCache.clear();
++    /* Clear the cache, destroy all backends */
++    while (!m_deviceCache.isEmpty()) {
++        QString udi= m_deviceCache.takeFirst();
++        DeviceBackend::destroyBackend(udi);
++    }
 +
 +    introspect("/org/freedesktop/UDisks2/block_devices", true /*checkOptical*/);
 +    introspect("/org/freedesktop/UDisks2/drives");
@@ -2245,6 +2288,7 @@ index 0000000..69e053f
 +    if (!udi.isEmpty() && (interfaces.isEmpty() || device.interfaces().isEmpty() || device.interfaces().contains(UD2_DBUS_INTERFACE_FILESYSTEM))) {
 +        Q_EMIT deviceRemoved(udi);
 +        m_deviceCache.removeAll(udi);
++        DeviceBackend::destroyBackend(udi);
 +    }
 +}
 +
@@ -2267,6 +2311,7 @@ index 0000000..69e053f
 +    if (m_deviceCache.contains(udi) && size == 0) {  // we know the optdisc, got removed
 +        Q_EMIT deviceRemoved(udi);
 +        m_deviceCache.removeAll(udi);
++        DeviceBackend::destroyBackend(udi);
 +    }
 +}
 +
@@ -2734,7 +2779,7 @@ index 0000000..0cdcc66
 +#endif // UDISKS2OPTICALDISC_H
 diff --git a/solid/solid/backends/udisks2/udisksopticaldrive.cpp b/solid/solid/backends/udisks2/udisksopticaldrive.cpp
 new file mode 100644
-index 0000000..74a901e
+index 0000000..4df18b1
 --- /dev/null
 +++ b/solid/solid/backends/udisks2/udisksopticaldrive.cpp
 @@ -0,0 +1,226 @@
@@ -2803,7 +2848,7 @@ index 0000000..74a901e
 +    m_device->broadcastActionRequested("eject");
 +
 +    const QString path = m_device->udi();
-+    QDBusConnection c = QDBusConnection::systemBus();
++    QDBusConnection c = QDBusConnection::connectToBus(QDBusConnection::SystemBus, "Solid::Udisks2::OpticalDrive::" + path);
 +
 +    // if the device is mounted, unmount first
 +    QString blockPath;
@@ -3952,13 +3997,14 @@ index 0000000..2ca04d2
 +
 +#endif // UDISKS2STORAGEVOLUME_H
 diff --git a/solid/solid/managerbase.cpp b/solid/solid/managerbase.cpp
-index fb5a67c..c7005ff 100644
+index fb5a67c..dc1700d 100644
 --- a/solid/solid/managerbase.cpp
 +++ b/solid/solid/managerbase.cpp
-@@ -30,8 +30,11 @@
+@@ -29,9 +29,11 @@
+ 
  #if defined (Q_OS_MAC)
  #include "backends/iokit/iokitmanager.h"
- #elif defined (Q_OS_UNIX)
+-#elif defined (Q_OS_UNIX)
 -#include "backends/hal/halmanager.h"
 +#if defined (WITH_SOLID_UDISKS2)
 +#include "backends/udisks2/udisksmanager.h"
@@ -3968,7 +4014,7 @@ index fb5a67c..c7005ff 100644
  #include "backends/upower/upowermanager.h"
  
  #if defined (HUPNP_FOUND)
-@@ -71,22 +74,17 @@ void Solid::ManagerBasePrivate::loadBackends()
+@@ -71,22 +73,17 @@ void Solid::ManagerBasePrivate::loadBackends()
  #        elif defined(Q_WS_WIN) && defined(HAVE_WBEM) && !defined(_WIN32_WCE)
              m_backends << new Solid::Backends::Wmi::WmiManager(0);
  
@@ -3995,20 +4041,15 @@ index fb5a67c..c7005ff 100644
  #        endif
  
  #        if defined (HUPNP_FOUND)
-@@ -99,5 +97,3 @@ QList<QObject*> Solid::ManagerBasePrivate::managerBackends() const
- {
-     return m_backends;
- }
--
--
 diff --git a/solid/tests/CMakeLists.txt b/solid/tests/CMakeLists.txt
-index ef507d1..9212741 100644
+index ef507d1..b9f3720 100644
 --- a/solid/tests/CMakeLists.txt
 +++ b/solid/tests/CMakeLists.txt
-@@ -16,20 +16,6 @@ target_link_libraries(fakehardwaretest solid_static ${QT_QTCORE_LIBRARY} ${QT_QT
+@@ -15,21 +15,6 @@ endif(WIN32)
+ target_link_libraries(fakehardwaretest solid_static ${QT_QTCORE_LIBRARY} ${QT_QTDBUS_LIBRARY} ${QT_QTXML_LIBRARY} ${QT_QTTEST_LIBRARY} )
  add_definitions(-DTEST_DATA="\\"${CMAKE_CURRENT_SOURCE_DIR}/../solid/backends/fakehw/fakecomputer.xml\\"")
  
- 
+-
 -########### halbasictest ###############
 -
 -if(NOT WIN32 AND NOT APPLE)
diff --git a/kdelibs.spec b/kdelibs.spec
index 200ed90..0a90c8a 100644
--- a/kdelibs.spec
+++ b/kdelibs.spec
@@ -596,8 +596,9 @@ rm -rf %{buildroot}
 
 
 %changelog
-* Mon Dec 03 2012 Than Ngo <than at redhat.com> - 4.9.4-1
+* Mon Dec 03 2012 Than Ngo <than at redhat.com> - 6:4.9.4-1
 - 4.9.4
+- update udisks2 backend patch to fix ghost devices
 
 * Fri Nov 30 2012 Dan Vrátil <dvratil at redhat.com> - 6:4.9.3-7
 - update udisks2 backend patch


More information about the scm-commits mailing list