Update error handling on sdbusplus calls
Error handling was added to the bus.call() routine,
which effectively deprecated the is_method_error()
check. Errors should now be handled in a try/catch
block.
Change-Id: I1dcbbaa91db443445ef6bb2f616bd37f06731d36
Signed-off-by: Anthony Wilson <wilsonan@us.ibm.com>
diff --git a/bmc_state_manager.cpp b/bmc_state_manager.cpp
index 6651d2f..20efcf0 100644
--- a/bmc_state_manager.cpp
+++ b/bmc_state_manager.cpp
@@ -1,5 +1,6 @@
#include <cassert>
#include <phosphor-logging/log.hpp>
+#include <sdbusplus/exception.hpp>
#include <sys/sysinfo.h>
#include "bmc_state_manager.hpp"
@@ -14,6 +15,7 @@
namespace server = sdbusplus::xyz::openbmc_project::State::server;
using namespace phosphor::logging;
+using sdbusplus::exception::SdBusError;
constexpr auto obmcStandbyTarget = "obmc-standby.target";
constexpr auto signalDone = "done";
@@ -38,17 +40,17 @@
method.append(obmcStandbyTarget);
- auto result = this->bus.call(method);
-
- // Check that the bus call didn't result in an error
- if (result.is_method_error())
+ try
{
- log<level::ERR>("Error in bus call.");
+ auto result = this->bus.call(method);
+ result.read(unitTargetPath);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in GetUnit call", entry("ERROR=%s", e.what()));
return;
}
- result.read(unitTargetPath);
-
method = this->bus.new_method_call(
SYSTEMD_SERVICE,
static_cast<const std::string&>(unitTargetPath).c_str(),
@@ -56,19 +58,23 @@
method.append("org.freedesktop.systemd1.Unit", "ActiveState");
- result = this->bus.call(method);
-
- // Check that the bus call didn't result in an error
- if (result.is_method_error())
+ try
{
- log<level::INFO>("Error in bus call.");
+ auto result = this->bus.call(method);
+
+ // Is obmc-standby.target active or inactive?
+ result.read(currentState);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::INFO>("Error in ActiveState Get",
+ entry("ERROR=%s", e.what()));
return;
}
- // Is obmc-standby.target active or inactive?
- result.read(currentState);
-
- if (currentState == activeState)
+ auto currentStateStr =
+ sdbusplus::message::variant_ns::get<std::string>(currentState);
+ if (currentStateStr == activeState)
{
log<level::INFO>("Setting the BMCState field",
entry("CURRENT_BMC_STATE=%s", "BMC_READY"));
@@ -77,8 +83,16 @@
// Unsubscribe so we stop processing all other signals
method = this->bus.new_method_call(SYSTEMD_SERVICE, SYSTEMD_OBJ_PATH,
SYSTEMD_INTERFACE, "Unsubscribe");
- this->bus.call(method);
- this->stateSignal.release();
+ try
+ {
+ this->bus.call(method);
+ this->stateSignal.release();
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::INFO>("Error in Unsubscribe",
+ entry("ERROR=%s", e.what()));
+ }
}
else
{
@@ -94,7 +108,15 @@
{
auto method = this->bus.new_method_call(SYSTEMD_SERVICE, SYSTEMD_OBJ_PATH,
SYSTEMD_INTERFACE, "Subscribe");
- this->bus.call(method);
+
+ try
+ {
+ this->bus.call(method);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::INFO>("Error in Subscribe", entry("ERROR=%s", e.what()));
+ }
return;
}
@@ -114,7 +136,16 @@
// needs to be irreversible once started
method.append(sysdUnit, "replace-irreversibly");
- this->bus.call(method);
+ try
+ {
+ this->bus.call(method);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::INFO>("Error in StartUnit - replace-irreversibly",
+ entry("ERROR=%s", e.what()));
+ }
+
return;
}
@@ -138,8 +169,17 @@
auto method =
this->bus.new_method_call(SYSTEMD_SERVICE, SYSTEMD_OBJ_PATH,
SYSTEMD_INTERFACE, "Unsubscribe");
- this->bus.call(method);
- this->stateSignal.release();
+
+ try
+ {
+ this->bus.call(method);
+ this->stateSignal.release();
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::INFO>("Error in Unsubscribe",
+ entry("ERROR=%s", e.what()));
+ }
}
return 0;
diff --git a/chassis_state_manager.cpp b/chassis_state_manager.cpp
index 16cecec..8fd56b8 100644
--- a/chassis_state_manager.cpp
+++ b/chassis_state_manager.cpp
@@ -70,24 +70,7 @@
try
{
auto reply = this->bus.call(method);
- if (reply.is_method_error())
- {
- log<level::ERR>(
- "Error in response message - could not get initial pgood");
- goto fail;
- }
-
- try
- {
- reply.read(pgood);
- }
- catch (const SdBusError& e)
- {
- log<level::ERR>("Error in bus response - bad encoding of pgood",
- entry("ERROR=%s", e.what()),
- entry("REPLY_SIG=%s", reply.get_signature()));
- goto fail;
- }
+ reply.read(pgood);
if (sdbusplus::message::variant_ns::get<int>(pgood) == 1)
{
@@ -165,25 +148,15 @@
SYSTEMD_INTERFACE, "GetUnit");
method.append(target);
- auto result = this->bus.call(method);
-
- // Check that the bus call didn't result in an error
- if (result.is_method_error())
- {
- log<level::ERR>("Error in bus call - could not resolve GetUnit for:",
- entry(" %s", SYSTEMD_INTERFACE));
- return false;
- }
try
{
+ auto result = this->bus.call(method);
result.read(unitTargetPath);
}
catch (const SdBusError& e)
{
- log<level::ERR>("Error in bus response - bad encoding for GetUnit",
- entry("ERROR=%s", e.what()),
- entry("REPLY_SIG=%s", result.get_signature()));
+ log<level::ERR>("Error in GetUnit call", entry("ERROR=%s", e.what()));
return false;
}
@@ -193,17 +166,19 @@
SYSTEMD_PROPERTY_IFACE, "Get");
method.append(SYSTEMD_INTERFACE_UNIT, "ActiveState");
- result = this->bus.call(method);
- // Check that the bus call didn't result in an error
- if (result.is_method_error())
+ try
{
- log<level::ERR>("Error in bus call - could not resolve Get for:",
- entry(" %s", SYSTEMD_PROPERTY_IFACE));
+ auto result = this->bus.call(method);
+ result.read(currentState);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in ActiveState Get",
+ entry("ERROR=%s", e.what()));
return false;
}
- result.read(currentState);
const auto& currentStateStr =
sdbusplus::message::variant_ns::get<std::string>(currentState);
return currentStateStr == ACTIVE_STATE ||
diff --git a/discover_system_state.cpp b/discover_system_state.cpp
index 92d3416..3a38152 100644
--- a/discover_system_state.cpp
+++ b/discover_system_state.cpp
@@ -4,6 +4,7 @@
#include <string>
#include <config.h>
#include <systemd/sd-bus.h>
+#include <sdbusplus/exception.hpp>
#include <sdbusplus/server.hpp>
#include <phosphor-logging/log.hpp>
#include <phosphor-logging/elog-errors.hpp>
@@ -22,6 +23,7 @@
using namespace phosphor::logging;
using namespace sdbusplus::xyz::openbmc_project::Common::Error;
using namespace sdbusplus::xyz::openbmc_project::Control::Power::server;
+using sdbusplus::exception::SdBusError;
constexpr auto MAPPER_BUSNAME = "xyz.openbmc_project.ObjectMapper";
constexpr auto MAPPER_PATH = "/xyz/openbmc_project/object_mapper";
@@ -36,23 +38,27 @@
MAPPER_INTERFACE, "GetObject");
mapper.append(path, std::vector<std::string>({interface}));
- auto mapperResponseMsg = bus.call(mapper);
-
- if (mapperResponseMsg.is_method_error())
- {
- log<level::ERR>("Error in mapper call", entry("PATH=%s", path.c_str()),
- entry("INTERFACE=%s", interface.c_str()));
- throw std::runtime_error("Error in mapper call");
- }
std::map<std::string, std::vector<std::string>> mapperResponse;
- mapperResponseMsg.read(mapperResponse);
- if (mapperResponse.empty())
+ try
{
- log<level::ERR>("Error reading mapper response",
+ auto mapperResponseMsg = bus.call(mapper);
+
+ mapperResponseMsg.read(mapperResponse);
+ if (mapperResponse.empty())
+ {
+ log<level::ERR>("Error reading mapper response",
+ entry("PATH=%s", path.c_str()),
+ entry("INTERFACE=%s", interface.c_str()));
+ throw std::runtime_error("Error reading mapper response");
+ }
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in mapper call", entry("ERROR=%s", e.what()),
entry("PATH=%s", path.c_str()),
entry("INTERFACE=%s", interface.c_str()));
- throw std::runtime_error("Error reading mapper response");
+ throw;
}
return mapperResponse.begin()->first;
@@ -68,16 +74,18 @@
PROPERTY_INTERFACE, "Get");
method.append(interface, propertyName);
- auto reply = bus.call(method);
- if (reply.is_method_error())
+ try
{
- log<level::ERR>("Error in property Get",
- entry("PROPERTY=%s", propertyName.c_str()));
- throw std::runtime_error("Error in property Get");
+ auto reply = bus.call(method);
+ reply.read(property);
}
-
- reply.read(property);
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in property Get", entry("ERROR=%s", e.what()),
+ entry("PROPERTY=%s", propertyName.c_str()));
+ throw;
+ }
if (sdbusplus::message::variant_ns::get<std::string>(property).empty())
{
@@ -148,15 +156,20 @@
settings.powerRestorePolicy.c_str(), "org.freedesktop.DBus.Properties",
"Get");
method.append(powerRestoreIntf, "PowerRestorePolicy");
- auto reply = bus.call(method);
- if (reply.is_method_error())
+
+ sdbusplus::message::variant<std::string> result;
+ try
{
- log<level::ERR>("Error in PowerRestorePolicy Get");
+ auto reply = bus.call(method);
+ reply.read(result);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in PowerRestorePolicy Get",
+ entry("ERROR=%s", e.what()));
elog<InternalFailure>();
}
- sdbusplus::message::variant<std::string> result;
- reply.read(result);
auto powerPolicy = sdbusplus::message::variant_ns::get<std::string>(result);
log<level::INFO>("Host power is off, checking power policy",
diff --git a/host_check_main.cpp b/host_check_main.cpp
index 54a182a..311e6e2 100644
--- a/host_check_main.cpp
+++ b/host_check_main.cpp
@@ -4,6 +4,7 @@
#include <fstream>
#include <cstdio>
#include <sdbusplus/bus.hpp>
+#include <sdbusplus/exception.hpp>
#include <phosphor-logging/log.hpp>
#include <xyz/openbmc_project/Control/Host/server.hpp>
#include <config.h>
@@ -11,6 +12,7 @@
using namespace std::literals;
using namespace phosphor::logging;
using namespace sdbusplus::xyz::openbmc_project::Control::server;
+using sdbusplus::exception::SdBusError;
// Required strings for sending the msg to check on host
constexpr auto MAPPER_BUSNAME = "xyz.openbmc_project.ObjectMapper";
@@ -59,17 +61,21 @@
mapper.append(CONTROL_HOST_PATH,
std::vector<std::string>({CONTROL_HOST_INTERFACE}));
- auto mapperResponseMsg = bus.call(mapper);
-
- if (mapperResponseMsg.is_method_error())
- {
- log<level::ERR>("Error in mapper call for control host");
- // TODO openbmc/openbmc#851 - Once available, throw returned error
- throw std::runtime_error("Error in mapper call for control host");
- }
std::map<std::string, std::vector<std::string>> mapperResponse;
- mapperResponseMsg.read(mapperResponse);
+
+ try
+ {
+ auto mapperResponseMsg = bus.call(mapper);
+ mapperResponseMsg.read(mapperResponse);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in mapper call for control host",
+ entry("ERROR=%s", e.what()));
+ throw;
+ }
+
if (mapperResponse.empty())
{
log<level::ERR>("Error reading mapper resp for control host");
@@ -83,11 +89,15 @@
CONTROL_HOST_INTERFACE, "Execute");
method.append(convertForMessage(Host::Command::Heartbeat).c_str());
- auto reply = bus.call(method);
- if (reply.is_method_error())
+ try
{
- log<level::ERR>("Error in call to control host Execute");
- throw std::runtime_error("Error in call to control host Execute");
+ auto reply = bus.call(method);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in call to control host Execute",
+ entry("ERROR=%s", e.what()));
+ throw;
}
return;
diff --git a/host_state_manager.cpp b/host_state_manager.cpp
index aaac46c..7d661dd 100644
--- a/host_state_manager.cpp
+++ b/host_state_manager.cpp
@@ -8,6 +8,7 @@
#include <cereal/types/tuple.hpp>
#include <cereal/archives/json.hpp>
#include <fstream>
+#include <sdbusplus/exception.hpp>
#include <sdbusplus/server.hpp>
#include <phosphor-logging/log.hpp>
#include <phosphor-logging/elog-errors.hpp>
@@ -34,6 +35,7 @@
sdbusplus::xyz::openbmc_project::State::OperatingSystem::server;
using namespace phosphor::logging;
namespace fs = std::experimental::filesystem;
+using sdbusplus::exception::SdBusError;
// host-shutdown notifies host of shutdown and that leads to host-stop being
// called so initiate a host shutdown with the -shutdown target and consider the
@@ -133,35 +135,37 @@
SYSTEMD_INTERFACE, "GetUnit");
method.append(target);
- auto result = this->bus.call(method);
- // Check that the bus call didn't result in an error
- if (result.is_method_error())
+ try
{
- log<level::ERR>("Error in bus call - could not resolve GetUnit for:",
- entry(" %s", SYSTEMD_INTERFACE));
+ auto result = this->bus.call(method);
+ result.read(unitTargetPath);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in GetUnit call", entry("ERROR=%s", e.what()));
return false;
}
- result.read(unitTargetPath);
-
method = this->bus.new_method_call(
SYSTEMD_SERVICE,
static_cast<const std::string&>(unitTargetPath).c_str(),
SYSTEMD_PROPERTY_IFACE, "Get");
method.append(SYSTEMD_INTERFACE_UNIT, "ActiveState");
- result = this->bus.call(method);
- // Check that the bus call didn't result in an error
- if (result.is_method_error())
+ try
{
- log<level::ERR>("Error in bus call - could not resolve Get for:",
- entry(" %s", SYSTEMD_PROPERTY_IFACE));
+ auto result = this->bus.call(method);
+ result.read(currentState);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in ActiveState Get",
+ entry("ERROR=%s", e.what()));
return false;
}
- result.read(currentState);
const auto& currentStateStr =
sdbusplus::message::variant_ns::get<std::string>(currentState);
return currentStateStr == ACTIVE_STATE ||
@@ -176,44 +180,48 @@
settings.service(settings.autoReboot, autoRebootIntf).c_str(),
settings.autoReboot.c_str(), "org.freedesktop.DBus.Properties", "Get");
method.append(autoRebootIntf, "AutoReboot");
- auto reply = bus.call(method);
- if (reply.is_method_error())
- {
- log<level::ERR>("Error in AutoReboot Get");
- return false;
- }
- sdbusplus::message::variant<bool> result;
- reply.read(result);
- auto autoReboot = sdbusplus::message::variant_ns::get<bool>(result);
- auto rebootCounterParam = reboot::RebootAttempts::attemptsLeft();
-
- if (autoReboot)
+ try
{
- if (rebootCounterParam > 0)
+ auto reply = bus.call(method);
+
+ sdbusplus::message::variant<bool> result;
+ reply.read(result);
+ auto autoReboot = sdbusplus::message::variant_ns::get<bool>(result);
+ auto rebootCounterParam = reboot::RebootAttempts::attemptsLeft();
+
+ if (autoReboot)
{
- // Reduce BOOTCOUNT by 1
- log<level::INFO>("Auto reboot enabled, rebooting");
- return true;
- }
- else if (rebootCounterParam == 0)
- {
- // Reset reboot counter and go to quiesce state
- log<level::INFO>("Auto reboot enabled. "
- "HOST BOOTCOUNT already set to 0.");
- attemptsLeft(BOOT_COUNT_MAX_ALLOWED);
- return false;
+ if (rebootCounterParam > 0)
+ {
+ // Reduce BOOTCOUNT by 1
+ log<level::INFO>("Auto reboot enabled, rebooting");
+ return true;
+ }
+ else if (rebootCounterParam == 0)
+ {
+ // Reset reboot counter and go to quiesce state
+ log<level::INFO>("Auto reboot enabled. "
+ "HOST BOOTCOUNT already set to 0.");
+ attemptsLeft(BOOT_COUNT_MAX_ALLOWED);
+ return false;
+ }
+ else
+ {
+ log<level::INFO>("Auto reboot enabled. "
+ "HOST BOOTCOUNT has an invalid value.");
+ return false;
+ }
}
else
{
- log<level::INFO>("Auto reboot enabled. "
- "HOST BOOTCOUNT has an invalid value.");
+ log<level::INFO>("Auto reboot disabled.");
return false;
}
}
- else
+ catch (const SdBusError& e)
{
- log<level::INFO>("Auto reboot disabled.");
+ log<level::ERR>("Error in AutoReboot Get", entry("ERROR=%s", e.what()));
return false;
}
}
diff --git a/settings.cpp b/settings.cpp
index 70bf04e..7f9f236 100644
--- a/settings.cpp
+++ b/settings.cpp
@@ -1,5 +1,6 @@
#include <phosphor-logging/elog-errors.hpp>
#include <phosphor-logging/log.hpp>
+#include <sdbusplus/exception.hpp>
#include "xyz/openbmc_project/Common/error.hpp"
#include "settings.hpp"
@@ -8,6 +9,7 @@
using namespace phosphor::logging;
using namespace sdbusplus::xyz::openbmc_project::Common::Error;
+using sdbusplus::exception::SdBusError;
constexpr auto mapperService = "xyz.openbmc_project.ObjectMapper";
constexpr auto mapperPath = "/xyz/openbmc_project/object_mapper";
@@ -23,20 +25,26 @@
mapperCall.append(root);
mapperCall.append(depth);
mapperCall.append(settingsIntfs);
- auto response = bus.call(mapperCall);
- if (response.is_method_error())
- {
- log<level::ERR>("Error in mapper GetSubTree");
- elog<InternalFailure>();
- }
using Interfaces = std::vector<Interface>;
using MapperResponse = std::map<Path, std::map<Service, Interfaces>>;
MapperResponse result;
- response.read(result);
- if (result.empty())
+
+ try
{
- log<level::ERR>("Invalid response from mapper");
+ auto response = bus.call(mapperCall);
+
+ response.read(result);
+ if (result.empty())
+ {
+ log<level::ERR>("Invalid response from mapper");
+ elog<InternalFailure>();
+ }
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in mapper GetSubTree",
+ entry("ERROR=%s", e.what()));
elog<InternalFailure>();
}
@@ -69,15 +77,20 @@
mapperCall.append(path);
mapperCall.append(Interfaces({interface}));
- auto response = bus.call(mapperCall);
- if (response.is_method_error())
+ std::map<Service, Interfaces> result;
+
+ try
{
- log<level::ERR>("Error in mapper GetObject");
+ auto response = bus.call(mapperCall);
+ response.read(result);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in mapper GetObject",
+ entry("ERROR=%s", e.what()));
elog<InternalFailure>();
}
- std::map<Service, Interfaces> result;
- response.read(result);
if (result.empty())
{
log<level::ERR>("Invalid response from mapper");