Delay return from updateUbootEnvVars() until service file finishes
This commit, as the title might suggest, delays the return from the
function updateUbootEnvVars() until the called service file completes
running. The reason for this change is because the service file runs
asynchronously, so activation seemingly finishes before the priorities
are actually changed.
This is accomplished by adding a generic function waitForServiceFile()
which waits until the completion of a supplied one-shot service file or
until a timeout is reached.
Resolves openbmc/openbmc#2857
Change-Id: I7c0333f510f8239a0b66cc5a20159c91aa2dbdee
Signed-off-by: Michael Tritz <mtritz@us.ibm.com>
Signed-off-by: Saqib Khan <khansa@us.ibm.com>
diff --git a/item_updater.cpp b/item_updater.cpp
index df73fc5..a4198ab 100644
--- a/item_updater.cpp
+++ b/item_updater.cpp
@@ -575,6 +575,10 @@
log<level::ERR>("Failed to update u-boot env variables",
entry("VERSIONID=%s", versionId.c_str()));
}
+
+ // On average this service takes about 5 seconds. Therefore setting
+ // it to 3x average time should be more than sufficient.
+ waitForServiceFile(updateEnvVarsFile, 15);
}
void ItemUpdater::resetUbootEnvVars()
diff --git a/serialize.cpp b/serialize.cpp
index b59ff46..29bd640 100644
--- a/serialize.cpp
+++ b/serialize.cpp
@@ -2,8 +2,11 @@
#include <experimental/filesystem>
#include <cereal/archives/json.hpp>
#include <fstream>
+#include <unistd.h>
#include "serialize.hpp"
#include <sdbusplus/server.hpp>
+#include <phosphor-logging/log.hpp>
+#include <chrono>
namespace phosphor
{
@@ -14,6 +17,9 @@
namespace fs = std::experimental::filesystem;
+using namespace phosphor::logging;
+using namespace std::chrono_literals;
+
void storeToFile(std::string versionId, uint8_t priority)
{
auto bus = sdbusplus::bus::new_default();
@@ -34,6 +40,10 @@
SYSTEMD_INTERFACE, "StartUnit");
method.append(serviceFile, "replace");
bus.call_noreply(method);
+
+ // On average it takes 1-2 seconds for the service to complete.
+ // Therefore timeout is set to 3x average completion time.
+ waitForServiceFile(serviceFile, 6);
}
bool restoreFromFile(std::string versionId, uint8_t& priority)
@@ -104,6 +114,37 @@
}
}
+void waitForServiceFile(const std::string& serviceFile, int timeout)
+{
+ auto bus = sdbusplus::bus::new_default();
+
+ std::time_t start = time(0);
+ std::time_t end = time(0);
+
+ while (end - start < timeout)
+ {
+ auto method = bus.new_method_call(SYSTEMD_BUSNAME, SYSTEMD_PATH,
+ SYSTEMD_INTERFACE, "GetUnit");
+ method.append(serviceFile);
+ auto result = bus.call(method);
+
+ // "GetUnit" returns the unit object path for a unit name.
+ // If the unit is not found because it hasn't been loaded yet,
+ // or in this case because it has finished and exited, the call
+ // will fail.
+ if (result.is_method_error())
+ {
+ return;
+ }
+
+ std::this_thread::sleep_for(1s);
+ end = time(0);
+ }
+
+ log<level::ERR>("Service file timed out!",
+ entry("FILENAME=%s", serviceFile.c_str()));
+}
+
} // namespace phosphor
} // namespace software
} // namespace openpower
diff --git a/serialize.hpp b/serialize.hpp
index 03eea99..604aada 100644
--- a/serialize.hpp
+++ b/serialize.hpp
@@ -30,6 +30,13 @@
**/
void removeFile(std::string versionId);
+/** @brief Delays until the completion of the supplied service file or until
+ * specified timeout is reached.
+ * @param[in] serviceFile - Service file to wait for.
+ * @param[in] timeout - Fallback timeout so we don't get stuck.
+ **/
+void waitForServiceFile(const std::string& serviceFile, int timeout);
+
} // namespace phosphor
} // namespace software
} // namespace openpower