Fsync directory contents
Fsync dirname is not enough for changes to be flushed to disk. Fsync all
entries as well to ensure atomicity.
This change effectively makes update_engine resumable at most points of
interruption.
1. Outside checkpointing (straightforward and has always been resumable)
2. During checkpointing
a. prefs dir deletion (we will resume using prefs_tmp)
b. after rename (If changes aren't flushed, we can continue
using prefs_tmp).
c. during fsync() (This will lead to corrupt prefs on disk)
d. after fsync() -> same as finishing checkpointing
With this change, only an interruption during point c will lead to
corrupt prefs on disk. Since the fsync is relatively fast, update_engine
will essentially be 99% resistant to interrupts. In the off chance that
an interrupt happens during fsync(), we are still able to re-try the OTA
as a fallback
Test: trigger kernel crash at various points during checkpointing
Change-Id: I68fa56b971ed6b5677c47231b51102ae6d01bb9b
diff --git a/common/utils.cc b/common/utils.cc
index c2c72d4..53ef8d0 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -370,7 +370,7 @@
}
off_t FileSize(int fd) {
- struct stat stbuf {};
+ struct stat stbuf{};
int rc = fstat(fd, &stbuf);
CHECK_EQ(rc, 0);
if (rc < 0) {
@@ -425,7 +425,41 @@
return true;
}
+bool FsyncDirectoryContents(const char* dirname) {
+ std::filesystem::path dir_path(dirname);
+
+ std::error_code ec;
+ if (!std::filesystem::exists(dir_path, ec) ||
+ !std::filesystem::is_directory(dir_path, ec)) {
+ LOG(ERROR) << "Error: Invalid directory path: " << dirname << std::endl;
+ return false;
+ }
+
+ for (const auto& entry : std::filesystem::directory_iterator(dirname, ec)) {
+ if (entry.is_regular_file()) {
+ int fd = open(entry.path().c_str(), O_RDONLY | O_CLOEXEC);
+ if (fd == -1) {
+ LOG(ERROR) << "open failed: " << entry.path();
+ return false;
+ }
+
+ if (fsync(fd) == -1) {
+ LOG(ERROR) << "fsync failed";
+ return false;
+ }
+
+ close(fd);
+ }
+ }
+
+ return true;
+}
+
bool FsyncDirectory(const char* dirname) {
+ if (!FsyncDirectoryContents(dirname)) {
+ LOG(ERROR) << "failed to fsync directory contents";
+ return false;
+ }
android::base::unique_fd fd(
TEMP_FAILURE_RETRY(open(dirname, O_RDONLY | O_CLOEXEC)));
if (fd == -1) {
@@ -553,17 +587,17 @@
}
bool FileExists(const char* path) {
- struct stat stbuf {};
+ struct stat stbuf{};
return 0 == lstat(path, &stbuf);
}
bool IsSymlink(const char* path) {
- struct stat stbuf {};
+ struct stat stbuf{};
return lstat(path, &stbuf) == 0 && S_ISLNK(stbuf.st_mode) != 0;
}
bool IsRegFile(const char* path) {
- struct stat stbuf {};
+ struct stat stbuf{};
return lstat(path, &stbuf) == 0 && S_ISREG(stbuf.st_mode) != 0;
}
@@ -718,8 +752,7 @@
}
bool IsMountpoint(const std::string& mountpoint) {
- struct stat stdir {
- }, stparent{};
+ struct stat stdir{}, stparent{};
// Check whether the passed mountpoint is a directory and the /.. is in the
// same device or not. If mountpoint/.. is in a different device it means that
@@ -1163,7 +1196,7 @@
}
string GetTimeAsString(time_t utime) {
- struct tm tm {};
+ struct tm tm{};
CHECK_EQ(localtime_r(&utime, &tm), &tm);
char str[16];
CHECK_EQ(strftime(str, sizeof(str), "%Y%m%d-%H%M%S", &tm), 15u);