Fsync data after writing prefs
update_engine is relying on prefs stored on filesystem to resume OTA
progress. Partners report loss of update progress after forced reboot.
This might be I/O cache issue.
Test: th
Bug: 259174530
Change-Id: I7b4b8281cfe2cc45c51e4d6d60e09edb12f3ed86
diff --git a/common/prefs.cc b/common/prefs.cc
index f33a8a9..41a7b24 100644
--- a/common/prefs.cc
+++ b/common/prefs.cc
@@ -25,6 +25,7 @@
#include <base/strings/string_split.h>
#include <base/strings/string_util.h>
+#include "brillo/file_utils.h"
#include "update_engine/common/utils.h"
using std::string;
@@ -202,8 +203,8 @@
// to parent directories where we might not have permission to write to.
TEST_AND_RETURN_FALSE(base::CreateDirectory(filename.DirName()));
}
- TEST_AND_RETURN_FALSE(base::WriteFile(filename, value.data(), value.size()) ==
- static_cast<int>(value.size()));
+ TEST_AND_RETURN_FALSE(
+ utils::WriteStringToFileAtomic(filename.value(), value));
return true;
}
diff --git a/common/utils.cc b/common/utils.cc
index e3ffa1c..6b8bc54 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -35,6 +35,7 @@
#include <unistd.h>
#include <algorithm>
+#include <filesystem>
#include <utility>
#include <vector>
@@ -409,6 +410,53 @@
return true;
}
+bool FsyncDirectory(const char* dirname) {
+ android::base::unique_fd fd(
+ TEMP_FAILURE_RETRY(open(dirname, O_RDONLY | O_CLOEXEC)));
+ if (fd == -1) {
+ PLOG(ERROR) << "Failed to open " << dirname;
+ return false;
+ }
+ if (fsync(fd) == -1) {
+ if (errno == EROFS || errno == EINVAL) {
+ PLOG(WARNING) << "Skip fsync " << dirname
+ << " on a file system does not support synchronization";
+ } else {
+ PLOG(ERROR) << "Failed to fsync " << dirname;
+ return false;
+ }
+ }
+ return true;
+}
+
+bool WriteStringToFileAtomic(const std::string& path,
+ std::string_view content) {
+ const std::string tmp_path = path + ".tmp";
+ {
+ const int flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC;
+ android::base::unique_fd fd(
+ TEMP_FAILURE_RETRY(open(tmp_path.c_str(), flags, 0644)));
+ if (fd == -1) {
+ PLOG(ERROR) << "Failed to open " << path;
+ return false;
+ }
+ if (!WriteAll(fd.get(), content.data(), content.size())) {
+ PLOG(ERROR) << "Failed to write to fd " << fd;
+ return false;
+ }
+ // rename() without fsync() is not safe. Data could still be living on page
+ // cache. To ensure atomiticity, call fsync()
+ if (fsync(fd) != 0) {
+ PLOG(ERROR) << "Failed to fsync " << tmp_path;
+ }
+ }
+ if (rename(tmp_path.c_str(), path.c_str()) == -1) {
+ PLOG(ERROR) << "rename failed from " << tmp_path << " to " << path;
+ return false;
+ }
+ return FsyncDirectory(std::filesystem::path(path).parent_path().c_str());
+}
+
void HexDumpArray(const uint8_t* const arr, const size_t length) {
LOG(INFO) << "Logging array of length: " << length;
const unsigned int bytes_per_line = 16;
diff --git a/common/utils.h b/common/utils.h
index 201e47e..0087794 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -162,6 +162,9 @@
bool SendFile(int out_fd, int in_fd, size_t count);
+bool FsyncDirectory(const char* dirname);
+bool WriteStringToFileAtomic(const std::string& path, std::string_view content);
+
// Returns true if the file exists for sure. Returns false if it doesn't exist,
// or an error occurs.
bool FileExists(const char* path);