diff --git a/fs_mgr/libfiemap_writer/fiemap_writer.cpp b/fs_mgr/libfiemap_writer/fiemap_writer.cpp index 6b742e66c..8b880e6ee 100644 --- a/fs_mgr/libfiemap_writer/fiemap_writer.cpp +++ b/fs_mgr/libfiemap_writer/fiemap_writer.cpp @@ -200,9 +200,8 @@ static bool PerformFileChecks(const std::string& file_path, uint64_t file_size, return false; } - if (file_size % sfs.f_bsize) { - LOG(ERROR) << "File size " << file_size << " is not aligned to optimal block size " - << sfs.f_bsize << " for file " << file_path; + if (!sfs.f_bsize) { + LOG(ERROR) << "Unsupported block size: " << sfs.f_bsize; return false; } @@ -500,6 +499,11 @@ FiemapUniquePtr FiemapWriter::Open(const std::string& file_path, uint64_t file_s return nullptr; } + // Align up to the nearest block size. + if (file_size % blocksz) { + file_size += blocksz - (file_size % blocksz); + } + if (create) { if (!AllocateFile(file_fd, abs_path, blocksz, file_size, std::move(progress))) { LOG(ERROR) << "Failed to allocate file: " << abs_path << " of size: " << file_size diff --git a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp index 3e8381b50..d1c0aadd7 100644 --- a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp +++ b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -43,6 +44,7 @@ using LoopDevice = android::dm::LoopDevice; std::string gTestDir; uint64_t testfile_size = 536870912; // default of 512MiB +size_t gBlockSize = 0; class FiemapWriterTest : public ::testing::Test { protected: @@ -71,16 +73,15 @@ TEST_F(FiemapWriterTest, CreateUnalignedFile) { // Try creating a file of size 4097 bytes which is guaranteed // to be unaligned to all known block sizes. The creation must // fail. - FiemapUniquePtr fptr = FiemapWriter::Open(testfile, 4097); - EXPECT_EQ(fptr, nullptr); - EXPECT_EQ(access(testfile.c_str(), F_OK), -1); - EXPECT_EQ(errno, ENOENT); + FiemapUniquePtr fptr = FiemapWriter::Open(testfile, gBlockSize + 1); + ASSERT_NE(fptr, nullptr); + ASSERT_EQ(fptr->size(), gBlockSize * 2); } TEST_F(FiemapWriterTest, CheckFilePath) { - FiemapUniquePtr fptr = FiemapWriter::Open(testfile, 4096); + FiemapUniquePtr fptr = FiemapWriter::Open(testfile, gBlockSize); ASSERT_NE(fptr, nullptr); - EXPECT_EQ(fptr->size(), 4096); + EXPECT_EQ(fptr->size(), gBlockSize); EXPECT_EQ(fptr->file_path(), testfile); EXPECT_EQ(access(testfile.c_str(), F_OK), 0); } @@ -88,18 +89,18 @@ TEST_F(FiemapWriterTest, CheckFilePath) { TEST_F(FiemapWriterTest, CheckProgress) { std::vector expected{ 0, - 4096, + gBlockSize, }; size_t invocations = 0; auto callback = [&](uint64_t done, uint64_t total) -> bool { EXPECT_LT(invocations, expected.size()); EXPECT_EQ(done, expected[invocations]); - EXPECT_EQ(total, 4096); + EXPECT_EQ(total, gBlockSize); invocations++; return true; }; - auto ptr = FiemapWriter::Open(testfile, 4096, true, std::move(callback)); + auto ptr = FiemapWriter::Open(testfile, gBlockSize, true, std::move(callback)); EXPECT_NE(ptr, nullptr); EXPECT_EQ(invocations, 2); } @@ -111,8 +112,8 @@ TEST_F(FiemapWriterTest, CheckPinning) { } TEST_F(FiemapWriterTest, CheckBlockDevicePath) { - FiemapUniquePtr fptr = FiemapWriter::Open(testfile, 4096); - EXPECT_EQ(fptr->size(), 4096); + FiemapUniquePtr fptr = FiemapWriter::Open(testfile, gBlockSize); + EXPECT_EQ(fptr->size(), gBlockSize); EXPECT_EQ(fptr->bdev_path().find("/dev/block/"), size_t(0)); EXPECT_EQ(fptr->bdev_path().find("/dev/block/dm-"), string::npos); } @@ -139,44 +140,23 @@ TEST_F(FiemapWriterTest, CheckFileExtents) { EXPECT_GT(fptr->extents().size(), 0); } -class TestExistingFile : public ::testing::Test { - protected: - void SetUp() override { - unaligned_file_ = gTestDir + "/unaligned_file"; - file_4k_ = gTestDir + "/file_4k"; - file_32k_ = gTestDir + "/file_32k"; - - CleanupFiles(); - fptr_unaligned = FiemapWriter::Open(unaligned_file_, 4097); - fptr_4k = FiemapWriter::Open(file_4k_, 4096); - fptr_32k = FiemapWriter::Open(file_32k_, 32768); +TEST_F(FiemapWriterTest, ExistingFile) { + // Create the file. + { ASSERT_NE(FiemapWriter::Open(testfile, gBlockSize), nullptr); } + // Test that we can still open it. + { + auto ptr = FiemapWriter::Open(testfile, 0, false); + ASSERT_NE(ptr, nullptr); + EXPECT_GT(ptr->extents().size(), 0); } +} - void TearDown() { CleanupFiles(); } - - void CleanupFiles() { - unlink(unaligned_file_.c_str()); - unlink(file_4k_.c_str()); - unlink(file_32k_.c_str()); - } - - std::string unaligned_file_; - std::string file_4k_; - std::string file_32k_; - FiemapUniquePtr fptr_unaligned; - FiemapUniquePtr fptr_4k; - FiemapUniquePtr fptr_32k; -}; - -TEST_F(TestExistingFile, ErrorChecks) { - EXPECT_EQ(fptr_unaligned, nullptr); - EXPECT_NE(fptr_4k, nullptr); - EXPECT_NE(fptr_32k, nullptr); - - EXPECT_EQ(fptr_4k->size(), 4096); - EXPECT_EQ(fptr_32k->size(), 32768); - EXPECT_GT(fptr_4k->extents().size(), 0); - EXPECT_GT(fptr_32k->extents().size(), 0); +TEST_F(FiemapWriterTest, FileDeletedOnError) { + auto callback = [](uint64_t, uint64_t) -> bool { return false; }; + auto ptr = FiemapWriter::Open(testfile, gBlockSize, true, std::move(callback)); + EXPECT_EQ(ptr, nullptr); + EXPECT_EQ(access(testfile.c_str(), F_OK), -1); + EXPECT_EQ(errno, ENOENT); } class VerifyBlockWritesExt4 : public ::testing::Test { @@ -263,6 +243,21 @@ class VerifyBlockWritesF2fs : public ::testing::Test { std::string fs_path; }; +bool DetermineBlockSize() { + struct statfs s; + if (statfs(gTestDir.c_str(), &s)) { + std::cerr << "Could not call statfs: " << strerror(errno) << "\n"; + return false; + } + if (!s.f_bsize) { + std::cerr << "Invalid block size: " << s.f_bsize << "\n"; + return false; + } + + gBlockSize = s.f_bsize; + return true; +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); if (argc <= 1) { @@ -275,7 +270,7 @@ int main(int argc, char** argv) { std::string tempdir = argv[1] + "/XXXXXX"s; if (!mkdtemp(tempdir.data())) { - cerr << "unable to create tempdir on " << argv[1]; + cerr << "unable to create tempdir on " << argv[1] << "\n"; exit(EXIT_FAILURE); } gTestDir = tempdir; @@ -287,6 +282,10 @@ int main(int argc, char** argv) { } } + if (!DetermineBlockSize()) { + exit(EXIT_FAILURE); + } + auto result = RUN_ALL_TESTS(); std::string cmd = "rm -rf " + gTestDir; diff --git a/fs_mgr/libfiemap_writer/include/libfiemap_writer/fiemap_writer.h b/fs_mgr/libfiemap_writer/include/libfiemap_writer/fiemap_writer.h index b5472c9f7..3ba68e9f9 100644 --- a/fs_mgr/libfiemap_writer/include/libfiemap_writer/fiemap_writer.h +++ b/fs_mgr/libfiemap_writer/include/libfiemap_writer/fiemap_writer.h @@ -41,6 +41,9 @@ class FiemapWriter final { // invoked, if create is true, while the file is being initialized. It receives the bytes // written and the number of total bytes. If the callback returns false, the operation will // fail. + // + // Note: when create is true, the file size will be aligned up to the nearest file system + // block. static FiemapUniquePtr Open(const std::string& file_path, uint64_t file_size, bool create = true, std::function progress = {});