PEL: Read the compatible system names property
The chassis object in the inventory has (or rather at the time of this
writing: will have) a Names property that contains a list of the
compatible system types for the current system. An example is:
["company-systemA-4G", "company-systemA", "company"].
Add this to the DataInterface class, and remove the previous
'getSystemType' API that was there but was still stubbed out.
Also change all the calls from getSystemType() to the new call
getSystemNames(), and check against all entries in that array to see if
the desired name applies to the current system.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I86aa0c15c564153fea612f407c161dfe9041fce6
diff --git a/extensions/openpower-pels/data_interface.cpp b/extensions/openpower-pels/data_interface.cpp
index f84e80f..74da2e2 100644
--- a/extensions/openpower-pels/data_interface.cpp
+++ b/extensions/openpower-pels/data_interface.cpp
@@ -34,6 +34,7 @@
{
constexpr auto objectMapper = "/xyz/openbmc_project/object_mapper";
constexpr auto systemInv = "/xyz/openbmc_project/inventory/system";
+constexpr auto chassisInv = "/xyz/openbmc_project/inventory/system/chassis";
constexpr auto baseInv = "/xyz/openbmc_project/inventory";
constexpr auto bmcState = "/xyz/openbmc_project/state/bmc0";
constexpr auto chassisState = "/xyz/openbmc_project/state/chassis0";
@@ -58,6 +59,8 @@
"xyz.openbmc_project.Inventory.Item.Board.Motherboard";
constexpr auto viniRecordVPD = "com.ibm.ipzvpd.VINI";
constexpr auto locCode = "com.ibm.ipzvpd.Location";
+constexpr auto invCompatible =
+ "xyz.openbmc_project.Inventory.Decorator.Compatible";
} // namespace interface
using namespace sdbusplus::xyz::openbmc_project::State::OperatingSystem::server;
@@ -142,6 +145,13 @@
*this, [this](const auto& value) {
this->_hostState = std::get<std::string>(value);
}));
+
+ // Watch the compatible system names property
+ _properties.emplace_back(std::make_unique<PropertyWatcher<DataInterface>>(
+ bus, object_path::chassisInv, interface::invCompatible, "Names", *this,
+ [this](const auto& value) {
+ this->_systemNames = std::get<std::vector<std::string>>(value);
+ }));
}
DBusPropertyMap
diff --git a/extensions/openpower-pels/data_interface.hpp b/extensions/openpower-pels/data_interface.hpp
index d6153b0..a2dd8f2 100644
--- a/extensions/openpower-pels/data_interface.hpp
+++ b/extensions/openpower-pels/data_interface.hpp
@@ -235,15 +235,13 @@
getLocationCode(const std::string& inventoryPath) const = 0;
/**
- * @brief Gets the system type from Entity Manager
+ * @brief Get the list of system type names the system is called.
*
- * @param[in] std::string - The system type string
+ * @return std::vector<std::string> - The list of names
*/
- virtual std::string getSystemType() const
+ virtual const std::vector<std::string>& getSystemNames() const
{
- // TODO, not implemented by entity manager yet, but adding now
- // so it can be mocked.
- return _systemType;
+ return _systemNames;
}
protected:
@@ -348,9 +346,9 @@
std::string _motherboardCCIN;
/**
- * @brief The system type
+ * @brief The compatible system names array
*/
- std::string _systemType;
+ std::vector<std::string> _systemNames;
};
/**
diff --git a/extensions/openpower-pels/dbus_types.hpp b/extensions/openpower-pels/dbus_types.hpp
index 1a95c8b..8743f8f 100644
--- a/extensions/openpower-pels/dbus_types.hpp
+++ b/extensions/openpower-pels/dbus_types.hpp
@@ -9,7 +9,8 @@
namespace openpower::pels
{
-using DBusValue = std::variant<std::string, bool, std::vector<uint8_t>>;
+using DBusValue = std::variant<std::string, bool, std::vector<uint8_t>,
+ std::vector<std::string>>;
using DBusProperty = std::string;
using DBusInterface = std::string;
using DBusService = std::string;
diff --git a/extensions/openpower-pels/registry.cpp b/extensions/openpower-pels/registry.cpp
index 9b2262d..22cdc27 100644
--- a/extensions/openpower-pels/registry.cpp
+++ b/extensions/openpower-pels/registry.cpp
@@ -335,8 +335,9 @@
* ]
* }
*/
-const nlohmann::json& findCalloutList(const nlohmann::json& json,
- const std::string& systemType)
+const nlohmann::json&
+ findCalloutList(const nlohmann::json& json,
+ const std::vector<std::string>& systemNames)
{
const nlohmann::json* callouts = nullptr;
@@ -352,7 +353,9 @@
{
if (calloutList.contains("System"))
{
- if (systemType == calloutList["System"].get<std::string>())
+ if (std::find(systemNames.begin(), systemNames.end(),
+ calloutList["System"].get<std::string>()) !=
+ systemNames.end())
{
callouts = &calloutList["CalloutList"];
break;
@@ -367,13 +370,16 @@
if (!callouts)
{
+ std::string types;
+ std::for_each(systemNames.begin(), systemNames.end(),
+ [&types](const auto& t) { types += t + '|'; });
log<level::WARNING>(
- "No matching system type entry or default system type entry "
+ "No matching system name entry or default system name entry "
" for PEL callout list",
- entry("SYSTEMTYPE=%s", systemType.c_str()));
+ entry("SYSTEMNAMES=%s", types.c_str()));
throw std::runtime_error{
- "Could not find a CalloutList JSON for this error and system type"};
+ "Could not find a CalloutList JSON for this error and system name"};
}
return *callouts;
@@ -451,17 +457,18 @@
* ]
*
* @param[in] json - The callout JSON
- * @param[in] systemType - The system type from EntityManager
+ * @param[in] systemNames - List of compatible system type names
*
* @return std::vector<RegistryCallout> - The callouts to use
*/
-std::vector<RegistryCallout> getCalloutsWithoutAD(const nlohmann::json& json,
- const std::string& systemType)
+std::vector<RegistryCallout>
+ getCalloutsWithoutAD(const nlohmann::json& json,
+ const std::vector<std::string>& systemNames)
{
std::vector<RegistryCallout> calloutEntries;
// Find the CalloutList to use based on the system type
- const auto& calloutList = findCalloutList(json, systemType);
+ const auto& calloutList = findCalloutList(json, systemNames);
// We finally found the callouts, make the objects.
for (const auto& callout : calloutList)
@@ -503,14 +510,14 @@
* entry used when there is no AdditionalData key.
*
* @param[in] json - The callout JSON
- * @param[in] systemType - The system type from EntityManager
+ * @param[in] systemNames - List of compatible system type names
* @param[in] additionalData - The AdditionalData property
*
* @return std::vector<RegistryCallout> - The callouts to use
*/
std::vector<RegistryCallout>
getCalloutsUsingAD(const nlohmann::json& json,
- const std::string& systemType,
+ const std::vector<std::string>& systemNames,
const AdditionalData& additionalData)
{
// This indicates which AD field we'll be using
@@ -550,7 +557,7 @@
}
// Proceed to find the callouts possibly based on system type.
- return getCalloutsWithoutAD((*it)["Callouts"], systemType);
+ return getCalloutsWithoutAD((*it)["Callouts"], systemNames);
}
} // namespace helper
@@ -725,18 +732,18 @@
std::vector<RegistryCallout>
Registry::getCallouts(const nlohmann::json& calloutJSON,
- const std::string& systemType,
+ const std::vector<std::string>& systemNames,
const AdditionalData& additionalData)
{
// The JSON may either use an AdditionalData key
// as an index, or not.
if (helper::calloutUsesAdditionalData(calloutJSON))
{
- return helper::getCalloutsUsingAD(calloutJSON, systemType,
+ return helper::getCalloutsUsingAD(calloutJSON, systemNames,
additionalData);
}
- return helper::getCalloutsWithoutAD(calloutJSON, systemType);
+ return helper::getCalloutsWithoutAD(calloutJSON, systemNames);
}
} // namespace message
diff --git a/extensions/openpower-pels/registry.hpp b/extensions/openpower-pels/registry.hpp
index b145311..673e2c2 100644
--- a/extensions/openpower-pels/registry.hpp
+++ b/extensions/openpower-pels/registry.hpp
@@ -274,14 +274,14 @@
* Throws exceptions on failures.
*
* @param[in] calloutJSON - Where to look up the callouts
- * @param[in] systemType - The system type from EntityManager
+ * @param[in] systemNames - List of compatible system type names
* @param[in] additionalData - The AdditionalData property
*
* @return std::vector<RegistryCallout> - The callouts to use
*/
static std::vector<RegistryCallout>
getCallouts(const nlohmann::json& calloutJSON,
- const std::string& systemType,
+ const std::vector<std::string>& systemNames,
const AdditionalData& additionalData);
private:
diff --git a/extensions/openpower-pels/src.cpp b/extensions/openpower-pels/src.cpp
index c2edd90..13a3ae5 100644
--- a/extensions/openpower-pels/src.cpp
+++ b/extensions/openpower-pels/src.cpp
@@ -571,10 +571,10 @@
{
try
{
- auto systemType = dataIface.getSystemType();
+ auto systemNames = dataIface.getSystemNames();
auto regCallouts = message::Registry::getCallouts(
- regEntry.callouts.value(), systemType, additionalData);
+ regEntry.callouts.value(), systemNames, additionalData);
for (const auto& regCallout : regCallouts)
{
diff --git a/extensions/openpower-pels/user_header.cpp b/extensions/openpower-pels/user_header.cpp
index 07c3de3..0e53acf 100644
--- a/extensions/openpower-pels/user_header.cpp
+++ b/extensions/openpower-pels/user_header.cpp
@@ -70,7 +70,7 @@
{
// Find the severity possibly dependent on the system type.
auto sev =
- getSeverity(entry.severity.value(), dataIface.getSystemType());
+ getSeverity(entry.severity.value(), dataIface.getSystemNames());
if (sev)
{
_eventSeverity = *sev;
@@ -78,11 +78,15 @@
else
{
// Someone screwed up the message registry.
+ std::string types;
+ const auto& compatibles = dataIface.getSystemNames();
+ std::for_each(compatibles.begin(), compatibles.end(),
+ [&types](const auto& t) { types += t + '|'; });
+
log<level::ERR>(
- "No severity entry found for this error and system type",
+ "No severity entry found for this error and system name",
phosphor::logging::entry("ERROR=%s", entry.name.c_str()),
- phosphor::logging::entry("SYSTEMTYPE=%s",
- dataIface.getSystemType().c_str()));
+ phosphor::logging::entry("SYSTEMNAMES=%s", types.c_str()));
// Have to choose something, just use informational.
_eventSeverity = 0;
@@ -194,7 +198,7 @@
std::optional<uint8_t> UserHeader::getSeverity(
const std::vector<message::RegistrySeverity>& severities,
- const std::string& systemType) const
+ const std::vector<std::string>& systemNames) const
{
const uint8_t* s = nullptr;
@@ -202,7 +206,8 @@
// entry (where no system type is specified).
for (const auto& sev : severities)
{
- if (sev.system == systemType)
+ if (std::find(systemNames.begin(), systemNames.end(), sev.system) !=
+ systemNames.end())
{
s = &sev.severity;
break;
diff --git a/extensions/openpower-pels/user_header.hpp b/extensions/openpower-pels/user_header.hpp
index a033ac0..50c5824 100644
--- a/extensions/openpower-pels/user_header.hpp
+++ b/extensions/openpower-pels/user_header.hpp
@@ -244,11 +244,11 @@
*
* @param[in] severities - The array of {systype, severity}
* structures to find an entry in.
- * @param[in] systemType - The system type from DataInterface.
+ * @param[in] systemNames - List of compatible system type names
*/
std::optional<uint8_t>
getSeverity(const std::vector<message::RegistrySeverity>& severities,
- const std::string& systemType) const;
+ const std::vector<std::string>& systemNames) const;
/**
* @brief The subsystem associated with the event.
*/
diff --git a/test/openpower-pels/mocks.hpp b/test/openpower-pels/mocks.hpp
index 1fd679f..be3291d 100644
--- a/test/openpower-pels/mocks.hpp
+++ b/test/openpower-pels/mocks.hpp
@@ -32,9 +32,10 @@
MOCK_METHOD(void, getHWCalloutFields,
(const std::string&, std::string&, std::string&, std::string&),
(const override));
- MOCK_METHOD(std::string, getSystemType, (), (const override));
MOCK_METHOD(std::string, getLocationCode, (const std::string&),
(const override));
+ MOCK_METHOD(const std::vector<std::string>&, getSystemNames, (),
+ (const override));
void changeHostState(bool newState)
{
diff --git a/test/openpower-pels/registry_test.cpp b/test/openpower-pels/registry_test.cpp
index ec930e9..fa1aa0c 100644
--- a/test/openpower-pels/registry_test.cpp
+++ b/test/openpower-pels/registry_test.cpp
@@ -362,6 +362,8 @@
// Test when callouts are in the JSON.
TEST_F(RegistryTest, TestGetCallouts)
{
+ std::vector<std::string> names;
+
{
// Callouts without AD, that depend on system type,
// where there is a default entry without a system type.
@@ -403,8 +405,9 @@
])"_json;
AdditionalData ad;
+ names.push_back("system1");
- auto callouts = Registry::getCallouts(json, "system1", ad);
+ auto callouts = Registry::getCallouts(json, names, ad);
EXPECT_EQ(callouts.size(), 3);
EXPECT_EQ(callouts[0].priority, "high");
EXPECT_EQ(callouts[0].locCode, "P1-C1");
@@ -423,7 +426,8 @@
EXPECT_EQ(callouts[2].symbolicFRUTrusted, "");
// system2 isn't in the JSON, so it will pick the default one
- callouts = Registry::getCallouts(json, "system2", ad);
+ names[0] = "system2";
+ callouts = Registry::getCallouts(json, names, ad);
EXPECT_EQ(callouts.size(), 2);
EXPECT_EQ(callouts[0].priority, "medium");
EXPECT_EQ(callouts[0].locCode, "");
@@ -440,7 +444,8 @@
{
auto json = R"([])"_json;
AdditionalData ad;
- EXPECT_THROW(Registry::getCallouts(json, "system1", ad),
+ names[0] = "system1";
+ EXPECT_THROW(Registry::getCallouts(json, names, ad),
std::runtime_error);
}
@@ -479,8 +484,9 @@
])"_json;
AdditionalData ad;
+ names[0] = "system1";
- auto callouts = Registry::getCallouts(json, "system1", ad);
+ auto callouts = Registry::getCallouts(json, names, ad);
EXPECT_EQ(callouts.size(), 2);
EXPECT_EQ(callouts[0].priority, "high");
EXPECT_EQ(callouts[0].locCode, "P1-C1");
@@ -493,7 +499,8 @@
EXPECT_EQ(callouts[1].symbolicFRU, "1234567");
EXPECT_EQ(callouts[1].symbolicFRUTrusted, "");
- callouts = Registry::getCallouts(json, "system2", ad);
+ names[0] = "system2";
+ callouts = Registry::getCallouts(json, names, ad);
EXPECT_EQ(callouts.size(), 1);
EXPECT_EQ(callouts[0].priority, "medium");
EXPECT_EQ(callouts[0].locCode, "P7");
@@ -503,7 +510,8 @@
// There is no entry for system3 or a default system,
// so this should fail.
- EXPECT_THROW(Registry::getCallouts(json, "system3", ad),
+ names[0] = "system3";
+ EXPECT_THROW(Registry::getCallouts(json, names, ad),
std::runtime_error);
}
@@ -574,8 +582,9 @@
// Find callouts for PROC_NUM 0 on system3
std::vector<std::string> adData{"PROC_NUM=0"};
AdditionalData ad{adData};
+ names[0] = "system3";
- auto callouts = Registry::getCallouts(json, "system3", ad);
+ auto callouts = Registry::getCallouts(json, names, ad);
EXPECT_EQ(callouts.size(), 3);
EXPECT_EQ(callouts[0].priority, "high");
EXPECT_EQ(callouts[0].locCode, "P1-C5");
@@ -594,7 +603,9 @@
EXPECT_EQ(callouts[2].symbolicFRUTrusted, "");
// Find callouts for PROC_NUM 0 that uses the default system entry.
- callouts = Registry::getCallouts(json, "system99", ad);
+ names[0] = "system99";
+
+ callouts = Registry::getCallouts(json, names, ad);
EXPECT_EQ(callouts.size(), 1);
EXPECT_EQ(callouts[0].priority, "low");
EXPECT_EQ(callouts[0].locCode, "P55");
@@ -606,8 +617,9 @@
// Find callouts for PROC_NUM 1 that uses a default system entry.
std::vector<std::string> adData{"PROC_NUM=1"};
AdditionalData ad{adData};
+ names[0] = "system1";
- auto callouts = Registry::getCallouts(json, "system1", ad);
+ auto callouts = Registry::getCallouts(json, names, ad);
EXPECT_EQ(callouts.size(), 1);
EXPECT_EQ(callouts[0].priority, "high");
EXPECT_EQ(callouts[0].locCode, "P1-C6");
@@ -620,7 +632,7 @@
std::vector<std::string> adData{"PROC_NUM=2"};
AdditionalData ad{adData};
- EXPECT_THROW(Registry::getCallouts(json, "system1", ad),
+ EXPECT_THROW(Registry::getCallouts(json, names, ad),
std::runtime_error);
}
}
diff --git a/test/openpower-pels/src_test.cpp b/test/openpower-pels/src_test.cpp
index e9abb8e..d767c53 100644
--- a/test/openpower-pels/src_test.cpp
+++ b/test/openpower-pels/src_test.cpp
@@ -26,6 +26,7 @@
using ::testing::InvokeWithoutArgs;
using ::testing::NiceMock;
using ::testing::Return;
+using ::testing::ReturnRef;
using ::testing::SetArgReferee;
namespace fs = std::filesystem;
@@ -517,7 +518,9 @@
// Call out a symbolic FRU and a procedure
AdditionalData ad;
NiceMock<MockDataInterface> dataIface;
- EXPECT_CALL(dataIface, getSystemType).WillOnce(Return("systemA"));
+ std::vector<std::string> names{"systemA"};
+
+ EXPECT_CALL(dataIface, getSystemNames).WillOnce(ReturnRef(names));
SRC src{entry, ad, dataIface};
@@ -551,7 +554,9 @@
// another one without.
AdditionalData ad;
NiceMock<MockDataInterface> dataIface;
- EXPECT_CALL(dataIface, getSystemType).WillOnce(Return("systemB"));
+ std::vector<std::string> names{"systemB"};
+
+ EXPECT_CALL(dataIface, getSystemNames).WillOnce(ReturnRef(names));
SRC src{entry, ad, dataIface};
diff --git a/test/openpower-pels/user_header_test.cpp b/test/openpower-pels/user_header_test.cpp
index f37b085..546b480 100644
--- a/test/openpower-pels/user_header_test.cpp
+++ b/test/openpower-pels/user_header_test.cpp
@@ -24,6 +24,7 @@
using namespace openpower::pels;
using ::testing::Return;
+using ::testing::ReturnRef;
TEST(UserHeaderTest, SizeTest)
{
@@ -112,7 +113,9 @@
regEntry.eventScope = 2;
MockDataInterface dataIface;
- EXPECT_CALL(dataIface, getSystemType).WillOnce(Return("systemA"));
+ std::vector<std::string> names{"systemA"};
+
+ EXPECT_CALL(dataIface, getSystemNames).WillOnce(ReturnRef(names));
UserHeader uh(regEntry, phosphor::logging::Entry::Level::Error,
dataIface);
@@ -143,10 +146,14 @@
regEntry.severity = {{"", 0x20}, {"systemB", 0x10}, {"systemA", 0x00}};
MockDataInterface dataIface;
- EXPECT_CALL(dataIface, getSystemType)
- .WillOnce(Return("systemA"))
- .WillOnce(Return("systemB"))
- .WillOnce(Return("systemC"));
+ std::vector<std::string> names1{"systemA"};
+ std::vector<std::string> names2{"systemB"};
+ std::vector<std::string> names3{"systemC"};
+
+ EXPECT_CALL(dataIface, getSystemNames)
+ .WillOnce(ReturnRef(names1))
+ .WillOnce(ReturnRef(names2))
+ .WillOnce(ReturnRef(names3));
{
UserHeader uh(regEntry, phosphor::logging::Entry::Level::Error,
@@ -204,6 +211,9 @@
MockDataInterface dataIface;
+ std::vector<std::string> names{"systemA"};
+ EXPECT_CALL(dataIface, getSystemNames).WillOnce(ReturnRef(names));
+
UserHeader uh(regEntry, phosphor::logging::Entry::Level::Error, dataIface);
ASSERT_EQ(uh.eventType(), 0);