diff options
author | Nan Zhou <nanzhoumails@gmail.com> | 2022-05-18 00:12:43 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2022-05-19 22:56:00 +0300 |
commit | 9a5aceac206a550930204f102f1b84d808579407 (patch) | |
tree | fa0e1a8af6e4175f343bb0d70dafd7d38be15837 | |
parent | 6a409c12ced1ae98773ea018767e386ace2ed742 (diff) | |
download | bmcweb-9a5aceac206a550930204f102f1b84d808579407.tar.xz |
memory: move long code blocks in callbacks into separate functions
It is a bit cleaner to have separate functions rather than keep codes in
the callback, as callback normally have deeper indent.
The main reason is that this helps code review of later changes that
make Expand at MemoryCollection efficient.
Tested:
1. Tested on my mock environment,
```
URI: /redfish/v1/Systems/system/Memory/dimm0
{
"@odata.id": "/redfish/v1/Systems/system/Memory/dimm0",
"@odata.type": "#Memory.v1_11_0.Memory",
"AllowedSpeedsMHz": [],
"BaseModuleType": "RDIMM",
"BusWidthBits": 0,
"CapacityMiB": 1024,
"DataWidthBits": 0,
"ErrorCorrection": "NoECC",
"FirmwareRevision": "0",
"Id": "dimm0",
"Name": "DIMM Slot",
"OperatingSpeedMhz": 0,
"RankCount": 0,
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Enabled"
}
}
```
2. No new Redfish Validator failures on MemoryCollection on real
hardware.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I7693388049aeffa6ebd285b958e5ca6622e5d3b6
-rw-r--r-- | redfish-core/lib/memory.hpp | 524 |
1 files changed, 257 insertions, 267 deletions
diff --git a/redfish-core/lib/memory.hpp b/redfish-core/lib/memory.hpp index 6e4ab679c0..f84a17a709 100644 --- a/redfish-core/lib/memory.hpp +++ b/redfish-core/lib/memory.hpp @@ -426,295 +426,285 @@ inline void getPersistentMemoryProperties( } } -inline void getDimmDataByService(std::shared_ptr<bmcweb::AsyncResp> aResp, - const std::string& dimmId, - const std::string& service, - const std::string& objPath) +inline void + assembleDimmProperties(std::string_view dimmId, + const std::shared_ptr<bmcweb::AsyncResp>& aResp, + const dbus::utility::DBusPropertiesMap& properties) { - auto health = std::make_shared<HealthPopulate>(aResp); - health->selfPath = objPath; - health->populate(); + aResp->res.jsonValue["Id"] = dimmId; + aResp->res.jsonValue["Name"] = "DIMM Slot"; + aResp->res.jsonValue["Status"]["State"] = "Enabled"; + aResp->res.jsonValue["Status"]["Health"] = "OK"; - BMCWEB_LOG_DEBUG << "Get available system components."; - crow::connections::systemBus->async_method_call( - [dimmId, aResp{std::move(aResp)}]( - const boost::system::error_code ec, - const dbus::utility::DBusPropertiesMap& properties) { - if (ec) + for (const auto& property : properties) + { + if (property.first == "MemoryDataWidth") + { + const uint16_t* value = std::get_if<uint16_t>(&property.second); + if (value == nullptr) { - BMCWEB_LOG_DEBUG << "DBUS response error"; + continue; + } + aResp->res.jsonValue["DataWidthBits"] = *value; + } + else if (property.first == "MemorySizeInKB") + { + const size_t* memorySize = std::get_if<size_t>(&property.second); + if (memorySize == nullptr) + { + // Important property not in desired type messages::internalError(aResp->res); return; } - aResp->res.jsonValue["Id"] = dimmId; - aResp->res.jsonValue["Name"] = "DIMM Slot"; - aResp->res.jsonValue["Status"]["State"] = "Enabled"; - aResp->res.jsonValue["Status"]["Health"] = "OK"; + aResp->res.jsonValue["CapacityMiB"] = (*memorySize >> 10); + } + else if (property.first == "PartNumber") + { + const std::string* value = + std::get_if<std::string>(&property.second); + if (value == nullptr) + { + continue; + } + aResp->res.jsonValue["PartNumber"] = *value; + } + else if (property.first == "SerialNumber") + { + const std::string* value = + std::get_if<std::string>(&property.second); + if (value == nullptr) + { + continue; + } + aResp->res.jsonValue["SerialNumber"] = *value; + } + else if (property.first == "Manufacturer") + { + const std::string* value = + std::get_if<std::string>(&property.second); + if (value == nullptr) + { + continue; + } + aResp->res.jsonValue["Manufacturer"] = *value; + } + else if (property.first == "RevisionCode") + { + const uint16_t* value = std::get_if<uint16_t>(&property.second); - for (const auto& property : properties) + if (value == nullptr) { - if (property.first == "MemoryDataWidth") - { - const uint16_t* value = - std::get_if<uint16_t>(&property.second); - if (value == nullptr) - { - continue; - } - aResp->res.jsonValue["DataWidthBits"] = *value; - } - else if (property.first == "MemorySizeInKB") - { - const size_t* memorySize = - std::get_if<size_t>(&property.second); - if (memorySize == nullptr) - { - // Important property not in desired type - messages::internalError(aResp->res); - return; - } - aResp->res.jsonValue["CapacityMiB"] = (*memorySize >> 10); - } - else if (property.first == "PartNumber") - { - const std::string* value = - std::get_if<std::string>(&property.second); - if (value == nullptr) - { - continue; - } - aResp->res.jsonValue["PartNumber"] = *value; - } - else if (property.first == "SerialNumber") - { - const std::string* value = - std::get_if<std::string>(&property.second); - if (value == nullptr) - { - continue; - } - aResp->res.jsonValue["SerialNumber"] = *value; - } - else if (property.first == "Manufacturer") - { - const std::string* value = - std::get_if<std::string>(&property.second); - if (value == nullptr) - { - continue; - } - aResp->res.jsonValue["Manufacturer"] = *value; - } - else if (property.first == "RevisionCode") - { - const uint16_t* value = - std::get_if<uint16_t>(&property.second); + messages::internalError(aResp->res); + BMCWEB_LOG_DEBUG << "Invalid property type for RevisionCode"; + return; + } + aResp->res.jsonValue["FirmwareRevision"] = std::to_string(*value); + } + else if (property.first == "Present") + { + const bool* value = std::get_if<bool>(&property.second); + if (value == nullptr) + { + messages::internalError(aResp->res); + BMCWEB_LOG_DEBUG << "Invalid property type for Dimm Presence"; + return; + } + if (!*value) + { + aResp->res.jsonValue["Status"]["State"] = "Absent"; + } + } + else if (property.first == "MemoryTotalWidth") + { + const uint16_t* value = std::get_if<uint16_t>(&property.second); + if (value == nullptr) + { + continue; + } + aResp->res.jsonValue["BusWidthBits"] = *value; + } + else if (property.first == "ECC") + { + const std::string* value = + std::get_if<std::string>(&property.second); + if (value == nullptr) + { + messages::internalError(aResp->res); + BMCWEB_LOG_DEBUG << "Invalid property type for ECC"; + return; + } + constexpr const std::array<const char*, 4> values{ + "NoECC", "SingleBitECC", "MultiBitECC", "AddressParity"}; - if (value == nullptr) - { - messages::internalError(aResp->res); - BMCWEB_LOG_DEBUG - << "Invalid property type for RevisionCode"; - return; - } - aResp->res.jsonValue["FirmwareRevision"] = - std::to_string(*value); - } - else if (property.first == "Present") - { - const bool* value = std::get_if<bool>(&property.second); - if (value == nullptr) - { - messages::internalError(aResp->res); - BMCWEB_LOG_DEBUG - << "Invalid property type for Dimm Presence"; - return; - } - if (!*value) - { - aResp->res.jsonValue["Status"]["State"] = "Absent"; - } - } - else if (property.first == "MemoryTotalWidth") - { - const uint16_t* value = - std::get_if<uint16_t>(&property.second); - if (value == nullptr) - { - continue; - } - aResp->res.jsonValue["BusWidthBits"] = *value; - } - else if (property.first == "ECC") + for (const char* v : values) + { + if (boost::ends_with(*value, v)) { - const std::string* value = - std::get_if<std::string>(&property.second); - if (value == nullptr) - { - messages::internalError(aResp->res); - BMCWEB_LOG_DEBUG << "Invalid property type for ECC"; - return; - } - constexpr const std::array<const char*, 4> values{ - "NoECC", "SingleBitECC", "MultiBitECC", - "AddressParity"}; - - for (const char* v : values) - { - if (boost::ends_with(*value, v)) - { - aResp->res.jsonValue["ErrorCorrection"] = v; - break; - } - } + aResp->res.jsonValue["ErrorCorrection"] = v; + break; } - else if (property.first == "FormFactor") - { - const std::string* value = - std::get_if<std::string>(&property.second); - if (value == nullptr) - { - messages::internalError(aResp->res); - BMCWEB_LOG_DEBUG - << "Invalid property type for FormFactor"; - return; - } - constexpr const std::array<const char*, 11> values{ - "RDIMM", "UDIMM", "SO_DIMM", - "LRDIMM", "Mini_RDIMM", "Mini_UDIMM", - "SO_RDIMM_72b", "SO_UDIMM_72b", "SO_DIMM_16b", - "SO_DIMM_32b", "Die"}; + } + } + else if (property.first == "FormFactor") + { + const std::string* value = + std::get_if<std::string>(&property.second); + if (value == nullptr) + { + messages::internalError(aResp->res); + BMCWEB_LOG_DEBUG << "Invalid property type for FormFactor"; + return; + } + constexpr const std::array<const char*, 11> values{ + "RDIMM", "UDIMM", "SO_DIMM", "LRDIMM", + "Mini_RDIMM", "Mini_UDIMM", "SO_RDIMM_72b", "SO_UDIMM_72b", + "SO_DIMM_16b", "SO_DIMM_32b", "Die"}; - for (const char* v : values) - { - if (boost::ends_with(*value, v)) - { - aResp->res.jsonValue["BaseModuleType"] = v; - break; - } - } - } - else if (property.first == "AllowedSpeedsMT") + for (const char* v : values) + { + if (boost::ends_with(*value, v)) { - const std::vector<uint16_t>* value = - std::get_if<std::vector<uint16_t>>(&property.second); - if (value == nullptr) - { - continue; - } - nlohmann::json& jValue = - aResp->res.jsonValue["AllowedSpeedsMHz"]; - jValue = nlohmann::json::array(); - for (uint16_t subVal : *value) - { - jValue.push_back(subVal); - } + aResp->res.jsonValue["BaseModuleType"] = v; + break; } - else if (property.first == "MemoryAttributes") - { - const uint8_t* value = - std::get_if<uint8_t>(&property.second); + } + } + else if (property.first == "AllowedSpeedsMT") + { + const std::vector<uint16_t>* value = + std::get_if<std::vector<uint16_t>>(&property.second); + if (value == nullptr) + { + continue; + } + nlohmann::json& jValue = aResp->res.jsonValue["AllowedSpeedsMHz"]; + jValue = nlohmann::json::array(); + for (uint16_t subVal : *value) + { + jValue.push_back(subVal); + } + } + else if (property.first == "MemoryAttributes") + { + const uint8_t* value = std::get_if<uint8_t>(&property.second); - if (value == nullptr) - { - messages::internalError(aResp->res); - BMCWEB_LOG_DEBUG - << "Invalid property type for MemoryAttributes"; - return; - } - aResp->res.jsonValue["RankCount"] = - static_cast<uint64_t>(*value); - } - else if (property.first == "MemoryConfiguredSpeedInMhz") - { - const uint16_t* value = - std::get_if<uint16_t>(&property.second); - if (value == nullptr) - { - continue; - } - aResp->res.jsonValue["OperatingSpeedMhz"] = *value; - } - else if (property.first == "MemoryType") - { - const auto* value = - std::get_if<std::string>(&property.second); - if (value != nullptr) - { - std::string memoryDeviceType = - translateMemoryTypeToRedfish(*value); - // Values like "Unknown" or "Other" will return empty - // so just leave off - if (!memoryDeviceType.empty()) - { - aResp->res.jsonValue["MemoryDeviceType"] = - memoryDeviceType; - } - if (value->find("DDR") != std::string::npos) - { - aResp->res.jsonValue["MemoryType"] = "DRAM"; - } - else if (boost::ends_with(*value, "Logical")) - { - aResp->res.jsonValue["MemoryType"] = "IntelOptane"; - } - } - } - // memory location interface - else if (property.first == "Channel" || - property.first == "MemoryController" || - property.first == "Slot" || property.first == "Socket") - { - const std::string* value = - std::get_if<std::string>(&property.second); - if (value == nullptr) - { - messages::internalError(aResp->res); - return; - } - aResp->res.jsonValue["MemoryLocation"][property.first] = - *value; - } - else if (property.first == "SparePartNumber") - { - const std::string* value = - std::get_if<std::string>(&property.second); - if (value == nullptr) - { - messages::internalError(aResp->res); - return; - } - aResp->res.jsonValue["SparePartNumber"] = *value; - } - else if (property.first == "Model") + if (value == nullptr) + { + messages::internalError(aResp->res); + BMCWEB_LOG_DEBUG + << "Invalid property type for MemoryAttributes"; + return; + } + aResp->res.jsonValue["RankCount"] = static_cast<uint64_t>(*value); + } + else if (property.first == "MemoryConfiguredSpeedInMhz") + { + const uint16_t* value = std::get_if<uint16_t>(&property.second); + if (value == nullptr) + { + continue; + } + aResp->res.jsonValue["OperatingSpeedMhz"] = *value; + } + else if (property.first == "MemoryType") + { + const auto* value = std::get_if<std::string>(&property.second); + if (value != nullptr) + { + std::string memoryDeviceType = + translateMemoryTypeToRedfish(*value); + // Values like "Unknown" or "Other" will return empty + // so just leave off + if (!memoryDeviceType.empty()) { - const std::string* value = - std::get_if<std::string>(&property.second); - if (value == nullptr) - { - messages::internalError(aResp->res); - return; - } - aResp->res.jsonValue["Model"] = *value; + aResp->res.jsonValue["MemoryDeviceType"] = memoryDeviceType; } - else if (property.first == "LocationCode") + if (value->find("DDR") != std::string::npos) { - const std::string* value = - std::get_if<std::string>(&property.second); - if (value == nullptr) - { - messages::internalError(aResp->res); - return; - } - aResp->res - .jsonValue["Location"]["PartLocation"]["ServiceLabel"] = - *value; + aResp->res.jsonValue["MemoryType"] = "DRAM"; } - else + else if (boost::ends_with(*value, "Logical")) { - getPersistentMemoryProperties(aResp, property); + aResp->res.jsonValue["MemoryType"] = "IntelOptane"; } } + } + // memory location interface + else if (property.first == "Channel" || + property.first == "MemoryController" || + property.first == "Slot" || property.first == "Socket") + { + const std::string* value = + std::get_if<std::string>(&property.second); + if (value == nullptr) + { + messages::internalError(aResp->res); + return; + } + aResp->res.jsonValue["MemoryLocation"][property.first] = *value; + } + else if (property.first == "SparePartNumber") + { + const std::string* value = + std::get_if<std::string>(&property.second); + if (value == nullptr) + { + messages::internalError(aResp->res); + return; + } + aResp->res.jsonValue["SparePartNumber"] = *value; + } + else if (property.first == "Model") + { + const std::string* value = + std::get_if<std::string>(&property.second); + if (value == nullptr) + { + messages::internalError(aResp->res); + return; + } + aResp->res.jsonValue["Model"] = *value; + } + else if (property.first == "LocationCode") + { + const std::string* value = + std::get_if<std::string>(&property.second); + if (value == nullptr) + { + messages::internalError(aResp->res); + return; + } + aResp->res.jsonValue["Location"]["PartLocation"]["ServiceLabel"] = + *value; + } + else + { + getPersistentMemoryProperties(aResp, property); + } + } +} + +inline void getDimmDataByService(std::shared_ptr<bmcweb::AsyncResp> aResp, + const std::string& dimmId, + const std::string& service, + const std::string& objPath) +{ + auto health = std::make_shared<HealthPopulate>(aResp); + health->selfPath = objPath; + health->populate(); + + BMCWEB_LOG_DEBUG << "Get available system components."; + crow::connections::systemBus->async_method_call( + [dimmId, aResp{std::move(aResp)}]( + const boost::system::error_code ec, + const dbus::utility::DBusPropertiesMap& properties) { + if (ec) + { + BMCWEB_LOG_DEBUG << "DBUS response error"; + messages::internalError(aResp->res); + return; + } + assembleDimmProperties(dimmId, aResp, properties); }, service, objPath, "org.freedesktop.DBus.Properties", "GetAll", ""); } |