libdm: add an overload of DeleteDevice accepting a timeout_ms
In some scenarios (e.g. apexd or userspace reboot), dm-devices are
getting deleted and re-created. Since this operation can be racy (newly
created device can get the same path as the previously deleted one,
resulting in the unexpected ENOENT errors on a system call to the path),
it will be nice to have an API that blocks until ueventd processes
corresponding udev events.
Test: libdm_test
Bug: 143970043
Bug: 122059364
Change-Id: I31a19afd9e245bf5e3554011bdde1c3cc4878f1c
diff --git a/fs_mgr/libdm/dm.cpp b/fs_mgr/libdm/dm.cpp
index e7a3ff2..824ee43 100644
--- a/fs_mgr/libdm/dm.cpp
+++ b/fs_mgr/libdm/dm.cpp
@@ -20,6 +20,7 @@
#include <sys/sysmacros.h>
#include <sys/types.h>
+#include <chrono>
#include <functional>
#include <thread>
@@ -79,14 +80,24 @@
return true;
}
-bool DeviceMapper::DeleteDeviceIfExists(const std::string& name) {
+bool DeviceMapper::DeleteDeviceIfExists(const std::string& name,
+ const std::chrono::milliseconds& timeout_ms) {
if (GetState(name) == DmDeviceState::INVALID) {
return true;
}
- return DeleteDevice(name);
+ return DeleteDevice(name, timeout_ms);
}
-bool DeviceMapper::DeleteDevice(const std::string& name) {
+bool DeviceMapper::DeleteDeviceIfExists(const std::string& name) {
+ return DeleteDeviceIfExists(name, 0ms);
+}
+
+bool DeviceMapper::DeleteDevice(const std::string& name,
+ const std::chrono::milliseconds& timeout_ms) {
+ std::string unique_path;
+ if (!GetDeviceUniquePath(name, &unique_path)) {
+ LOG(ERROR) << "Failed to get unique path for device " << name;
+ }
struct dm_ioctl io;
InitIo(&io, name);
@@ -100,9 +111,23 @@
CHECK(io.flags & DM_UEVENT_GENERATED_FLAG)
<< "Didn't generate uevent for [" << name << "] removal";
+ if (timeout_ms <= std::chrono::milliseconds::zero()) {
+ return true;
+ }
+ if (unique_path.empty()) {
+ return false;
+ }
+ if (!WaitForFileDeleted(unique_path, timeout_ms)) {
+ LOG(ERROR) << "Timeout out waiting for " << unique_path << " to be deleted";
+ return false;
+ }
return true;
}
+bool DeviceMapper::DeleteDevice(const std::string& name) {
+ return DeleteDevice(name, 0ms);
+}
+
static std::string GenerateUuid() {
uuid_t uuid_bytes;
uuid_generate(uuid_bytes);
diff --git a/fs_mgr/libdm/dm_test.cpp b/fs_mgr/libdm/dm_test.cpp
index ed2fa83..1695953 100644
--- a/fs_mgr/libdm/dm_test.cpp
+++ b/fs_mgr/libdm/dm_test.cpp
@@ -520,3 +520,27 @@
ASSERT_TRUE(target.Valid());
ASSERT_EQ(target.GetParameterString(), "AES-256-XTS abcdef0123456789 /dev/loop0 0");
}
+
+TEST(libdm, DeleteDeviceWithTimeout) {
+ unique_fd tmp(CreateTempFile("file_1", 4096));
+ ASSERT_GE(tmp, 0);
+ LoopDevice loop(tmp, 10s);
+ ASSERT_TRUE(loop.valid());
+
+ DmTable table;
+ ASSERT_TRUE(table.Emplace<DmTargetLinear>(0, 1, loop.device(), 0));
+ ASSERT_TRUE(table.valid());
+ TempDevice dev("libdm-test-dm-linear", table);
+ ASSERT_TRUE(dev.valid());
+
+ DeviceMapper& dm = DeviceMapper::Instance();
+
+ std::string path;
+ ASSERT_TRUE(dm.GetDmDevicePathByName("libdm-test-dm-linear", &path));
+ ASSERT_EQ(0, access(path.c_str(), F_OK));
+
+ ASSERT_TRUE(dm.DeleteDevice("libdm-test-dm-linear", 5s));
+ ASSERT_EQ(DmDeviceState::INVALID, dm.GetState("libdm-test-dm-linear"));
+ ASSERT_NE(0, access(path.c_str(), F_OK));
+ ASSERT_EQ(ENOENT, errno);
+}
diff --git a/fs_mgr/libdm/include/libdm/dm.h b/fs_mgr/libdm/include/libdm/dm.h
index e25ce7f..830c5e8 100644
--- a/fs_mgr/libdm/include/libdm/dm.h
+++ b/fs_mgr/libdm/include/libdm/dm.h
@@ -90,6 +90,10 @@
// Returns 'true' on success, false otherwise.
bool DeleteDevice(const std::string& name);
bool DeleteDeviceIfExists(const std::string& name);
+ // Removes a device mapper device with the given name and waits for |timeout_ms| milliseconds
+ // for the corresponding block device to be deleted.
+ bool DeleteDevice(const std::string& name, const std::chrono::milliseconds& timeout_ms);
+ bool DeleteDeviceIfExists(const std::string& name, const std::chrono::milliseconds& timeout_ms);
// Fetches and returns the complete state of the underlying device mapper
// device with given name.
diff --git a/fs_mgr/libdm/utility.cpp b/fs_mgr/libdm/utility.cpp
index eccf2fb..f252565 100644
--- a/fs_mgr/libdm/utility.cpp
+++ b/fs_mgr/libdm/utility.cpp
@@ -52,5 +52,15 @@
return WaitForCondition(condition, timeout_ms);
}
+bool WaitForFileDeleted(const std::string& path, const std::chrono::milliseconds& timeout_ms) {
+ auto condition = [&]() -> WaitResult {
+ if (access(path.c_str(), F_OK) == 0 || errno != ENOENT) {
+ return WaitResult::Wait;
+ }
+ return WaitResult::Done;
+ };
+ return WaitForCondition(condition, timeout_ms);
+}
+
} // namespace dm
} // namespace android
diff --git a/fs_mgr/libdm/utility.h b/fs_mgr/libdm/utility.h
index f1dce9e..58fa96b 100644
--- a/fs_mgr/libdm/utility.h
+++ b/fs_mgr/libdm/utility.h
@@ -23,6 +23,7 @@
enum class WaitResult { Wait, Done, Fail };
bool WaitForFile(const std::string& path, const std::chrono::milliseconds& timeout_ms);
+bool WaitForFileDeleted(const std::string& path, const std::chrono::milliseconds& timeout_ms);
bool WaitForCondition(const std::function<WaitResult()>& condition,
const std::chrono::milliseconds& timeout_ms);