Fix Present property update in prime inventory
This commit adds changes in vpd-manager prime inventory flow to skip
updating Present property for FRUs for which vpd-manager doesn't
handle Present property.
This commit also adds changes to skip updating Present property for a
FRU if vpd-manager is not supposed to handle Present property for the
FRU, in the scenario where VPD parsing fails for the FRU.
Test:
```
Tested on an Everest system.
- Reboot BMC with Chassis Off and fan3 plugged in.
After reboot, fan3 name and Present property appears properly on GUI.
- Reboot BMC with Chassis Off and fan3 plugged out.
After reboot, fan3 name and Present property appears properly on GUI.
Fan Health shows critical on GUI.
- Reboot BMC with Chassis On and fan3 plugged out.
After reboot, fan3 name and Present property appears properly on GUI.
fan3 Health shows critical on GUI.
- Reboot BMC with Chassis On and fan3 plugged in.
After reboot, fan3 name and Present property appears properly on GUI.
- Recreate genesis boot scenario and reboot BMC with fan3 plugged in.
After reboot, fan3 name and Present property appears properly on GUI.
- Recreate genesis boot scenario and reboot BMC with fan3 plugged out.
After reboot, fan3 name and Present property appears properly on GUI.
fan3 Health shows critical on GUI.
```
Change-Id: Ifa6a8909df059a7d3bddd34338d89f0fd8dd5098
Signed-off-by: Souvik Roy <souvikroyofficial10@gmail.com>
diff --git a/vpd-manager/include/worker.hpp b/vpd-manager/include/worker.hpp
index f364964..68ebb3a 100644
--- a/vpd-manager/include/worker.hpp
+++ b/vpd-manager/include/worker.hpp
@@ -519,6 +519,10 @@
/**
* @brief API to set present property.
*
+ * This API updates the present property of the given FRU with the given
+ * value. Note: It is the responsibility of the caller to determine whether
+ * the present property for the FRU should be updated or not.
+ *
* @param[in] i_vpdPath - EEPROM or inventory path.
* @param[in] i_value - value to be set.
*/
@@ -536,6 +540,25 @@
*/
bool skipPathForCollection(const std::string& i_vpdFilePath);
+ /**
+ * @brief API to check if present property should be handled for given FRU.
+ *
+ * vpd-manager should update present property for a FRU if and only if it's
+ * not synthesized and vpd-manager handles present property for the FRU.
+ * This API assumes "handlePresence" tag is a subset of "synthesized" tag.
+ *
+ * @param[in] i_fru - JSON block for a single FRU.
+ *
+ * @return true if present property should be handled, false otherwise.
+ */
+ inline bool isPresentPropertyHandlingRequired(
+ const nlohmann::json& i_fru) const noexcept
+ {
+ // TODO: revisit this to see if this logic can be optimized.
+ return !i_fru.value("synthesized", false) &&
+ i_fru.value("handlePresence", true);
+ }
+
// Parsed JSON file.
nlohmann::json m_parsedJson{};
diff --git a/vpd-manager/src/worker.cpp b/vpd-manager/src/worker.cpp
index f1758fb..82afb3a 100644
--- a/vpd-manager/src/worker.cpp
+++ b/vpd-manager/src/worker.cpp
@@ -822,9 +822,14 @@
continue;
}
- // Clear data under PIM if already exists.
- vpdSpecificUtility::resetDataUnderPIM(
- std::string(l_Fru["inventoryPath"]), l_interfaces);
+ // Reset data under PIM for this FRU only if the FRU is not synthesized
+ // and we handle it's Present property.
+ if (isPresentPropertyHandlingRequired(l_Fru))
+ {
+ // Clear data under PIM if already exists.
+ vpdSpecificUtility::resetDataUnderPIM(
+ std::string(l_Fru["inventoryPath"]), l_interfaces);
+ }
// Add extra interfaces mentioned in the Json config file
if (l_Fru.contains("extraInterfaces"))
@@ -834,16 +839,22 @@
}
types::PropertyMap l_propertyValueMap;
- l_propertyValueMap.emplace("Present", false);
- // TODO: Present based on file will be taken care in future.
- // By default present is set to false for FRU at the time of
- // priming. Once collection goes through, it will be set to true in that
- // flow.
- /*if (std::filesystem::exists(i_vpdFilePath))
+ // Update Present property for this FRU only if we handle Present
+ // property for the FRU.
+ if (isPresentPropertyHandlingRequired(l_Fru))
{
- l_propertyValueMap["Present"] = true;
- }*/
+ l_propertyValueMap.emplace("Present", false);
+
+ // TODO: Present based on file will be taken care in future.
+ // By default present is set to false for FRU at the time of
+ // priming. Once collection goes through, it will be set to true in
+ // that flow.
+ /*if (std::filesystem::exists(i_vpdFilePath))
+ {
+ l_propertyValueMap["Present"] = true;
+ }*/
+ }
vpdSpecificUtility::insertOrMerge(l_interfaces,
"xyz.openbmc_project.Inventory.Item",
@@ -1500,7 +1511,13 @@
// set present property to false for any error case. In future this will
// be replaced by presence logic.
- setPresentProperty(i_vpdFilePath, false);
+ // Update Present property for this FRU only if we handle Present
+ // property for the FRU.
+ if (isPresentPropertyHandlingRequired(
+ m_parsedJson["frus"][i_vpdFilePath].at(0)))
+ {
+ setPresentProperty(i_vpdFilePath, false);
+ }
m_semaphore.release();
return std::make_tuple(false, i_vpdFilePath);