PEL: Use the real system names property
There is now a 'Names' property on the new
xyz.openbmc_project.Configuration.IBMCompatibleSystem interface that the
DataInterface::getSystemNames function will read. The existing code was
mostly a placeholder until the actual interface showed up on D-Bus.
Since the path that holds the interface is system specific, and the PEL
code should only rarely need this, the code was changed to read it on
demand instead of at caching it startup.
Since getSystemNames() no longer returns a cached value, the signature
was changed slightly to remove the reference from the returned
std::vector<std::string> value.
The UserHeader constructor used to make a call to getSystemNames every
time, and that was changed to only call it when actually necessary,
which is when there is a severity value defined in the message registry
that depends on the system name.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I1d4f307ef38287b631f94dae2b8929307d129029
diff --git a/extensions/openpower-pels/data_interface.cpp b/extensions/openpower-pels/data_interface.cpp
index 4cad337..de6591a 100644
--- a/extensions/openpower-pels/data_interface.cpp
+++ b/extensions/openpower-pels/data_interface.cpp
@@ -64,8 +64,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";
+constexpr auto compatible =
+ "xyz.openbmc_project.Configuration.IBMCompatibleSystem";
constexpr auto vpdManager = "com.ibm.VPD.Manager";
constexpr auto association = "xyz.openbmc_project.Association";
constexpr auto ledGroup = "xyz.openbmc_project.Led.Group";
@@ -153,13 +153,6 @@
*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
@@ -444,5 +437,32 @@
_bus.call(method);
}
+std::vector<std::string> DataInterface::getSystemNames() const
+{
+ DBusSubTree subtree;
+ DBusValue names;
+
+ auto method = _bus.new_method_call(service_name::objectMapper,
+ object_path::objectMapper,
+ interface::objectMapper, "GetSubTree");
+ method.append(std::string{"/"}, 0,
+ std::vector<std::string>{interface::compatible});
+ auto reply = _bus.call(method);
+
+ reply.read(subtree);
+ if (subtree.empty())
+ {
+ throw std::runtime_error("Compatible interface not on D-Bus");
+ }
+
+ const auto& object = *(subtree.begin());
+ const auto& path = object.first;
+ const auto& service = object.second.begin()->first;
+
+ getProperty(service, path, interface::compatible, "Names", names);
+
+ return std::get<std::vector<std::string>>(names);
+}
+
} // namespace pels
} // namespace openpower
diff --git a/extensions/openpower-pels/data_interface.hpp b/extensions/openpower-pels/data_interface.hpp
index 09bcf59..9a48721 100644
--- a/extensions/openpower-pels/data_interface.hpp
+++ b/extensions/openpower-pels/data_interface.hpp
@@ -239,10 +239,7 @@
*
* @return std::vector<std::string> - The list of names
*/
- virtual const std::vector<std::string>& getSystemNames() const
- {
- return _systemNames;
- }
+ virtual std::vector<std::string> getSystemNames() const = 0;
/**
* @brief Fills in the placeholder 'Ufcs' in the passed in location
@@ -401,11 +398,6 @@
* @brief The motherboard CCIN
*/
std::string _motherboardCCIN;
-
- /**
- * @brief The compatible system names array
- */
- std::vector<std::string> _systemNames;
};
/**
@@ -492,6 +484,13 @@
getLocationCode(const std::string& inventoryPath) const override;
/**
+ * @brief Get the list of system type names the system is called.
+ *
+ * @return std::vector<std::string> - The list of names
+ */
+ std::vector<std::string> getSystemNames() const override;
+
+ /**
* @brief Fills in the placeholder 'Ufcs' in the passed in location
* code with the machine feature code and serial number, which
* is needed to create a valid location code.
diff --git a/extensions/openpower-pels/dbus_types.hpp b/extensions/openpower-pels/dbus_types.hpp
index 8743f8f..9532177 100644
--- a/extensions/openpower-pels/dbus_types.hpp
+++ b/extensions/openpower-pels/dbus_types.hpp
@@ -19,5 +19,7 @@
using DBusPathList = std::vector<DBusPath>;
using DBusPropertyMap = std::map<DBusProperty, DBusValue>;
using DBusInterfaceMap = std::map<DBusInterface, DBusPropertyMap>;
+using DBusSubTree =
+ std::map<DBusPath, std::map<DBusService, DBusInterfaceList>>;
} // namespace openpower::pels
diff --git a/extensions/openpower-pels/user_header.cpp b/extensions/openpower-pels/user_header.cpp
index a687c55..276c506 100644
--- a/extensions/openpower-pels/user_header.cpp
+++ b/extensions/openpower-pels/user_header.cpp
@@ -69,24 +69,19 @@
else
{
// Find the severity possibly dependent on the system type.
- auto sev =
- getSeverity(entry.severity.value(), dataIface.getSystemNames());
+ auto sev = getSeverity(entry.severity.value(), dataIface);
if (sev)
{
_eventSeverity = *sev;
}
else
{
- // Someone screwed up the message registry.
+ // Either someone screwed up the message registry
+ // or getSystemNames failed.
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 name",
- phosphor::logging::entry("ERROR=%s", entry.name.c_str()),
- phosphor::logging::entry("SYSTEMNAMES=%s", types.c_str()));
+ "Failed finding the severity in the message registry",
+ phosphor::logging::entry("ERROR=%s", entry.name.c_str()));
// Have to choose something, just use informational.
_eventSeverity = 0;
@@ -199,9 +194,27 @@
std::optional<uint8_t> UserHeader::getSeverity(
const std::vector<message::RegistrySeverity>& severities,
- const std::vector<std::string>& systemNames) const
+ const DataInterfaceBase& dataIface) const
{
const uint8_t* s = nullptr;
+ std::vector<std::string> systemNames;
+
+ // getSystemNames makes D-Bus calls, so only call it if we
+ // know we'll need it because there is a system name in the sev list
+ if (std::any_of(severities.begin(), severities.end(),
+ [](const auto& sev) { return !sev.system.empty(); }))
+ {
+ try
+ {
+ systemNames = dataIface.getSystemNames();
+ }
+ catch (const std::exception& e)
+ {
+ log<level::ERR>("Failed trying to look up system names on D-Bus",
+ entry("ERROR=%s", e.what()));
+ return std::nullopt;
+ }
+ }
// Find the severity to use for this system type, or use the default
// entry (where no system type is specified).
diff --git a/extensions/openpower-pels/user_header.hpp b/extensions/openpower-pels/user_header.hpp
index 07b74ae..da9be6c 100644
--- a/extensions/openpower-pels/user_header.hpp
+++ b/extensions/openpower-pels/user_header.hpp
@@ -245,11 +245,12 @@
*
* @param[in] severities - The array of {systype, severity}
* structures to find an entry in.
- * @param[in] systemNames - List of compatible system type names
+ * @param[in] dataIface - The DataInterface object
*/
std::optional<uint8_t>
getSeverity(const std::vector<message::RegistrySeverity>& severities,
- const std::vector<std::string>& systemNames) const;
+ const DataInterfaceBase& dataIface) const;
+
/**
* @brief The subsystem associated with the event.
*/