Make unit tests less timing sensitive when unmounting busy loop devices.
This CL modifies utils::UnmountFilesystem() to retry when umount()
fails to unmount a busy loop device and replaces several unit tests to
use the modified utils::UnmountFilesystem() instead of calling
System("umount").
BUG=chromium-os:35112
TEST=Run unit tests repeatedly.
Change-Id: I42978f02b4797c68acbec6c351ea2663b9ec7b59
Reviewed-on: https://gerrit.chromium.org/gerrit/34862
Reviewed-by: Ben Chan <[email protected]>
Tested-by: Ben Chan <[email protected]>
Commit-Ready: Ben Chan <[email protected]>
diff --git a/filesystem_iterator_unittest.cc b/filesystem_iterator_unittest.cc
index aca95ff..aed16e5 100644
--- a/filesystem_iterator_unittest.cc
+++ b/filesystem_iterator_unittest.cc
@@ -57,9 +57,9 @@
set<string> expected_paths(expected_paths_vector.begin(),
expected_paths_vector.end());
VerifyAllPaths(kMountPath, expected_paths);
-
- EXPECT_EQ(0, System(string("umount ") + kMountPath + "/some_dir/mnt"));
- EXPECT_EQ(0, System(string("umount ") + kMountPath));
+
+ EXPECT_TRUE(utils::UnmountFilesystem(kMountPath + string("/some_dir/mnt")));
+ EXPECT_TRUE(utils::UnmountFilesystem(kMountPath));
EXPECT_EQ(0, System(string("rm -f ") + first_image + " " + sub_image));
}
diff --git a/postinstall_runner_action_unittest.cc b/postinstall_runner_action_unittest.cc
index 8f42ddd..65f378e 100644
--- a/postinstall_runner_action_unittest.cc
+++ b/postinstall_runner_action_unittest.cc
@@ -119,7 +119,7 @@
ASSERT_TRUE(WriteFileString(mountpoint + "/postinst", script));
ASSERT_EQ(0, System(string("chmod a+x ") + mountpoint + "/postinst"));
- ASSERT_EQ(0, System(string("umount ") + mountpoint));
+ ASSERT_TRUE(utils::UnmountFilesystem(mountpoint));
ASSERT_EQ(0, System(string("rm -f ") + cwd + "/postinst_called"));
diff --git a/test_utils.cc b/test_utils.cc
index fddd2d1..5379979 100644
--- a/test_utils.cc
+++ b/test_utils.cc
@@ -217,7 +217,7 @@
kMountPath, kMountPath)));
EXPECT_EQ(0, System(StringPrintf("ln -s bogus %s/boguslink",
kMountPath)));
- EXPECT_EQ(0, System(StringPrintf("umount %s", kMountPath)));
+ EXPECT_TRUE(utils::UnmountFilesystem(kMountPath));
if (out_paths) {
out_paths->clear();
diff --git a/utils.cc b/utils.cc
index e0d9244..1e87262 100644
--- a/utils.cc
+++ b/utils.cc
@@ -41,6 +41,16 @@
namespace chromeos_update_engine {
+namespace {
+
+// The following constants control how UnmountFilesystem should retry if
+// umount() fails with an errno EBUSY, i.e. retry 5 times over the course of
+// one second.
+const int kUnmountMaxNumOfRetries = 5;
+const int kUnmountRetryIntervalInMicroseconds = 200 * 1000; // 200 ms
+
+} // namespace
+
namespace utils {
static const char kDevImageMarker[] = "/root/.dev_mode";
@@ -497,7 +507,14 @@
}
bool UnmountFilesystem(const string& mountpoint) {
- TEST_AND_RETURN_FALSE_ERRNO(umount(mountpoint.c_str()) == 0);
+ for (int num_retries = 0; ; ++num_retries) {
+ if (umount(mountpoint.c_str()) == 0)
+ break;
+
+ TEST_AND_RETURN_FALSE_ERRNO(errno == EBUSY &&
+ num_retries < kUnmountMaxNumOfRetries);
+ usleep(kUnmountRetryIntervalInMicroseconds);
+ }
return true;
}
