PEL: Update D-Bus event sev based on PEL severity
Put in checks to ensure the D-Bus event log severity agrees with the
final PEL severity for PELs created by the BMC. The D-bus property is
what is used in the Redfish event log, and we want to avoid cases like
having a Critical event log for an informational PEL.
This could happen in the case where the PEL message registry entry for
the event has a hardcoded or manufacturing specific severity value that
is different than the severity the D-Bus event log is first created
with.
This commit ensures that:
* If the PEL is nonError/recovered, the D-Bus severity is one of the
non error ones.
* If the PEL isn't nonError/recovered, then the D-Bus severity also
isn't.
* If the PEL is critical, the D-Bus severity is also critical.
It doesn't try to update the D-Bus severity for every PEL severity
because there isn't a one to one mapping - e.g. Notice, Debug, and
Informational D-Bus severities all map to a PEL severity of
nonError(informational).
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I6b0006034090e6d8e33e9f8b136ae50cce489f6e
diff --git a/extensions/openpower-pels/manager.cpp b/extensions/openpower-pels/manager.cpp
index d44fe35..152e4fb 100644
--- a/extensions/openpower-pels/manager.cpp
+++ b/extensions/openpower-pels/manager.cpp
@@ -20,6 +20,7 @@
#include "pel.hpp"
#include "pel_entry.hpp"
#include "service_indicators.hpp"
+#include "severity.hpp"
#include <fmt/format.h>
#include <sys/inotify.h>
@@ -403,6 +404,7 @@
auto policy = service_indicators::getPolicy(*_dataIface);
policy->activate(*pel);
+ updateDBusSeverity(*pel);
updateEventId(pel);
updateResolution(*pel);
createPELEntry(obmcLogID);
@@ -841,6 +843,40 @@
return false;
}
+void Manager::updateDBusSeverity(const openpower::pels::PEL& pel)
+{
+ // The final severity of the PEL may not agree with the
+ // original severity of the D-Bus event log. Update the
+ // D-Bus property to match in some cases. This is to
+ // ensure there isn't a Critical or Warning Redfish event
+ // log for an informational or recovered PEL (or vice versa).
+ // This doesn't make an explicit call to serialize the new
+ // event log property value because updateEventId() is called
+ // right after this and will do it.
+ auto sevType =
+ static_cast<SeverityType>(pel.userHeader().severity() & 0xF0);
+
+ auto entryN = _logManager.entries.find(pel.obmcLogID());
+ if (entryN != _logManager.entries.end())
+ {
+ auto newSeverity =
+ fixupLogSeverity(entryN->second->severity(), sevType);
+ if (newSeverity)
+ {
+ log<level::INFO>(
+ fmt::format(
+ "Changing event log {} severity from {} "
+ "to {} to match PEL",
+ entryN->second->id(),
+ Entry::convertLevelToString(entryN->second->severity()),
+ Entry::convertLevelToString(*newSeverity))
+ .c_str());
+
+ entryN->second->severity(*newSeverity, true);
+ }
+ }
+}
+
void Manager::setEntryPath(uint32_t obmcLogID)
{
Repository::LogID id{Repository::LogID::Obmc(obmcLogID)};
diff --git a/extensions/openpower-pels/manager.hpp b/extensions/openpower-pels/manager.hpp
index fa8ac0f..e6ae00f 100644
--- a/extensions/openpower-pels/manager.hpp
+++ b/extensions/openpower-pels/manager.hpp
@@ -439,6 +439,15 @@
bool updateResolution(const openpower::pels::PEL& pel);
/**
+ * @brief Check if the D-Bus severity property for the event log
+ * needs to be updated based on the final PEL severity,
+ * and update the property accordingly.
+ *
+ * @param[in] pel - The PEL to operate on.
+ */
+ void updateDBusSeverity(const openpower::pels::PEL& pel);
+
+ /**
* @brief Create PELEntry Interface with supported properties
*
* Create PELEntry Interface and update all the properties which are
diff --git a/extensions/openpower-pels/severity.cpp b/extensions/openpower-pels/severity.cpp
index 2c7c9df..acac0f5 100644
--- a/extensions/openpower-pels/severity.cpp
+++ b/extensions/openpower-pels/severity.cpp
@@ -15,8 +15,6 @@
*/
#include "severity.hpp"
-#include "pel_types.hpp"
-
namespace openpower
{
namespace pels
@@ -24,6 +22,33 @@
using LogSeverity = phosphor::logging::Entry::Level;
+namespace
+{
+
+LogSeverity convertPELSeverityToOBMC(SeverityType pelSeverity)
+{
+ LogSeverity logSeverity = LogSeverity::Error;
+
+ const std::map<SeverityType, LogSeverity> severities{
+ {SeverityType::nonError, LogSeverity::Informational},
+ {SeverityType::recovered, LogSeverity::Informational},
+ {SeverityType::predictive, LogSeverity::Warning},
+ {SeverityType::unrecoverable, LogSeverity::Error},
+ {SeverityType::critical, LogSeverity::Critical},
+ {SeverityType::diagnostic, LogSeverity::Error},
+ {SeverityType::symptom, LogSeverity::Warning}};
+
+ auto s = severities.find(pelSeverity);
+ if (s != severities.end())
+ {
+ logSeverity = s->second;
+ }
+
+ return logSeverity;
+}
+
+} // namespace
+
uint8_t convertOBMCSeverityToPEL(LogSeverity severity)
{
uint8_t pelSeverity = static_cast<uint8_t>(SeverityType::unrecoverable);
@@ -52,5 +77,41 @@
return pelSeverity;
}
+
+std::optional<LogSeverity> fixupLogSeverity(LogSeverity obmcSeverity,
+ SeverityType pelSeverity)
+{
+ bool isNonErrPelSev = (pelSeverity == SeverityType::nonError) ||
+ (pelSeverity == SeverityType::recovered);
+
+ bool isNonErrObmcSev = (obmcSeverity == LogSeverity::Notice) ||
+ (obmcSeverity == LogSeverity::Informational) ||
+ (obmcSeverity == LogSeverity::Debug);
+
+ // If a nonError/recovered PEL, then the LogSeverity must be
+ // Notice/Informational/Debug, otherwise set it to Informational.
+ if (isNonErrPelSev && !isNonErrObmcSev)
+ {
+ return LogSeverity::Informational;
+ }
+
+ // If a Notice/Informational/Debug LogSeverity, then the PEL
+ // severity must be nonError/recovered, otherwise set it
+ // to an appropriate value based on the actual PEL severity.
+ if (isNonErrObmcSev && !isNonErrPelSev)
+ {
+ return convertPELSeverityToOBMC(pelSeverity);
+ }
+
+ // If PEL is critical, the LogSeverity should be as well.
+ if ((obmcSeverity != LogSeverity::Critical) &&
+ (pelSeverity == SeverityType::critical))
+ {
+ return LogSeverity::Critical;
+ }
+
+ return std::nullopt;
+}
+
} // namespace pels
} // namespace openpower
diff --git a/extensions/openpower-pels/severity.hpp b/extensions/openpower-pels/severity.hpp
index f2b9921..a1c1a59 100644
--- a/extensions/openpower-pels/severity.hpp
+++ b/extensions/openpower-pels/severity.hpp
@@ -1,6 +1,9 @@
#pragma once
#include "elog_entry.hpp"
+#include "pel_types.hpp"
+
+#include <optional>
namespace openpower
{
@@ -16,5 +19,25 @@
*/
uint8_t convertOBMCSeverityToPEL(phosphor::logging::Entry::Level severity);
+/**
+ * @brief Possibly calculate a new LogSeverity value based on the
+ * current LogSeverity and PEL severity.
+ *
+ * Just handles cases where the LogSeverity value is clearly out of
+ * sync with the PEL severity:
+ * - critical PEL but non critical LogSeverity
+ * - info PEL but non info LogSeverity
+ * - non info PEL but info LogSeverity
+ *
+ * @param[in] obmcSeverity - The current LogSeverity
+ * @param[in] pelSeverity - The PEL severity
+ *
+ * @return optional<LogSeverity> The new LogSeverity to use if one was
+ * found, otherwise std::nullopt which means the original one
+ * is good enough.
+ */
+std::optional<phosphor::logging::Entry::Level>
+ fixupLogSeverity(phosphor::logging::Entry::Level obmcSeverity,
+ SeverityType pelSeverity);
} // namespace pels
} // namespace openpower
diff --git a/test/openpower-pels/severity_test.cpp b/test/openpower-pels/severity_test.cpp
index 29c3bc3..b006aa9 100644
--- a/test/openpower-pels/severity_test.cpp
+++ b/test/openpower-pels/severity_test.cpp
@@ -31,3 +31,43 @@
ASSERT_EQ(convertOBMCSeverityToPEL(LogSeverity::Alert), 0x40);
ASSERT_EQ(convertOBMCSeverityToPEL(LogSeverity::Error), 0x40);
}
+
+TEST(SeverityTest, fixupLogSeverityTest)
+{
+ struct TestParams
+ {
+ LogSeverity sevIn;
+ SeverityType pelSevIn;
+ std::optional<LogSeverity> sevOut;
+ };
+
+ const std::vector<TestParams> testParams{
+ // Convert nonInfo sevs to info
+ {LogSeverity::Error, SeverityType::nonError,
+ LogSeverity::Informational},
+ {LogSeverity::Critical, SeverityType::recovered,
+ LogSeverity::Informational},
+ {LogSeverity::Warning, SeverityType::nonError,
+ LogSeverity::Informational},
+
+ // Convert info sevs to nonInfo
+ {LogSeverity::Informational, SeverityType::predictive,
+ LogSeverity::Warning},
+ {LogSeverity::Notice, SeverityType::unrecoverable, LogSeverity::Error},
+ {LogSeverity::Debug, SeverityType::critical, LogSeverity::Critical},
+
+ // Convert non-critical to critical
+ {LogSeverity::Warning, SeverityType::critical, LogSeverity::Critical},
+
+ // No change
+ {LogSeverity::Informational, SeverityType::nonError, std::nullopt},
+ {LogSeverity::Debug, SeverityType::recovered, std::nullopt},
+ {LogSeverity::Notice, SeverityType::nonError, std::nullopt},
+ {LogSeverity::Error, SeverityType::unrecoverable, std::nullopt},
+ {LogSeverity::Critical, SeverityType::unrecoverable, std::nullopt}};
+
+ for (const auto& test : testParams)
+ {
+ EXPECT_EQ(fixupLogSeverity(test.sevIn, test.pelSevIn), test.sevOut);
+ }
+}