Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5df00923 authored by David Anderson's avatar David Anderson
Browse files

libfiemap_writer: Fix the progress bar on VFAT partitions.

We allocate VFAT file space by seeking and writing a single byte. It
turns out this is not fast enough to avoid invoking the progress
callback, so instead, we should seek in increments.

Bug: 126230649
Test: fiemap_writer_test gtest
Change-Id: I2b363af4ebde873411e8a3acd22ca68d175afe3f
parent 9a00f596
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
@@ -41,6 +41,9 @@ cc_library_static {

cc_test {
    name: "fiemap_writer_test",
    cflags: [
        "-D_FILE_OFFSET_BITS=64",
    ],
    static_libs: [
        "libbase",
        "libdm",
+46 −22
Original line number Diff line number Diff line
@@ -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));
            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;
        }
            return FallocateFallback(file_fd, blocksz, file_size, file_path, on_progress);
        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;
            }
+15 −14
Original line number Diff line number Diff line
@@ -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";
+1 −0
Original line number Diff line number Diff line
@@ -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;
+4 −0
Original line number Diff line number Diff line
@@ -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