Unmount old postinstall mountpoint from previous runs.
am: f411650403
Change-Id: I76867c7997e632dd0b1393ac27a4dbe8b93ccffd
diff --git a/common/utils.cc b/common/utils.cc
index 1cb37e5..ea748c1 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -703,6 +703,32 @@
return true;
}
+bool IsMountpoint(const std::string& mountpoint) {
+ 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
+ // there is a filesystem mounted there. If it is not, but they both point to
+ // the same inode it basically is the special case of /.. pointing to /. This
+ // test doesn't play well with bind mount but that's out of the scope of what
+ // we want to detect here.
+ if (lstat(mountpoint.c_str(), &stdir) != 0) {
+ PLOG(ERROR) << "Error stat'ing " << mountpoint;
+ return false;
+ }
+ if (!S_ISDIR(stdir.st_mode))
+ return false;
+
+ base::FilePath parent(mountpoint);
+ parent = parent.Append("..");
+ if (lstat(parent.value().c_str(), &stparent) != 0) {
+ PLOG(ERROR) << "Error stat'ing " << parent.value();
+ return false;
+ }
+ return S_ISDIR(stparent.st_mode) &&
+ (stparent.st_dev != stdir.st_dev || stparent.st_ino == stdir.st_ino);
+}
+
// Tries to parse the header of an ELF file to obtain a human-readable
// description of it on the |output| string.
static bool GetFileFormatELF(const uint8_t* buffer, size_t size,
diff --git a/common/utils.h b/common/utils.h
index d551b7b..8cccc24 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -192,6 +192,12 @@
const std::string& fs_mount_options);
bool UnmountFilesystem(const std::string& mountpoint);
+// Return whether the passed |mountpoint| path is a directory where a filesystem
+// is mounted. Due to detection mechanism limitations, when used on directories
+// where another part of the tree was bind mounted returns true only if bind
+// mounted on top of a different filesystem (not inside the same filesystem).
+bool IsMountpoint(const std::string& mountpoint);
+
// Returns a human-readable string with the file format based on magic constants
// on the header of the file.
std::string GetFileFormat(const std::string& path);
diff --git a/common/utils_unittest.cc b/common/utils_unittest.cc
index 7852910..6e9a911 100644
--- a/common/utils_unittest.cc
+++ b/common/utils_unittest.cc
@@ -478,17 +478,37 @@
test_utils::ScopedLoopbackDeviceBinder loop_binder(
tmp_image, true, &loop_dev);
+ EXPECT_FALSE(utils::IsMountpoint(mnt_dir.path().value()));
// This is the actual test part. While we hold a file descriptor open for the
// mounted filesystem, umount should still succeed.
EXPECT_TRUE(utils::MountFilesystem(
loop_dev, mnt_dir.path().value(), MS_RDONLY, "ext4", ""));
+ // Verify the directory is a mount point now.
+ EXPECT_TRUE(utils::IsMountpoint(mnt_dir.path().value()));
+
string target_file = mnt_dir.path().Append("empty-file").value();
int fd = HANDLE_EINTR(open(target_file.c_str(), O_RDONLY));
EXPECT_GE(fd, 0);
EXPECT_TRUE(utils::UnmountFilesystem(mnt_dir.path().value()));
+ // The filesystem should be already unmounted at this point.
+ EXPECT_FALSE(utils::IsMountpoint(mnt_dir.path().value()));
IGNORE_EINTR(close(fd));
// The filesystem was already unmounted so this call should fail.
EXPECT_FALSE(utils::UnmountFilesystem(mnt_dir.path().value()));
}
+TEST(UtilsTest, IsMountpointTest) {
+ EXPECT_TRUE(utils::IsMountpoint("/"));
+ EXPECT_FALSE(utils::IsMountpoint("/path/to/nowhere"));
+
+ base::ScopedTempDir mnt_dir;
+ EXPECT_TRUE(mnt_dir.CreateUniqueTempDir());
+ EXPECT_FALSE(utils::IsMountpoint(mnt_dir.path().value()));
+
+ base::FilePath file;
+ EXPECT_TRUE(base::CreateTemporaryFile(&file));
+ ScopedPathUnlinker unlinker(file.value());
+ EXPECT_FALSE(utils::IsMountpoint(file.value()));
+}
+
} // namespace chromeos_update_engine
diff --git a/payload_consumer/postinstall_runner_action.cc b/payload_consumer/postinstall_runner_action.cc
index a1b6f25..11eec34 100644
--- a/payload_consumer/postinstall_runner_action.cc
+++ b/payload_consumer/postinstall_runner_action.cc
@@ -118,6 +118,13 @@
fs_mount_dir_ = temp_dir.value();
#endif // __ANDROID__
+ // Double check that the fs_mount_dir is not busy with a previous mounted
+ // filesystem from a previous crashed postinstall step.
+ if (utils::IsMountpoint(fs_mount_dir_)) {
+ LOG(INFO) << "Found previously mounted filesystem at " << fs_mount_dir_;
+ utils::UnmountFilesystem(fs_mount_dir_);
+ }
+
base::FilePath postinstall_path(partition.postinstall_path);
if (postinstall_path.IsAbsolute()) {
LOG(ERROR) << "Invalid absolute path passed to postinstall, use a relative"
