Call update u-boot env once when a priority is changed
The recursive nature of calling the free priority function
would trigger setting the u-boot env multiple times.
Make a change so that the priorities are sorted and
updated once.
- Create a non-overriden priority setter function to be called
by the free priority function and by the function that creates
the dbus objects after a bmc reboot. There's no need to call
to free the priorities after reboot since the priorities are
preserved on the bmc, and if they're not they default to 0 or 255.
- When a dbus request is made to update the priority, update
the value, then call the free priority function, which will
sort the versions by priority and bump the priority of any
duplicate ones.
Resolves openbmc/openbmc#2535
Change-Id: Ib92cc0ca6c4d5f6e986f3cde7156d63b53844b46
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
diff --git a/item_updater.cpp b/item_updater.cpp
index 33f4096..151986f 100644
--- a/item_updater.cpp
+++ b/item_updater.cpp
@@ -1,4 +1,5 @@
#include <fstream>
+#include <set>
#include <string>
#include <phosphor-logging/log.hpp>
#include <phosphor-logging/elog.hpp>
@@ -254,7 +255,8 @@
bus,
path,
*(activations.find(id)->second),
- priority);
+ priority,
+ false);
}
}
}
@@ -396,18 +398,58 @@
void ItemUpdater::freePriority(uint8_t value, const std::string& versionId)
{
- //TODO openbmc/openbmc#1896 Improve the performance of this function
+ std::map<std::string, uint8_t> priorityMap;
+
+ // Insert the requested version and priority, it may not exist yet.
+ priorityMap.insert(std::make_pair(versionId, value));
+
for (const auto& intf : activations)
{
if (intf.second->redundancyPriority)
{
- if (intf.second->redundancyPriority.get()->priority() == value &&
- intf.second->versionId != versionId)
- {
- intf.second->redundancyPriority.get()->priority(value + 1);
- }
+ priorityMap.insert(std::make_pair(
+ intf.first,
+ intf.second->redundancyPriority.get()->priority()));
}
}
+
+ // Lambda function to compare 2 priority values, use <= to allow duplicates
+ typedef std::function<bool(
+ std::pair<std::string, uint8_t>,
+ std::pair<std::string, uint8_t>)> cmpPriority;
+ cmpPriority cmpPriorityFunc = [](
+ std::pair<std::string, uint8_t> priority1,
+ std::pair<std::string, uint8_t> priority2)
+ {
+ return priority1.second <= priority2.second;
+ };
+
+ // Sort versions by ascending priority
+ std::set<std::pair<std::string, uint8_t>, cmpPriority> prioritySet(
+ priorityMap.begin(), priorityMap.end(), cmpPriorityFunc);
+
+ auto freePriorityValue = value;
+ for (auto& element : prioritySet)
+ {
+ if (element.first == versionId)
+ {
+ continue;
+ }
+ if (element.second == freePriorityValue)
+ {
+ ++freePriorityValue;
+ auto it = activations.find(element.first);
+ it->second->redundancyPriority.get()->sdbusPriority(
+ freePriorityValue);
+ }
+ }
+
+ auto lowestVersion = prioritySet.begin()->first;
+ if (value == prioritySet.begin()->second)
+ {
+ lowestVersion = versionId;
+ }
+ updateUbootEnvVars(lowestVersion);
}
void ItemUpdater::reset()
@@ -587,6 +629,26 @@
return true;
}
+void ItemUpdater::updateUbootEnvVars(const std::string& versionId)
+{
+ auto method = bus.new_method_call(
+ SYSTEMD_BUSNAME,
+ SYSTEMD_PATH,
+ SYSTEMD_INTERFACE,
+ "StartUnit");
+ auto updateEnvVarsFile = "obmc-flash-bmc-updateubootvars@" + versionId +
+ ".service";
+ method.append(updateEnvVarsFile, "replace");
+ auto result = bus.call(method);
+
+ //Check that the bus call didn't result in an error
+ if (result.is_method_error())
+ {
+ log<level::ERR>("Failed to update u-boot env variables",
+ entry("VERSIONID=%s", versionId));
+ }
+}
+
void ItemUpdater::resetUbootEnvVars()
{
decltype(activations.begin()->second->redundancyPriority.get()->priority())
@@ -609,8 +671,7 @@
}
// Update the U-boot environment variable to point to the lowest priority
- auto it = activations.find(lowestPriorityVersion);
- it->second->updateUbootEnvVars();
+ updateUbootEnvVars(lowestPriorityVersion);
}
} // namespace updater