bios: On DBus failure fall back to persisted value When building the BIOS attribute/value tables at startup, buildAndStoreAttrTables() reads the BaseBIOSTable property from bios-settings-manager and passes the stored currentValue to each attribute's constructEntry() as optAttributeValue. For attributes backed by a D-Bus property (dBusMap is set), the live D-Bus value is the ground truth and should be preferred. However, if the D-Bus read fails (e.g. the object has not yet appeared on the bus), the correct fallback is the last known persisted value from BaseBIOSTable. Fix all three attribute types (integer, string, enum) by adding an optional persisted value into the getAttrValue() / getAttrValueIndex() helpers. The priority order is now: 1. Live D-Bus value (when dBusMap is set and the read succeeds) 2. Persisted value (optAttributeValue from BaseBIOSTable) 3. JSON default (last resort) Non-D-Bus attributes (no dBusMap) follow the same priority 2 & 3 path they always did. Change-Id: I4c1f52bbd47712846b59e0b25619369b4e0f4a6d Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
diff --git a/libpldmresponder/bios_enum_attribute.cpp b/libpldmresponder/bios_enum_attribute.cpp index 0ea636a..3be76c8 100644 --- a/libpldmresponder/bios_enum_attribute.cpp +++ b/libpldmresponder/bios_enum_attribute.cpp
@@ -128,11 +128,25 @@ } } -uint8_t BIOSEnumAttribute::getAttrValueIndex() +uint8_t BIOSEnumAttribute::getAttrValueIndex( + std::optional<std::string> persisted) { auto defaultValueIndex = getValueIndex(defaultValue, possibleValues); if (!dBusMap.has_value()) { + if (persisted.has_value()) + { + try + { + return getValueIndex(*persisted, possibleValues); + } + catch (const std::exception&) + { + warning("Persisted value '{VALUE}' not in possible values for " + "attribute '{ATTRIBUTE}', using default", + "VALUE", *persisted, "ATTRIBUTE", name); + } + } return defaultValueIndex; } @@ -151,6 +165,19 @@ } catch (const std::exception&) { + if (persisted.has_value()) + { + try + { + return getValueIndex(*persisted, possibleValues); + } + catch (const std::exception&) + { + warning("Persisted value '{VALUE}' not in possible values for " + "attribute '{ATTRIBUTE}', using default", + "VALUE", *persisted, "ATTRIBUTE", name); + } + } return defaultValueIndex; } } @@ -223,25 +250,14 @@ populateValueDisplayNamesMap(attrHandle); - std::vector<uint8_t> currValueIndices(1, 0); + std::optional<std::string> persisted; + if (optAttributeValue.has_value() && optAttributeValue->index() == 1) + { + persisted = std::get<std::string>(*optAttributeValue); + } - if (optAttributeValue.has_value()) - { - auto attributeValue = optAttributeValue.value(); - if (attributeValue.index() == 1) - { - auto currValue = std::get<std::string>(attributeValue); - currValueIndices[0] = getValueIndex(currValue, possibleValues); - } - else - { - currValueIndices[0] = getAttrValueIndex(); - } - } - else - { - currValueIndices[0] = getAttrValueIndex(); - } + std::vector<uint8_t> currValueIndices( + 1, getAttrValueIndex(std::move(persisted))); table::attribute_value::constructEnumEntry(attrValueTable, attrHandle, attrType, currValueIndices); @@ -277,7 +293,7 @@ std::string value = std::get<std::string>(attributevalue); entry->attr_type = 0; entry->value[0] = 1; // number of current values, default 1 - entry->value[1] = getAttrValueIndex(value); + entry->value[1] = getAttrValueIndex(PropertyValue{value}); } } // namespace bios
diff --git a/libpldmresponder/bios_enum_attribute.hpp b/libpldmresponder/bios_enum_attribute.hpp index 459abda..8bf5c77 100644 --- a/libpldmresponder/bios_enum_attribute.hpp +++ b/libpldmresponder/bios_enum_attribute.hpp
@@ -103,10 +103,13 @@ */ void buildValMap(const Json& dbusVals); - /** @brief Get index of the current value in possible values + /** @brief Get index of the current value in possible values, falling back + * to the persisted value then default on D-Bus failure. + * @param[in] persisted - optional persisted string from BaseBIOSTable * @return The index of the current value in possible values */ - uint8_t getAttrValueIndex(); + uint8_t getAttrValueIndex( + std::optional<std::string> persisted = std::nullopt); /** @brief Get index of the property value in possible values * @param[in] propValue - property values
diff --git a/libpldmresponder/bios_integer_attribute.cpp b/libpldmresponder/bios_integer_attribute.cpp index c19d633..36ad13a 100644 --- a/libpldmresponder/bios_integer_attribute.cpp +++ b/libpldmresponder/bios_integer_attribute.cpp
@@ -118,23 +118,13 @@ auto [attrHandle, attrType, _] = table::attribute::decodeHeader(attrTableEntry); - int64_t currentValue{}; - if (optAttributeValue.has_value()) + std::optional<int64_t> persisted; + if (optAttributeValue.has_value() && optAttributeValue->index() == 0) { - auto attributeValue = optAttributeValue.value(); - if (attributeValue.index() == 0) - { - currentValue = std::get<int64_t>(attributeValue); - } - else - { - currentValue = getAttrValue(); - } + persisted = std::get<int64_t>(*optAttributeValue); } - else - { - currentValue = getAttrValue(); - } + + auto currentValue = getAttrValue(persisted); table::attribute_value::constructIntegerEntry(attrValueTable, attrHandle, attrType, currentValue); @@ -184,11 +174,12 @@ return value; } -uint64_t BIOSIntegerAttribute::getAttrValue() +uint64_t BIOSIntegerAttribute::getAttrValue(std::optional<int64_t> persisted) { if (!dBusMap.has_value()) { - return integerInfo.defaultValue; + return static_cast<uint64_t>( + persisted.value_or(static_cast<int64_t>(integerInfo.defaultValue))); } try @@ -205,7 +196,8 @@ "Error getting integer attribute '{ATTRIBUTE}' at path '{PATH}' and interface '{INTERFACE}' for property '{PROPERTY}', error - {ERROR}", "ATTRIBUTE", name, "PATH", dBusMap->objectPath, "INTERFACE", dBusMap->interface, "PROPERTY", dBusMap->propertyName, "ERROR", e); - return integerInfo.defaultValue; + return static_cast<uint64_t>( + persisted.value_or(static_cast<int64_t>(integerInfo.defaultValue))); } }
diff --git a/libpldmresponder/bios_integer_attribute.hpp b/libpldmresponder/bios_integer_attribute.hpp index 6ed9c6b..446ab4b 100644 --- a/libpldmresponder/bios_integer_attribute.hpp +++ b/libpldmresponder/bios_integer_attribute.hpp
@@ -70,8 +70,11 @@ /** @brief Get pldm value from dbus propertyValue */ uint64_t getAttrValue(const pldm::utils::PropertyValue& value); - /** @brief Get value on dbus */ - uint64_t getAttrValue(); + /** @brief Get value on dbus, falling back to persisted value then default + * on failure. + * @param[in] persisted - optional persisted value from BaseBIOSTable + */ + uint64_t getAttrValue(std::optional<int64_t> persisted = std::nullopt); }; } // namespace bios
diff --git a/libpldmresponder/bios_string_attribute.cpp b/libpldmresponder/bios_string_attribute.cpp index 88ea911..f4e22c3 100644 --- a/libpldmresponder/bios_string_attribute.cpp +++ b/libpldmresponder/bios_string_attribute.cpp
@@ -74,11 +74,12 @@ dbusHandler->setDbusProperty(*dBusMap, value); } -std::string BIOSStringAttribute::getAttrValue() +std::string BIOSStringAttribute::getAttrValue( + std::optional<std::string> persisted) { if (!dBusMap.has_value()) { - return stringInfo.defString; + return persisted.value_or(stringInfo.defString); } try { @@ -92,7 +93,7 @@ "Failed to get string attribute '{ATTRIBUTE}' at path '{PATH}' and interface '{INTERFACE}' for property '{PROPERTY}', error - {ERROR}", "ATTRIBUTE", name, "PATH", dBusMap->objectPath, "INTERFACE", dBusMap->interface, "PROPERTY", dBusMap->propertyName, "ERROR", e); - return stringInfo.defString; + return persisted.value_or(stringInfo.defString); } } @@ -112,23 +113,13 @@ auto [attrHandle, attrType, _] = table::attribute::decodeHeader(attrTableEntry); - std::string currStr{}; - if (optAttributeValue.has_value()) + std::optional<std::string> persisted; + if (optAttributeValue.has_value() && optAttributeValue->index() == 1) { - auto attributeValue = optAttributeValue.value(); - if (attributeValue.index() == 1) - { - currStr = std::get<std::string>(attributeValue); - } - else - { - currStr = getAttrValue(); - } + persisted = std::get<std::string>(*optAttributeValue); } - else - { - currStr = getAttrValue(); - } + + auto currStr = getAttrValue(std::move(persisted)); table::attribute_value::constructStringEntry(attrValueTable, attrHandle, attrType, currStr);
diff --git a/libpldmresponder/bios_string_attribute.hpp b/libpldmresponder/bios_string_attribute.hpp index 5e65595..18be953 100644 --- a/libpldmresponder/bios_string_attribute.hpp +++ b/libpldmresponder/bios_string_attribute.hpp
@@ -90,8 +90,12 @@ /** @brief string field from json */ table::attribute::StringField stringInfo; - /** @brief Get attribute value on dbus */ - std::string getAttrValue(); + /** @brief Get attribute value on dbus, falling back to persisted value + * then default on failure. + * @param[in] persisted - optional persisted value from BaseBIOSTable + */ + std::string getAttrValue( + std::optional<std::string> persisted = std::nullopt); }; } // namespace bios