sysd_monitor: Monitor and log errors
Note some aspects of the code are non-optimal to allow better unit
testing.
Tested:
- Verified failure detected and correct error created in qemu
Change-Id: I1d4c9638fc13147508168278cc5ab90c37e1fb8e
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
diff --git a/Makefile.am b/Makefile.am
index aba2d6b..d62c61b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -13,10 +13,12 @@
AM_CFLAGS = $(CODE_COVERAGE_CFLAGS)
AM_CXXFLAGS = $(CODE_COVERAGE_CXXFLAGS) \
-DBOOST_SYSTEM_NO_DEPRECATED -DBOOST_ERROR_CODE_HEADER_ONLY \
- -DBOOST_ALL_NO_LIB
+ -DBOOST_ALL_NO_LIB $(SDEVENTPLUS_CFLAGS) \
+ $(PHOSPHOR_DBUS_INTERFACES_CFLAGS) $(PHOSPHOR_LOGGING_CFLAGS)
AM_LDFLAGS = $(GMOCK_LIBS) -lgmock_main \
$(GTEST_LIBS) $(OESDK_TESTCASE_FLAGS) $(PTHREAD_LIBS) \
-$(SDBUSPLUS_LIBS) -lboost_system
+ $(SDBUSPLUS_LIBS) -lboost_system $(SDEVENTPLUS_LIBS) \
+ $(PHOSPHOR_DBUS_INTERFACES_LIBS) $(PHOSPHOR_LOGGING_LIBS)
bin_PROGRAMS = \
phosphor-host-state-manager \
@@ -51,7 +53,8 @@
phosphor_systemd_target_monitor_SOURCES = \
systemd_target_monitor.cpp \
- systemd_target_parser.cpp
+ systemd_target_parser.cpp \
+ systemd_target_signal.cpp
generic_cxxflags = \
$(SYSTEMD_CFLAGS) \
diff --git a/systemd_target_monitor.cpp b/systemd_target_monitor.cpp
index 2c58769..dc94951 100644
--- a/systemd_target_monitor.cpp
+++ b/systemd_target_monitor.cpp
@@ -4,6 +4,7 @@
#include <sdbusplus/bus.hpp>
#include <sdeventplus/event.hpp>
#include <systemd_target_parser.hpp>
+#include <systemd_target_signal.hpp>
#include <vector>
using phosphor::logging::level;
@@ -70,8 +71,10 @@
dump_targets(targetData);
}
- // TODO - Begin monitoring for systemd unit changes and logging appropriate
- // errors
+ phosphor::state::manager::SystemdTargetLogging targetMon(targetData, bus);
+
+ // Subscribe to systemd D-bus signals indicating target completions
+ targetMon.subscribeToSystemdSignals();
return event.loop();
}
diff --git a/systemd_target_signal.cpp b/systemd_target_signal.cpp
new file mode 100644
index 0000000..009a9d0
--- /dev/null
+++ b/systemd_target_signal.cpp
@@ -0,0 +1,111 @@
+#include <phosphor-logging/log.hpp>
+#include <phosphor-logging/elog-errors.hpp>
+#include <sdbusplus/server/manager.hpp>
+#include <sdeventplus/event.hpp>
+#include <sdbusplus/exception.hpp>
+#include <systemd_target_signal.hpp>
+#include <xyz/openbmc_project/Common/error.hpp>
+
+namespace phosphor
+{
+namespace state
+{
+namespace manager
+{
+
+using phosphor::logging::elog;
+using phosphor::logging::entry;
+using phosphor::logging::level;
+using phosphor::logging::log;
+using sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
+
+void SystemdTargetLogging::logError(const std::string& error,
+ const std::string& result)
+{
+ auto method = this->bus.new_method_call(
+ "xyz.openbmc_project.Logging", "/xyz/openbmc_project/logging",
+ "xyz.openbmc_project.Logging.Create", "Create");
+ // Signature is ssa{ss}
+ method.append(error);
+ method.append("xyz.openbmc_project.Logging.Entry.Level.Critical");
+ method.append(std::array<std::pair<std::string, std::string>, 1>(
+ {std::pair<std::string, std::string>({"SYSTEMD_RESULT", result})}));
+ try
+ {
+ this->bus.call_noreply(method);
+ }
+ catch (const sdbusplus::exception::SdBusError& e)
+ {
+ log<level::ERR>("Failed to create systemd target error",
+ entry("ERROR=%s", error.c_str()),
+ entry("RESULT=%s", result.c_str()),
+ entry("SDBUSERR=%s", e.what()));
+ }
+}
+
+const std::string* SystemdTargetLogging::processError(const std::string& unit,
+ const std::string& result)
+{
+ auto targetEntry = this->targetData.find(unit);
+ if (targetEntry != this->targetData.end())
+ {
+ // Check if its result matches any of our monitored errors
+ if (std::find(targetEntry->second.errorsToMonitor.begin(),
+ targetEntry->second.errorsToMonitor.end(),
+ result) != targetEntry->second.errorsToMonitor.end())
+ {
+ log<level::INFO>("Monitored systemd unit has hit an error",
+ entry("UNIT=%s", unit.c_str()),
+ entry("RESULT=%s", result.c_str()));
+ return (&targetEntry->second.errorToLog);
+ }
+ }
+ return {nullptr};
+}
+
+void SystemdTargetLogging::systemdUnitChange(sdbusplus::message::message& msg)
+{
+ uint32_t id;
+ sdbusplus::message::object_path objPath;
+ std::string unit{};
+ std::string result{};
+
+ msg.read(id, objPath, unit, result);
+
+ // In most cases it will just be success, in which case just return
+ if (result != "done")
+ {
+ const std::string* error = processError(unit, result);
+
+ // If this is a monitored error then log it
+ if (error)
+ {
+ logError(*error, result);
+ }
+ }
+ return;
+}
+
+void SystemdTargetLogging::subscribeToSystemdSignals()
+{
+ auto method = this->bus.new_method_call(
+ "org.freedesktop.systemd1", "/org/freedesktop/systemd1",
+ "org.freedesktop.systemd1.Manager", "Subscribe");
+
+ try
+ {
+ this->bus.call(method);
+ }
+ catch (const sdbusplus::exception::SdBusError& e)
+ {
+ log<level::ERR>("Failed to subscribe to systemd signals",
+ entry("SDBUSERR=%s", e.what()));
+ elog<InternalFailure>();
+ }
+
+ return;
+}
+
+} // namespace manager
+} // namespace state
+} // namespace phosphor
diff --git a/systemd_target_signal.hpp b/systemd_target_signal.hpp
new file mode 100644
index 0000000..8dc8faf
--- /dev/null
+++ b/systemd_target_signal.hpp
@@ -0,0 +1,97 @@
+#pragma once
+
+#include <sdbusplus/bus.hpp>
+#include <sdbusplus/bus/match.hpp>
+#include <systemd_target_parser.hpp>
+
+extern bool gVerbose;
+
+namespace phosphor
+{
+namespace state
+{
+namespace manager
+{
+/** @class SystemdTargetLogging
+ * @brief Object to monitor input systemd targets and create corresponding
+ * input errors for on failures
+ */
+class SystemdTargetLogging
+{
+ public:
+ SystemdTargetLogging() = delete;
+ SystemdTargetLogging(const SystemdTargetLogging&) = delete;
+ SystemdTargetLogging& operator=(const SystemdTargetLogging&) = delete;
+ SystemdTargetLogging(SystemdTargetLogging&&) = delete;
+ SystemdTargetLogging& operator=(SystemdTargetLogging&&) = delete;
+ virtual ~SystemdTargetLogging() = default;
+
+ SystemdTargetLogging(const TargetErrorData& targetData,
+ sdbusplus::bus::bus& bus) :
+ targetData(targetData),
+ bus(bus),
+ systemdSignals(
+ bus,
+ sdbusplus::bus::match::rules::type::signal() +
+ sdbusplus::bus::match::rules::member("JobRemoved") +
+ sdbusplus::bus::match::rules::path(
+ "/org/freedesktop/systemd1") +
+ sdbusplus::bus::match::rules::interface(
+ "org.freedesktop.systemd1.Manager"),
+ std::bind(std::mem_fn(&SystemdTargetLogging::systemdUnitChange),
+ this, std::placeholders::_1))
+ {
+ }
+
+ /**
+ * @brief subscribe to the systemd signals
+ *
+ * This object needs to monitor systemd target changes so it can create
+ * the required error logs on failures
+ *
+ **/
+ void subscribeToSystemdSignals();
+
+ /** @brief Process the target fail and return error to log
+ *
+ * @note This is public for unit testing purposes
+ *
+ * @param[in] unit - The systemd unit that failed
+ * @param[in] result - The failure code from the system unit
+ *
+ * @return valid pointer to error to log, otherwise nullptr
+ */
+ const std::string* processError(const std::string& unit,
+ const std::string& result);
+
+ private:
+ /** @brief Call phosphor-logging to create error
+ *
+ * @param[in] error - The error to log
+ * @param[in] result - The failure code from the systemd unit
+ */
+ void logError(const std::string& error, const std::string& result);
+
+ /** @brief Check if systemd state change is one to monitor
+ *
+ * Instance specific interface to handle the detected systemd state
+ * change
+ *
+ * @param[in] msg - Data associated with subscribed signal
+ *
+ */
+ void systemdUnitChange(sdbusplus::message::message& msg);
+
+ /** @brief Systemd targets to monitor and error logs to create */
+ const TargetErrorData& targetData;
+
+ /** @brief Persistent sdbusplus DBus bus connection. */
+ sdbusplus::bus::bus& bus;
+
+ /** @brief Used to subscribe to dbus systemd signals **/
+ sdbusplus::bus::match_t systemdSignals;
+};
+
+} // namespace manager
+} // namespace state
+} // namespace phosphor
diff --git a/test/Makefile.am.include b/test/Makefile.am.include
index 95bf636..ba8d106 100644
--- a/test/Makefile.am.include
+++ b/test/Makefile.am.include
@@ -1,4 +1,8 @@
test_systemd_parser_SOURCES = %reldir%/systemd_parser.cpp systemd_target_parser.cpp
+test_systemd_signal_SOURCES = %reldir%/systemd_signal.cpp systemd_target_signal.cpp
check_PROGRAMS += \
%reldir%/systemd_parser
+
+check_PROGRAMS += \
+ %reldir%/systemd_signal
diff --git a/test/systemd_signal.cpp b/test/systemd_signal.cpp
new file mode 100644
index 0000000..ce0a299
--- /dev/null
+++ b/test/systemd_signal.cpp
@@ -0,0 +1,44 @@
+#include <gtest/gtest.h>
+#include <sdbusplus/bus.hpp>
+#include <sdeventplus/event.hpp>
+#include <systemd_target_signal.hpp>
+
+#include <iostream>
+
+// Enable debug by default for debug when needed
+bool gVerbose = true;
+
+TEST(TargetSignalData, BasicPaths)
+{
+
+ // Create default data structure for testing
+ TargetErrorData targetData = {
+ {"multi-user.target",
+ {"xyz.openbmc_project.State.BMC.Error.MultiUserTargetFailure",
+ {"default"}}},
+ {"obmc-chassis-poweron@0.target",
+ {"xyz.openbmc_project.State.Chassis.Error.PowerOnTargetFailure",
+ {"timeout", "failed"}}}};
+
+ auto bus = sdbusplus::bus::new_default();
+ auto event = sdeventplus::Event::get_default();
+ bus.attach_event(event.get(), SD_EVENT_PRIORITY_NORMAL);
+
+ phosphor::state::manager::SystemdTargetLogging targetMon(targetData, bus);
+
+ std::string invalidUnit = "invalid_unit";
+ std::string validError = "timeout";
+ const std::string* errorToLog =
+ targetMon.processError(invalidUnit, validError);
+ EXPECT_EQ(errorToLog, nullptr);
+
+ std::string validUnit = "obmc-chassis-poweron@0.target";
+ std::string invalidError = "invalid_error";
+ errorToLog = targetMon.processError(validUnit, invalidError);
+ EXPECT_EQ(errorToLog, nullptr);
+
+ errorToLog = targetMon.processError(validUnit, validError);
+ EXPECT_NE(errorToLog, nullptr);
+ EXPECT_EQ(*errorToLog,
+ "xyz.openbmc_project.State.Chassis.Error.PowerOnTargetFailure");
+}