bmc-ready: provide special error return on BMCNotReady
A new feature has been proposed[1] and implemented[2] which can be
optionally enabled on a system to not allow a chassis or host power on
operation when the BMC is not in a "Ready" state.
In those situations, if a power on operation is requested, the D-Bus
error response will be a specific BMCNotReady error. In those
situations, respond to the user with a more targeted error asking them
to retry in 10 seconds. The 10s retry is based on my experience with
OpenBMC based systems, the longest time between bmcweb being up and
running and BMC Ready is around 30s.
Tested:
- Enabled BMC Ready feature, manually put BMC in NotReady state,
and requested a:
```
/redfish/v1/Chassis/chassis/Actions/Chassis.Reset -d '{"ResetType": "PowerCycle"}'
```
- Confirmed new response message:
```
"Message": "The service is temporarily unavailable. Retry in 10 seconds."
```
- Stopped Chassis state service and verified expected "internal service
error" on same request
- Ran similar test with Systems/system/Actions/ComputerSystem.Reset API
- Confirmed good paths still worked as expected
[1]: https://lists.ozlabs.org/pipermail/openbmc/2023-May/033383.html
[2]: https://gerrit.openbmc.org/q/topic:bmc-ready-check
Change-Id: I6a6e5774c96b4f37c794ba49a5e06d3e51156d09
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
diff --git a/redfish-core/lib/chassis.hpp b/redfish-core/lib/chassis.hpp
index d5b8d1f..99e305b 100644
--- a/redfish-core/lib/chassis.hpp
+++ b/redfish-core/lib/chassis.hpp
@@ -30,6 +30,7 @@
#include <boost/system/error_code.hpp>
#include <boost/url/format.hpp>
#include <sdbusplus/asio/property.hpp>
+#include <sdbusplus/message.hpp>
#include <sdbusplus/unpack_properties.hpp>
#include <array>
@@ -632,6 +633,36 @@
std::bind_front(handleChassisPatch, std::ref(app)));
}
+/**
+ * Handle error responses from d-bus for chassis power cycles
+ */
+inline void handleChassisPowerCycleError(const boost::system::error_code& ec,
+ const sdbusplus::message_t& eMsg,
+ crow::Response& res)
+{
+ if (eMsg.get_error() == nullptr)
+ {
+ BMCWEB_LOG_ERROR << "D-Bus response error: " << ec;
+ messages::internalError(res);
+ return;
+ }
+ std::string_view errorMessage = eMsg.get_error()->name;
+
+ // If operation failed due to BMC not being in Ready state, tell
+ // user to retry in a bit
+ if (errorMessage ==
+ std::string_view("xyz.openbmc_project.State.Chassis.Error.BMCNotReady"))
+ {
+ BMCWEB_LOG_DEBUG << "BMC not ready, operation not allowed right now";
+ messages::serviceTemporarilyUnavailable(res, "10");
+ return;
+ }
+
+ BMCWEB_LOG_ERROR << "Chassis Power Cycle fail " << ec
+ << " sdbusplus:" << errorMessage;
+ messages::internalError(res);
+}
+
inline void
doChassisPowerCycle(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
@@ -669,12 +700,13 @@
}
crow::connections::systemBus->async_method_call(
- [asyncResp](const boost::system::error_code& ec2) {
+ [asyncResp](const boost::system::error_code& ec2,
+ sdbusplus::message_t& sdbusErrMsg) {
// Use "Set" method to set the property value.
if (ec2)
{
- BMCWEB_LOG_ERROR << "[Set] Bad D-Bus request error: " << ec2;
- messages::internalError(asyncResp->res);
+ handleChassisPowerCycleError(ec2, sdbusErrMsg, asyncResp->res);
+
return;
}
diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp
index 19444b2..81db36b 100644
--- a/redfish-core/lib/systems.hpp
+++ b/redfish-core/lib/systems.hpp
@@ -32,10 +32,12 @@
#include "utils/sw_utils.hpp"
#include "utils/time_utils.hpp"
+#include <boost/asio/error.hpp>
#include <boost/container/flat_map.hpp>
#include <boost/system/error_code.hpp>
#include <boost/url/format.hpp>
#include <sdbusplus/asio/property.hpp>
+#include <sdbusplus/message.hpp>
#include <sdbusplus/unpack_properties.hpp>
#include <array>
@@ -2855,6 +2857,46 @@
}
/**
+ * Handle error responses from d-bus for system power requests
+ */
+inline void handleSystemActionResetError(const boost::system::error_code& ec,
+ const sdbusplus::message_t& eMsg,
+ std::string_view resetType,
+ crow::Response& res)
+{
+ if (ec.value() == boost::asio::error::invalid_argument)
+ {
+ messages::actionParameterNotSupported(res, resetType, "Reset");
+ return;
+ }
+
+ if (eMsg.get_error() == nullptr)
+ {
+ BMCWEB_LOG_ERROR << "D-Bus response error: " << ec;
+ messages::internalError(res);
+ return;
+ }
+ std::string_view errorMessage = eMsg.get_error()->name;
+
+ // If operation failed due to BMC not being in Ready state, tell
+ // user to retry in a bit
+ if ((errorMessage ==
+ std::string_view(
+ "xyz.openbmc_project.State.Chassis.Error.BMCNotReady")) ||
+ (errorMessage ==
+ std::string_view("xyz.openbmc_project.State.Host.Error.BMCNotReady")))
+ {
+ BMCWEB_LOG_DEBUG << "BMC not ready, operation not allowed right now";
+ messages::serviceTemporarilyUnavailable(res, "10");
+ return;
+ }
+
+ BMCWEB_LOG_ERROR << "System Action Reset transition fail " << ec
+ << " sdbusplus:" << errorMessage;
+ messages::internalError(res);
+}
+
+/**
* SystemActionsReset class supports handle POST method for Reset action.
* The class retrieves and sends data directly to D-Bus.
*/
@@ -2931,19 +2973,13 @@
if (hostCommand)
{
crow::connections::systemBus->async_method_call(
- [asyncResp, resetType](const boost::system::error_code& ec) {
+ [asyncResp, resetType](const boost::system::error_code& ec,
+ sdbusplus::message_t& sdbusErrMsg) {
if (ec)
{
- BMCWEB_LOG_ERROR << "D-Bus responses error: " << ec;
- if (ec.value() == boost::asio::error::invalid_argument)
- {
- messages::actionParameterNotSupported(
- asyncResp->res, resetType, "Reset");
- }
- else
- {
- messages::internalError(asyncResp->res);
- }
+ handleSystemActionResetError(ec, sdbusErrMsg, resetType,
+ asyncResp->res);
+
return;
}
messages::success(asyncResp->res);
@@ -2957,19 +2993,12 @@
else
{
crow::connections::systemBus->async_method_call(
- [asyncResp, resetType](const boost::system::error_code& ec) {
+ [asyncResp, resetType](const boost::system::error_code& ec,
+ sdbusplus::message_t& sdbusErrMsg) {
if (ec)
{
- BMCWEB_LOG_ERROR << "D-Bus responses error: " << ec;
- if (ec.value() == boost::asio::error::invalid_argument)
- {
- messages::actionParameterNotSupported(
- asyncResp->res, resetType, "Reset");
- }
- else
- {
- messages::internalError(asyncResp->res);
- }
+ handleSystemActionResetError(ec, sdbusErrMsg, resetType,
+ asyncResp->res);
return;
}
messages::success(asyncResp->res);