diff options
author | David Anderson <dvander@google.com> | 2019-03-13 13:34:56 -0700 |
---|---|---|
committer | David Anderson <dvander@google.com> | 2019-03-13 13:58:12 -0700 |
commit | 5df00923536ccc75731e380828323681bb93c660 (patch) | |
tree | 7731bce549636c07f9cfc642803402fa3d7453a7 /fs_mgr | |
parent | 9a00f596d3376ed750eb58cf07c04d575b540c2b (diff) | |
download | system_core-5df00923536ccc75731e380828323681bb93c660.tar.gz system_core-5df00923536ccc75731e380828323681bb93c660.tar.bz2 system_core-5df00923536ccc75731e380828323681bb93c660.zip |
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
Diffstat (limited to 'fs_mgr')
-rw-r--r-- | fs_mgr/libfiemap_writer/Android.bp | 3 | ||||
-rw-r--r-- | fs_mgr/libfiemap_writer/fiemap_writer.cpp | 68 | ||||
-rw-r--r-- | fs_mgr/libfiemap_writer/fiemap_writer_test.cpp | 29 | ||||
-rw-r--r-- | fs_mgr/libfiemap_writer/include/libfiemap_writer/split_fiemap_writer.h | 1 | ||||
-rw-r--r-- | fs_mgr/libfiemap_writer/split_fiemap_writer.cpp | 4 |
5 files changed, 69 insertions, 36 deletions
diff --git a/fs_mgr/libfiemap_writer/Android.bp b/fs_mgr/libfiemap_writer/Android.bp index 746381050..32fc3d21c 100644 --- a/fs_mgr/libfiemap_writer/Android.bp +++ b/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", diff --git a/fs_mgr/libfiemap_writer/fiemap_writer.cpp b/fs_mgr/libfiemap_writer/fiemap_writer.cpp index 5b7728021..45b997ffe 100644 --- a/fs_mgr/libfiemap_writer/fiemap_writer.cpp +++ b/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)); - 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; } diff --git a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp index 66eb9ae35..4a1a5ab6c 100644 --- a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp +++ b/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"; diff --git a/fs_mgr/libfiemap_writer/include/libfiemap_writer/split_fiemap_writer.h b/fs_mgr/libfiemap_writer/include/libfiemap_writer/split_fiemap_writer.h index 07f3c1023..7b977e1bd 100644 --- a/fs_mgr/libfiemap_writer/include/libfiemap_writer/split_fiemap_writer.h +++ b/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; diff --git a/fs_mgr/libfiemap_writer/split_fiemap_writer.cpp b/fs_mgr/libfiemap_writer/split_fiemap_writer.cpp index dbb67a88d..a0ccc10f7 100644 --- a/fs_mgr/libfiemap_writer/split_fiemap_writer.cpp +++ b/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 |