summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEd Tanous <ed@tanous.net>2024-02-14 01:43:34 +0300
committerAsmitha Karunanithi <asmitk01@in.ibm.com>2024-03-28 22:50:43 +0300
commitd02aad3988620c648d6e696b67d6c542857f5bfc (patch)
tree3da513c4a205709519d7c360d4cf56df46917a66
parentf0b59af46a6aa84890d2181b08d4e1af5ce5002f (diff)
downloadbmcweb-d02aad3988620c648d6e696b67d6c542857f5bfc.tar.xz
Create Redfish specific setProperty call
There are currently 78 sdbusplus::asio::setProperty calls in redfish-core. The error handler for nearly all of them looks something like: ``` if (ec) { const sd_bus_error* dbusError = msg.get_error(); if ((dbusError != nullptr) && (dbusError->name == std::string_view( "xyz.openbmc_project.Common.Error.InvalidArgument"))) { BMCWEB_LOG_WARNING("DBUS response error: {}", ec); messages::propertyValueIncorrect(asyncResp->res, "<PropertyName>", <PropertyValue>); return; } messages::internalError(asyncResp->res); return; } messages::success(asyncResp->res); ``` In some cases there are more errors handled that translate to more error messages, but the vast majority only handle InvalidArgument. Many of these, like the ones in account_service.hpp, do the error handling in a lambda, which causes readability problems. This commit starts to make things more consistent, and easier for trivial property sets. This commit invents a setDbusProperty method in the redfish namespace that tries to handle all DBus errors in a consistent manner. Looking for input on whether this will work before changing over the other 73 calls. Overall this is less code, fewer inline lambdas, and defaults that should work for MOST use cases of calling an OpenBMC daemon, and fall back to more generic errors when calling a "normal" dbus daemon. As part of this, I've ported over several examples. Some things that might be up in the air: 1. Do we always return 204 no_content on property sets? Today there's a mix of 200, with a Base::Success message, and 204, with an empty body. 2. Do all DBus response codes map to the same error? A majority are covered by xyz.openbmc_project.Common.Error.InvalidArgument, but there are likely differences. If we allow any daemon to return any return code, does that cause compatibility problems later? Tested: ``` curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"HostName":"openbmc@#"}' https://192.168.7.2/redfish/v1/Managers/bmc/EthernetInterfaces/eth0 ``` Returns the appropriate error in the response Base.1.16.0.PropertyValueIncorrect Change-Id: If033a1112ba516792c9386c997d090c8f9094f3a Signed-off-by: Ed Tanous <ed@tanous.net>
-rw-r--r--meson.build2
-rw-r--r--redfish-core/include/utils/dbus_utils.hpp42
-rw-r--r--redfish-core/lib/account_service.hpp365
-rw-r--r--redfish-core/lib/chassis.hpp47
-rw-r--r--redfish-core/lib/ethernet.hpp141
-rw-r--r--redfish-core/lib/sensors.hpp41
-rw-r--r--redfish-core/lib/systems.hpp79
-rw-r--r--redfish-core/lib/update_service.hpp19
-rw-r--r--redfish-core/src/utils/dbus_utils.cpp76
-rw-r--r--test/redfish-core/include/utils/dbus_utils.cpp74
10 files changed, 328 insertions, 558 deletions
diff --git a/meson.build b/meson.build
index 0c5277e0e9..e9223dde34 100644
--- a/meson.build
+++ b/meson.build
@@ -385,6 +385,7 @@ fs = import('fs')
srcfiles_bmcweb = files(
'redfish-core/src/error_messages.cpp',
'redfish-core/src/registries.cpp',
+ 'redfish-core/src/utils/dbus_utils.cpp',
'redfish-core/src/utils/time_utils.cpp',
'redfish-core/src/utils/json_utils.cpp',
'src/boost_asio_ssl.cpp',
@@ -444,6 +445,7 @@ srcfiles_unittest = files(
'test/redfish-core/include/redfish_aggregator_test.cpp',
'test/redfish-core/include/registries_test.cpp',
'test/redfish-core/include/utils/hex_utils_test.cpp',
+ 'test/redfish-core/include/utils/dbus_utils.cpp',
'test/redfish-core/include/utils/ip_utils_test.cpp',
'test/redfish-core/include/utils/json_utils_test.cpp',
'test/redfish-core/include/utils/query_param_test.cpp',
diff --git a/redfish-core/include/utils/dbus_utils.hpp b/redfish-core/include/utils/dbus_utils.hpp
index 298a56b6ec..cd7e0e2ca2 100644
--- a/redfish-core/include/utils/dbus_utils.hpp
+++ b/redfish-core/include/utils/dbus_utils.hpp
@@ -1,9 +1,18 @@
#pragma once
+#include "async_resp.hpp"
+#include "dbus_singleton.hpp"
+#include "error_messages.hpp"
#include "logging.hpp"
+#include <nlohmann/json.hpp>
+#include <sdbusplus/asio/property.hpp>
+#include <sdbusplus/message.hpp>
#include <sdbusplus/unpack_properties.hpp>
+#include <memory>
+#include <string_view>
+
namespace redfish
{
namespace dbus_utils
@@ -22,4 +31,37 @@ struct UnpackErrorPrinter
};
} // namespace dbus_utils
+
+namespace details
+{
+void afterSetProperty(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& redfishPropertyName,
+ const nlohmann::json& propertyValue,
+ const boost::system::error_code& ec,
+ const sdbusplus::message_t& msg);
+}
+
+template <typename PropertyType>
+void setDbusProperty(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ std::string_view processName,
+ const sdbusplus::message::object_path& path,
+ std::string_view interface, std::string_view dbusProperty,
+ std::string_view redfishPropertyName,
+ const PropertyType& prop)
+{
+ std::string processNameStr(processName);
+ std::string interfaceStr(interface);
+ std::string dbusPropertyStr(dbusProperty);
+
+ sdbusplus::asio::setProperty(
+ *crow::connections::systemBus, processNameStr, path.str, interfaceStr,
+ dbusPropertyStr, prop,
+ [asyncResp, redfishPropertyNameStr = std::string{redfishPropertyName},
+ jsonProp = nlohmann::json(prop)](const boost::system::error_code& ec,
+ const sdbusplus::message_t& msg) {
+ details::afterSetProperty(asyncResp, redfishPropertyNameStr, jsonProp,
+ ec, msg);
+ });
+}
+
} // namespace redfish
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 23f1616964..edf3cf7ef2 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -258,19 +258,9 @@ inline void
// logged.
return;
}
-
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.User.Manager",
- dbusObjectPath, "xyz.openbmc_project.User.Attributes", "UserGroups",
- updatedUserGroups, [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_ERROR("D-Bus responses error: {}", ec);
- messages::internalError(asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
+ setDbusProperty(asyncResp, "xyz.openbmc_project.User.Manager",
+ dbusObjectPath, "xyz.openbmc_project.User.Attributes",
+ "UserGroups", "AccountTypes", updatedUserGroups);
}
inline void userErrorMessageHandler(
@@ -433,70 +423,23 @@ inline void handleRoleMapPatch(
// If "RemoteGroup" info is provided
if (remoteGroup)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, ldapDbusService,
- roleMapObjData[index].first,
+ setDbusProperty(
+ asyncResp, ldapDbusService, roleMapObjData[index].first,
"xyz.openbmc_project.User.PrivilegeMapperEntry",
- "GroupName", *remoteGroup,
- [asyncResp, roleMapObjData, serverType, index,
- remoteGroup](const boost::system::error_code& ec,
- const sdbusplus::message_t& msg) {
- if (ec)
- {
- const sd_bus_error* dbusError = msg.get_error();
- if ((dbusError != nullptr) &&
- (dbusError->name ==
- std::string_view(
- "xyz.openbmc_project.Common.Error.InvalidArgument")))
- {
- BMCWEB_LOG_WARNING("DBUS response error: {}",
- ec);
- messages::propertyValueIncorrect(asyncResp->res,
- "RemoteGroup",
- *remoteGroup);
- return;
- }
- messages::internalError(asyncResp->res);
- return;
- }
- asyncResp->res
- .jsonValue[serverType]["RemoteRoleMapping"][index]
- ["RemoteGroup"] = *remoteGroup;
- });
+ "GroupName",
+ std::format("RemoteRoleMapping/{}/RemoteGroup", index),
+ *remoteGroup);
}
// If "LocalRole" info is provided
if (localRole)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, ldapDbusService,
- roleMapObjData[index].first,
+ setDbusProperty(
+ asyncResp, ldapDbusService, roleMapObjData[index].first,
"xyz.openbmc_project.User.PrivilegeMapperEntry",
- "Privilege", *localRole,
- [asyncResp, roleMapObjData, serverType, index,
- localRole](const boost::system::error_code& ec,
- const sdbusplus::message_t& msg) {
- if (ec)
- {
- const sd_bus_error* dbusError = msg.get_error();
- if ((dbusError != nullptr) &&
- (dbusError->name ==
- std::string_view(
- "xyz.openbmc_project.Common.Error.InvalidArgument")))
- {
- BMCWEB_LOG_WARNING("DBUS response error: {}",
- ec);
- messages::propertyValueIncorrect(
- asyncResp->res, "LocalRole", *localRole);
- return;
- }
- messages::internalError(asyncResp->res);
- return;
- }
- asyncResp->res
- .jsonValue[serverType]["RemoteRoleMapping"][index]
- ["LocalRole"] = *localRole;
- });
+ "Privilege",
+ std::format("RemoteRoleMapping/{}/LocalRole", index),
+ *localRole);
}
}
// Create a new RoleMapping Object.
@@ -805,40 +748,10 @@ inline void handleServiceAddressPatch(
const std::string& ldapServerElementName,
const std::string& ldapConfigObject)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, ldapDbusService, ldapConfigObject,
- ldapConfigInterface, "LDAPServerURI", serviceAddressList.front(),
- [asyncResp, ldapServerElementName, serviceAddressList](
- const boost::system::error_code& ec, sdbusplus::message_t& msg) {
- if (ec)
- {
- const sd_bus_error* dbusError = msg.get_error();
- if ((dbusError != nullptr) &&
- (dbusError->name ==
- std::string_view(
- "xyz.openbmc_project.Common.Error.InvalidArgument")))
- {
- BMCWEB_LOG_WARNING(
- "Error Occurred in updating the service address");
- messages::propertyValueIncorrect(asyncResp->res,
- "ServiceAddresses",
- serviceAddressList.front());
- return;
- }
- messages::internalError(asyncResp->res);
- return;
- }
- std::vector<std::string> modifiedserviceAddressList = {
- serviceAddressList.front()};
- asyncResp->res.jsonValue[ldapServerElementName]["ServiceAddresses"] =
- modifiedserviceAddressList;
- if ((serviceAddressList).size() > 1)
- {
- messages::propertyValueModified(asyncResp->res, "ServiceAddresses",
- serviceAddressList.front());
- }
- BMCWEB_LOG_DEBUG("Updated the service address");
- });
+ setDbusProperty(asyncResp, ldapDbusService, ldapConfigObject,
+ ldapConfigInterface, "LDAPServerURI",
+ ldapServerElementName + "/ServiceAddress",
+ serviceAddressList.front());
}
/**
* @brief updates the LDAP Bind DN and updates the
@@ -855,21 +768,10 @@ inline void
const std::string& ldapServerElementName,
const std::string& ldapConfigObject)
{
- sdbusplus::asio::setProperty(*crow::connections::systemBus, ldapDbusService,
- ldapConfigObject, ldapConfigInterface,
- "LDAPBindDN", username,
- [asyncResp, username, ldapServerElementName](
- const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_DEBUG("Error occurred in updating the username");
- messages::internalError(asyncResp->res);
- return;
- }
- asyncResp->res.jsonValue[ldapServerElementName]["Authentication"]
- ["Username"] = username;
- BMCWEB_LOG_DEBUG("Updated the username");
- });
+ setDbusProperty(asyncResp, ldapDbusService, ldapConfigObject,
+ ldapConfigInterface, "LDAPBindDN",
+ ldapServerElementName + "/Authentication/Username",
+ username);
}
/**
@@ -886,21 +788,10 @@ inline void
const std::string& ldapServerElementName,
const std::string& ldapConfigObject)
{
- sdbusplus::asio::setProperty(*crow::connections::systemBus, ldapDbusService,
- ldapConfigObject, ldapConfigInterface,
- "LDAPBindDNPassword", password,
- [asyncResp, password, ldapServerElementName](
- const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_DEBUG("Error occurred in updating the password");
- messages::internalError(asyncResp->res);
- return;
- }
- asyncResp->res.jsonValue[ldapServerElementName]["Authentication"]
- ["Password"] = "";
- BMCWEB_LOG_DEBUG("Updated the password");
- });
+ setDbusProperty(asyncResp, ldapDbusService, ldapConfigObject,
+ ldapConfigInterface, "LDAPBindDNPassword",
+ ldapServerElementName + "/Authentication/Password",
+ password);
}
/**
@@ -918,41 +809,11 @@ inline void
const std::string& ldapServerElementName,
const std::string& ldapConfigObject)
{
- sdbusplus::asio::setProperty(*crow::connections::systemBus, ldapDbusService,
- ldapConfigObject, ldapConfigInterface,
- "LDAPBaseDN", baseDNList.front(),
- [asyncResp, baseDNList, ldapServerElementName](
- const boost::system::error_code& ec,
- const sdbusplus::message_t& msg) {
- if (ec)
- {
- BMCWEB_LOG_DEBUG("Error Occurred in Updating the base DN");
- const sd_bus_error* dbusError = msg.get_error();
- if ((dbusError != nullptr) &&
- (dbusError->name ==
- std::string_view(
- "xyz.openbmc_project.Common.Error.InvalidArgument")))
- {
- messages::propertyValueIncorrect(asyncResp->res,
- "BaseDistinguishedNames",
- baseDNList.front());
- return;
- }
- messages::internalError(asyncResp->res);
- return;
- }
- auto& serverTypeJson = asyncResp->res.jsonValue[ldapServerElementName];
- auto& searchSettingsJson =
- serverTypeJson["LDAPService"]["SearchSettings"];
- std::vector<std::string> modifiedBaseDNList = {baseDNList.front()};
- searchSettingsJson["BaseDistinguishedNames"] = modifiedBaseDNList;
- if (baseDNList.size() > 1)
- {
- messages::propertyValueModified(
- asyncResp->res, "BaseDistinguishedNames", baseDNList.front());
- }
- BMCWEB_LOG_DEBUG("Updated the base DN");
- });
+ setDbusProperty(asyncResp, ldapDbusService, ldapConfigObject,
+ ldapConfigInterface, "LDAPBaseDN",
+ ldapServerElementName +
+ "/LDAPService/SearchSettings/BaseDistinguishedNames",
+ baseDNList.front());
}
/**
* @brief updates the LDAP user name attribute and updates the
@@ -969,24 +830,11 @@ inline void
const std::string& ldapServerElementName,
const std::string& ldapConfigObject)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, ldapDbusService, ldapConfigObject,
- ldapConfigInterface, "UserNameAttribute", userNameAttribute,
- [asyncResp, userNameAttribute,
- ldapServerElementName](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_DEBUG("Error Occurred in Updating the "
- "username attribute");
- messages::internalError(asyncResp->res);
- return;
- }
- auto& serverTypeJson = asyncResp->res.jsonValue[ldapServerElementName];
- auto& searchSettingsJson =
- serverTypeJson["LDAPService"]["SearchSettings"];
- searchSettingsJson["UsernameAttribute"] = userNameAttribute;
- BMCWEB_LOG_DEBUG("Updated the user name attr.");
- });
+ setDbusProperty(asyncResp, ldapDbusService, ldapConfigObject,
+ ldapConfigInterface, "UserNameAttribute",
+ ldapServerElementName +
+ "LDAPService/SearchSettings/UsernameAttribute",
+ userNameAttribute);
}
/**
* @brief updates the LDAP group attribute and updates the
@@ -1003,24 +851,11 @@ inline void handleGroupNameAttrPatch(
const std::string& ldapServerElementName,
const std::string& ldapConfigObject)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, ldapDbusService, ldapConfigObject,
- ldapConfigInterface, "GroupNameAttribute", groupsAttribute,
- [asyncResp, groupsAttribute,
- ldapServerElementName](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_DEBUG("Error Occurred in Updating the "
- "groupname attribute");
- messages::internalError(asyncResp->res);
- return;
- }
- auto& serverTypeJson = asyncResp->res.jsonValue[ldapServerElementName];
- auto& searchSettingsJson =
- serverTypeJson["LDAPService"]["SearchSettings"];
- searchSettingsJson["GroupsAttribute"] = groupsAttribute;
- BMCWEB_LOG_DEBUG("Updated the groupname attr");
- });
+ setDbusProperty(asyncResp, ldapDbusService, ldapConfigObject,
+ ldapConfigInterface, "GroupNameAttribute",
+ ldapServerElementName +
+ "/LDAPService/SearchSettings/GroupsAttribute",
+ groupsAttribute);
}
/**
* @brief updates the LDAP service enable and updates the
@@ -1036,21 +871,9 @@ inline void handleServiceEnablePatch(
const std::string& ldapServerElementName,
const std::string& ldapConfigObject)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, ldapDbusService, ldapConfigObject,
- ldapEnableInterface, "Enabled", serviceEnabled,
- [asyncResp, serviceEnabled,
- ldapServerElementName](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_DEBUG("Error Occurred in Updating the service enable");
- messages::internalError(asyncResp->res);
- return;
- }
- asyncResp->res.jsonValue[ldapServerElementName]["ServiceEnabled"] =
- serviceEnabled;
- BMCWEB_LOG_DEBUG("Updated Service enable = {}", serviceEnabled);
- });
+ setDbusProperty(asyncResp, ldapDbusService, ldapConfigObject,
+ ldapEnableInterface, "Enabled",
+ ldapServerElementName + "/ServiceEnabled", serviceEnabled);
}
inline void
@@ -1359,19 +1182,10 @@ inline void updateUserProperties(
if (enabled)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus,
- "xyz.openbmc_project.User.Manager", dbusObjectPath,
- "xyz.openbmc_project.User.Attributes", "UserEnabled", *enabled,
- [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_ERROR("D-Bus responses error: {}", ec);
- messages::internalError(asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
+ setDbusProperty(asyncResp, "xyz.openbmc_project.User.Manager",
+ dbusObjectPath,
+ "xyz.openbmc_project.User.Attributes",
+ "UserEnabled", "Enabled", *enabled);
}
if (roleId)
@@ -1383,20 +1197,10 @@ inline void updateUserProperties(
"Locked");
return;
}
-
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus,
- "xyz.openbmc_project.User.Manager", dbusObjectPath,
- "xyz.openbmc_project.User.Attributes", "UserPrivilege", priv,
- [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_ERROR("D-Bus responses error: {}", ec);
- messages::internalError(asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
+ setDbusProperty(asyncResp, "xyz.openbmc_project.User.Manager",
+ dbusObjectPath,
+ "xyz.openbmc_project.User.Attributes",
+ "UserPrivilege", "RoleId", priv);
}
if (locked)
@@ -1410,21 +1214,10 @@ inline void updateUserProperties(
"Locked");
return;
}
-
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus,
- "xyz.openbmc_project.User.Manager", dbusObjectPath,
- "xyz.openbmc_project.User.Attributes",
- "UserLockedForFailedAttempt", *locked,
- [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_ERROR("D-Bus responses error: {}", ec);
- messages::internalError(asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
+ setDbusProperty(asyncResp, "xyz.openbmc_project.User.Manager",
+ dbusObjectPath,
+ "xyz.openbmc_project.User.Attributes",
+ "UserLockedForFailedAttempt", "Locked", *locked);
}
if (accountTypes)
@@ -1594,19 +1387,11 @@ inline void handleAccountServicePatch(
if (minPasswordLength)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.User.Manager",
- "/xyz/openbmc_project/user",
+ setDbusProperty(
+ asyncResp, "xyz.openbmc_project.User.Manager",
+ sdbusplus::message::object_path("/xyz/openbmc_project/user"),
"xyz.openbmc_project.User.AccountPolicy", "MinPasswordLength",
- *minPasswordLength,
- [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- messages::internalError(asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
+ "MinPasswordLength", *minPasswordLength);
}
if (maxPasswordLength)
@@ -1642,34 +1427,20 @@ inline void handleAccountServicePatch(
if (unlockTimeout)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.User.Manager",
- "/xyz/openbmc_project/user",
+ setDbusProperty(
+ asyncResp, "xyz.openbmc_project.User.Manager",
+ sdbusplus::message::object_path("/xyz/openbmc_project/user"),
"xyz.openbmc_project.User.AccountPolicy", "AccountUnlockTimeout",
- *unlockTimeout, [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- messages::internalError(asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
+ "AccountLockoutDuration", *unlockTimeout);
}
if (lockoutThreshold)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.User.Manager",
- "/xyz/openbmc_project/user",
+ setDbusProperty(
+ asyncResp, "xyz.openbmc_project.User.Manager",
+ sdbusplus::message::object_path("/xyz/openbmc_project/user"),
"xyz.openbmc_project.User.AccountPolicy",
- "MaxLoginAttemptBeforeLockout", *lockoutThreshold,
- [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- messages::internalError(asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
+ "MaxLoginAttemptBeforeLockout", "AccountLockoutThreshold",
+ *lockoutThreshold);
}
}
diff --git a/redfish-core/lib/chassis.hpp b/redfish-core/lib/chassis.hpp
index 02bd7a43af..6038384201 100644
--- a/redfish-core/lib/chassis.hpp
+++ b/redfish-core/lib/chassis.hpp
@@ -798,36 +798,6 @@ inline void requestRoutesChassis(App& app)
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 {} sdbusplus:{}", ec,
- errorMessage);
- messages::internalError(res);
-}
-
inline void
doChassisPowerCycle(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
@@ -863,21 +833,8 @@ inline void
objectPath = "/xyz/openbmc_project/state/chassis0";
}
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, processName, objectPath,
- interfaceName, destProperty, propertyValue,
- [asyncResp](const boost::system::error_code& ec2,
- sdbusplus::message_t& sdbusErrMsg) {
- // Use "Set" method to set the property value.
- if (ec2)
- {
- handleChassisPowerCycleError(ec2, sdbusErrMsg, asyncResp->res);
-
- return;
- }
-
- messages::success(asyncResp->res);
- });
+ setDbusProperty(asyncResp, processName, objectPath, interfaceName,
+ destProperty, "ResetType", propertyValue);
});
}
diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index d1d47f5d42..b73da3e737 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -729,18 +729,12 @@ inline void updateIPv4DefaultGateway(
const std::string& ifaceId, const std::string& gateway,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.Network",
- "/xyz/openbmc_project/network/" + ifaceId,
+ setDbusProperty(
+ asyncResp, "xyz.openbmc_project.Network",
+ sdbusplus::message::object_path("/xyz/openbmc_project/network") /
+ ifaceId,
"xyz.openbmc_project.Network.EthernetInterface", "DefaultGateway",
- gateway, [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- messages::internalError(asyncResp->res);
- return;
- }
- asyncResp->res.result(boost::beast::http::status::no_content);
- });
+ "Gateway", gateway);
}
/**
* @brief Creates a static IPv4 entry
@@ -1224,33 +1218,22 @@ inline void
"HostName");
return;
}
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.Network",
- "/xyz/openbmc_project/network/config",
- "xyz.openbmc_project.Network.SystemConfiguration", "HostName", hostname,
- [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- messages::internalError(asyncResp->res);
- }
- });
+ setDbusProperty(
+ asyncResp, "xyz.openbmc_project.Network",
+ sdbusplus::message::object_path("/xyz/openbmc_project/network/config"),
+ "xyz.openbmc_project.Network.SystemConfiguration", "HostName",
+ "HostName", hostname);
}
inline void
handleMTUSizePatch(const std::string& ifaceId, const size_t mtuSize,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
- sdbusplus::message::object_path objPath = "/xyz/openbmc_project/network/" +
- ifaceId;
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.Network", objPath,
- "xyz.openbmc_project.Network.EthernetInterface", "MTU", mtuSize,
- [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- messages::internalError(asyncResp->res);
- }
- });
+ sdbusplus::message::object_path objPath("/xyz/openbmc_project/network");
+ objPath /= ifaceId;
+ setDbusProperty(asyncResp, "xyz.openbmc_project.Network", objPath,
+ "xyz.openbmc_project.Network.EthernetInterface", "MTU",
+ "MTUSize", mtuSize);
}
inline void
@@ -1259,16 +1242,12 @@ inline void
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
std::vector<std::string> vectorDomainname = {domainname};
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.Network",
- "/xyz/openbmc_project/network/" + ifaceId,
- "xyz.openbmc_project.Network.EthernetInterface", "DomainName",
- vectorDomainname, [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- messages::internalError(asyncResp->res);
- }
- });
+ setDbusProperty(
+ asyncResp, "xyz.openbmc_project.Network",
+ sdbusplus::message::object_path("/xyz/openbmc_project/network") /
+ ifaceId,
+ "xyz.openbmc_project.Network.EthernetInterface", "DomainName", "FQDN",
+ vectorDomainname);
}
inline bool isHostnameValid(const std::string& hostname)
@@ -1335,32 +1314,12 @@ inline void
const std::string& macAddress,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
- static constexpr std::string_view dbusNotAllowedError =
- "xyz.openbmc_project.Common.Error.NotAllowed";
-
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.Network",
- "/xyz/openbmc_project/network/" + ifaceId,
- "xyz.openbmc_project.Network.MACAddress", "MACAddress", macAddress,
- [asyncResp](const boost::system::error_code& ec,
- const sdbusplus::message_t& msg) {
- if (ec)
- {
- const sd_bus_error* err = msg.get_error();
- if (err == nullptr)
- {
- messages::internalError(asyncResp->res);
- return;
- }
- if (err->name == dbusNotAllowedError)
- {
- messages::propertyNotWritable(asyncResp->res, "MACAddress");
- return;
- }
- messages::internalError(asyncResp->res);
- return;
- }
- });
+ setDbusProperty(
+ asyncResp, "xyz.openbmc_project.Network",
+ sdbusplus::message::object_path("/xyz/openbmc_project/network") /
+ ifaceId,
+ "xyz.openbmc_project.Network.MACAddress", "MACAddress", "MACAddress",
+ macAddress);
}
inline void setDHCPEnabled(const std::string& ifaceId,
@@ -1369,37 +1328,12 @@ inline void setDHCPEnabled(const std::string& ifaceId,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
const std::string dhcp = getDhcpEnabledEnumeration(v4Value, v6Value);
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.Network",
- "/xyz/openbmc_project/network/" + ifaceId,
- "xyz.openbmc_project.Network.EthernetInterface", propertyName, dhcp,
- [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_ERROR("D-Bus responses error: {}", ec);
- messages::internalError(asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
-}
-
-inline void setEthernetInterfaceBoolProperty(
- const std::string& ifaceId, const std::string& propertyName,
- const bool& value, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
-{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.Network",
- "/xyz/openbmc_project/network/" + ifaceId,
- "xyz.openbmc_project.Network.EthernetInterface", propertyName, value,
- [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_ERROR("D-Bus responses error: {}", ec);
- messages::internalError(asyncResp->res);
- return;
- }
- });
+ setDbusProperty(
+ asyncResp, "xyz.openbmc_project.Network",
+ sdbusplus::message::object_path("/xyz/openbmc_project/network") /
+ ifaceId,
+ "xyz.openbmc_project.Network.EthernetInterface", propertyName, "DHCPv4",
+ dhcp);
}
enum class NetworkType
@@ -2434,8 +2368,13 @@ inline void requestEthernetInterfacesRoutes(App& app)
if (interfaceEnabled)
{
- setEthernetInterfaceBoolProperty(ifaceId, "NICEnabled",
- *interfaceEnabled, asyncResp);
+ setDbusProperty(asyncResp, "xyz.openbmc_project.Network",
+ sdbusplus::message::object_path(
+ "/xyz/openbmc_project/network") /
+ ifaceId,
+ "xyz.openbmc_project.Network.EthernetInterface",
+ "NICEnabled", "InterfaceEnabled",
+ *interfaceEnabled);
}
if (mtuSize)
diff --git a/redfish-core/lib/sensors.hpp b/redfish-core/lib/sensors.hpp
index e6de44d793..002ee3fbdb 100644
--- a/redfish-core/lib/sensors.hpp
+++ b/redfish-core/lib/sensors.hpp
@@ -2637,7 +2637,7 @@ inline void setSensorsOverride(
BMCWEB_LOG_INFO("setSensorsOverride for subNode{}",
sensorAsyncResp->chassisSubNode);
- const char* propertyValueName = nullptr;
+ std::string_view propertyValueName;
std::unordered_map<std::string, std::pair<double, std::string>> overrideMap;
std::string memberId;
double value = 0.0;
@@ -2669,7 +2669,8 @@ inline void setSensorsOverride(
}
auto getChassisSensorListCb =
- [sensorAsyncResp, overrideMap](
+ [sensorAsyncResp, overrideMap,
+ propertyValueNameStr = std::string(propertyValueName)](
const std::shared_ptr<std::set<std::string>>& sensorsList) {
// Match sensor names in the PATCH request to those managed by the
// chassis node
@@ -2691,10 +2692,10 @@ inline void setSensorsOverride(
}
// Get the connection to which the memberId belongs
auto getObjectsWithConnectionCb =
- [sensorAsyncResp,
- overrideMap](const std::set<std::string>& /*connections*/,
- const std::set<std::pair<std::string, std::string>>&
- objectsWithConnection) {
+ [sensorAsyncResp, overrideMap, propertyValueNameStr](
+ const std::set<std::string>& /*connections*/,
+ const std::set<std::pair<std::string, std::string>>&
+ objectsWithConnection) {
if (objectsWithConnection.size() != overrideMap.size())
{
BMCWEB_LOG_INFO(
@@ -2731,30 +2732,10 @@ inline void setSensorsOverride(
messages::internalError(sensorAsyncResp->asyncResp->res);
return;
}
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, item.second, item.first,
- "xyz.openbmc_project.Sensor.Value", "Value",
- iterator->second.first,
- [sensorAsyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- if (ec.value() ==
- boost::system::errc::permission_denied)
- {
- BMCWEB_LOG_WARNING(
- "Manufacturing mode is not Enabled...can't "
- "Override the sensor value. ");
-
- messages::insufficientPrivilege(
- sensorAsyncResp->asyncResp->res);
- return;
- }
- BMCWEB_LOG_DEBUG(
- "setOverrideValueStatus DBUS error: {}", ec);
- messages::internalError(
- sensorAsyncResp->asyncResp->res);
- }
- });
+ setDbusProperty(sensorAsyncResp->asyncResp, item.second,
+ item.first, "xyz.openbmc_project.Sensor.Value",
+ "Value", propertyValueNameStr,
+ iterator->second.first);
}
};
// Get object with connection for the given sensor name
diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp
index a8751b00a6..37ec29c63a 100644
--- a/redfish-core/lib/systems.hpp
+++ b/redfish-core/lib/systems.hpp
@@ -3071,46 +3071,6 @@ inline void doNMI(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
serviceName, objectPath, interfaceName, method);
}
-/**
- * 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 {} sdbusplus:{}", ec,
- errorMessage);
- messages::internalError(res);
-}
-
inline void handleComputerSystemResetActionPost(
crow::App& app, const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
@@ -3183,43 +3143,20 @@ inline void handleComputerSystemResetActionPost(
messages::actionParameterUnknown(asyncResp->res, "Reset", resetType);
return;
}
+ sdbusplus::message::object_path statePath("/xyz/openbmc_project/state");
if (hostCommand)
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.State.Host",
- "/xyz/openbmc_project/state/host0",
- "xyz.openbmc_project.State.Host", "RequestedHostTransition",
- command,
- [asyncResp, resetType](const boost::system::error_code& ec,
- sdbusplus::message_t& sdbusErrMsg) {
- if (ec)
- {
- handleSystemActionResetError(ec, sdbusErrMsg, resetType,
- asyncResp->res);
-
- return;
- }
- messages::success(asyncResp->res);
- });
+ setDbusProperty(asyncResp, "xyz.openbmc_project.State.Host",
+ statePath / "host0", "xyz.openbmc_project.State.Host",
+ "RequestedHostTransition", "Reset", command);
}
else
{
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.State.Chassis",
- "/xyz/openbmc_project/state/chassis0",
- "xyz.openbmc_project.State.Chassis", "RequestedPowerTransition",
- command,
- [asyncResp, resetType](const boost::system::error_code& ec,
- sdbusplus::message_t& sdbusErrMsg) {
- if (ec)
- {
- handleSystemActionResetError(ec, sdbusErrMsg, resetType,
- asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
+ setDbusProperty(asyncResp, "xyz.openbmc_project.State.Chassis",
+ statePath / "chassis0",
+ "xyz.openbmc_project.State.Chassis",
+ "RequestedPowerTransition", "Reset", command);
}
}
diff --git a/redfish-core/lib/update_service.hpp b/redfish-core/lib/update_service.hpp
index 62591149a3..fa2dc9e101 100644
--- a/redfish-core/lib/update_service.hpp
+++ b/redfish-core/lib/update_service.hpp
@@ -620,20 +620,11 @@ inline void setApplyTime(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
return;
}
- // Set the requested image apply time value
- sdbusplus::asio::setProperty(
- *crow::connections::systemBus, "xyz.openbmc_project.Settings",
- "/xyz/openbmc_project/software/apply_time",
- "xyz.openbmc_project.Software.ApplyTime", "RequestedApplyTime",
- applyTimeNewVal, [asyncResp](const boost::system::error_code& ec) {
- if (ec)
- {
- BMCWEB_LOG_ERROR("D-Bus responses error: {}", ec);
- messages::internalError(asyncResp->res);
- return;
- }
- messages::success(asyncResp->res);
- });
+ setDbusProperty(asyncResp, "xyz.openbmc_project.Settings",
+ sdbusplus::message::object_path(
+ "/xyz/openbmc_project/software/apply_time"),
+ "xyz.openbmc_project.Software.ApplyTime",
+ "RequestedApplyTime", "ApplyTime", applyTimeNewVal);
}
inline void
diff --git a/redfish-core/src/utils/dbus_utils.cpp b/redfish-core/src/utils/dbus_utils.cpp
new file mode 100644
index 0000000000..59ddb9d6ba
--- /dev/null
+++ b/redfish-core/src/utils/dbus_utils.cpp
@@ -0,0 +1,76 @@
+#include "utils/dbus_utils.hpp"
+
+#include "async_resp.hpp"
+
+#include <boost/system/error_code.hpp>
+#include <nlohmann/json.hpp>
+#include <sdbusplus/message.hpp>
+
+#include <memory>
+#include <string>
+#include <string_view>
+
+namespace redfish
+{
+namespace details
+{
+
+void afterSetProperty(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& redfishPropertyName,
+ const nlohmann::json& propertyValue,
+ const boost::system::error_code& ec,
+ const sdbusplus::message_t& msg)
+{
+ if (ec)
+ {
+ if (ec.value() == boost::system::errc::permission_denied)
+ {
+ messages::insufficientPrivilege(asyncResp->res);
+ }
+ const sd_bus_error* dbusError = msg.get_error();
+ if (dbusError != nullptr)
+ {
+ std::string_view errorName(dbusError->name);
+
+ if (errorName == "xyz.openbmc_project.Common.Error.InvalidArgument")
+ {
+ BMCWEB_LOG_WARNING("DBUS response error: {}", ec);
+ messages::propertyValueIncorrect(
+ asyncResp->res, redfishPropertyName, propertyValue);
+ return;
+ }
+ if (errorName ==
+ "xyz.openbmc_project.State.Chassis.Error.BMCNotReady")
+ {
+ BMCWEB_LOG_WARNING(
+ "BMC not ready, operation not allowed right now");
+ messages::serviceTemporarilyUnavailable(asyncResp->res, "10");
+ return;
+ }
+ if (errorName == "xyz.openbmc_project.State.Host.Error.BMCNotReady")
+ {
+ BMCWEB_LOG_WARNING(
+ "BMC not ready, operation not allowed right now");
+ messages::serviceTemporarilyUnavailable(asyncResp->res, "10");
+ return;
+ }
+ if (errorName == "xyz.openbmc_project.Common.Error.NotAllowed")
+ {
+ messages::propertyNotWritable(asyncResp->res,
+ redfishPropertyName);
+ return;
+ }
+ }
+ BMCWEB_LOG_ERROR("D-Bus error setting Redfish Property {} ec={}",
+ redfishPropertyName, ec);
+ messages::internalError(asyncResp->res);
+ return;
+ }
+ // Only set 204 if another erro hasn't already happened.
+ if (asyncResp->res.result() == boost::beast::http::status::ok)
+ {
+ asyncResp->res.result(boost::beast::http::status::no_content);
+ }
+};
+} // namespace details
+} // namespace redfish
diff --git a/test/redfish-core/include/utils/dbus_utils.cpp b/test/redfish-core/include/utils/dbus_utils.cpp
new file mode 100644
index 0000000000..00c53dec7f
--- /dev/null
+++ b/test/redfish-core/include/utils/dbus_utils.cpp
@@ -0,0 +1,74 @@
+
+#include "utils/dbus_utils.hpp"
+
+#include "http_request.hpp"
+#include "http_response.hpp"
+
+#include <boost/beast/http/status.hpp>
+#include <nlohmann/json.hpp>
+
+#include <cstdint>
+#include <optional>
+#include <string>
+#include <system_error>
+#include <vector>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace redfish::details
+{
+namespace
+{
+
+TEST(DbusUtils, AfterPropertySetSuccess)
+{
+ std::shared_ptr<bmcweb::AsyncResp> asyncResp =
+ std::make_shared<bmcweb::AsyncResp>();
+
+ boost::system::error_code ec;
+ sdbusplus::message_t msg;
+ afterSetProperty(asyncResp, "MyRedfishProperty",
+ nlohmann::json("MyRedfishValue"), ec, msg);
+
+ EXPECT_EQ(asyncResp->res.result(), boost::beast::http::status::no_content);
+ EXPECT_TRUE(asyncResp->res.jsonValue.empty());
+}
+
+TEST(DbusUtils, AfterPropertySetInternalError)
+{
+ std::shared_ptr<bmcweb::AsyncResp> asyncResp =
+ std::make_shared<bmcweb::AsyncResp>();
+
+ boost::system::error_code ec =
+ boost::system::errc::make_error_code(boost::system::errc::timed_out);
+ sdbusplus::message_t msg;
+ afterSetProperty(asyncResp, "MyRedfishProperty",
+ nlohmann::json("MyRedfishValue"), ec, msg);
+
+ EXPECT_EQ(asyncResp->res.result(),
+ boost::beast::http::status::internal_server_error);
+ EXPECT_EQ(asyncResp->res.jsonValue.size(), 1);
+ using nlohmann::literals::operator""_json;
+
+ EXPECT_EQ(asyncResp->res.jsonValue,
+ R"({
+ "error": {
+ "@Message.ExtendedInfo": [
+ {
+ "@odata.type": "#Message.v1_1_1.Message",
+ "Message": "The request failed due to an internal service error. The service is still operational.",
+ "MessageArgs": [],
+ "MessageId": "Base.1.16.0.InternalError",
+ "MessageSeverity": "Critical",
+ "Resolution": "Resubmit the request. If the problem persists, consider resetting the service."
+ }
+ ],
+ "code": "Base.1.16.0.InternalError",
+ "message": "The request failed due to an internal service error. The service is still operational."
+ }
+ })"_json);
+}
+
+} // namespace
+} // namespace redfish::details