summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNan Zhou <nanzhoumails@gmail.com>2022-05-18 00:12:43 +0300
committerEd Tanous <ed@tanous.net>2022-05-19 22:56:00 +0300
commit9a5aceac206a550930204f102f1b84d808579407 (patch)
treefa0e1a8af6e4175f343bb0d70dafd7d38be15837
parent6a409c12ced1ae98773ea018767e386ace2ed742 (diff)
downloadbmcweb-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.hpp524
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", "");
}