Remove the block size alignment restriction.

Rather than require block-size alignment, instead bump the requested
file size to the necessary alignment. This ensures that the final block
is usable without placing onerous restrictions on the caller to figure
out the file system's block size.

This will require callers (namely, gsid) to track the actual desired
image size separately from the flie size.

This patch also updates tests to use the actual filesize of the
filesystem, rather than hardcoded 4096.

Bug: 126230649
Test: fiemap_writer_test gtest
Change-Id: I000cca274718c3ceac526d7c3392fe3a23bb42bc

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

fs_mgr/libfiemap_writer/fiemap_writer_test.cpp

@@ -22,6 +22,7 @@
 #include <sys/mount.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <sys/vfs.h>
 #include <unistd.h>
 
 #include <string>
@@ -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<uint64_t> 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;

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<bool(uint64_t, uint64_t)> progress = {});