Merge "libfiemap_writer: Fix the progress bar on VFAT partitions."

fs_mgr/libfiemap_writer/Android.bp

@@ -41,6 +41,9 @@ cc_library_static {
 
 cc_test {
     name: "fiemap_writer_test",
+    cflags: [
+        "-D_FILE_OFFSET_BITS=64",
+    ],
     static_libs: [
         "libbase",
         "libdm",

fs_mgr/libfiemap_writer/fiemap_writer.cpp

@@ -51,6 +51,9 @@ static constexpr const uint32_t kUnsupportedExtentFlags =
         FIEMAP_EXTENT_NOT_ALIGNED | FIEMAP_EXTENT_DATA_INLINE | FIEMAP_EXTENT_DATA_TAIL |
         FIEMAP_EXTENT_UNWRITTEN | FIEMAP_EXTENT_SHARED | FIEMAP_EXTENT_MERGED;
 
+// Large file support must be enabled.
+static_assert(sizeof(off_t) == sizeof(uint64_t));
+
 static inline void cleanup(const std::string& file_path, bool created) {
     if (created) {
         unlink(file_path.c_str());
@@ -229,6 +232,42 @@ static bool PerformFileChecks(const std::string& file_path, uint64_t file_size,
     return true;
 }
 
+static bool FallocateFallback(int file_fd, uint64_t block_size, uint64_t file_size,
+                              const std::string& file_path,
+                              const std::function<bool(uint64_t, uint64_t)>& on_progress) {
+    // Even though this is much faster than writing zeroes, it is still slow
+    // enough that we need to fire the progress callback periodically. To
+    // easily achieve this, we seek in chunks. We use 1000 chunks since
+    // normally we only fire the callback on 1/1000th increments.
+    uint64_t bytes_per_chunk = std::max(file_size / 1000, block_size);
+
+    // Seek just to the end of each chunk and write a single byte, causing
+    // the filesystem to allocate blocks.
+    off_t cursor = 0;
+    off_t end = static_cast<off_t>(file_size);
+    while (cursor < end) {
+        cursor = std::min(static_cast<off_t>(cursor + bytes_per_chunk), end);
+        auto rv = TEMP_FAILURE_RETRY(lseek(file_fd, cursor - 1, SEEK_SET));
+        if (rv < 0) {
+            PLOG(ERROR) << "Failed to lseek " << file_path;
+            return false;
+        }
+        if (rv != cursor - 1) {
+            LOG(ERROR) << "Seek returned wrong offset " << rv << " for file " << file_path;
+            return false;
+        }
+        char buffer[] = {0};
+        if (!android::base::WriteFully(file_fd, buffer, 1)) {
+            PLOG(ERROR) << "Write failed: " << file_path;
+            return false;
+        }
+        if (on_progress && !on_progress(cursor, file_size)) {
+            return false;
+        }
+    }
+    return true;
+}
+
 static bool AllocateFile(int file_fd, const std::string& file_path, uint64_t blocksz,
                          uint64_t file_size, unsigned int fs_type,
                          std::function<bool(uint64_t, uint64_t)> on_progress) {
@@ -245,28 +284,10 @@ static bool AllocateFile(int file_fd, const std::string& file_path, uint64_t blo
                 return false;
             }
             break;
-        case MSDOS_SUPER_MAGIC: {
+        case MSDOS_SUPER_MAGIC:
             // fallocate() is not supported, and not needed, since VFAT does not support holes.
             // Instead we can perform a much faster allocation.
-            auto offset = TEMP_FAILURE_RETRY(lseek(file_fd, file_size - 1, SEEK_SET));
+            return FallocateFallback(file_fd, blocksz, file_size, file_path, on_progress);
-            if (offset < 0) {
-                PLOG(ERROR) << "Failed to lseek " << file_path;
-                return false;
-            }
-            if (offset != file_size - 1) {
-                LOG(ERROR) << "Seek returned wrong offset " << offset << " for file " << file_path;
-                return false;
-            }
-            char buffer[] = {0};
-            if (!android::base::WriteFully(file_fd, buffer, 1)) {
-                PLOG(ERROR) << "Write failed: " << file_path;
-                return false;
-            }
-            if (on_progress && !on_progress(file_size, file_size)) {
-                return false;
-            }
-            return true;
-        }
         default:
             LOG(ERROR) << "Missing fallocate() support for file system " << fs_type;
             return false;
@@ -288,16 +309,19 @@ static bool AllocateFile(int file_fd, const std::string& file_path, uint64_t blo
     }
 
     int permille = -1;
-    for (; offset < file_size; offset += blocksz) {
+    while (offset < file_size) {
         if (!::android::base::WriteFully(file_fd, buffer.get(), blocksz)) {
             PLOG(ERROR) << "Failed to write" << blocksz << " bytes at offset" << offset
                         << " in file " << file_path;
             return false;
         }
+
+        offset += blocksz;
+
         // Don't invoke the callback every iteration - wait until a significant
         // chunk (here, 1/1000th) of the data has been processed.
         int new_permille = (static_cast<uint64_t>(offset) * 1000) / file_size;
-        if (new_permille != permille) {
+        if (new_permille != permille && static_cast<uint64_t>(offset) != file_size) {
             if (on_progress && !on_progress(offset, file_size)) {
                 return false;
             }

fs_mgr/libfiemap_writer/fiemap_writer_test.cpp

@@ -108,28 +108,29 @@ TEST_F(FiemapWriterTest, CheckFilePath) {
     EXPECT_EQ(access(testfile.c_str(), F_OK), 0);
 }
 
+TEST_F(FiemapWriterTest, CheckFileSize) {
+    // Create a large-ish file and test that the expected size matches.
+    FiemapUniquePtr fptr = FiemapWriter::Open(testfile, 1024 * 1024 * 16);
+    ASSERT_NE(fptr, nullptr);
+
+    struct stat s;
+    ASSERT_EQ(stat(testfile.c_str(), &s), 0);
+    EXPECT_EQ(static_cast<uint64_t>(s.st_size), fptr->size());
+}
+
 TEST_F(FiemapWriterTest, CheckProgress) {
     std::vector<uint64_t> expected;
     size_t invocations = 0;
     auto callback = [&](uint64_t done, uint64_t total) -> bool {
-        EXPECT_LT(invocations, expected.size());
+        if (invocations >= expected.size()) {
+            return false;
+        }
         EXPECT_EQ(done, expected[invocations]);
         EXPECT_EQ(total, gBlockSize);
         invocations++;
         return true;
     };
 
-    uint32_t fs_type;
-    {
-        auto ptr = FiemapWriter::Open(testfile, gBlockSize, true);
-        ASSERT_NE(ptr, nullptr);
-        fs_type = ptr->fs_type();
-    }
-    ASSERT_EQ(unlink(testfile.c_str()), 0);
-
-    if (fs_type != MSDOS_SUPER_MAGIC) {
-        expected.push_back(0);
-    }
     expected.push_back(gBlockSize);
 
     auto ptr = FiemapWriter::Open(testfile, gBlockSize, true, std::move(callback));
@@ -163,7 +164,7 @@ TEST_F(FiemapWriterTest, CheckFileSizeActual) {
 
     struct stat sb;
     ASSERT_EQ(stat(testfile.c_str(), &sb), 0);
-    EXPECT_EQ(sb.st_size, testfile_size);
+    EXPECT_GE(sb.st_size, testfile_size);
 }
 
 TEST_F(FiemapWriterTest, CheckFileExtents) {
@@ -227,7 +228,7 @@ TEST_F(SplitFiemapTest, Open) {
 }
 
 TEST_F(SplitFiemapTest, DeleteOnFail) {
-    auto ptr = SplitFiemap::Create(testfile, 1024 * 1024 * 10, 1);
+    auto ptr = SplitFiemap::Create(testfile, 1024 * 1024 * 100, 1);
     ASSERT_EQ(ptr, nullptr);
 
     std::string first_file = testfile + ".0001";

fs_mgr/libfiemap_writer/include/libfiemap_writer/split_fiemap_writer.h

@@ -69,6 +69,7 @@ class SplitFiemap final {
     const std::vector<struct fiemap_extent>& extents();
     uint32_t block_size() const;
     uint64_t size() const { return total_size_; }
+    const std::string& bdev_path() const;
 
     // Non-copyable & Non-movable
     SplitFiemap(const SplitFiemap&) = delete;

fs_mgr/libfiemap_writer/split_fiemap_writer.cpp

@@ -289,5 +289,9 @@ uint32_t SplitFiemap::block_size() const {
     return files_[0]->block_size();
 }
 
+const std::string& SplitFiemap::bdev_path() const {
+    return files_[0]->bdev_path();
+}
+
 }  // namespace fiemap_writer
 }  // namespace android