diff options
| -rw-r--r-- | meson.build | 2 | ||||
| -rw-r--r-- | redfish-core/include/utils/dbus_utils.hpp | 42 | ||||
| -rw-r--r-- | redfish-core/lib/account_service.hpp | 365 | ||||
| -rw-r--r-- | redfish-core/lib/chassis.hpp | 47 | ||||
| -rw-r--r-- | redfish-core/lib/ethernet.hpp | 141 | ||||
| -rw-r--r-- | redfish-core/lib/sensors.hpp | 41 | ||||
| -rw-r--r-- | redfish-core/lib/systems.hpp | 79 | ||||
| -rw-r--r-- | redfish-core/lib/update_service.hpp | 19 | ||||
| -rw-r--r-- | redfish-core/src/utils/dbus_utils.cpp | 76 | ||||
| -rw-r--r-- | test/redfish-core/include/utils/dbus_utils.cpp | 74 |
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 |