Async request to create PEL
To avoid any deadlocks between phosphor-logging and vpd-manager
service a wrapper around request to create PEL is created
which enables vpd-manager to make async requests to create PEL.
It also implements a new exception type to throw any GPIO
related exception. This was required to throw specific
exception.
Test:
Scenario 1:
While vpd-manager logs a pel with call out to an inventory
item.
It should be checked that both the services do not enter into
a deadlock situation.
Scenario 2:
Any third process is requesting to log PEL with call out
to an inventory item in a loop.
In the mean time vpd-manager also requests to log a PEL
with or without call out to inventory item.
All the pels should be logged correctly in the system.
No deadlock should appear.
Signed-off-by: Sunny Srivastava <sunnsr25@in.ibm.com>
Change-Id: I0efaf6918e679ca94808fc70d747c843a50eaf78
diff --git a/ibm_vpd_app.cpp b/ibm_vpd_app.cpp
index fd9713f..b625568 100644
--- a/ibm_vpd_app.cpp
+++ b/ibm_vpd_app.cpp
@@ -484,40 +484,50 @@
return;
}
- if (executePreAction(json, file))
+ try
{
- if (json["frus"][file].at(0).find("devAddress") !=
- json["frus"][file].at(0).end())
+ if (executePreAction(json, file))
{
- // Now bind the device
- string bind = json["frus"][file].at(0).value("devAddress", "");
- cout << "Binding device " << bind << endl;
- string bindCmd = string("echo \"") + bind +
- string("\" > /sys/bus/i2c/drivers/at24/bind");
- cout << bindCmd << endl;
- executeCmd(bindCmd);
-
- // Check if device showed up (test for file)
- if (!fs::exists(file))
+ if (json["frus"][file].at(0).find("devAddress") !=
+ json["frus"][file].at(0).end())
{
- cerr << "EEPROM " << file
- << " does not exist. Take failure action" << endl;
- // If not, then take failure postAction
+ // Now bind the device
+ string bind = json["frus"][file].at(0).value("devAddress", "");
+ cout << "Binding device " << bind << endl;
+ string bindCmd = string("echo \"") + bind +
+ string("\" > /sys/bus/i2c/drivers/at24/bind");
+ cout << bindCmd << endl;
+ executeCmd(bindCmd);
+
+ // Check if device showed up (test for file)
+ if (!fs::exists(file))
+ {
+ cerr << "EEPROM " << file
+ << " does not exist. Take failure action" << endl;
+ // If not, then take failure postAction
+ executePostFailAction(json, file);
+ }
+ }
+ else
+ {
+ // missing required informations
+ cerr << "VPD inventory JSON missing basic informations of "
+ "preAction "
+ "for this FRU : ["
+ << file << "]. Executing executePostFailAction." << endl;
+
+ // Take failure postAction
executePostFailAction(json, file);
+ return;
}
}
- else
- {
- // missing required informations
- cerr
- << "VPD inventory JSON missing basic informations of preAction "
- "for this FRU : ["
- << file << "]. Executing executePostFailAction." << endl;
-
- // Take failure postAction
- executePostFailAction(json, file);
- return;
- }
+ }
+ catch (const GpioException& e)
+ {
+ PelAdditionalData additionalData{};
+ additionalData.emplace("DESCRIPTION", e.what());
+ createPEL(additionalData, PelSeverity::WARNING, errIntfForGpioError,
+ nullptr);
}
}
@@ -755,7 +765,7 @@
PelAdditionalData additionalData{};
additionalData.emplace("DESCRIPTION", err);
createPEL(additionalData, PelSeverity::WARNING,
- errIntfForInvalidSystemType);
+ errIntfForInvalidSystemType, nullptr);
exit(-1);
}
@@ -859,7 +869,7 @@
additionalData.emplace("DESCRIPTION", errMsg);
createPEL(additionalData, PelSeverity::WARNING,
- errIntfForInvalidVPD);
+ errIntfForInvalidVPD, nullptr);
}
}
else
@@ -902,7 +912,7 @@
// log PEL TODO: Block IPL
createPEL(additionalData, PelSeverity::ERROR,
- errIntfForBlankSystemVPD);
+ errIntfForBlankSystemVPD, nullptr);
continue;
}
}
@@ -1436,7 +1446,7 @@
{
additionalData.emplace("JSON_PATH", ex.getJsonPath());
additionalData.emplace("DESCRIPTION", ex.what());
- createPEL(additionalData, pelSeverity, errIntfForJsonFailure);
+ createPEL(additionalData, pelSeverity, errIntfForJsonFailure, nullptr);
cerr << ex.what() << "\n";
rc = -1;
@@ -1446,7 +1456,7 @@
additionalData.emplace("DESCRIPTION", "ECC check failed");
additionalData.emplace("CALLOUT_INVENTORY_PATH",
INVENTORY_PATH + baseFruInventoryPath);
- createPEL(additionalData, pelSeverity, errIntfForEccCheckFail);
+ createPEL(additionalData, pelSeverity, errIntfForEccCheckFail, nullptr);
dumpBadVpd(file, vpdVector);
cerr << ex.what() << "\n";
rc = -1;
@@ -1475,7 +1485,8 @@
additionalData.emplace("DESCRIPTION", errorMsg);
additionalData.emplace("CALLOUT_INVENTORY_PATH",
INVENTORY_PATH + baseFruInventoryPath);
- createPEL(additionalData, pelSeverity, errIntfForInvalidVPD);
+ createPEL(additionalData, pelSeverity, errIntfForInvalidVPD,
+ nullptr);
rc = -1;
}
diff --git a/ibm_vpd_utils.cpp b/ibm_vpd_utils.cpp
index cc46aa2..de25528 100644
--- a/ibm_vpd_utils.cpp
+++ b/ibm_vpd_utils.cpp
@@ -161,7 +161,52 @@
}
void createPEL(const std::map<std::string, std::string>& additionalData,
- const Severity& sev, const std::string& errIntf)
+ const Severity& sev, const std::string& errIntf, sd_bus* sdBus)
+{
+ // This pointer will be NULL in case the call is made from ibm-read-vpd. In
+ // that case a sync call will do.
+ if (sdBus == nullptr)
+ {
+ createSyncPEL(additionalData, sev, errIntf);
+ }
+ else
+ {
+ std::string errDescription{};
+ auto pos = additionalData.find("DESCRIPTION");
+ if (pos != additionalData.end())
+ {
+ errDescription = pos->second;
+ }
+ else
+ {
+ errDescription = "Description field missing in additional data";
+ }
+
+ std::string pelSeverity =
+ "xyz.openbmc_project.Logging.Entry.Level.Error";
+ auto itr = sevMap.find(sev);
+ if (itr != sevMap.end())
+ {
+ pelSeverity = itr->second;
+ }
+
+ // Implies this is a call from Manager. Hence we need to make an async
+ // call to avoid deadlock with Phosphor-logging.
+ auto rc = sd_bus_call_method_async(
+ sdBus, NULL, loggerService, loggerObjectPath, loggerCreateInterface,
+ "Create", NULL, NULL, "ssa{ss}", errIntf.c_str(),
+ pelSeverity.c_str(), 1, "DESCRIPTION", errDescription.c_str());
+
+ if (rc < 0)
+ {
+ log<level::ERR>("Error calling sd_bus_call_method_async",
+ entry("RC=%d", rc), entry("MSG=%s", strerror(-rc)));
+ }
+ }
+}
+
+void createSyncPEL(const std::map<std::string, std::string>& additionalData,
+ const Severity& sev, const std::string& errIntf)
{
try
{
@@ -678,7 +723,7 @@
if (!outputLine)
{
- throw std::runtime_error(
+ throw GpioException(
"Couldn't find output line for the GPIO. Skipping "
"this GPIO action.");
}
@@ -701,9 +746,7 @@
errMsg += " i2cBusAddress: " + i2cBusAddr;
}
- PelAdditionalData additionalData{};
- additionalData.emplace("DESCRIPTION", errMsg);
- createPEL(additionalData, PelSeverity::WARNING, errIntfForGpioError);
+ throw GpioException(e.what());
}
return;
@@ -734,7 +777,7 @@
std::cerr << "Couldn't find the presence line for - "
<< presPinName << std::endl;
- throw std::runtime_error(
+ throw GpioException(
"Couldn't find the presence line for the "
"GPIO. Skipping this GPIO action.");
}
@@ -761,14 +804,9 @@
errMsg += " i2cBusAddress: " + i2cBusAddr;
}
- PelAdditionalData additionalData{};
- additionalData.emplace("DESCRIPTION", errMsg);
- createPEL(additionalData, PelSeverity::WARNING,
- errIntfForGpioError);
-
// Take failure postAction
executePostFailAction(json, file);
- return false;
+ throw GpioException(errMsg);
}
}
else
@@ -819,7 +857,7 @@
{
std::cerr << "Couldn't find the line for output pin - "
<< pinName << std::endl;
- throw std::runtime_error(
+ throw GpioException(
"Couldn't find output line for the GPIO. "
"Skipping this GPIO action.");
}
@@ -842,15 +880,9 @@
errMsg += " i2cBusAddress: " + i2cBusAddr;
}
- PelAdditionalData additionalData{};
- additionalData.emplace("DESCRIPTION", errMsg);
- createPEL(additionalData, PelSeverity::WARNING,
- errIntfForGpioError);
-
// Take failure postAction
executePostFailAction(json, file);
-
- return false;
+ throw GpioException(errMsg);
}
}
else
@@ -863,7 +895,6 @@
// Take failure postAction
executePostFailAction(json, file);
-
return false;
}
}
diff --git a/ibm_vpd_utils.hpp b/ibm_vpd_utils.hpp
index 60903f9..2b75c7d 100644
--- a/ibm_vpd_utils.hpp
+++ b/ibm_vpd_utils.hpp
@@ -138,12 +138,32 @@
/**
* @brief API to create PEL entry
+ * The api makes synchronous call to phosphor-logging create api.
* @param[in] additionalData - Map holding the additional data
* @param[in] sev - Severity
* @param[in] errIntf - error interface
*/
+void createSyncPEL(const std::map<std::string, std::string>& additionalData,
+ const constants::PelSeverity& sev,
+ const std::string& errIntf);
+
+/**
+ * @brief Api to create PEL.
+ * A wrapper api through which sync/async call to phosphor-logging create api
+ * can be made as and when required.
+ * sdBus as nullptr will result in sync call else async call will be made with
+ * just "DESCRIPTION" key/value pair in additional data.
+ * To make asyn call with more fields in additional data call
+ * "sd_bus_call_method_async" in place.
+ *
+ * @param[in] additionalData - Map of additional data.
+ * @param[in] sev - severity of the PEL.
+ * @param[in] errIntf - Error interface to be used in PEL.
+ * @param[in] sdBus - Pointer to Sd-Bus
+ */
void createPEL(const std::map<std::string, std::string>& additionalData,
- const constants::PelSeverity& sev, const std::string& errIntf);
+ const constants::PelSeverity& sev, const std::string& errIntf,
+ sd_bus* sdBus);
/**
* @brief getVpdFilePath
diff --git a/impl.cpp b/impl.cpp
index 99bc074..900e0e8 100644
--- a/impl.cpp
+++ b/impl.cpp
@@ -289,7 +289,7 @@
additionalData.emplace("DESCRIPTION", errMsg);
additionalData.emplace("CALLOUT_INVENTORY_PATH", inventoryPath);
createPEL(additionalData, PelSeverity::WARNING,
- errIntfForEccCheckFail);
+ errIntfForEccCheckFail, nullptr);
}
catch (const VpdDataException& ex)
{
@@ -300,7 +300,7 @@
additionalData.emplace("DESCRIPTION", errMsg);
additionalData.emplace("CALLOUT_INVENTORY_PATH", inventoryPath);
createPEL(additionalData, PelSeverity::WARNING,
- errIntfForInvalidVPD);
+ errIntfForInvalidVPD, nullptr);
}
#endif
diff --git a/vpd-manager/manager.cpp b/vpd-manager/manager.cpp
index 7fa09ee..35d9ddf 100644
--- a/vpd-manager/manager.cpp
+++ b/vpd-manager/manager.cpp
@@ -449,15 +449,28 @@
if ((jsonFile["frus"][item].at(0)).find("preAction") !=
jsonFile["frus"][item].at(0).end())
{
- if (!executePreAction(jsonFile, item))
+ try
{
- // if the FRU has preAction defined then its execution should
- // pass to ensure bind/unbind of data.
- // preAction execution failed. should not call bind/unbind.
- log<level::ERR>(
- "Pre-Action execution failed for the FRU",
- entry("ERROR=%s",
- ("Inventory path: " + inventoryPath).c_str()));
+ if (!executePreAction(jsonFile, item))
+ {
+ // if the FRU has preAction defined then its execution
+ // should pass to ensure bind/unbind of data.
+ // preAction execution failed. should not call
+ // bind/unbind.
+ log<level::ERR>(
+ "Pre-Action execution failed for the FRU",
+ entry("ERROR=%s",
+ ("Inventory path: " + inventoryPath).c_str()));
+ continue;
+ }
+ }
+ catch (const GpioException& e)
+ {
+ log<level::ERR>(e.what());
+ PelAdditionalData additionalData{};
+ additionalData.emplace("DESCRIPTION", e.what());
+ createPEL(additionalData, PelSeverity::WARNING,
+ errIntfForGpioError, sdBus);
continue;
}
prePostActionRequired = true;
@@ -529,8 +542,18 @@
// Check if device showed up (test for file)
if (!filesystem::exists(item))
{
- // If not, then take failure postAction
- executePostFailAction(jsonFile, item);
+ try
+ {
+ // If not, then take failure postAction
+ executePostFailAction(jsonFile, item);
+ }
+ catch (const GpioException& e)
+ {
+ PelAdditionalData additionalData{};
+ additionalData.emplace("DESCRIPTION", e.what());
+ createPEL(additionalData, PelSeverity::WARNING,
+ errIntfForGpioError, sdBus);
+ }
}
}
}
diff --git a/vpd_exceptions.hpp b/vpd_exceptions.hpp
index 2ea2cd5..f3473e5 100644
--- a/vpd_exceptions.hpp
+++ b/vpd_exceptions.hpp
@@ -134,6 +134,30 @@
}; // class VpdJSonException
+/** @class GpioException
+ * @brief This class extends Exceptions class and define
+ * type for GPIO related exception in VPD
+ */
+class GpioException : public VPDException
+{
+ public:
+ // deleted methods
+ GpioException() = delete;
+ GpioException(const GpioException&) = delete;
+ GpioException(GpioException&&) = delete;
+ GpioException& operator=(const GpioException&) = delete;
+
+ // default destructor
+ ~GpioException() = default;
+
+ /** @brief constructor
+ * @param[in] msg - string to define exception
+ */
+ explicit GpioException(const std::string& msg) : VPDException(msg)
+ {
+ }
+};
+
} // namespace exceptions
} // namespace vpd
} // namespace openpower
\ No newline at end of file