diff --git a/CMakeLists.txt b/CMakeLists.txt index 55ce06a..9250b9a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,7 +4,7 @@ cmake_minimum_required(VERSION 3.16) project(RemoteDiscHealthMonitor VERSION 0.2.0) set(CMAKE_CXX_EXTENSIONS ON) -set(CMAKE_CXX_STANDARD_REQUIRED OFF) +set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_STANDARD 23) option(BUILD_AGENT "Build the agent component" ON) @@ -37,7 +37,16 @@ set(INSTALL_GTEST OFF CACHE BOOL "" FORCE) add_subdirectory(external/googletest EXCLUDE_FROM_ALL) if(BUILD_MONITOR) - add_subdirectory(external/QtZeroConf) + add_subdirectory(external/QtZeroConf EXCLUDE_FROM_ALL) + set_target_properties(QtZeroConf PROPERTIES + EXCLUDE_FROM_ALL FALSE + PUBLIC_HEADER "" + ) + + install(TARGETS QtZeroConf + RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} + ) set(CppRestAPI_QtBackend ON CACHE BOOL "" FORCE) set(CppRestAPI_GitHub OFF CACHE BOOL "" FORCE) diff --git a/packaging/arch/rdhm-agent/rdhm-agent.install b/packaging/arch/rdhm-agent/rdhm-agent.install index 094b4ca..d55cb88 100644 --- a/packaging/arch/rdhm-agent/rdhm-agent.install +++ b/packaging/arch/rdhm-agent/rdhm-agent.install @@ -1,23 +1,29 @@ +run_systemctl() { + if command -v systemctl >/dev/null 2>&1; then + systemctl "$@" || true + fi +} + post_install() { - systemctl daemon-reload - systemctl enable rdhm-agent.service + run_systemctl daemon-reload + run_systemctl enable rdhm-agent.service echo ">>> Enable and start the agent with: systemctl start rdhm-agent" } pre_upgrade() { - systemctl stop rdhm-agent.service || true + run_systemctl stop rdhm-agent.service } post_upgrade() { - systemctl daemon-reload - systemctl restart rdhm-agent.service || true + run_systemctl daemon-reload + run_systemctl restart rdhm-agent.service } pre_remove() { - systemctl stop rdhm-agent.service || true - systemctl disable rdhm-agent.service || true + run_systemctl stop rdhm-agent.service + run_systemctl disable rdhm-agent.service } post_remove() { - systemctl daemon-reload + run_systemctl daemon-reload } diff --git a/packaging/deb/rdhm-agent/debian/rdhm-agent.postinst b/packaging/deb/rdhm-agent/debian/rdhm-agent.postinst index 926465e..c3d4344 100644 --- a/packaging/deb/rdhm-agent/debian/rdhm-agent.postinst +++ b/packaging/deb/rdhm-agent/debian/rdhm-agent.postinst @@ -1,7 +1,13 @@ #!/bin/sh set -e +run_systemctl() { + if command -v systemctl >/dev/null 2>&1; then + systemctl "$@" || true + fi +} + if [ "$1" = "configure" ]; then - systemctl daemon-reload - systemctl enable rdhm-agent.service + run_systemctl daemon-reload + run_systemctl enable rdhm-agent.service fi diff --git a/packaging/deb/rdhm-agent/debian/rdhm-agent.prerm b/packaging/deb/rdhm-agent/debian/rdhm-agent.prerm index f7cbc84..6ecc7be 100644 --- a/packaging/deb/rdhm-agent/debian/rdhm-agent.prerm +++ b/packaging/deb/rdhm-agent/debian/rdhm-agent.prerm @@ -1,7 +1,13 @@ #!/bin/sh set -e +run_systemctl() { + if command -v systemctl >/dev/null 2>&1; then + systemctl "$@" || true + fi +} + if [ "$1" = "remove" ] || [ "$1" = "deconfigure" ]; then - systemctl stop rdhm-agent.service || true - systemctl disable rdhm-agent.service || true + run_systemctl stop rdhm-agent.service + run_systemctl disable rdhm-agent.service fi diff --git a/packaging/rpm/rdhm-monitor.spec b/packaging/rpm/rdhm-monitor.spec index eae8550..4ea5e7c 100644 --- a/packaging/rpm/rdhm-monitor.spec +++ b/packaging/rpm/rdhm-monitor.spec @@ -38,8 +38,6 @@ network via mDNS/ZeroConf and displays disk health status in real time. %license LICENSE %{_bindir}/rdhm-monitor %{_libdir}/libQtZeroConf.so* -%exclude %{_includedir}/QtZeroConf -%exclude %{_libdir}/cmake/QtZeroConf %changelog * Sat Mar 14 2026 Michał Walenciak - 0.2.0-1 diff --git a/packaging/systemd/rdhm-agent.service b/packaging/systemd/rdhm-agent.service index bb0c659..d73d7a9 100644 --- a/packaging/systemd/rdhm-agent.service +++ b/packaging/systemd/rdhm-agent.service @@ -6,6 +6,7 @@ After=network.target [Service] Type=simple EnvironmentFile=/etc/rdhm/agent.conf +RuntimeDirectory=rdhm ExecStart=/usr/sbin/rdhm-agent -n ${AGENT_NAME} Restart=on-failure RestartSec=5 diff --git a/src/agent/CMakeLists.txt b/src/agent/CMakeLists.txt index 06fb269..68856c0 100644 --- a/src/agent/CMakeLists.txt +++ b/src/agent/CMakeLists.txt @@ -27,6 +27,10 @@ ADD_EXECUTABLE( agent target_include_directories(agent PRIVATE ${PROJECT_SOURCE_DIR}/src +) + +target_include_directories(agent + SYSTEM PRIVATE ${PROJECT_SOURCE_DIR}/external/cpp-httplib ${PROJECT_SOURCE_DIR}/external/nlohmann-json/single_include ${PROJECT_SOURCE_DIR}/external/mdns diff --git a/src/agent/Disk.cpp b/src/agent/Disk.cpp index 94a2de6..49a3302 100644 --- a/src/agent/Disk.cpp +++ b/src/agent/Disk.cpp @@ -1,6 +1,7 @@ -#include "Disk.h" - -#include +#include "Disk.h" + +#include +#include Disk::Disk(const std::string& _deviceId) : m_deviceId(_deviceId) diff --git a/src/agent/HttpServer.cpp b/src/agent/HttpServer.cpp index 62c992a..819c809 100644 --- a/src/agent/HttpServer.cpp +++ b/src/agent/HttpServer.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -21,9 +22,9 @@ namespace { bool isValidDiskName(const std::string& name) { - return !name.empty() && name.size() <= 64 && + return !name.empty() && name.size() <= 128 && std::ranges::all_of(name, [](char c) { - return std::isalnum(static_cast(c)) || c == '-' || c == '_'; + return std::isalnum(static_cast(c)) || c == '-' || c == '_' || c == '.' || c == '\\' || c == ':'; }); } } @@ -45,7 +46,8 @@ struct HttpServer::Impl // Refresh cooldown static constexpr int RefreshCooldownSeconds = 600; // 10 minutes - std::chrono::steady_clock::time_point lastRefreshTime{}; + std::chrono::steady_clock::time_point lastRefreshTime = + std::chrono::steady_clock::now() - std::chrono::seconds{RefreshCooldownSeconds}; std::mutex refreshMutex; // SSE support @@ -237,12 +239,12 @@ void HttpServer::setStatusData(GeneralHealth::Health overallHealth, std::vector< m_impl->disks = std::move(disks); const auto now = std::chrono::system_clock::now(); - const auto time_t = std::chrono::system_clock::to_time_t(now); + const auto nowTime = std::chrono::system_clock::to_time_t(now); std::tm tm_buf{}; #ifdef _WIN32 - gmtime_s(&tm_buf, &time_t); + gmtime_s(&tm_buf, &nowTime); #else - gmtime_r(&time_t, &tm_buf); + gmtime_r(&nowTime, &tm_buf); #endif std::ostringstream oss; oss << std::put_time(&tm_buf, "%Y-%m-%dT%H:%M:%SZ"); diff --git a/src/agent/MdnsNetwork.cpp b/src/agent/MdnsNetwork.cpp index a406013..d67c6e2 100644 --- a/src/agent/MdnsNetwork.cpp +++ b/src/agent/MdnsNetwork.cpp @@ -134,8 +134,8 @@ timeval toTimeval(std::chrono::microseconds timeout) const auto microseconds = timeout - seconds; timeval result{}; - result.tv_sec = static_cast(seconds.count()); - result.tv_usec = static_cast(microseconds.count()); + result.tv_sec = seconds.count(); + result.tv_usec = microseconds.count(); return result; } @@ -189,29 +189,30 @@ std::vector openMdnsServiceSockets() std::vector waitForReadableSockets(const std::vector& sockets, std::chrono::microseconds timeout) { - if (sockets.empty()) - return {}; + std::vector ready; - fd_set readable; - FD_ZERO(&readable); + if (!sockets.empty()) + { + fd_set readable; + FD_ZERO(&readable); - int nfds = 0; - for (int socket : sockets) { - if (socket >= nfds) - nfds = socket + 1; - FD_SET(socket, &readable); - } + int nfds = 0; + for (int socket : sockets) { + if (socket >= nfds) + nfds = socket + 1; + FD_SET(socket, &readable); + } - timeval tv = toTimeval(timeout); - if (select(nfds, &readable, nullptr, nullptr, &tv) <= 0) - return {}; + timeval tv = toTimeval(timeout); + if (select(nfds, &readable, nullptr, nullptr, &tv) > 0) + { + ready.reserve(sockets.size()); - std::vector ready; - ready.reserve(sockets.size()); - - for (int socket : sockets) { - if (FD_ISSET(socket, &readable)) - ready.push_back(socket); + for (int socket : sockets) { + if (FD_ISSET(socket, &readable)) + ready.push_back(socket); + } + } } return ready; diff --git a/src/agent/SmartHealthAnalyzer.cpp b/src/agent/SmartHealthAnalyzer.cpp index 56e6f63..a74aba8 100644 --- a/src/agent/SmartHealthAnalyzer.cpp +++ b/src/agent/SmartHealthAnalyzer.cpp @@ -146,37 +146,39 @@ GeneralHealth::Health SmartHealthAnalyzer::GetStatus(const Disk& disk) const nlohmann::json SmartHealthAnalyzer::GetRawData(const Disk& disk) const { - auto it = m_cachedSmartData.find(disk.GetDeviceId()); - if (it == m_cachedSmartData.end()) - return nlohmann::json{{"type", "smart"}, {"attributes", nlohmann::json::array()}}; - - const auto& smart = it->second; + auto j = nlohmann::json{{"type", "smart"}, {"attributes", nlohmann::json::array()}}; - nlohmann::json attrs = nlohmann::json::array(); - for (const auto& attr : smart.attributes) + auto it = m_cachedSmartData.find(disk.GetDeviceId()); + if (it != m_cachedSmartData.end()) { - attrs.push_back({ - {"id", attr.id}, - {"name", attr.name}, - {"value", attr.value}, - {"worst", attr.worst}, - {"threshold", attr.threshold}, - {"rawVal", attr.rawVal} - }); - } + const auto& smart = it->second; + + nlohmann::json attrs = nlohmann::json::array(); + for (const auto& attr : smart.attributes) + { + attrs.push_back({ + {"id", attr.id}, + {"name", attr.name}, + {"value", attr.value}, + {"worst", attr.worst}, + {"threshold", attr.threshold}, + {"rawVal", attr.rawVal} + }); + } - auto j = nlohmann::json{{"type", "smart"}, {"attributes", attrs}}; + j["attributes"] = attrs; - auto testIt = m_cachedTestStatus.find(disk.GetDeviceId()); - SmartTestStatus testStatus; - if (testIt != m_cachedTestStatus.end()) - testStatus = testIt->second; + auto testIt = m_cachedTestStatus.find(disk.GetDeviceId()); + SmartTestStatus testStatus; + if (testIt != m_cachedTestStatus.end()) + testStatus = testIt->second; - j["selfTestStatus"] = { - {"running", testStatus.running}, - {"percentRemaining", testStatus.percentRemaining}, - {"lastResult", testStatus.lastResult} - }; + j["selfTestStatus"] = { + {"running", testStatus.running}, + {"percentRemaining", testStatus.percentRemaining}, + {"lastResult", testStatus.lastResult} + }; + } return j; } diff --git a/src/agent/linux/DmesgParser.cpp b/src/agent/linux/DmesgParser.cpp index 4f41c03..85dcecf 100644 --- a/src/agent/linux/DmesgParser.cpp +++ b/src/agent/linux/DmesgParser.cpp @@ -9,10 +9,23 @@ namespace { const std::vector ErrorPatterns = { - std::regex("Buffer I/O error on device ([a-z0-9]*)") + std::regex(R"(Buffer I/O error on (?:dev(?:ice)? )?([A-Za-z0-9_.-]+))"), + std::regex(R"((?:I/O error|critical medium error|medium error|uncorrectable error).*dev ([A-Za-z0-9_.-]+))"), + std::regex(R"(EXT[234]-fs error.*\(device ([A-Za-z0-9_.-]+)\))") }; -} + std::string physicalDeviceFor(const std::string& deviceName, + const IPartitionsManager& partitionsManager) + { + if (partitionsManager.isPartition(deviceName)) + { + const auto physicalDevice = partitionsManager.diskForPartition(deviceName); + return physicalDevice.empty() ? deviceName : physicalDevice; + } + else + return deviceName; + } +} std::map> DmesgParser::parse(const std::string& output, const IPartitionsManager& paritionsManager) @@ -29,9 +42,7 @@ std::map> DmesgParser::parse(const std::string& outp if (std::regex_search(line, errorMatch, errorRegex)) { const std::string dev = errorMatch[1].str(); - const std::string physicalDev = paritionsManager.isPartition(dev)? - paritionsManager.diskForPartition(dev): - dev; + const std::string physicalDev = physicalDeviceFor(dev, paritionsManager); const Disk disk(physicalDev); diff --git a/src/agent/linux/LinGeneralAnalyzer.cpp b/src/agent/linux/LinGeneralAnalyzer.cpp index 9adcb46..9146f97 100644 --- a/src/agent/linux/LinGeneralAnalyzer.cpp +++ b/src/agent/linux/LinGeneralAnalyzer.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include "common/GeneralHealth.h" #include "LinGeneralAnalyzer.h" @@ -12,6 +13,12 @@ namespace { + struct CommandResult + { + std::string output; + bool success = false; + }; + std::string getCursorFilePath() { if (const char* runDir = std::getenv("XDG_RUNTIME_DIR")) @@ -19,6 +26,42 @@ namespace return "/run/rdhm/journal-cursor"; } + + std::string shellQuote(const std::string& value) + { + std::string quoted = "'"; + for (char c : value) + { + if (c == '\'') + quoted += "'\\''"; + else + quoted += c; + } + quoted += "'"; + return quoted; + } + + bool exitStatusOk(int status) + { + return WIFEXITED(status) && WEXITSTATUS(status) == 0; + } + + CommandResult runCommand(const std::string& cmd) + { + CommandResult result; + std::array buffer; + + FILE* pipe = popen(cmd.c_str(), "r"); + if (pipe) + { + while (fgets(buffer.data(), buffer.size(), pipe) != nullptr) + result.output += buffer.data(); + + result.success = exitStatusOk(pclose(pipe)); + } + + return result; + } } @@ -26,13 +69,7 @@ namespace LinGeneralAnalyzer::LinGeneralAnalyzer(std::shared_ptr manager) : m_partitionsManager(manager) { - FILE* pipe = popen("which journalctl", "r"); - if (pipe) - { - char buf[256]; - m_useJournalctl = (fgets(buf, sizeof(buf), pipe) != nullptr); - pclose(pipe); - } + m_useJournalctl = runCommand("command -v journalctl 2>/dev/null").success; } @@ -75,28 +112,25 @@ nlohmann::json LinGeneralAnalyzer::GetRawData(const Disk& disk) const void LinGeneralAnalyzer::Refresh(const std::vector&) { - std::string output; - std::array buffer; - const auto cursorPath = getCursorFilePath(); - const auto journalCmd = "journalctl -k --cursor-file=" + cursorPath + - " --no-pager -q --output=short"; + const auto journalCmd = "journalctl -k --cursor-file=" + shellQuote(cursorPath) + + " --no-pager -q --output=short 2>/dev/null"; - const char* cmd = m_useJournalctl - ? journalCmd.c_str() - : "dmesg"; + CommandResult commandResult; + bool usedJournalctl = false; - FILE* pipe = popen(cmd, "r"); - if (pipe) + if (m_useJournalctl) { - while (fgets(buffer.data(), buffer.size(), pipe) != nullptr) - output += buffer.data(); - pclose(pipe); + commandResult = runCommand(journalCmd); + usedJournalctl = commandResult.success; } - auto newErrors = DmesgParser::parse(output, *m_partitionsManager); + if (!usedJournalctl) + commandResult = runCommand("dmesg 2>/dev/null"); - if (m_useJournalctl) + auto newErrors = DmesgParser::parse(commandResult.output, *m_partitionsManager); + + if (usedJournalctl) { // journalctl with cursor-file returns only new entries since last read. // Errors are accumulated permanently — once a disk reports an error it stays BAD diff --git a/src/agent/linux/LinuxSmartReader.cpp b/src/agent/linux/LinuxSmartReader.cpp index 46f2c13..c64dd60 100644 --- a/src/agent/linux/LinuxSmartReader.cpp +++ b/src/agent/linux/LinuxSmartReader.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include "../SmartReader.h" @@ -20,22 +21,23 @@ namespace std::string runSmartctl(const Disk& disk) { + std::string output; + if (!isValidDeviceId(disk.GetDeviceId())) { std::cerr << "Invalid device ID, skipping smartctl: " << disk.GetDeviceId() << '\n'; - return {}; } - - std::string output; - std::array buffer; - - const std::string cmd = "smartctl -a /dev/" + disk.GetDeviceId(); - FILE* pipe = popen(cmd.c_str(), "r"); - if (pipe) + else { - while (fgets(buffer.data(), buffer.size(), pipe) != nullptr) - output += buffer.data(); - pclose(pipe); + std::array buffer; + const std::string cmd = "smartctl -a /dev/" + disk.GetDeviceId(); + FILE* pipe = popen(cmd.c_str(), "r"); + if (pipe) + { + while (fgets(buffer.data(), buffer.size(), pipe) != nullptr) + output += buffer.data(); + pclose(pipe); + } } return output; diff --git a/src/agent/linux/LsblkOutputParser.cpp b/src/agent/linux/LsblkOutputParser.cpp index 28b6355..9eb7bbe 100644 --- a/src/agent/linux/LsblkOutputParser.cpp +++ b/src/agent/linux/LsblkOutputParser.cpp @@ -1,8 +1,9 @@ #include +#include #include #include -#include +#include #include "agent/OutputParsersUtils.h" #include "LsblkOutputParser.h" @@ -19,33 +20,70 @@ namespace int minor; }; + std::vector splitByWhitespace(const std::string& str) + { + std::vector parts; + std::istringstream stream(str); + std::string part; + + while (stream >> part) + parts.push_back(part); + + return parts; + } + std::vector splitString(const std::string& str, char delim) { std::vector parts; std::istringstream stream(str); std::string part; + while (std::getline(stream, part, delim)) parts.push_back(part); + return parts; } - RawLsblkEntry parseLine(const std::string& line) + std::optional parseLine(const std::string& line) { - const auto cols = splitString(line, ' '); - assert(cols.size() >= 6); + const auto cols = splitByWhitespace(line); + if (cols.size() < 6) + return std::nullopt; const auto major_minor = splitString(cols[1], ':'); - assert(major_minor.size() == 2); - - const RawLsblkEntry rawEntry{ - .name = cols[0], - .type = cols[5], - .size = std::stoull(cols[3]), - .major = std::stoi(major_minor[0]), - .minor = std::stoi(major_minor[1]) - }; + if (major_minor.size() != 2) + return std::nullopt; - return rawEntry; + try + { + return RawLsblkEntry{ + .name = cols[0], + .type = cols[5], + .size = std::stoull(cols[3]), + .major = std::stoi(major_minor[0]), + .minor = std::stoi(major_minor[1]) + }; + } + catch (const std::exception&) + { + return std::nullopt; + } + } + + bool isPartitionOfDisk(const std::string& partitionName, + const std::string& diskName) + { + if (partitionName.size() <= diskName.size() || partitionName.find(diskName) != 0) + return false; + + const auto suffix = partitionName.substr(diskName.size()); + + if (!suffix.empty() && std::isdigit(static_cast(suffix.front()))) + return true; + + return suffix.size() > 1 && + suffix.front() == 'p' && + std::isdigit(static_cast(suffix[1])); } LsblkOutputParser::LsblkEntry fromRaw(const RawLsblkEntry& raw) @@ -77,14 +115,21 @@ std::vector LsblkOutputParser::parse(const std::s { if (line.empty()) continue; - const RawLsblkEntry entry = parseLine(line); + const auto parsedEntry = parseLine(line); + if (!parsedEntry) + { + std::cerr << "Skipping malformed lsblk line: " << line << std::endl; + continue; + } + + const RawLsblkEntry& entry = *parsedEntry; if (entry.type == "disk") entries.push_back(fromRaw(entry)); else if (entry.type == "part") { auto entryIt = std::find_if(entries.begin(), entries.end(), [part_name = entry.name](const auto& disk) { - return part_name.find(disk.name) == 0; + return isPartitionOfDisk(part_name, disk.name); }); if (entryIt == entries.end()) diff --git a/src/agent/linux/SmartCtlOutputParser.cpp b/src/agent/linux/SmartCtlOutputParser.cpp index 07fa1e5..b83268e 100644 --- a/src/agent/linux/SmartCtlOutputParser.cpp +++ b/src/agent/linux/SmartCtlOutputParser.cpp @@ -282,9 +282,7 @@ namespace SmartCtlOutputParser return parseNvmeHealth(cleanLines); const auto attributeLines = smartAttributes(cleanLines); - const auto table = parseRawTable(attributeLines); - - return table; + return parseRawTable(attributeLines); } SmartTestStatus parseTestStatus(const std::string& smartCtlOutput) diff --git a/src/agent/main.cpp b/src/agent/main.cpp index 69d0163..12f243b 100644 --- a/src/agent/main.cpp +++ b/src/agent/main.cpp @@ -25,6 +25,7 @@ namespace std::atomic g_running{true}; std::mutex g_bgMutex; std::condition_variable g_bgCv; + bool g_refreshRequested = false; void signalHandler(int) { @@ -166,6 +167,15 @@ namespace server.setStatusData(overall, std::move(diskInfos)); } + void requestBackgroundRefresh() + { + { + std::lock_guard lock(g_bgMutex); + g_refreshRequested = true; + } + g_bgCv.notify_one(); + } + // Lock hierarchy (acquire in this order to avoid deadlocks): // refreshMutex (HttpServer::Impl) → g_probeMutex → dataMutex → sseMutex std::mutex g_probeMutex; @@ -237,7 +247,7 @@ int main(int argc, char** argv) std::lock_guard lock(g_probeMutex); publishFromCache(server, probeEntries, disks); } - g_bgCv.notify_one(); + requestBackgroundRefresh(); }); // Initial proactive data collection @@ -265,7 +275,8 @@ int main(int argc, char** argv) { std::unique_lock lock(g_bgMutex); g_bgCv.wait_for(lock, std::chrono::minutes(1), - [] { return !g_running.load(); }); + [] { return !g_running.load() || g_refreshRequested; }); + g_refreshRequested = false; } if (!g_running) diff --git a/src/agent/windows/CMDCommunication.cpp b/src/agent/windows/CMDCommunication.cpp index aeb4978..b646acb 100644 --- a/src/agent/windows/CMDCommunication.cpp +++ b/src/agent/windows/CMDCommunication.cpp @@ -1,13 +1,14 @@ -#include "CMDCommunication.h" -#include -#include +#include "CMDCommunication.h" +#include +#include +#include #include #include namespace { - std::string runCommand(const std::string& cmd) - { + std::string runCommand(const std::string& cmd) + { std::string output; std::array buffer; @@ -18,10 +19,26 @@ namespace output += buffer.data(); _pclose(pipe); } - - return output; - } -} + + return output; + } + + std::string tokenAfterDeviceId(const std::string& output, const std::string& deviceId) + { + auto diskPos = output.find(deviceId); + if (diskPos == std::string::npos) + return {}; + + auto valueStart = output.find_first_not_of(" \t\r\n", diskPos + deviceId.size()); + if (valueStart == std::string::npos) + return {}; + + auto valueStop = output.find_first_of(" \t\r\n", valueStart); + return output.substr(valueStart, valueStop == std::string::npos + ? std::string::npos + : valueStop - valueStart); + } +} GeneralHealth::Health CMDCommunication::CollectDiskStatus(const Disk& _disk) { @@ -39,10 +56,12 @@ GeneralHealth::Health CMDCommunication::CollectDiskStatus(const Disk& _disk) bool CMDCommunication::CompareDeviceIdWithInstanceName(const Disk& _disk, std::string _instanceName) { - std::string diskInstanceName = GetInstanceName(_disk); - - auto diskPos = _instanceName.find("_0"); - _instanceName = _instanceName.substr(0, diskPos); + std::string diskInstanceName = GetInstanceName(_disk); + if (diskInstanceName.empty() || _instanceName.empty()) + return false; + + auto diskPos = _instanceName.find("_0"); + _instanceName = _instanceName.substr(0, diskPos); ChangeStringToLowerCase(_instanceName); ChangeStringToLowerCase(diskInstanceName); @@ -50,35 +69,21 @@ bool CMDCommunication::CompareDeviceIdWithInstanceName(const Disk& _disk, std::s return ( diskInstanceName.compare(_instanceName) == 0 ); } -std::string CMDCommunication::ExecuteDiscStatusCommand(const Disk& _disk) const -{ - std::string ret = runCommand("wmic diskdrive get deviceid,status"); - - auto diskPos = ret.find(_disk.GetDeviceId()); - if (diskPos == std::string::npos) - return {}; - - auto statusPosStart = ret.find_first_not_of(' ', diskPos + (_disk.GetDeviceId()).size()); - auto statusPosStop = ret.find_first_of(" \r\n", statusPosStart); - ret = ret.substr(statusPosStart, statusPosStop - statusPosStart); - return ret; -} - -std::string CMDCommunication::GetInstanceName(const Disk& _disk) const -{ - std::string ret = runCommand("wmic diskdrive get DeviceID,PNPDeviceID"); - - auto diskPos = ret.find(_disk.GetDeviceId()); - if (diskPos == std::string::npos) - return {}; - - auto statusPosStart = ret.find_first_not_of(' ', diskPos + (_disk.GetDeviceId()).size()); - auto statusPosStop = ret.find_first_of(" \r\n", statusPosStart); - ret = ret.substr(statusPosStart, statusPosStop - statusPosStart); - return ret; -} - -void CMDCommunication::ChangeStringToLowerCase(std::string& _string) const -{ - std::transform(_string.begin(), _string.end(), _string.begin(), ::tolower); -} +std::string CMDCommunication::ExecuteDiscStatusCommand(const Disk& _disk) const +{ + const std::string ret = runCommand("wmic diskdrive get deviceid,status"); + return tokenAfterDeviceId(ret, _disk.GetDeviceId()); +} + +std::string CMDCommunication::GetInstanceName(const Disk& _disk) const +{ + const std::string ret = runCommand("wmic diskdrive get DeviceID,PNPDeviceID"); + return tokenAfterDeviceId(ret, _disk.GetDeviceId()); +} + +void CMDCommunication::ChangeStringToLowerCase(std::string& _string) const +{ + std::transform(_string.begin(), _string.end(), _string.begin(), [](unsigned char c) { + return static_cast(std::tolower(c)); + }); +} diff --git a/src/agent/windows/WMICommunication.cpp b/src/agent/windows/WMICommunication.cpp index 45ffa5f..54ec6e3 100644 --- a/src/agent/windows/WMICommunication.cpp +++ b/src/agent/windows/WMICommunication.cpp @@ -6,117 +6,92 @@ #include #include -WMICommunication::~WMICommunication() -{ - m_initialLocatorToWMI->Release(); - m_services->Release(); - m_pEnumerator->Release(); - CoUninitialize(); -} - -bool WMICommunication::WMIInit(const WmiNamespace _namespace) -{ - try { - HRESULT hres = CoInitializeEx(0, COINIT_MULTITHREADED); - - if (FAILED(hres)) - { - throw std::exception("Failed to initialize COM library. Error code = 0x"); - } - - hres = CoInitializeSecurity( - NULL, - -1, // COM authentication - NULL, // Authentication services - NULL, // Reserved - RPC_C_AUTHN_LEVEL_PKT, // Default authentication - RPC_C_IMP_LEVEL_IMPERSONATE, // Default Impersonation - NULL, // Authentication info - EOAC_NONE, // Additional capabilities - NULL // Reserved - ); - - if (FAILED(hres)) - { - throw std::exception("Failed to initialize security. Error code = 0x"); - } - - hres = CoCreateInstance( - CLSID_WbemLocator, - 0, - CLSCTX_INPROC_SERVER, - IID_IWbemLocator, (LPVOID*)&m_initialLocatorToWMI); - - if (FAILED(hres)) - { - throw std::exception("Failed to create IWbemLocator object. Err code = 0x"); - } - - _bstr_t wmiNamespace; - if (_namespace == Smart) - { - wmiNamespace = L"ROOT\\WMI"; - } - else if (_namespace == Discs) - { - wmiNamespace = L"ROOT\\cimv2"; - } - - hres = m_initialLocatorToWMI->ConnectServer( - wmiNamespace, // Object path of WMI namespace - NULL, // User name. NULL = current user - NULL, // User password. NULL = current - 0, // Locale. NULL indicates current - NULL, // Security flags. - 0, // Authority (for example, Kerberos) - 0, // Context object - &m_services // pointer to IWbemServices proxy - ); - - if (FAILED(hres)) - { - throw std::exception("Could not connect. Error code = 0x"); - } - - hres = CoSetProxyBlanket( - m_services, // Indicates the proxy to set - RPC_C_AUTHN_WINNT, // RPC_C_AUTHN_xxx - RPC_C_AUTHZ_NONE, // RPC_C_AUTHZ_xxx - NULL, // Server principal name - RPC_C_AUTHN_LEVEL_CALL, // RPC_C_AUTHN_LEVEL_xxx - RPC_C_IMP_LEVEL_IMPERSONATE, // RPC_C_IMP_LEVEL_xxx - NULL, // client identity - EOAC_NONE // proxy capabilities - ); - - if (FAILED(hres)) - { - throw std::exception("Could not set proxy blanket. Error code = 0x"); - } - - return true; - } - catch (const std::exception& e) - { - CoUninitialize(); - if (m_initialLocatorToWMI != NULL) - { - m_initialLocatorToWMI->Release(); - } - - if (m_services != NULL) - { - m_services->Release(); - } - - return false; - } -} - -bool WMICommunication::CollectSMARTDataViaWMI(const Disk& _disk) -{ - try { - HRESULT hres = m_services->ExecQuery( +WMICommunication::~WMICommunication() +{ + ReleaseComObjects(); + if (m_comInitialized) + CoUninitialize(); +} + +bool WMICommunication::WMIInit(const WmiNamespace _namespace) +{ + ReleaseComObjects(); + + HRESULT hres = CoInitializeEx(0, COINIT_MULTITHREADED); + if (FAILED(hres)) + return false; + + m_comInitialized = true; + + hres = CoInitializeSecurity( + NULL, + -1, // COM authentication + NULL, // Authentication services + NULL, // Reserved + RPC_C_AUTHN_LEVEL_PKT, // Default authentication + RPC_C_IMP_LEVEL_IMPERSONATE, // Default Impersonation + NULL, // Authentication info + EOAC_NONE, // Additional capabilities + NULL // Reserved + ); + + if (FAILED(hres) && hres != RPC_E_TOO_LATE) + return false; + + hres = CoCreateInstance( + CLSID_WbemLocator, + 0, + CLSCTX_INPROC_SERVER, + IID_IWbemLocator, (LPVOID*)&m_initialLocatorToWMI); + + if (FAILED(hres)) + return false; + + _bstr_t wmiNamespace; + if (_namespace == Smart) + wmiNamespace = L"ROOT\\WMI"; + else if (_namespace == Discs) + wmiNamespace = L"ROOT\\cimv2"; + + hres = m_initialLocatorToWMI->ConnectServer( + wmiNamespace, // Object path of WMI namespace + NULL, // User name. NULL = current user + NULL, // User password. NULL = current + 0, // Locale. NULL indicates current + NULL, // Security flags. + 0, // Authority (for example, Kerberos) + 0, // Context object + &m_services // pointer to IWbemServices proxy + ); + + if (FAILED(hres)) + return false; + + hres = CoSetProxyBlanket( + m_services, // Indicates the proxy to set + RPC_C_AUTHN_WINNT, // RPC_C_AUTHN_xxx + RPC_C_AUTHZ_NONE, // RPC_C_AUTHZ_xxx + NULL, // Server principal name + RPC_C_AUTHN_LEVEL_CALL, // RPC_C_AUTHN_LEVEL_xxx + RPC_C_IMP_LEVEL_IMPERSONATE, // RPC_C_IMP_LEVEL_xxx + NULL, // client identity + EOAC_NONE // proxy capabilities + ); + + if (FAILED(hres)) + return false; + + return true; +} + +bool WMICommunication::CollectSMARTDataViaWMI(const Disk& _disk) +{ + if (m_services == nullptr) + return false; + + try { + ReleaseEnumerator(); + HRESULT hres = m_services->ExecQuery( bstr_t("WQL"), bstr_t("SELECT * FROM MSStorageDriver_FailurePredictData"), WBEM_FLAG_FORWARD_ONLY | WBEM_FLAG_RETURN_IMMEDIATELY, @@ -141,15 +116,23 @@ bool WMICommunication::CollectSMARTDataViaWMI(const Disk& _disk) break; } - VARIANT vtInstanceName; - hr = pclsObj->Get(L"InstanceName", 0, &vtInstanceName, 0, 0); - - CMDCommunication communicator; - std::wstring instanceName = (vtInstanceName.bstrVal); - if ( communicator.CompareDeviceIdWithInstanceName( _disk, std::string( instanceName.begin(), instanceName.end() ) ) ) - { - VARIANT vtProp; - hr = pclsObj->Get(L"VendorSpecific", 0, &vtProp, 0, 0); + VARIANT vtInstanceName; + VariantInit(&vtInstanceName); + hr = pclsObj->Get(L"InstanceName", 0, &vtInstanceName, 0, 0); + if (FAILED(hr) || vtInstanceName.vt != VT_BSTR) + { + VariantClear(&vtInstanceName); + pclsObj->Release(); + continue; + } + + CMDCommunication communicator; + std::wstring instanceName = (vtInstanceName.bstrVal); + if ( communicator.CompareDeviceIdWithInstanceName( _disk, std::string( instanceName.begin(), instanceName.end() ) ) ) + { + VARIANT vtProp; + VariantInit(&vtProp); + hr = pclsObj->Get(L"VendorSpecific", 0, &vtProp, 0, 0); if (V_ISARRAY(&vtProp)) { @@ -204,29 +187,21 @@ bool WMICommunication::CollectSMARTDataViaWMI(const Disk& _disk) } return true; - } - catch (const std::exception& e) - { - if (m_services != NULL) - { - m_services->Release(); - } - - if (m_initialLocatorToWMI != NULL) - { - m_initialLocatorToWMI->Release(); - } - - CoUninitialize(); - - return false; - } -} - -bool WMICommunication::CollectInfoAboutDiscsViaWMI() -{ - try { - HRESULT hres = m_services->ExecQuery( + } + catch (const std::exception& e) + { + return false; + } +} + +bool WMICommunication::CollectInfoAboutDiscsViaWMI() +{ + if (m_services == nullptr) + return false; + + try { + ReleaseEnumerator(); + HRESULT hres = m_services->ExecQuery( bstr_t("WQL"), bstr_t("SELECT * FROM Win32_DiskDrive"), WBEM_FLAG_FORWARD_ONLY | WBEM_FLAG_RETURN_IMMEDIATELY, @@ -251,17 +226,31 @@ bool WMICommunication::CollectInfoAboutDiscsViaWMI() break; } - VARIANT vtPropDeviceId; - hr = pclsObj->Get(L"DeviceID", 0, &vtPropDeviceId, 0, 0); - - VARIANT vtPropModel; - hr = pclsObj->Get(L"Model", 0, &vtPropModel, 0, 0); - - VARIANT vtPropSize; - hr = pclsObj->Get(L"Size", 0, &vtPropSize, 0, 0); - - VARIANT vtPropInterfaceType; - hr = pclsObj->Get(L"InterfaceType", 0, &vtPropInterfaceType, 0, 0); + VARIANT vtPropDeviceId; + VariantInit(&vtPropDeviceId); + hr = pclsObj->Get(L"DeviceID", 0, &vtPropDeviceId, 0, 0); + + VARIANT vtPropModel; + VariantInit(&vtPropModel); + hr = pclsObj->Get(L"Model", 0, &vtPropModel, 0, 0); + + VARIANT vtPropSize; + VariantInit(&vtPropSize); + hr = pclsObj->Get(L"Size", 0, &vtPropSize, 0, 0); + + VARIANT vtPropInterfaceType; + VariantInit(&vtPropInterfaceType); + hr = pclsObj->Get(L"InterfaceType", 0, &vtPropInterfaceType, 0, 0); + + if (vtPropDeviceId.vt != VT_BSTR) + { + VariantClear(&vtPropInterfaceType); + VariantClear(&vtPropSize); + VariantClear(&vtPropModel); + VariantClear(&vtPropDeviceId); + pclsObj->Release(); + continue; + } std::string model; if (vtPropModel.vt == VT_BSTR) @@ -290,24 +279,12 @@ bool WMICommunication::CollectInfoAboutDiscsViaWMI() } return true; - } - catch (const std::exception& e) - { - if (m_services != NULL) - { - m_services->Release(); - } - - if (m_initialLocatorToWMI != NULL) - { - m_initialLocatorToWMI->Release(); - } - - CoUninitialize(); - - return false; - } -} + } + catch (const std::exception& e) + { + return false; + } +} const SmartData& WMICommunication::GetSMARTData() const { @@ -345,10 +322,14 @@ void WMICommunication::FeedSmartDataStructure(const std::vector& _data, co } -bool WMICommunication::CollectThresholdsViaWMI(const Disk& _disk) -{ - try { - HRESULT hres = m_services->ExecQuery( +bool WMICommunication::CollectThresholdsViaWMI(const Disk& _disk) +{ + if (m_services == nullptr) + return false; + + try { + ReleaseEnumerator(); + HRESULT hres = m_services->ExecQuery( bstr_t("WQL"), bstr_t("SELECT * FROM MSStorageDriver_FailurePredictThresholds"), WBEM_FLAG_FORWARD_ONLY | WBEM_FLAG_RETURN_IMMEDIATELY, @@ -369,15 +350,23 @@ bool WMICommunication::CollectThresholdsViaWMI(const Disk& _disk) if (0 == uReturn) break; - VARIANT vtInstanceName; - hr = pclsObj->Get(L"InstanceName", 0, &vtInstanceName, 0, 0); - - CMDCommunication communicator; - std::wstring instanceName = (vtInstanceName.bstrVal); - if (communicator.CompareDeviceIdWithInstanceName(_disk, std::string(instanceName.begin(), instanceName.end()))) - { - VARIANT vtProp; - hr = pclsObj->Get(L"VendorSpecific", 0, &vtProp, 0, 0); + VARIANT vtInstanceName; + VariantInit(&vtInstanceName); + hr = pclsObj->Get(L"InstanceName", 0, &vtInstanceName, 0, 0); + if (FAILED(hr) || vtInstanceName.vt != VT_BSTR) + { + VariantClear(&vtInstanceName); + pclsObj->Release(); + continue; + } + + CMDCommunication communicator; + std::wstring instanceName = (vtInstanceName.bstrVal); + if (communicator.CompareDeviceIdWithInstanceName(_disk, std::string(instanceName.begin(), instanceName.end()))) + { + VARIANT vtProp; + VariantInit(&vtProp); + hr = pclsObj->Get(L"VendorSpecific", 0, &vtProp, 0, 0); if (V_ISARRAY(&vtProp)) { @@ -421,8 +410,8 @@ bool WMICommunication::CollectThresholdsViaWMI(const Disk& _disk) } } -void WMICommunication::FeedThresholds(const std::vector& _data, const LONG& _dataSize) -{ +void WMICommunication::FeedThresholds(const std::vector& _data, const LONG& _dataSize) +{ // Build a map of attribute ID → threshold value from the blob. // Layout follows the same 12-byte chunking as FeedSmartDataStructure: // byte[i+2] = attribute ID, byte[i+3] = threshold value. @@ -438,12 +427,38 @@ void WMICommunication::FeedThresholds(const std::vector& _data, const LONG { if (auto it = thresholds.find(attr.id); it != thresholds.end()) attr.threshold = it->second; - } -} - -std::string WMICommunication::StringFromVariant(VARIANT& vt) -{ - _bstr_t bs(vt); + } +} + +void WMICommunication::ReleaseEnumerator() +{ + if (m_pEnumerator != nullptr) + { + m_pEnumerator->Release(); + m_pEnumerator = nullptr; + } +} + +void WMICommunication::ReleaseComObjects() +{ + ReleaseEnumerator(); + + if (m_services != nullptr) + { + m_services->Release(); + m_services = nullptr; + } + + if (m_initialLocatorToWMI != nullptr) + { + m_initialLocatorToWMI->Release(); + m_initialLocatorToWMI = nullptr; + } +} + +std::string WMICommunication::StringFromVariant(VARIANT& vt) +{ + _bstr_t bs(vt); return std::string(static_cast(bs)); } diff --git a/src/agent/windows/WMICommunication.h b/src/agent/windows/WMICommunication.h index 1d78383..f4550fb 100644 --- a/src/agent/windows/WMICommunication.h +++ b/src/agent/windows/WMICommunication.h @@ -30,12 +30,15 @@ class WMICommunication const std::vector GetDisksCollection() const; private: - IWbemLocator* m_initialLocatorToWMI; - IWbemServices* m_services; - IEnumWbemClassObject* m_pEnumerator; + IWbemLocator* m_initialLocatorToWMI = nullptr; + IWbemServices* m_services = nullptr; + IEnumWbemClassObject* m_pEnumerator = nullptr; + bool m_comInitialized = false; SmartData m_smartData; std::vector m_discsCollection; + void ReleaseEnumerator(); + void ReleaseComObjects(); void FeedSmartDataStructure(const std::vector& _data, const LONG& _dataSize); void FeedThresholds(const std::vector& _data, const LONG& _dataSize); std::string StringFromVariant(VARIANT& vt); diff --git a/src/agent/windows/WinDiskCollector.cpp b/src/agent/windows/WinDiskCollector.cpp index ef8ae4f..6ee162d 100644 --- a/src/agent/windows/WinDiskCollector.cpp +++ b/src/agent/windows/WinDiskCollector.cpp @@ -2,9 +2,13 @@ #include "WMICommunication.h" std::vector WinDiskCollector::GetDisksList() -{ - WMICommunication wmi; - wmi.WMIInit(WMICommunication::WmiNamespace::Discs); - wmi.CollectInfoAboutDiscsViaWMI(); - return wmi.GetDisksCollection(); -} +{ + WMICommunication wmi; + if (!wmi.WMIInit(WMICommunication::WmiNamespace::Discs)) + return {}; + + if (!wmi.CollectInfoAboutDiscsViaWMI()) + return {}; + + return wmi.GetDisksCollection(); +} diff --git a/src/agent/windows/WindowsSmartReader.cpp b/src/agent/windows/WindowsSmartReader.cpp index c1c43f5..456dbee 100644 --- a/src/agent/windows/WindowsSmartReader.cpp +++ b/src/agent/windows/WindowsSmartReader.cpp @@ -7,8 +7,12 @@ SmartData SmartReader::ReadSMARTData(const Disk& _disk) { WMICommunication wmi; - wmi.WMIInit(); - wmi.CollectSMARTDataViaWMI(_disk); + if (!wmi.WMIInit()) + return {}; + + if (!wmi.CollectSMARTDataViaWMI(_disk)) + return {}; + wmi.CollectThresholdsViaWMI(_disk); return wmi.GetSMARTData(); diff --git a/src/common/Utils.h b/src/common/Utils.h index 73b68c3..18625d7 100644 --- a/src/common/Utils.h +++ b/src/common/Utils.h @@ -3,6 +3,7 @@ #include #include +#include #include diff --git a/src/monitor/AgentsList.cpp b/src/monitor/AgentsList.cpp index 6349530..cf8e953 100644 --- a/src/monitor/AgentsList.cpp +++ b/src/monitor/AgentsList.cpp @@ -28,15 +28,25 @@ void AgentsList::addAgent(const AgentInformation& info) { auto it = std::find(m_agents.begin(), m_agents.end(), info); - // we do not need duplicates if (it == m_agents.end()) { - beginInsertRows({}, m_agents.size(), m_agents.size()); + const int row = rowCount({}); + + beginInsertRows({}, row, row); m_agents.append(info); endInsertRows(); m_statusProvider.observe(info); } + else if (info.detectionSource() == AgentInformation::DetectionSource::Hardcoded && + it->detectionSource() == AgentInformation::DetectionSource::ZeroConf) + { + const int pos = static_cast(std::distance(m_agents.begin(), it)); + m_agents[pos] = info; + + const QModelIndex idx = index(pos, 0); + emit dataChanged(idx, idx, {AgentDetectionTypeRole}); + } } @@ -46,15 +56,24 @@ void AgentsList::removeAgent(const AgentInformation& info) if (it != m_agents.end()) { - const int pos = std::distance(m_agents.begin(), it); + if (info.detectionSource() == AgentInformation::DetectionSource::ZeroConf && + it->detectionSource() == AgentInformation::DetectionSource::Hardcoded) + { + return; + } - removeAgentAt(pos); + const int pos = static_cast(std::distance(m_agents.begin(), it)); + + removeAgentAt(pos); } } void AgentsList::removeAgentAt(int position) { + if (position < 0 || position >= rowCount({})) + return; + const AgentInformation info = m_agents[position]; beginRemoveRows({}, position, position); @@ -77,7 +96,7 @@ const QVector& AgentsList::agents() const int AgentsList::rowCount(const QModelIndex& parent) const { - return parent.isValid()? 0: m_agents.size(); + return parent.isValid()? 0: static_cast(m_agents.size()); } @@ -85,7 +104,7 @@ QVariant AgentsList::data(const QModelIndex& index, int role) const { QVariant result; - if (index.column() == 0 && index.row() < m_agents.size()) + if (index.column() == 0 && index.row() >= 0 && index.row() < rowCount({})) { const int row = index.row(); @@ -198,7 +217,7 @@ void AgentsList::updateAgentHealth(const AgentInformation& info, const GeneralHe { m_health[info] = health; - const int pos = std::distance(m_agents.begin(), it); + const int pos = static_cast(std::distance(m_agents.begin(), it)); const QModelIndex idx = index(pos, 0); emit dataChanged(idx, idx, {AgentHealthRole}); @@ -213,7 +232,7 @@ void AgentsList::updateAgentDiskInfoCollection(const AgentInformation& _info, co { m_diskInfoCollection[_info] = _diskInfoCollection; - const int pos = std::distance(m_agents.begin(), it); + const int pos = static_cast(std::distance(m_agents.begin(), it)); const QModelIndex idx = index(pos, 0); emit dataChanged(idx, idx, { AgentDiskInfoNamesRole }); @@ -229,7 +248,7 @@ void AgentsList::updateConnectionState(const AgentInformation& info, ConnectionS { m_connectionStates[info] = state; - const int pos = std::distance(m_agents.begin(), it); + const int pos = static_cast(std::distance(m_agents.begin(), it)); const QModelIndex idx = index(pos, 0); emit dataChanged(idx, idx, {AgentConnectionStateRole}); @@ -244,7 +263,7 @@ void AgentsList::updateLastRefreshed(const AgentInformation& info, const QString { m_lastRefreshed[info] = timestamp; - const int pos = std::distance(m_agents.begin(), it); + const int pos = static_cast(std::distance(m_agents.begin(), it)); const QModelIndex idx = index(pos, 0); emit dataChanged(idx, idx, {AgentLastRefreshedRole}); diff --git a/src/monitor/CMakeLists.txt b/src/monitor/CMakeLists.txt index a301ce4..52c3f91 100644 --- a/src/monitor/CMakeLists.txt +++ b/src/monitor/CMakeLists.txt @@ -1,6 +1,10 @@ find_package(Qt6 REQUIRED COMPONENTS Quick Gui Network QuickControls2 Widgets) +if(COMMAND qt_policy AND QT_KNOWN_POLICY_QTP0004) + qt_policy(SET QTP0004 NEW) +endif() + qt_add_executable(monitor AgentsList.cpp AgentsList.hpp diff --git a/src/monitor/Qml/AddAgentDialog.qml b/src/monitor/Qml/AddAgentDialog.qml index 0b80880..a3daa41 100644 --- a/src/monitor/Qml/AddAgentDialog.qml +++ b/src/monitor/Qml/AddAgentDialog.qml @@ -6,27 +6,25 @@ Dialog { id: dialog title: qsTr("Add Agent") - standardButtons: Dialog.Ok | Dialog.Cancel modal: true anchors.centerIn: parent width: 360 - onAccepted: { - agentsValidator.addNewAgent(nameField.text, ipField.text, portField.text) + function resetFields() { nameField.text = "" ipField.text = "" portField.text = "1630" errorLabel.text = "" } - onRejected: { - nameField.text = "" - ipField.text = "" - portField.text = "1630" + function submitAgent() { errorLabel.text = "" + agentsValidator.addNewAgent(nameField.text, ipField.text, portField.text) } + onRejected: resetFields() + Connections { target: agentsValidator @@ -35,10 +33,25 @@ Dialog { } function onAgentDiscovered() { + dialog.resetFields() dialog.close() } } + footer: DialogButtonBox { + Button { + text: qsTr("OK") + DialogButtonBox.buttonRole: DialogButtonBox.ActionRole + onClicked: dialog.submitAgent() + } + + Button { + text: qsTr("Cancel") + DialogButtonBox.buttonRole: DialogButtonBox.RejectRole + onClicked: dialog.reject() + } + } + contentItem: GridLayout { columns: 2 columnSpacing: 12 diff --git a/src/monitor/Qml/Main.qml b/src/monitor/Qml/Main.qml index d6b4a45..c4fd4cf 100644 --- a/src/monitor/Qml/Main.qml +++ b/src/monitor/Qml/Main.qml @@ -12,9 +12,14 @@ ApplicationWindow { visible: true title: qsTr("Remote Disc Health Monitor") + property bool quitRequested: false + color: palette.window onClosing: function(close) { + if (root.quitRequested) + return + close.accepted = false root.hide() } diff --git a/src/monitor/TrayIcon.cpp b/src/monitor/TrayIcon.cpp index 047947b..0c6a67a 100644 --- a/src/monitor/TrayIcon.cpp +++ b/src/monitor/TrayIcon.cpp @@ -33,7 +33,12 @@ TrayIcon::TrayIcon(AgentsList& agents, QObject* parent) } }); - connect(quitAction, &QAction::triggered, qApp, &QApplication::quit); + connect(quitAction, &QAction::triggered, this, [this]() { + if (m_window) + m_window->setProperty("quitRequested", true); + + qApp->quit(); + }); m_trayIcon.setContextMenu(m_menu.get()); m_trayIcon.setToolTip(tr("Remote Disc Health Monitor")); diff --git a/src/monitor/main.cpp b/src/monitor/main.cpp index 4d401e0..033d587 100644 --- a/src/monitor/main.cpp +++ b/src/monitor/main.cpp @@ -57,6 +57,8 @@ int main(int argc, char** argv) QObject::connect(&agentsEnumerator, &AgentsExplorer::agentDiscovered, &activeAgents, &AgentsList::addAgent); QObject::connect(&agentsEnumerator, &AgentsExplorer::agentLost, &activeAgents, &AgentsList::removeAgent); + restoreHardcodedAgents(config, activeAgents); + agentsEnumerator.startListening(); QQmlApplicationEngine engine; @@ -72,8 +74,6 @@ int main(int argc, char** argv) auto* window = qobject_cast(engine.rootObjects().first()); trayIcon.setWindow(window); - restoreHardcodedAgents(config, activeAgents); - const int exitCode = app.exec(); storeHardcodedAgents(config, activeAgents); diff --git a/tests/agent/DmesgParserTests.cpp b/tests/agent/DmesgParserTests.cpp index f864c6d..d4e575e 100644 --- a/tests/agent/DmesgParserTests.cpp +++ b/tests/agent/DmesgParserTests.cpp @@ -35,7 +35,7 @@ TEST(DmesgParserTest, NoErrors) TEST(DmesgParserTest, BufferIOError) { - IPartitionsManagerMock pm; + NiceMock pm; ON_CALL(pm, isPartition(std::string("sdb1"))).WillByDefault(Return(true)); ON_CALL(pm, diskForPartition(std::string("sdb1"))).WillByDefault(Return("sdb")); @@ -60,3 +60,45 @@ TEST(DmesgParserTest, BufferIOError) ASSERT_EQ(errors.size(), 1); ASSERT_NE(errors.find(failedDisk), errors.end()); } + + +TEST(DmesgParserTest, MapsCommonKernelIoErrorsToPhysicalDisk) +{ + NiceMock pm; + ON_CALL(pm, isPartition(std::string("nvme0n1p1"))).WillByDefault(Return(true)); + ON_CALL(pm, diskForPartition(std::string("nvme0n1p1"))).WillByDefault(Return("nvme0n1")); + ON_CALL(pm, isPartition(std::string("sdb1"))).WillByDefault(Return(true)); + ON_CALL(pm, diskForPartition(std::string("sdb1"))).WillByDefault(Return("sdb")); + + const auto errors = DmesgParser::parse( + R"( + [ 100.123456] blk_update_request: I/O error, dev nvme0n1p1, sector 123456 op 0x0:(READ) + [ 101.123456] blk_update_request: critical medium error, dev sda, sector 999 op 0x0:(READ) + [ 102.123456] EXT4-fs error (device sdb1): ext4_find_entry:1456: inode #2: comm systemd: reading directory lblock 0 + )", + pm + ); + + ASSERT_EQ(errors.size(), 3); + EXPECT_NE(errors.find(Disk("nvme0n1")), errors.end()); + EXPECT_NE(errors.find(Disk("sda")), errors.end()); + EXPECT_NE(errors.find(Disk("sdb")), errors.end()); +} + + +TEST(DmesgParserTest, KeepsOriginalDeviceWhenPartitionMappingIsMissing) +{ + NiceMock pm; + ON_CALL(pm, isPartition(std::string("dm-0"))).WillByDefault(Return(true)); + ON_CALL(pm, diskForPartition(std::string("dm-0"))).WillByDefault(Return("")); + + const auto errors = DmesgParser::parse( + R"( + [ 100.123456] Buffer I/O error on dev dm-0, logical block 12345 + )", + pm + ); + + ASSERT_EQ(errors.size(), 1); + EXPECT_NE(errors.find(Disk("dm-0")), errors.end()); +} diff --git a/tests/agent/LsblkOutputParserTests.cpp b/tests/agent/LsblkOutputParserTests.cpp index 4080957..c00f0b6 100644 --- a/tests/agent/LsblkOutputParserTests.cpp +++ b/tests/agent/LsblkOutputParserTests.cpp @@ -38,3 +38,39 @@ TEST(LsblkOutputParserTest, fullOutput) )); } + +TEST(LsblkOutputParserTest, ignoresMalformedRows) +{ + const auto result = LsblkOutputParser::parse( + R"(NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT + sda 8:0 0 120034123776 0 disk + malformed-row + sda1 8:1 0 209715200 0 part /boot + sdb 8:16 0 not-a-number 0 disk + )" + ); + + EXPECT_THAT(result, UnorderedElementsAre( + LsblkOutputParser::LsblkEntry{ .name = "sda", .type = "disk", .size = 120034123776, .partitions = {"sda1"}, .major = 8, .minor = 0 } + )); +} + + +TEST(LsblkOutputParserTest, doesNotMatchSiblingDiskByPrefix) +{ + const auto result = LsblkOutputParser::parse( + R"(NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT + sda 8:0 0 1000 0 disk + sdaa 65:160 0 2000 0 disk + sdaa1 65:161 0 1900 0 part /data + nvme0n1 259:0 0 3000 0 disk + nvme0n1p1 259:1 0 2900 0 part /fast + )" + ); + + EXPECT_THAT(result, UnorderedElementsAre( + LsblkOutputParser::LsblkEntry{ .name = "sda", .type = "disk", .size = 1000, .partitions = {}, .major = 8, .minor = 0 }, + LsblkOutputParser::LsblkEntry{ .name = "sdaa", .type = "disk", .size = 2000, .partitions = {"sdaa1"}, .major = 65, .minor = 160 }, + LsblkOutputParser::LsblkEntry{ .name = "nvme0n1", .type = "disk", .size = 3000, .partitions = {"nvme0n1p1"}, .major = 259, .minor = 0 } + )); +} diff --git a/tests/agent/SmartHealthAnalyzerTests.cpp b/tests/agent/SmartHealthAnalyzerTests.cpp index 43bd742..9ce60bf 100644 --- a/tests/agent/SmartHealthAnalyzerTests.cpp +++ b/tests/agent/SmartHealthAnalyzerTests.cpp @@ -179,9 +179,9 @@ TEST_F(SmartHealthAnalyzerTest, SeagateProfileMasksSeekErrorRate) EXPECT_CALL(*m_reader, ReadSMARTData(_)).WillOnce(Return(data)); EXPECT_CALL(*m_reader, ReadTestStatus(_)).WillOnce(Return(SmartTestStatus{})); - m_analyzer->Refresh({m_disk}); + m_analyzer->Refresh({seagateDisk}); - EXPECT_EQ(GeneralHealth::GOOD, m_analyzer->GetStatus(m_disk)); + EXPECT_EQ(GeneralHealth::GOOD, m_analyzer->GetStatus(seagateDisk)); } @@ -198,9 +198,9 @@ TEST_F(SmartHealthAnalyzerTest, UnknownVendorUsesGenericProfile) EXPECT_CALL(*m_reader, ReadSMARTData(_)).WillOnce(Return(data)); EXPECT_CALL(*m_reader, ReadTestStatus(_)).WillOnce(Return(SmartTestStatus{})); - m_analyzer->Refresh({m_disk}); + m_analyzer->Refresh({unknownDisk}); - EXPECT_EQ(GeneralHealth::CHECK_STATUS, m_analyzer->GetStatus(m_disk)); + EXPECT_EQ(GeneralHealth::CHECK_STATUS, m_analyzer->GetStatus(unknownDisk)); } diff --git a/tests/monitor/AgentsListTests.cpp b/tests/monitor/AgentsListTests.cpp index 58d5d76..c2dfc2a 100644 --- a/tests/monitor/AgentsListTests.cpp +++ b/tests/monitor/AgentsListTests.cpp @@ -70,6 +70,24 @@ TEST(AgentsListTest, doubleAgentRemoveShouldBeSafe) } +TEST(AgentsListTest, removeAgentAtIgnoresInvalidIndexes) +{ + NiceMock statusProvider; + AgentsList aal(statusProvider); + + AgentInformation info("Krzysiu", QHostAddress("192.168.1.12"), 2300, AgentInformation::DetectionSource::Hardcoded); + + aal.addAgent(info); + aal.removeAgentAt(-1); + aal.removeAgentAt(1); + + ASSERT_EQ(aal.rowCount({}), 1); + + const auto& agents = aal.agents(); + EXPECT_THAT(agents, Contains(info)); +} + + TEST(AgentsListTest, agentRemoval) { NiceMock statusProvider; @@ -89,6 +107,48 @@ TEST(AgentsListTest, agentRemoval) } +TEST(AgentsListTest, zeroconfLossDoesNotRemoveMatchingHardcodedAgent) +{ + NiceMock statusProvider; + AgentsList aal(statusProvider); + + AgentInformation hardcoded("Agent", QHostAddress("192.168.1.12"), 2300, AgentInformation::DetectionSource::Hardcoded); + AgentInformation zeroconf("Agent", QHostAddress("192.168.1.12"), 2300, AgentInformation::DetectionSource::ZeroConf); + + aal.addAgent(hardcoded); + aal.removeAgent(zeroconf); + + ASSERT_EQ(aal.rowCount({}), 1); + + const QModelIndex idx = aal.index(0, 0); + EXPECT_EQ(idx.data(AgentsList::AgentDetectionTypeRole), + static_cast(AgentInformation::DetectionSource::Hardcoded)); +} + + +TEST(AgentsListTest, hardcodedDuplicatePromotesExistingZeroconfAgent) +{ + NiceMock statusProvider; + AgentsList aal(statusProvider); + + AgentInformation zeroconf("Agent", QHostAddress("192.168.1.12"), 2300, AgentInformation::DetectionSource::ZeroConf); + AgentInformation hardcoded("Agent", QHostAddress("192.168.1.12"), 2300, AgentInformation::DetectionSource::Hardcoded); + + aal.addAgent(zeroconf); + aal.addAgent(hardcoded); + + ASSERT_EQ(aal.rowCount({}), 1); + + const QModelIndex idx = aal.index(0, 0); + EXPECT_EQ(idx.data(AgentsList::AgentDetectionTypeRole), + static_cast(AgentInformation::DetectionSource::Hardcoded)); + + aal.removeAgent(zeroconf); + + EXPECT_EQ(aal.rowCount({}), 1); +} + + TEST(AgentsListTest, listofAvailableRoles) { NiceMock statusProvider; @@ -340,6 +400,9 @@ TEST(AgentsListTest, agentDetectionTypeRoleFetching) AgentInformation info1("John Connor", QHostAddress("192.168.1.15"), 1998, AgentInformation::DetectionSource::Hardcoded); AgentInformation info2("T-1000", QHostAddress("192.168.1.16"), 1998, AgentInformation::DetectionSource::ZeroConf); + EXPECT_CALL(statusProvider, observe(info1)).Times(1); + EXPECT_CALL(statusProvider, observe(info2)).Times(1); + aal.addAgent(info1); aal.addAgent(info2);