Check if mount point already exists
Typically, we create the mount point directory when we mount the
filesystem, and then we remove the directory when we unmount. Currently,
we aren't accounting for the case where the directory already exists,
e.g. if the BMC reboots while the filesystem is mounted.
This commit adds a check to see if the directory is already present. If
so, it won't try to create the directory again.
Tested:
1. Formatted an eMMC using the FormatLuks method, which also creates the
mount point and mounts the filesystem.
2. Rebooted the BMC
3. Ran the Unlock method to unlock the LUKS device and mount the
filesystem.
Signed-off-by: John Wedig <johnwedig@google.com>
Change-Id: I3e279c653b21f570b97e4d530a19e5ae30bf8719
diff --git a/include/filesystemInterface.hpp b/include/filesystemInterface.hpp
index d34ff06..0f276a7 100644
--- a/include/filesystemInterface.hpp
+++ b/include/filesystemInterface.hpp
@@ -70,6 +70,14 @@
* @returns true on success, false otherwise.
*/
virtual bool removeDirectory(const std::filesystem::path& p) = 0;
+
+ /** @brief Wrapper around std::filesystem::is_directory
+ *
+ * @param[in] p - path to directory that we want to query.
+ *
+ * @returns true if the path exists and represents a directory.
+ */
+ virtual bool directoryExists(const std::filesystem::path& p) = 0;
};
/** @class Filesystem
@@ -107,5 +115,10 @@
{
return std::filesystem::remove(p);
}
+
+ bool directoryExists(const std::filesystem::path& p) override
+ {
+ return std::filesystem::is_directory(std::filesystem::status(p));
+ }
};
} // namespace estoraged
diff --git a/src/estoraged.cpp b/src/estoraged.cpp
index 777232a..55a08a4 100644
--- a/src/estoraged.cpp
+++ b/src/estoraged.cpp
@@ -250,14 +250,22 @@
void eStoraged::mountFilesystem()
{
- /* Create directory for the filesystem. */
- bool success = fsIface->createDirectory(std::filesystem::path(mountPoint));
- if (!success)
+ /*
+ * Create directory for the filesystem, if it's not already present. It
+ * might already exist if, for example, the BMC reboots after creating the
+ * directory.
+ */
+ if (!fsIface->directoryExists(std::filesystem::path(mountPoint)))
{
- lg2::error("Failed to create mount point: {DIR}", "DIR", mountPoint,
- "REDFISH_MESSAGE_ID",
- std::string("OpenBMC.0.1.MountFilesystemFail"));
- throw InternalFailure();
+ bool success =
+ fsIface->createDirectory(std::filesystem::path(mountPoint));
+ if (!success)
+ {
+ lg2::error("Failed to create mount point: {DIR}", "DIR", mountPoint,
+ "REDFISH_MESSAGE_ID",
+ std::string("OpenBMC.0.1.MountFilesystemFail"));
+ throw InternalFailure();
+ }
}
/* Run the command to mount the filesystem. */
diff --git a/src/test/estoraged_test.cpp b/src/test/estoraged_test.cpp
index 6b93a88..3b23215 100644
--- a/src/test/estoraged_test.cpp
+++ b/src/test/estoraged_test.cpp
@@ -40,6 +40,9 @@
MOCK_METHOD(bool, removeDirectory, (const std::filesystem::path& p),
(override));
+
+ MOCK_METHOD(bool, directoryExists, (const std::filesystem::path& p),
+ (override));
};
class MockCryptsetupInterface : public estoraged::CryptsetupInterface
@@ -173,6 +176,9 @@
EXPECT_CALL(*mockFsIface, runMkfs(testLuksDevName)).WillOnce(Return(0));
+ EXPECT_CALL(*mockFsIface, directoryExists(path(esObject->getMountPoint())))
+ .WillOnce(Return(false));
+
EXPECT_CALL(*mockFsIface, createDirectory(path(esObject->getMountPoint())))
.WillOnce(Return(true));
@@ -197,6 +203,56 @@
EXPECT_TRUE(esObject->isLocked());
}
+/*
+ * Test case where the mount point directory already exists, so it shouldn't
+ * try to create it.
+ */
+TEST_F(eStoragedTest, MountPointExistsPass)
+{
+ EXPECT_CALL(sdbusMock,
+ sd_bus_emit_properties_changed_strv(
+ IsNull(), StrEq(TEST_PATH), StrEq(ESTORAGED_INTERFACE), _))
+ .WillRepeatedly(Return(0));
+
+ EXPECT_CALL(*mockCryptIface, cryptFormat(_, _, _, _, _, _, _, _)).Times(1);
+
+ EXPECT_CALL(*mockCryptIface, cryptKeyslotAddByVolumeKey(_, _, _, _, _, _))
+ .Times(1);
+
+ EXPECT_CALL(*mockCryptIface, cryptLoad(_, _, _)).Times(1);
+
+ EXPECT_CALL(*mockCryptIface, cryptActivateByPassphrase(_, _, _, _, _, _))
+ .Times(1);
+
+ EXPECT_CALL(*mockFsIface, runMkfs(testLuksDevName)).WillOnce(Return(0));
+
+ EXPECT_CALL(*mockFsIface, directoryExists(path(esObject->getMountPoint())))
+ .WillOnce(Return(true));
+
+ EXPECT_CALL(*mockFsIface, createDirectory(path(esObject->getMountPoint())))
+ .Times(0);
+
+ EXPECT_CALL(*mockFsIface,
+ doMount(ContainsRegex("/dev/mapper/"),
+ StrEq(esObject->getMountPoint()), _, _, _))
+ .WillOnce(Return(0));
+
+ EXPECT_CALL(*mockFsIface, doUnmount(StrEq(esObject->getMountPoint())))
+ .WillOnce(Return(0));
+
+ EXPECT_CALL(*mockFsIface, removeDirectory(path(esObject->getMountPoint())))
+ .WillOnce(Return(true));
+
+ EXPECT_CALL(*mockCryptIface, cryptDeactivate(_, _)).Times(1);
+
+ /* Format the encrypted device. */
+ esObject->formatLuks(password, Volume::FilesystemType::ext4);
+ EXPECT_FALSE(esObject->isLocked());
+
+ esObject->lock();
+ EXPECT_TRUE(esObject->isLocked());
+}
+
/* Test case where the device/file doesn't exist. */
TEST_F(eStoragedTest, FormatNoDeviceFail)
{
@@ -330,6 +386,9 @@
EXPECT_CALL(*mockFsIface, runMkfs(testLuksDevName)).WillOnce(Return(0));
+ EXPECT_CALL(*mockFsIface, directoryExists(path(esObject->getMountPoint())))
+ .WillOnce(Return(false));
+
EXPECT_CALL(*mockFsIface, createDirectory(path(esObject->getMountPoint())))
.WillOnce(Return(false));
@@ -358,6 +417,9 @@
EXPECT_CALL(*mockFsIface, runMkfs(testLuksDevName)).WillOnce(Return(0));
+ EXPECT_CALL(*mockFsIface, directoryExists(path(esObject->getMountPoint())))
+ .WillOnce(Return(false));
+
EXPECT_CALL(*mockFsIface, createDirectory(path(esObject->getMountPoint())))
.WillOnce(Return(true));
@@ -394,6 +456,9 @@
EXPECT_CALL(*mockFsIface, runMkfs(testLuksDevName)).WillOnce(Return(0));
+ EXPECT_CALL(*mockFsIface, directoryExists(path(esObject->getMountPoint())))
+ .WillOnce(Return(false));
+
EXPECT_CALL(*mockFsIface, createDirectory(path(esObject->getMountPoint())))
.WillOnce(Return(true));
@@ -432,6 +497,9 @@
EXPECT_CALL(*mockFsIface, runMkfs(testLuksDevName)).WillOnce(Return(0));
+ EXPECT_CALL(*mockFsIface, directoryExists(path(esObject->getMountPoint())))
+ .WillOnce(Return(false));
+
EXPECT_CALL(*mockFsIface, createDirectory(path(esObject->getMountPoint())))
.WillOnce(Return(true));
@@ -474,6 +542,9 @@
EXPECT_CALL(*mockFsIface, runMkfs(testLuksDevName)).WillOnce(Return(0));
+ EXPECT_CALL(*mockFsIface, directoryExists(path(esObject->getMountPoint())))
+ .WillOnce(Return(false));
+
EXPECT_CALL(*mockFsIface, createDirectory(path(esObject->getMountPoint())))
.WillOnce(Return(true));