functions: Remove GetObject to check if PLDM is running
In order to check if PLDM is running, the code was subscribing to Name
Owner change and calling GetObject to see if PLDM was running, but PLDM
starts and does not immediately create its Object Manager, therefore
there is a time window when if PLDM starts but hasn't created its Object
Manager, and the openpower service starts, it'll hang forever waiting
for the Name Owner change signal that will not be sent because PLDM
already started.
Instead of calling GetObject, just attempt to run the callback, in this
case setting the bios attribute, and forward the exception if it fails
(due to PLDM not running and the property not existing in D-Bus yet),
so that the openpower service continues to wait. Change the log severity
to informational since setting the bios attribute will fail if PLDM has
not started yet.
Tested: Stopped the PLDM and Entity Manager services and started these
services and the openpower service in various different orders to verify
the openpower service ran when both PLDM and EM had started.
Change-Id: Idd2f344beda8dfcf2b987b3cab6f8967c63cab4c
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
diff --git a/functions.cpp b/functions.cpp
index 38ebeca..fe112a4 100644
--- a/functions.cpp
+++ b/functions.cpp
@@ -13,6 +13,7 @@
#include <sdbusplus/exception.hpp>
#include <sdbusplus/message.hpp>
#include <sdeventplus/event.hpp>
+#include <xyz/openbmc_project/Common/error.hpp>
#include <filesystem>
#include <fstream>
@@ -37,37 +38,6 @@
std::map<sdbusplus::message::object_path, InterfacesPropertiesMap>;
/**
- * @brief GetObject function to find the service given an object path.
- * It is used to determine if a service is running, so there is no need
- * to specify interfaces as a parameter to constrain the search.
- */
-std::string getObject(sdbusplus::bus::bus& bus, const std::string& path)
-{
- auto method = bus.new_method_call(MAPPER_BUSNAME, MAPPER_PATH,
- MAPPER_BUSNAME, "GetObject");
- method.append(path);
- std::vector<std::string> interfaces;
- method.append(interfaces);
-
- std::vector<std::pair<std::string, std::vector<std::string>>> response;
-
- try
- {
- auto reply = bus.call(method);
- reply.read(response);
- if (response.empty())
- {
- return std::string{};
- }
- }
- catch (const sdbusplus::exception::exception& e)
- {
- return std::string{};
- }
- return response[0].first;
-}
-
-/**
* @brief Returns the managed objects for a given service
*/
ManagedObjectType getManagedObjects(sdbusplus::bus::bus& bus,
@@ -436,10 +406,11 @@
reply.read(response);
if (response.empty())
{
- log<level::ERR>("Error reading mapper response",
- entry("PATH=%s", biosConfigPath),
- entry("INTERFACE=%s", biosConfigIntf));
- return;
+ log<level::INFO>("Error reading mapper response",
+ entry("PATH=%s", biosConfigPath),
+ entry("INTERFACE=%s", biosConfigIntf));
+ throw sdbusplus::xyz::openbmc_project::Common::Error::
+ InternalFailure();
}
auto method = bus.new_method_call((response.begin()->first).c_str(),
biosConfigPath,
@@ -450,10 +421,10 @@
}
catch (const sdbusplus::exception::exception& e)
{
- log<level::ERR>("Error setting the bios attribute",
- entry("ERROR=%s", e.what()),
- entry("ATTRIBUTE=%s", dbusAttrName));
- return;
+ log<level::INFO>("Error setting the bios attribute",
+ entry("ERROR=%s", e.what()),
+ entry("ATTRIBUTE=%s", dbusAttrName));
+ throw;
}
}
@@ -505,7 +476,14 @@
std::get<std::vector<std::string>>(propertyIterator->second);
if (callback)
{
- callback(ibmCompatibleSystem);
+ try
+ {
+ callback(ibmCompatibleSystem);
+ }
+ catch (const sdbusplus::exception::exception& e)
+ {
+ return false;
+ }
}
// IBMCompatibleSystem found and callback issued.
@@ -593,7 +571,14 @@
if (getExtensionsForIbmCompatibleSystem(extensionMap, ibmCompatibleSystem,
extensions))
{
- setBiosAttr(elementsJsonFilePath, extensions);
+ try
+ {
+ setBiosAttr(elementsJsonFilePath, extensions);
+ }
+ catch (const sdbusplus::exception::exception& e)
+ {
+ throw;
+ }
}
}
@@ -755,13 +740,14 @@
std::make_shared<decltype(elementsJsonFilePath)>(
std::move(elementsJsonFilePath));
- // Entity Manager is needed to get the list of supported extensions. Add a
- // match to monitor interfaces added in case it's not running yet.
auto maybeSetAttrWithArgsBound =
std::bind(maybeSetBiosAttr, std::cref(*pExtensionMap),
std::cref(*pElementsJsonFilePath), std::placeholders::_1);
std::vector<std::shared_ptr<void>> matches;
+
+ // Entity Manager is needed to get the list of supported extensions. Add a
+ // match to monitor interfaces added in case it's not running yet.
matches.emplace_back(std::make_shared<sdbusplus::bus::match::match>(
bus,
sdbusplus::bus::match::rules::interfacesAdded() +
@@ -769,17 +755,15 @@
"xyz.openbmc_project.EntityManager"),
[pldmPath, pExtensionMap, pElementsJsonFilePath,
maybeSetAttrWithArgsBound, &loop](auto& message) {
- auto bus = sdbusplus::bus::new_default();
- auto pldmObject = getObject(bus, pldmPath);
- if (pldmObject.empty())
- {
- return;
- }
if (maybeCallMessage(message, maybeSetAttrWithArgsBound))
{
loop.exit(0);
}
}));
+
+ // The BIOS attribute table can only be updated if PLDM is running because
+ // PLDM is the one that exposes this property. Add a match to monitor when
+ // the PLDM service starts.
matches.emplace_back(std::make_shared<sdbusplus::bus::match::match>(
bus,
sdbusplus::bus::match::rules::nameOwnerChanged() +
@@ -811,14 +795,6 @@
}
}));
- // The BIOS attribute table can only be updated if PLDM is running because
- // PLDM is the one that exposes this property. Return if it's not running.
- auto pldmObject = getObject(bus, pldmPath);
- if (pldmObject.empty())
- {
- return matches;
- }
-
InterfacesPropertiesMap interfacesAndProperties;
auto objects = getManagedObjects(bus, entityManagerServiceName);
for (const auto& pair : objects)
diff --git a/meson.build b/meson.build
index 2c0d3c4..a07be6d 100644
--- a/meson.build
+++ b/meson.build
@@ -296,6 +296,7 @@
'functions.cpp',
dependencies: [
dependency('gtest', main: true),
+ dependency('phosphor-dbus-interfaces'),
dependency('sdbusplus'),
dependency('sdeventplus'),
],