Commit 325018e2 authored by Harald Sitter's avatar Harald Sitter 🐕

add request queuing to kauth smartctl

kauth does not actually support making multiple requests at the same
time.
this effectively caused multiple devices to get the same reply blob
based on which device gets requested first, rendering the data largely
useless as they'd all reflect the state of a single device.

since the kauth test is a bit rubbish to test data consistency is now
asserted in the monitor

```
e.g.
::run(/dev/nvme0n1) -> job0
::run(/dev/sda) -> job1

|
kauth makes single request to action
|
v

helper(/dev/nvme0n1)
   			--> returns blob0

|
kauth returns all jobs for that action
|
v

job0 -> result(blob0)
job1 -> result(blob0)
```
parent 663d9730
......@@ -17,7 +17,7 @@
class MockCtl : public AbstractSMARTCtl
{
public:
void run(const QString &devicePath) const override
void run(const QString &devicePath) override
{
qDebug() << devicePath;
emit finished(devicePath, m_docs.value(devicePath));
......
......@@ -10,8 +10,17 @@
#include "kded_debug.h"
void SMARTCtl::run(const QString &devicePath) const
void SMARTCtl::run(const QString &devicePath)
{
// https://bugs.kde.org/show_bug.cgi?id=379215
// One cannot make any kind of concurrent requests to kauth, so we need
// a fairly awkward busy state and request queuing system.
if (m_busy) {
m_requestQueue.push(devicePath);
return;
}
m_busy = true;
KAuth::Action action(QStringLiteral("org.kde.kded.smart.smartctl"));
// This is technically never used unless the sysadmin forces our action
// to require authentication. In that case we'll want to give request context
......@@ -34,12 +43,22 @@ void SMARTCtl::run(const QString &devicePath) const
const auto data = job->data();
const auto code = data.value(QStringLiteral("exitCode"), QByteArray()).toInt();
const auto json = data.value(QStringLiteral("data"), QByteArray()).toByteArray();
QJsonDocument document;
if (json.isEmpty() || code & Failure::CmdLineParse || code & Failure::DeviceOpen) {
qCDebug(KDED) << "looks like we got no data back for" << devicePath << code << json.isEmpty();
} else {
document = QJsonDocument::fromJson(json);
}
// Queue the next pending request if there is any.
m_busy = false;
if (!m_requestQueue.empty()) {
auto request = m_requestQueue.front();
run(request);
m_requestQueue.pop();
}
emit finished(devicePath, document);
});
job->start();
......
......@@ -7,12 +7,14 @@
#include <QJsonDocument>
#include <QObject>
#include <queue>
class AbstractSMARTCtl : public QObject
{
Q_OBJECT
public:
virtual ~AbstractSMARTCtl() = default;
virtual void run(const QString &devicePath) const = 0;
virtual void run(const QString &devicePath) = 0;
signals:
void finished(const QString &devicePath, const QJsonDocument &document) const;
......@@ -55,7 +57,11 @@ public:
// The entire thing doesn't exceed 8 bits because it's a posix exit code.
};
void run(const QString &devicePath) const override;
void run(const QString &devicePath) override;
private:
bool m_busy = false;
std::queue<QString> m_requestQueue;
};
#endif // SMARTCTL_H
......@@ -87,6 +87,7 @@ void SMARTMonitor::onSMARTCtlFinished(const QString &devicePath, const QJsonDocu
}
SMARTData data(document);
Q_ASSERT(devicePath == data.m_device);
auto existingIt = std::find_if(m_devices.begin(), m_devices.end(), [&device](Device *existing) {
return *existing == *device;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment