[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: Re: [kdelibs/frameworks] tier1/solid/solid/backends/udisks2: more fixes
From: Stephen Kelly <steveire () gmail ! com>
Date: 2012-02-24 15:31:22
Message-ID: ji8aga$gvc$2 () dough ! gmane ! org
[Download RAW message or body]
Lukas Tinkl wrote:
> Git commit 7590ee0e18709f26717420b398bd411eaafb4099 by Lukas Tinkl.
> Committed on 24/02/2012 at 14:37.
> Pushed by lukas into branch 'frameworks'.
>
> more fixes
This isn't a very good commit message. The patch does too many different
things.
Consider learning to use git add -p to create multiple patches from a big
diff.
Thanks,
Steve.
>
> - more accurate and faster device enumeration and categorization, using
> introspection - better handling of encrypted storage access
>
> M +3 -0 tier1/solid/solid/backends/udisks2/udisks2.h
> M +77 -14 tier1/solid/solid/backends/udisks2/udisksdevice.cpp
> M +13 -3 tier1/solid/solid/backends/udisks2/udisksdevice.h
> M +0 -1 tier1/solid/solid/backends/udisks2/udisksmanager.cpp
> M +21 -28 tier1/solid/solid/backends/udisks2/udisksstorageaccess.cpp
> M +4 -3 tier1/solid/solid/backends/udisks2/udisksstorageaccess.h
> M +8 -4 tier1/solid/solid/backends/udisks2/udisksstoragevolume.cpp
>
> http://commits.kde.org/kdelibs/7590ee0e18709f26717420b398bd411eaafb4099
>
> diff --git a/tier1/solid/solid/backends/udisks2/udisks2.h
> b/tier1/solid/solid/backends/udisks2/udisks2.h index a038c09..39d8cb7
> 100644 --- a/tier1/solid/solid/backends/udisks2/udisks2.h
> +++ b/tier1/solid/solid/backends/udisks2/udisks2.h
> @@ -43,12 +43,15 @@ Q_DECLARE_METATYPE(DBUSManagerStruct)
> #define UD2_DBUS_PATH_MANAGER
> #"/org/freedesktop/UDisks2/Manager"
> #define UD2_DBUS_PATH_DRIVES
> #"/org/freedesktop/UDisks2/drives/"
> #define DBUS_INTERFACE_PROPS
> #"org.freedesktop.DBus.Properties"
> +#define DBUS_INTERFACE_INTROSPECT
> "org.freedesktop.DBus.Introspectable"
> #define DBUS_INTERFACE_MANAGER
> #"org.freedesktop.DBus.ObjectManager"
> #define UD2_DBUS_INTERFACE_BLOCK "org.freedesktop.UDisks2.Block"
> #define UD2_DBUS_INTERFACE_DRIVE "org.freedesktop.UDisks2.Drive"
> #define UD2_DBUS_INTERFACE_PARTITION
> #"org.freedesktop.UDisks2.Partition"
> +#define UD2_DBUS_INTERFACE_PARTITIONTABLE
> "org.freedesktop.UDisks2.PartitionTable"
> #define UD2_DBUS_INTERFACE_FILESYSTEM
> #"org.freedesktop.UDisks2.Filesystem"
> #define UD2_DBUS_INTERFACE_ENCRYPTED
> #"org.freedesktop.UDisks2.Encrypted"
> +#define UD2_DBUS_INTERFACE_SWAP
> "org.freedesktop.UDisks2.Swapspace"
>
> /* errors */
> #define UD2_ERROR_UNAUTHORIZED
> #"org.freedesktop.PolicyKit.Error.NotAuthorized"
> diff --git a/tier1/solid/solid/backends/udisks2/udisksdevice.cpp
> b/tier1/solid/solid/backends/udisks2/udisksdevice.cpp index
> b3be15b..4fdbbf0 100644 ---
> a/tier1/solid/solid/backends/udisks2/udisksdevice.cpp +++
> b/tier1/solid/solid/backends/udisks2/udisksdevice.cpp @@ -33,13 +33,14 @@
> #include <solid/deviceinterface.h>
> #include <solid/device.h>
>
> -#include <QtCore/QStringList>
> #include <QtCore/QDebug>
>
> #include <QtDBus/QDBusMessage>
> #include <QtDBus/QDBusMetaType>
> #include <QtDBus/QDBusPendingReply>
>
> +#include <QtXml/QDomDocument>
> +
> using namespace Solid::Backends::UDisks2;
>
> // Adapted from KLocale as Solid needs to be Qt-only
> @@ -96,9 +97,12 @@ Device::Device(const QString &udi)
> QString(), // no interface, we
> aggregate them
> QDBusConnection::systemBus());
>
> - if (m_device->isValid())
> + if (m_device->isValid()) {
> QDBusConnection::systemBus().connect(UD2_DBUS_SERVICE, m_udi,
> DBUS_INTERFACE_PROPS, "PropertiesChanged", this,
>
SLOT(slotPropertiesChanged(QString,QVariantMap,QStringList)));
> +
> + initInterfaces();
> + }
> }
>
> Device::~Device()
> @@ -150,7 +154,7 @@ bool Device::queryDeviceInterface(const
> Solid::DeviceInterface::Type& type) cons
> case Solid::DeviceInterface::Block:
> return isBlock();
> case Solid::DeviceInterface::StorageVolume: // partition
> - return isPartition();
> + return isStorageVolume();
> case Solid::DeviceInterface::StorageAccess: // filesystem
> return isStorageAccess();
> case Solid::DeviceInterface::StorageDrive:
> @@ -485,7 +489,7 @@ QString Device::volumeDescription() const
> const bool drive_is_hotpluggable = storageDrive.isHotpluggable();
>
> QString size_str = formatByteSize(storageVolume.size());
> - if (isEncrypted())
> + if (isEncryptedContainer())
> {
> if (!size_str.isEmpty())
> description = QObject::tr("%1 Encrypted Container", "%1 is
> the size").arg(size_str);
> @@ -672,6 +676,20 @@ void Device::checkCache(const QString &key) const
> }
> }
>
> +QString Device::introspect() const
> +{
> + QDBusMessage call = QDBusMessage::createMethodCall(UD2_DBUS_SERVICE,
> m_udi,
> +
> DBUS_INTERFACE_INTROSPECT, "Introspect");
> + QDBusPendingReply<QString> reply =
> QDBusConnection::systemBus().asyncCall(call);
> + reply.waitForFinished();
> +
> + if (reply.isValid())
> + return reply.value();
> + else {
> + return QString();
> + }
> +}
> +
> QVariant Device::prop(const QString &key) const
> {
> checkCache(key);
> @@ -699,9 +717,34 @@ QVariantMap Device::allProperties() const
> return m_cache;
> }
>
> +bool Device::hasInterface(const QString &name) const
> +{
> + return m_interfaces.contains(name);
> +}
> +
> +QStringList Device::interfaces() const
> +{
> + return m_interfaces;
> +}
> +
> +void Device::initInterfaces()
> +{
> + m_interfaces.clear();
> + const QString xmlData = introspect();
> + 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())
> + m_interfaces.append(ifaceElem.attribute("name"));
> + }
> +}
> +
> void Device::slotPropertiesChanged(const QString &ifaceName, const
> QVariantMap &changedProps, const QStringList &invalidatedProps)
> {
> Q_UNUSED(ifaceName);
> + Q_UNUSED(invalidatedProps);
>
> QMapIterator<QString, QVariant> i(changedProps);
> while (i.hasNext()) {
> @@ -709,10 +752,6 @@ void Device::slotPropertiesChanged(const QString
> &ifaceName, const QVariantMap &
> m_cache.insert(i.key(), i.value()); // replace the value
> }
>
> - Q_FOREACH(const QString & key, invalidatedProps) {
> - m_cache.remove(key);
> - }
> -
> Q_EMIT changed();
> }
>
> @@ -767,22 +806,32 @@ Solid::ErrorType Device::errorToSolidError(const
> QString & error) const
>
> bool Device::isBlock() const
> {
> - return prop("DeviceNumber").toULongLong() > 0;
> + return hasInterface(UD2_DBUS_INTERFACE_BLOCK);
> }
>
> bool Device::isPartition() const
> {
> - return isStorageAccess() || isOpticalDisc();
> + return hasInterface(UD2_DBUS_INTERFACE_PARTITION);
> +}
> +
> +bool Device::isPartitionTable() const
> +{
> + return hasInterface(UD2_DBUS_INTERFACE_PARTITIONTABLE);
> +}
> +
> +bool Device::isStorageVolume() const
> +{
> + return isPartition() || isPartitionTable() || isStorageAccess() ||
> isOpticalDisc();
> }
>
> bool Device::isStorageAccess() const
> {
> - return prop("IdUsage").toString()=="filesystem" ||
> prop("IdUsage").toString()=="crypto";
> + return hasInterface(UD2_DBUS_INTERFACE_FILESYSTEM) ||
> isEncryptedContainer();
> }
>
> bool Device::isDrive() const
> {
> - return m_udi.startsWith(UD2_DBUS_PATH_DRIVES);
> + return hasInterface(UD2_DBUS_INTERFACE_DRIVE);
> }
>
> bool Device::isOpticalDrive() const
> @@ -805,7 +854,21 @@ bool Device::isMounted() const
> return propertyExists("MountPoints") &&
> !prop("MountPoints").value<QByteArrayList>().isEmpty();
> }
>
> -bool Device::isEncrypted() const
> +bool Device::isEncryptedContainer() const
> +{
> + return hasInterface(UD2_DBUS_INTERFACE_ENCRYPTED);
> +}
> +
> +bool Device::isEncryptedCleartext() const
> +{
> + const QString holderDevice = prop("CryptoBackingDevice").toString();
> + if (holderDevice.isEmpty() || holderDevice == "/")
> + return false;
> + else
> + return true;
> +}
> +
> +bool Device::isSwap() const
> {
> - return prop("IdUsage").toString() == "crypto";
> + return hasInterface(UD2_DBUS_INTERFACE_SWAP);
> }
> diff --git a/tier1/solid/solid/backends/udisks2/udisksdevice.h
> b/tier1/solid/solid/backends/udisks2/udisksdevice.h index 6ad531e..aaacd3b
> 100644 --- a/tier1/solid/solid/backends/udisks2/udisksdevice.h
> +++ b/tier1/solid/solid/backends/udisks2/udisksdevice.h
> @@ -27,7 +27,7 @@
> #include <solid/solidnamespace.h>
>
> #include <QtDBus/QDBusInterface>
> -#include <QtCore/QSet>
> +#include <QtCore/QStringList>
>
> namespace Solid
> {
> @@ -43,7 +43,6 @@ public:
> Device(const QString &udi);
> virtual ~Device();
>
> -
> virtual QObject* createDeviceInterface(const
> Solid::DeviceInterface::Type& type); virtual bool
> queryDeviceInterface(const Solid::DeviceInterface::Type& type) const;
> virtual QString description() const;
> @@ -58,17 +57,24 @@ public:
> bool propertyExists(const QString &key) const;
> QVariantMap allProperties() const;
>
> + bool hasInterface(const QString & name) const;
> + QStringList interfaces() const;
> +
> QString errorToString(const QString & error) const;
> Solid::ErrorType errorToSolidError(const QString & error) const;
>
> bool isBlock() const;
> bool isPartition() const;
> + bool isPartitionTable() const;
> + bool isStorageVolume() const;
> bool isStorageAccess() const;
> bool isDrive() const;
> bool isOpticalDrive() const;
> bool isOpticalDisc() const;
> bool isMounted() const;
> - bool isEncrypted() const;
> + bool isEncryptedContainer() const;
> + bool isEncryptedCleartext() const;
> + bool isSwap() const;
>
> Q_SIGNALS:
> void changed();
> @@ -83,7 +89,11 @@ private:
> QString m_udi;
> mutable QVariantMap m_cache;
>
> + void initInterfaces();
> + QStringList m_interfaces;
> +
> void checkCache(const QString &key) const;
> + QString introspect() const;
> };
>
> }
> diff --git a/tier1/solid/solid/backends/udisks2/udisksmanager.cpp
> b/tier1/solid/solid/backends/udisks2/udisksmanager.cpp index
> 3fcc5fe..8b1e966 100644 ---
> a/tier1/solid/solid/backends/udisks2/udisksmanager.cpp +++
> b/tier1/solid/solid/backends/udisks2/udisksmanager.cpp @@ -135,7 +135,6 @@
> QStringList Manager::allDevices()
> const QString udi = path.path();
> if (udi == UD2_DBUS_PATH_MANAGER)
> continue;
> - const QVariantMapMap interfaces =
> reply.value().values().keys(); //FIXME change m_deviceCache to
> QMap<QString, QStringList>
> m_deviceCache.append(udi);
> }
> }
> diff --git a/tier1/solid/solid/backends/udisks2/udisksstorageaccess.cpp
> b/tier1/solid/solid/backends/udisks2/udisksstorageaccess.cpp index
> 113bebf..b7687d0 100644 ---
> a/tier1/solid/solid/backends/udisks2/udisksstorageaccess.cpp +++
> b/tier1/solid/solid/backends/udisks2/udisksstorageaccess.cpp @@ -32,7
> +32,7 @@ using namespace Solid::Backends::UDisks2;
> StorageAccess::StorageAccess(Device *device)
> : DeviceInterface(device), m_setupInProgress(false),
> : m_teardownInProgress(false), m_passphraseRequested(false)
> {
> - connect(device, SIGNAL(changed()), this, SLOT(slotChanged()));
> + connect(device, SIGNAL(changed()), this, SLOT(slotChanged())); //
> FIXME only listen to MountPoints changes
> updateCache();
>
> // Delay connecting to DBus signals to avoid the related time penalty
> @@ -58,13 +58,13 @@ void StorageAccess::connectDBusSignals()
> bool StorageAccess::isLuksDevice() const
> {
> // FIXME check
> - return m_device->isEncrypted(); // encrypted (and unlocked) device
> + return m_device->isEncryptedContainer(); // encrypted (and unlocked)
> device
> }
>
> bool StorageAccess::isAccessible() const
> {
> if (isLuksDevice()) { // check if the cleartext slave is mounted
> - Device
> holderDevice(m_device-
>prop("CryptoBackingDevice").value<QDBusObjectPath>().path());
> + Device holderDevice(m_clearTextPath);
> return holderDevice.isMounted();
> }
>
> @@ -79,10 +79,9 @@ QString StorageAccess::filePath() const
> QByteArrayList mntPoints;
>
> if (isLuksDevice()) { // encrypted (and unlocked) device
> - QString path =
> m_device->prop("CryptoBackingDevice").value<QDBusObjectPath>().path();
> - if (path.isEmpty() || path == "/")
> + if (m_clearTextPath.isEmpty() || m_clearTextPath == "/")
> return QString();
> - Device holderDevice(path);
> + Device holderDevice(m_clearTextPath);
> mntPoints =
> holderDevice.prop("MountPoints").value<QByteArrayList>(); if
> (!mntPoints.isEmpty())
> return QFile::decodeName(mntPoints.first()); // FIXME Solid
> doesn't support multiple mount points
> @@ -110,7 +109,7 @@ bool StorageAccess::setup()
> m_setupInProgress = true;
> m_device->broadcastActionRequested("setup");
>
> - if (m_device->isEncrypted())
> + if (m_device->isEncryptedContainer())
> return requestPassphrase();
> else
> return mount();
> @@ -144,13 +143,13 @@ void StorageAccess::updateCache()
>
> void StorageAccess::slotDBusReply( const QDBusMessage & reply )
> {
> - Q_UNUSED(reply);
> -
> if (m_setupInProgress)
> {
> - if (isLuksDevice() && !isAccessible()) // unlocked device, now
> mount it
> + if (isLuksDevice() && !isAccessible()) { // unlocked device, now
> mount it
> + if (reply.type() == QDBusMessage::ReplyMessage)
> + m_clearTextPath =
> reply.arguments().value(0).value<QDBusObjectPath>().path();
> mount();
> -
> + }
> else // Don't broadcast setupDone unless the setup is really
> done. (Fix kde#271156)
> {
> m_setupInProgress = false;
> @@ -161,17 +160,16 @@ void StorageAccess::slotDBusReply( const
> QDBusMessage & reply )
> }
> else if (m_teardownInProgress) // FIXME
> {
> - QString clearTextPath =
> m_device->prop("LuksHolder").value<QDBusObjectPath>().path();
> - if (isLuksDevice() && clearTextPath != "/") // unlocked device,
> lock it
> + if (isLuksDevice() && !m_clearTextPath.isEmpty() &&
> m_clearTextPath != "/") // unlocked device, lock it
> {
> callCryptoTeardown();
> }
> - else if (m_device->prop("DeviceIsLuksCleartext").toBool()) {
> + else if (!m_clearTextPath.isEmpty() && m_clearTextPath != "/") {
> callCryptoTeardown(true); // Lock crypted parent
> }
> else
> {
> - if (m_device->prop("Ejectable").toBool() &&
> !m_device->prop("Optical").toBool()) // optical drives have their Eject
> method
> + if (m_device->prop("Ejectable").toBool() &&
> !m_device->isOpticalDrive()) // optical drives have their Eject method
> {
> // try to "eject" (aka safely remove) from the (parent)
> drive, e.g. SD card from a reader QString drivePath =
> m_device->prop("Drive").value<QDBusObjectPath>().path();
> @@ -182,7 +180,6 @@ void StorageAccess::slotDBusReply( const QDBusMessage
> & reply )
> msg << QVariantMap(); // options, unused now
> c.call(msg, QDBus::NoBlock);
> }
> -
> }
>
> m_teardownInProgress = false;
> @@ -206,6 +203,7 @@ void StorageAccess::slotDBusError( const QDBusError &
> error )
> else if (m_teardownInProgress)
> {
> m_teardownInProgress = false;
> + m_clearTextPath.clear();
> m_device->broadcastActionDone("teardown",
> m_device->errorToSolidError(error.name()),
> m_device-
>errorToString(error.name())
> + ": " + error.message());
> slotChanged();
> @@ -233,27 +231,21 @@ void StorageAccess::slotTeardownRequested()
> void StorageAccess::slotTeardownDone(int error, const QString
> &errorString)
> {
> m_teardownInProgress = false;
> + m_clearTextPath.clear();
> Q_EMIT teardownDone(static_cast<Solid::ErrorType>(error),
> errorString, m_device->udi());
> }
>
> bool StorageAccess::mount()
> {
> QString path = m_device->udi();
> - QString fstype;
>
> if (isLuksDevice()) { // mount options for the cleartext volume
> - path =
> m_device->prop("LuksHolder").value<QDBusObjectPath>().path();
> - Device holderDevice(path);
> - fstype = holderDevice.prop("IdType").toString();
> + path = m_clearTextPath;
> }
>
> QDBusConnection c = QDBusConnection::systemBus();
> QDBusMessage msg = QDBusMessage::createMethodCall(UD2_DBUS_SERVICE,
> path, UD2_DBUS_INTERFACE_FILESYSTEM, "Mount");
>
> - if (m_device->prop("IdUsage").toString() == "filesystem")
> - fstype = m_device->prop("IdType").toString();
> -
> - msg << fstype;
> msg << QVariantMap(); // options, unused now
>
> return c.callWithCallback(msg, this,
> @@ -266,7 +258,7 @@ bool StorageAccess::unmount()
> QString path = m_device->udi();
>
> if (isLuksDevice()) { // unmount options for the cleartext volume
> - path =
> m_device->prop("LuksHolder").value<QDBusObjectPath>().path();
> + path = m_clearTextPath;
> }
>
> QDBusConnection c = QDBusConnection::systemBus();
> @@ -312,7 +304,7 @@ bool StorageAccess::requestPassphrase()
> return m_passphraseRequested;
> }
>
> -void StorageAccess::passphraseReply( const QString & passphrase )
> +void StorageAccess::passphraseReply(const QString & passphrase)
> {
> if (m_passphraseRequested)
> {
> @@ -328,7 +320,7 @@ void StorageAccess::passphraseReply( const QString &
> passphrase )
> }
> }
>
> -void StorageAccess::callCryptoSetup( const QString & passphrase )
> +void StorageAccess::callCryptoSetup(const QString & passphrase)
> {
> QDBusConnection c = QDBusConnection::systemBus();
> QDBusMessage msg = QDBusMessage::createMethodCall(UD2_DBUS_SERVICE,
> m_device->udi(), UD2_DBUS_INTERFACE_ENCRYPTED, "Unlock");
> @@ -349,8 +341,9 @@ bool StorageAccess::callCryptoTeardown(bool
> actOnParent)
>
UD2_DBUS_INTERFACE_ENCRYPTED,
> "Lock");
> msg << QVariantMap(); // options, unused now
>
> + m_clearTextPath.clear();
> +
> return c.callWithCallback(msg, this,
> SLOT(slotDBusReply(const QDBusMessage &)),
> SLOT(slotDBusError(const QDBusError &)));
> }
> -
> diff --git a/tier1/solid/solid/backends/udisks2/udisksstorageaccess.h
> b/tier1/solid/solid/backends/udisks2/udisksstorageaccess.h index
> f2d4c03..2f929ac 100644 ---
> a/tier1/solid/solid/backends/udisks2/udisksstorageaccess.h +++
> b/tier1/solid/solid/backends/udisks2/udisksstorageaccess.h @@ -57,12
> +57,12 @@ Q_SIGNALS:
> void teardownRequested(const QString &udi);
>
> public Q_SLOTS:
> - Q_SCRIPTABLE Q_NOREPLY void passphraseReply( const QString &
> passphrase );
> + Q_SCRIPTABLE Q_NOREPLY void passphraseReply(const QString &
> passphrase);
>
> private Q_SLOTS:
> void slotChanged();
> - void slotDBusReply( const QDBusMessage & reply );
> - void slotDBusError( const QDBusError & error );
> + void slotDBusReply(const QDBusMessage & reply);
> + void slotDBusError(const QDBusError & error);
>
> void connectDBusSignals();
>
> @@ -92,6 +92,7 @@ private:
> bool m_teardownInProgress;
> bool m_passphraseRequested;
> QString m_lastReturnObject;
> + QString m_clearTextPath; // path to the unlocked cleartext device
>
> static const int s_unmountTimeout = 0x7fffffff;
> };
> diff --git a/tier1/solid/solid/backends/udisks2/udisksstoragevolume.cpp
> b/tier1/solid/solid/backends/udisks2/udisksstoragevolume.cpp index
> a1672cb..f87b251 100644 ---
> a/tier1/solid/solid/backends/udisks2/udisksstoragevolume.cpp +++
> b/tier1/solid/solid/backends/udisks2/udisksstoragevolume.cpp @@ -20,6
> +20,7 @@
> */
>
> #include "udisksstoragevolume.h"
> +#include "udisks2.h"
>
> using namespace Solid::Backends::UDisks2;
>
> @@ -70,11 +71,11 @@ Solid::StorageVolume::UsageType StorageVolume::usage()
> const
> {
> const QString usage = m_device->prop("IdUsage").toString();
>
> - if (usage == "filesystem")
> + if (m_device->hasInterface(UD2_DBUS_INTERFACE_FILESYSTEM))
> {
> return Solid::StorageVolume::FileSystem;
> }
> - else if (usage == "partitiontable") // FIXME udisks2
> + else if (m_device->isPartitionTable())
> {
> return Solid::StorageVolume::PartitionTable;
> }
> @@ -82,7 +83,7 @@ Solid::StorageVolume::UsageType StorageVolume::usage()
> const
> {
> return Solid::StorageVolume::Raid;
> }
> - else if (usage == "crypto")
> + else if (m_device->isEncryptedContainer())
> {
> return Solid::StorageVolume::Encrypted;
> }
> @@ -98,5 +99,8 @@ Solid::StorageVolume::UsageType StorageVolume::usage()
> const
>
> bool StorageVolume::isIgnored() const
> {
> - return m_device->prop("HintIgnore").toBool(); // FIXME tune
> + const Solid::StorageVolume::UsageType usg = usage();
> + return m_device->prop("HintIgnore").toBool() || m_device->isSwap() ||
> + usg == Solid::StorageVolume::Unused || usg ==
> Solid::StorageVolume::Other ||
> + usg == Solid::StorageVolume::PartitionTable; // FIXME check
> for empty and audio CDs
> }
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic