From bc1d29de81216e99d0a73c5fd3b6bb7fd2194ba8 Mon Sep 17 00:00:00 2001 From: Krzysztof Grobelny Date: Tue, 9 Aug 2022 14:17:34 +0200 Subject: used sdbusplus::unpackPropertiesNoThrow part 3 used sdbusplus::unpackPropertiesNoThrow in systems.hpp, also replaced all usages of "GetAll" with sdbusplus::asio::getAllProperties bmcweb size: 2672984 -> 2668888 (-4096) compressed size: 1128611 -> 1127280 (-1331) Tested: Performed get on: - /redfish/v1/Systems/system - /redfish/v1/Systems/system/Memory/dimm0 Get result before and after the change was the same Change-Id: I11ca70bae07bdb17edd1935c539c68814d6ec172 Signed-off-by: Krzysztof Grobelny --- redfish-core/lib/systems.hpp | 487 ++++++++++++++++++++++--------------------- 1 file changed, 244 insertions(+), 243 deletions(-) diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp index 0c2c53c192..60bdbb96fa 100644 --- a/redfish-core/lib/systems.hpp +++ b/redfish-core/lib/systems.hpp @@ -28,6 +28,8 @@ #include #include #include +#include +#include #include #include @@ -164,37 +166,32 @@ inline void "xyz.openbmc_project.State.Decorator.OperationalStatus", "Functional", std::move(getCpuFunctionalState)); - for (const auto& property : properties) - { - - // TODO: Get Model + // TODO: Get Model - // Get CoreCount - if (property.first == "CoreCount") - { + const uint16_t* coreCount = nullptr; - // Get CPU CoreCount and add it to the total - const uint16_t* coreCountVal = - std::get_if(&property.second); + const bool success = sdbusplus::unpackPropertiesNoThrow( + dbus_utils::UnpackErrorPrinter(), properties, "CoreCount", coreCount); - if (coreCountVal == nullptr) - { - messages::internalError(aResp->res); - return; - } + if (!success) + { + messages::internalError(aResp->res); + return; + } - nlohmann::json& coreCount = - aResp->res.jsonValue["ProcessorSummary"]["CoreCount"]; - uint64_t* coreCountPtr = coreCount.get_ptr(); + if (coreCount != nullptr) + { + nlohmann::json& coreCountJson = + aResp->res.jsonValue["ProcessorSummary"]["CoreCount"]; + uint64_t* coreCountJsonPtr = coreCountJson.get_ptr(); - if (coreCountPtr == nullptr) - { - coreCount = *coreCountVal; - } - else - { - *coreCountPtr += *coreCountVal; - } + if (coreCountJsonPtr == nullptr) + { + coreCountJson = *coreCount; + } + else + { + *coreCountJsonPtr += *coreCount; } } } @@ -213,7 +210,9 @@ inline void getProcessorSummary(const std::shared_ptr& aResp, const std::string& path) { - crow::connections::systemBus->async_method_call( + sdbusplus::asio::getAllProperties( + *crow::connections::systemBus, service, path, + "xyz.openbmc_project.Inventory.Item.Cpu", [aResp, service, path](const boost::system::error_code ec2, const dbus::utility::DBusPropertiesMap& properties) { @@ -224,9 +223,7 @@ inline void getProcessorSummary(const std::shared_ptr& aResp, return; } getProcessorProperties(aResp, service, path, properties); - }, - service, path, "org.freedesktop.DBus.Properties", "GetAll", - "xyz.openbmc_project.Inventory.Item.Cpu"); + }); } /* @@ -289,7 +286,9 @@ inline void BMCWEB_LOG_DEBUG << "Found Dimm, now get its properties."; - crow::connections::systemBus->async_method_call( + sdbusplus::asio::getAllProperties( + *crow::connections::systemBus, connection.first, + path, "xyz.openbmc_project.Inventory.Item.Dimm", [aResp, service{connection.first}, path](const boost::system::error_code ec2, const dbus::utility::DBusPropertiesMap& @@ -304,45 +303,7 @@ inline void BMCWEB_LOG_DEBUG << "Got " << properties.size() << " Dimm properties."; - if (!properties.empty()) - { - for (const std::pair< - std::string, - dbus::utility::DbusVariantType>& - property : properties) - { - if (property.first != "MemorySizeInKB") - { - continue; - } - const uint32_t* value = - std::get_if(&property.second); - if (value == nullptr) - { - BMCWEB_LOG_DEBUG - << "Find incorrect type of MemorySize"; - continue; - } - nlohmann::json& totalMemory = - aResp->res - .jsonValue["MemorySummary"] - ["TotalSystemMemoryGiB"]; - const uint64_t* preValue = - totalMemory.get_ptr(); - if (preValue == nullptr) - { - continue; - } - aResp->res - .jsonValue["MemorySummary"] - ["TotalSystemMemoryGiB"] = - *value / (1024 * 1024) + *preValue; - aResp->res.jsonValue["MemorySummary"] - ["Status"]["State"] = - "Enabled"; - } - } - else + if (properties.empty()) { sdbusplus::asio::getProperty( *crow::connections::systemBus, service, @@ -360,11 +321,50 @@ inline void } updateDimmProperties(aResp, dimmState); }); + return; + } + + const uint32_t* memorySizeInKB = nullptr; + + const bool success = + sdbusplus::unpackPropertiesNoThrow( + dbus_utils::UnpackErrorPrinter(), + properties, "MemorySizeInKB", + memorySizeInKB); + + if (!success) + { + messages::internalError(aResp->res); + return; } - }, - connection.first, path, - "org.freedesktop.DBus.Properties", "GetAll", - "xyz.openbmc_project.Inventory.Item.Dimm"); + + if (memorySizeInKB != nullptr) + { + nlohmann::json& totalMemory = + aResp->res + .jsonValue["MemorySummary"] + ["TotalSystemMemoryGiB"]; + const uint64_t* preValue = + totalMemory.get_ptr(); + if (preValue == nullptr) + { + aResp->res + .jsonValue["MemorySummary"] + ["TotalSystemMemoryGiB"] = + *memorySizeInKB / (1024 * 1024); + } + else + { + aResp->res + .jsonValue["MemorySummary"] + ["TotalSystemMemoryGiB"] = + *memorySizeInKB / (1024 * 1024) + + *preValue; + } + aResp->res.jsonValue["MemorySummary"]["Status"] + ["State"] = "Enabled"; + } + }); memoryHealth->inventory.emplace_back(path); } @@ -382,7 +382,10 @@ inline void { BMCWEB_LOG_DEBUG << "Found UUID, now get its properties."; - crow::connections::systemBus->async_method_call( + + sdbusplus::asio::getAllProperties( + *crow::connections::systemBus, connection.first, + path, "xyz.openbmc_project.Common.UUID", [aResp](const boost::system::error_code ec3, const dbus::utility::DBusPropertiesMap& properties) { @@ -395,42 +398,42 @@ inline void } BMCWEB_LOG_DEBUG << "Got " << properties.size() << " UUID properties."; - for (const std::pair< - std::string, - dbus::utility::DbusVariantType>& property : - properties) + + const std::string* uUID = nullptr; + + const bool success = + sdbusplus::unpackPropertiesNoThrow( + dbus_utils::UnpackErrorPrinter(), + properties, "UUID", uUID); + + if (!success) { - if (property.first == "UUID") - { - const std::string* value = - std::get_if( - &property.second); + messages::internalError(aResp->res); + return; + } - if (value != nullptr) - { - std::string valueStr = *value; - if (valueStr.size() == 32) - { - valueStr.insert(8, 1, '-'); - valueStr.insert(13, 1, '-'); - valueStr.insert(18, 1, '-'); - valueStr.insert(23, 1, '-'); - } - BMCWEB_LOG_DEBUG << "UUID = " - << valueStr; - aResp->res.jsonValue["UUID"] = valueStr; - } + if (uUID != nullptr) + { + std::string valueStr = *uUID; + if (valueStr.size() == 32) + { + valueStr.insert(8, 1, '-'); + valueStr.insert(13, 1, '-'); + valueStr.insert(18, 1, '-'); + valueStr.insert(23, 1, '-'); } + BMCWEB_LOG_DEBUG << "UUID = " << valueStr; + aResp->res.jsonValue["UUID"] = valueStr; } - }, - connection.first, path, - "org.freedesktop.DBus.Properties", "GetAll", - "xyz.openbmc_project.Common.UUID"); + }); } else if (interfaceName == "xyz.openbmc_project.Inventory.Item.System") { - crow::connections::systemBus->async_method_call( + sdbusplus::asio::getAllProperties( + *crow::connections::systemBus, connection.first, + path, + "xyz.openbmc_project.Inventory.Decorator.Asset", [aResp](const boost::system::error_code ec2, const dbus::utility::DBusPropertiesMap& propertiesList) { @@ -442,38 +445,60 @@ inline void } BMCWEB_LOG_DEBUG << "Got " << propertiesList.size() << " properties for system"; - for (const std::pair< - std::string, - dbus::utility::DbusVariantType>& property : - propertiesList) + + const std::string* partNumber = nullptr; + const std::string* serialNumber = nullptr; + const std::string* manufacturer = nullptr; + const std::string* model = nullptr; + const std::string* subModel = nullptr; + + const bool success = + sdbusplus::unpackPropertiesNoThrow( + dbus_utils::UnpackErrorPrinter(), + propertiesList, "PartNumber", partNumber, + "SerialNumber", serialNumber, + "Manufacturer", manufacturer, "Model", + model, "SubModel", subModel); + + if (!success) { - const std::string& propertyName = - property.first; - if ((propertyName == "PartNumber") || - (propertyName == "SerialNumber") || - (propertyName == "Manufacturer") || - (propertyName == "Model") || - (propertyName == "SubModel")) - { - const std::string* value = - std::get_if( - &property.second); - if (value != nullptr) - { - aResp->res.jsonValue[propertyName] = - *value; - } - } + messages::internalError(aResp->res); + return; + } + + if (partNumber != nullptr) + { + aResp->res.jsonValue["PartNumber"] = + *partNumber; + } + + if (serialNumber != nullptr) + { + aResp->res.jsonValue["SerialNumber"] = + *serialNumber; + } + + if (manufacturer != nullptr) + { + aResp->res.jsonValue["Manufacturer"] = + *manufacturer; + } + + if (model != nullptr) + { + aResp->res.jsonValue["Model"] = *model; + } + + if (subModel != nullptr) + { + aResp->res.jsonValue["SubModel"] = *subModel; } // Grab the bios version sw_util::populateSoftwareInformation( aResp, sw_util::biosPurpose, "BiosVersion", false); - }, - connection.first, path, - "org.freedesktop.DBus.Properties", "GetAll", - "xyz.openbmc_project.Inventory.Decorator.Asset"); + }); sdbusplus::asio::getProperty( *crow::connections::systemBus, connection.first, @@ -494,6 +519,7 @@ inline void }); } } + break; } } }, @@ -1794,7 +1820,9 @@ inline void inline void getProvisioningStatus(std::shared_ptr aResp) { BMCWEB_LOG_DEBUG << "Get OEM information."; - crow::connections::systemBus->async_method_call( + sdbusplus::asio::getAllProperties( + *crow::connections::systemBus, "xyz.openbmc_project.PFR.Manager", + "/xyz/openbmc_project/pfr", "xyz.openbmc_project.PFR.Attributes", [aResp](const boost::system::error_code ec, const dbus::utility::DBusPropertiesMap& propertiesList) { nlohmann::json& oemPFR = @@ -1813,17 +1841,15 @@ inline void getProvisioningStatus(std::shared_ptr aResp) const bool* provState = nullptr; const bool* lockState = nullptr; - for (const std::pair& - property : propertiesList) + + const bool success = sdbusplus::unpackPropertiesNoThrow( + dbus_utils::UnpackErrorPrinter(), "UfmProvisioned", provState, + "UfmLocked", lockState); + + if (!success) { - if (property.first == "UfmProvisioned") - { - provState = std::get_if(&property.second); - } - else if (property.first == "UfmLocked") - { - lockState = std::get_if(&property.second); - } + messages::internalError(aResp->res); + return; } if ((provState == nullptr) || (lockState == nullptr)) @@ -1848,10 +1874,7 @@ inline void getProvisioningStatus(std::shared_ptr aResp) { oemPFR["ProvisioningStatus"] = "NotProvisioned"; } - }, - "xyz.openbmc_project.PFR.Manager", "/xyz/openbmc_project/pfr", - "org.freedesktop.DBus.Properties", "GetAll", - "xyz.openbmc_project.PFR.Attributes"); + }); } #endif @@ -2167,7 +2190,10 @@ inline void getHostWatchdogTimer(const std::shared_ptr& aResp) { BMCWEB_LOG_DEBUG << "Get host watchodg"; - crow::connections::systemBus->async_method_call( + sdbusplus::asio::getAllProperties( + *crow::connections::systemBus, "xyz.openbmc_project.Watchdog", + "/xyz/openbmc_project/watchdog/host0", + "xyz.openbmc_project.State.Watchdog", [aResp](const boost::system::error_code ec, const dbus::utility::DBusPropertiesMap& properties) { if (ec) @@ -2185,44 +2211,35 @@ inline void // watchdog service is running/enabled hostWatchdogTimer["Status"]["State"] = "Enabled"; - for (const auto& property : properties) + const bool* enabled = nullptr; + const std::string* expireAction = nullptr; + + const bool success = sdbusplus::unpackPropertiesNoThrow( + dbus_utils::UnpackErrorPrinter(), properties, "Enabled", enabled, + "ExpireAction", expireAction); + + if (!success) { - BMCWEB_LOG_DEBUG << "prop=" << property.first; - if (property.first == "Enabled") - { - const bool* state = std::get_if(&property.second); + messages::internalError(aResp->res); + return; + } - if (state == nullptr) - { - messages::internalError(aResp->res); - return; - } + if (enabled != nullptr) + { + hostWatchdogTimer["FunctionEnabled"] = *enabled; + } - hostWatchdogTimer["FunctionEnabled"] = *state; - } - else if (property.first == "ExpireAction") + if (expireAction != nullptr) + { + std::string action = dbusToRfWatchdogAction(*expireAction); + if (action.empty()) { - const std::string* s = - std::get_if(&property.second); - if (s == nullptr) - { - messages::internalError(aResp->res); - return; - } - - std::string action = dbusToRfWatchdogAction(*s); - if (action.empty()) - { - messages::internalError(aResp->res); - return; - } - hostWatchdogTimer["TimeoutAction"] = action; + messages::internalError(aResp->res); + return; } + hostWatchdogTimer["TimeoutAction"] = action; } - }, - "xyz.openbmc_project.Watchdog", "/xyz/openbmc_project/watchdog/host0", - "org.freedesktop.DBus.Properties", "GetAll", - "xyz.openbmc_project.State.Watchdog"); + }); } /** @@ -2301,70 +2318,54 @@ inline bool parseIpsProperties(const std::shared_ptr& aResp, const dbus::utility::DBusPropertiesMap& properties) { - for (const auto& property : properties) + const bool* enabled = nullptr; + const uint8_t* enterUtilizationPercent = nullptr; + const uint64_t* enterDwellTime = nullptr; + const uint8_t* exitUtilizationPercent = nullptr; + const uint64_t* exitDwellTime = nullptr; + + const bool success = sdbusplus::unpackPropertiesNoThrow( + dbus_utils::UnpackErrorPrinter(), properties, "Enabled", enabled, + "EnterUtilizationPercent", enterUtilizationPercent, + "ExitUtilizationPercent", exitUtilizationPercent, "ExitDwellTime", + exitDwellTime); + + if (!success) { - if (property.first == "Enabled") - { - const bool* state = std::get_if(&property.second); - if (state == nullptr) - { - return false; - } - aResp->res.jsonValue["IdlePowerSaver"][property.first] = *state; - } - else if (property.first == "EnterUtilizationPercent") - { - const uint8_t* util = std::get_if(&property.second); - if (util == nullptr) - { - return false; - } - aResp->res.jsonValue["IdlePowerSaver"][property.first] = *util; - } - else if (property.first == "EnterDwellTime") - { - // Convert Dbus time from milliseconds to seconds - const uint64_t* timeMilliseconds = - std::get_if(&property.second); - if (timeMilliseconds == nullptr) - { - return false; - } - const std::chrono::duration ms( - *timeMilliseconds); - aResp->res.jsonValue["IdlePowerSaver"]["EnterDwellTimeSeconds"] = - std::chrono::duration_cast>(ms) - .count(); - } - else if (property.first == "ExitUtilizationPercent") - { - const uint8_t* util = std::get_if(&property.second); - if (util == nullptr) - { - return false; - } - aResp->res.jsonValue["IdlePowerSaver"][property.first] = *util; - } - else if (property.first == "ExitDwellTime") - { - // Convert Dbus time from milliseconds to seconds - const uint64_t* timeMilliseconds = - std::get_if(&property.second); - if (timeMilliseconds == nullptr) - { - return false; - } - const std::chrono::duration ms( - *timeMilliseconds); - aResp->res.jsonValue["IdlePowerSaver"]["ExitDwellTimeSeconds"] = - std::chrono::duration_cast>(ms) - .count(); - } - else - { - BMCWEB_LOG_WARNING << "Unexpected IdlePowerSaver property: " - << property.first; - } + return false; + } + + if (enabled != nullptr) + { + aResp->res.jsonValue["IdlePowerSaver"]["Enabled"] = *enabled; + } + + if (enterUtilizationPercent != nullptr) + { + aResp->res.jsonValue["IdlePowerSaver"]["EnterUtilizationPercent"] = + *enterUtilizationPercent; + } + + if (enterDwellTime != nullptr) + { + const std::chrono::duration ms(*enterDwellTime); + aResp->res.jsonValue["IdlePowerSaver"]["EnterDwellTimeSeconds"] = + std::chrono::duration_cast>(ms) + .count(); + } + + if (exitUtilizationPercent != nullptr) + { + aResp->res.jsonValue["IdlePowerSaver"]["ExitUtilizationPercent"] = + *exitUtilizationPercent; + } + + if (exitDwellTime != nullptr) + { + const std::chrono::duration ms(*exitDwellTime); + aResp->res.jsonValue["IdlePowerSaver"]["ExitDwellTimeSeconds"] = + std::chrono::duration_cast>(ms) + .count(); } return true; @@ -2426,7 +2427,9 @@ inline void getIdlePowerSaver(const std::shared_ptr& aResp) } // Valid IdlePowerSaver object found, now read the current values - crow::connections::systemBus->async_method_call( + sdbusplus::asio::getAllProperties( + *crow::connections::systemBus, service, path, + "xyz.openbmc_project.Control.Power.IdlePowerSaver", [aResp](const boost::system::error_code ec2, const dbus::utility::DBusPropertiesMap& properties) { if (ec2) @@ -2442,9 +2445,7 @@ inline void getIdlePowerSaver(const std::shared_ptr& aResp) messages::internalError(aResp->res); return; } - }, - service, path, "org.freedesktop.DBus.Properties", "GetAll", - "xyz.openbmc_project.Control.Power.IdlePowerSaver"); + }); }, "xyz.openbmc_project.ObjectMapper", "/xyz/openbmc_project/object_mapper", -- cgit v1.2.3