diff options
author | David Anderson <dvander@google.com> | 2019-01-30 17:34:13 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2019-01-30 17:34:13 +0000 |
commit | 98910920ba5511a8185bc8c2776b4a191f0df87f (patch) | |
tree | edf5500cc61b2d13bd02d052159905fa527fbc1d | |
parent | 9747a653804105187b6c79172ec0bd4907b1776e (diff) | |
parent | 1326648f053c4cf67626c0ce223ca7ac98bfb07e (diff) | |
download | system_core-98910920ba5511a8185bc8c2776b4a191f0df87f.tar.gz system_core-98910920ba5511a8185bc8c2776b4a191f0df87f.tar.bz2 system_core-98910920ba5511a8185bc8c2776b4a191f0df87f.zip |
Merge "libfiemap_writer: Remove Flush and Write methods."
-rw-r--r-- | fs_mgr/libfiemap_writer/fiemap_writer.cpp | 122 | ||||
-rw-r--r-- | fs_mgr/libfiemap_writer/fiemap_writer_test.cpp | 150 | ||||
-rw-r--r-- | fs_mgr/libfiemap_writer/include/libfiemap_writer/fiemap_writer.h | 13 |
3 files changed, 20 insertions, 265 deletions
diff --git a/fs_mgr/libfiemap_writer/fiemap_writer.cpp b/fs_mgr/libfiemap_writer/fiemap_writer.cpp index 6ccdb5781..b9b75f87e 100644 --- a/fs_mgr/libfiemap_writer/fiemap_writer.cpp +++ b/fs_mgr/libfiemap_writer/fiemap_writer.cpp @@ -374,14 +374,6 @@ bool FiemapWriter::HasPinnedExtents(const std::string& file_path) { return IsFilePinned(fd, file_path, sfs.f_type); } -static void LogExtent(uint32_t num, const struct fiemap_extent& ext) { - LOG(INFO) << "Extent #" << num; - LOG(INFO) << " fe_logical: " << ext.fe_logical; - LOG(INFO) << " fe_physical: " << ext.fe_physical; - LOG(INFO) << " fe_length: " << ext.fe_length; - LOG(INFO) << " fe_flags: 0x" << std::hex << ext.fe_flags; -} - static bool ReadFiemap(int file_fd, const std::string& file_path, std::vector<struct fiemap_extent>* extents) { uint64_t fiemap_size = @@ -473,7 +465,7 @@ FiemapUniquePtr FiemapWriter::Open(const std::string& file_path, uint64_t file_s } ::android::base::unique_fd bdev_fd( - TEMP_FAILURE_RETRY(open(bdev_path.c_str(), O_RDWR | O_CLOEXEC))); + TEMP_FAILURE_RETRY(open(bdev_path.c_str(), O_RDONLY | O_CLOEXEC))); if (bdev_fd < 0) { PLOG(ERROR) << "Failed to open block device: " << bdev_path; cleanup(file_path, create); @@ -530,7 +522,6 @@ FiemapUniquePtr FiemapWriter::Open(const std::string& file_path, uint64_t file_s fmap->file_path_ = abs_path; fmap->bdev_path_ = bdev_path; fmap->file_fd_ = std::move(file_fd); - fmap->bdev_fd_ = std::move(bdev_fd); fmap->file_size_ = file_size; fmap->bdev_size_ = bdevsz; fmap->fs_type_ = fs_type; @@ -541,120 +532,9 @@ FiemapUniquePtr FiemapWriter::Open(const std::string& file_path, uint64_t file_s return fmap; } -bool FiemapWriter::Flush() const { - if (fsync(bdev_fd_)) { - PLOG(ERROR) << "Failed to flush " << bdev_path_ << " with fsync"; - return false; - } - return true; -} - -// TODO: Test with fs block_size > bdev block_size -bool FiemapWriter::Write(off64_t off, uint8_t* buffer, uint64_t size) { - if (!size || size > file_size_) { - LOG(ERROR) << "Failed write: size " << size << " is invalid for file's size " << file_size_; - return false; - } - - if (off + size > file_size_) { - LOG(ERROR) << "Failed write: Invalid offset " << off << " or size " << size - << " for file size " << file_size_; - return false; - } - - if ((off & (block_size_ - 1)) || (size & (block_size_ - 1))) { - LOG(ERROR) << "Failed write: Unaligned offset " << off << " or size " << size - << " for block size " << block_size_; - return false; - } - - if (!IsFilePinned(file_fd_, file_path_, fs_type_)) { - LOG(ERROR) << "Failed write: file " << file_path_ << " is not pinned"; - return false; - } - - // find extents that must be written to and then write one at a time. - uint32_t num_extent = 1; - uint32_t buffer_offset = 0; - for (auto& extent : extents_) { - uint64_t e_start = extent.fe_logical; - uint64_t e_end = extent.fe_logical + extent.fe_length; - // Do we write in this extent ? - if (off >= e_start && off < e_end) { - uint64_t written = WriteExtent(extent, buffer + buffer_offset, off, size); - if (written == 0) { - return false; - } - - buffer_offset += written; - off += written; - size -= written; - - // Paranoid check to make sure we are done with this extent now - if (size && (off >= e_start && off < e_end)) { - LOG(ERROR) << "Failed to write extent fully"; - LogExtent(num_extent, extent); - return false; - } - - if (size == 0) { - // done - break; - } - } - num_extent++; - } - - return true; -} - bool FiemapWriter::Read(off64_t off, uint8_t* buffer, uint64_t size) { return false; } -// private helpers - -// WriteExtent() Returns the total number of bytes written. It will always be multiple of -// block_size_. 0 is returned in one of the two cases. -// 1. Any write failed between logical_off & logical_off + length. -// 2. The logical_offset + length doesn't overlap with the extent passed. -// The function can either partially for fully write the extent depending on the -// logical_off + length. It is expected that alignment checks for size and offset are -// performed before calling into this function. -uint64_t FiemapWriter::WriteExtent(const struct fiemap_extent& ext, uint8_t* buffer, - off64_t logical_off, uint64_t length) { - uint64_t e_start = ext.fe_logical; - uint64_t e_end = ext.fe_logical + ext.fe_length; - if (logical_off < e_start || logical_off >= e_end) { - LOG(ERROR) << "Failed write extent, invalid offset " << logical_off << " and size " - << length; - LogExtent(0, ext); - return 0; - } - - off64_t bdev_offset = ext.fe_physical + (logical_off - e_start); - if (bdev_offset >= bdev_size_) { - LOG(ERROR) << "Failed write extent, invalid block # " << bdev_offset << " for block device " - << bdev_path_ << " of size " << bdev_size_ << " bytes"; - return 0; - } - if (TEMP_FAILURE_RETRY(lseek64(bdev_fd_, bdev_offset, SEEK_SET)) == -1) { - PLOG(ERROR) << "Failed write extent, seek offset for " << bdev_path_ << " offset " - << bdev_offset; - return 0; - } - - // Determine how much we want to write at once. - uint64_t logical_end = logical_off + length; - uint64_t write_size = (e_end <= logical_end) ? (e_end - logical_off) : length; - if (!android::base::WriteFully(bdev_fd_, buffer, write_size)) { - PLOG(ERROR) << "Failed write extent, write " << bdev_path_ << " at " << bdev_offset - << " size " << write_size; - return 0; - } - - return write_size; -} - } // namespace fiemap_writer } // namespace android diff --git a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp index 3d20ff32d..41fa959a6 100644 --- a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp +++ b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp @@ -138,36 +138,31 @@ TEST_F(FiemapWriterTest, CheckFileExtents) { EXPECT_GT(fptr->extents().size(), 0); } -TEST_F(FiemapWriterTest, CheckWriteError) { - FiemapUniquePtr fptr = FiemapWriter::Open(testfile, testfile_size); - ASSERT_NE(fptr, nullptr); - - // prepare buffer for writing the pattern - 0xa0 - uint64_t blocksize = fptr->block_size(); - auto buffer = std::unique_ptr<void, decltype(&free)>(calloc(1, blocksize), free); - ASSERT_NE(buffer, nullptr); - memset(buffer.get(), 0xa0, blocksize); - - uint8_t* p = static_cast<uint8_t*>(buffer.get()); - for (off64_t off = 0; off < testfile_size; off += blocksize) { - ASSERT_TRUE(fptr->Write(off, p, blocksize)); - } - - EXPECT_TRUE(fptr->Flush()); -} - class TestExistingFile : public ::testing::Test { protected: void SetUp() override { std::string exec_dir = ::android::base::GetExecutableDirectory(); - std::string unaligned_file = exec_dir + "/testdata/unaligned_file"; - std::string file_4k = exec_dir + "/testdata/file_4k"; - std::string file_32k = exec_dir + "/testdata/file_32k"; - fptr_unaligned = FiemapWriter::Open(unaligned_file, 4097, false); - fptr_4k = FiemapWriter::Open(file_4k, 4096, false); - fptr_32k = FiemapWriter::Open(file_32k, 32768, false); + unaligned_file_ = exec_dir + "/testdata/unaligned_file"; + file_4k_ = exec_dir + "/testdata/file_4k"; + file_32k_ = exec_dir + "/testdata/file_32k"; + + CleanupFiles(); + fptr_unaligned = FiemapWriter::Open(unaligned_file_, 4097); + fptr_4k = FiemapWriter::Open(file_4k_, 4096); + fptr_32k = FiemapWriter::Open(file_32k_, 32768); + } + + 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; @@ -184,33 +179,6 @@ TEST_F(TestExistingFile, ErrorChecks) { EXPECT_GT(fptr_32k->extents().size(), 0); } -TEST_F(TestExistingFile, CheckWriteError) { - ASSERT_NE(fptr_4k, nullptr); - // prepare buffer for writing the pattern - 0xa0 - uint64_t blocksize = fptr_4k->block_size(); - auto buff_4k = std::unique_ptr<void, decltype(&free)>(calloc(1, blocksize), free); - ASSERT_NE(buff_4k, nullptr); - memset(buff_4k.get(), 0xa0, blocksize); - - uint8_t* p = static_cast<uint8_t*>(buff_4k.get()); - for (off64_t off = 0; off < 4096; off += blocksize) { - ASSERT_TRUE(fptr_4k->Write(off, p, blocksize)); - } - EXPECT_TRUE(fptr_4k->Flush()); - - ASSERT_NE(fptr_32k, nullptr); - // prepare buffer for writing the pattern - 0xa0 - blocksize = fptr_32k->block_size(); - auto buff_32k = std::unique_ptr<void, decltype(&free)>(calloc(1, blocksize), free); - ASSERT_NE(buff_32k, nullptr); - memset(buff_32k.get(), 0xa0, blocksize); - p = static_cast<uint8_t*>(buff_32k.get()); - for (off64_t off = 0; off < 4096; off += blocksize) { - ASSERT_TRUE(fptr_32k->Write(off, p, blocksize)); - } - EXPECT_TRUE(fptr_32k->Flush()); -} - class VerifyBlockWritesExt4 : public ::testing::Test { // 2GB Filesystem and 4k block size by default static constexpr uint64_t block_size = 4096; @@ -253,46 +221,6 @@ class VerifyBlockWritesExt4 : public ::testing::Test { std::string fs_path; }; -TEST_F(VerifyBlockWritesExt4, CheckWrites) { - EXPECT_EQ(access(fs_path.c_str(), F_OK), 0); - - std::string file_path = mntpoint + "/testfile"; - uint64_t file_size = 100 * 1024 * 1024; - auto buffer = std::unique_ptr<void, decltype(&free)>(calloc(1, getpagesize()), free); - ASSERT_NE(buffer, nullptr); - memset(buffer.get(), 0xa0, getpagesize()); - { - // scoped fiemap writer - FiemapUniquePtr fptr = FiemapWriter::Open(file_path, file_size); - ASSERT_NE(fptr, nullptr); - uint8_t* p = static_cast<uint8_t*>(buffer.get()); - for (off64_t off = 0; off < file_size / getpagesize(); off += getpagesize()) { - ASSERT_TRUE(fptr->Write(off, p, getpagesize())); - } - EXPECT_TRUE(fptr->Flush()); - } - // unmount file system here to make sure we invalidated all page cache and - // remount the filesystem again for verification - ASSERT_EQ(umount(mntpoint.c_str()), 0); - - LoopDevice loop_dev(fs_path); - ASSERT_TRUE(loop_dev.valid()); - ASSERT_EQ(mount(loop_dev.device().c_str(), mntpoint.c_str(), "ext4", 0, nullptr), 0) - << "failed to mount: " << loop_dev.device() << " on " << mntpoint << ": " - << strerror(errno); - - ::android::base::unique_fd fd(open(file_path.c_str(), O_RDONLY | O_SYNC)); - ASSERT_NE(fd, -1); - auto filebuf = std::unique_ptr<void, decltype(&free)>(calloc(1, getpagesize()), free); - ASSERT_NE(filebuf, nullptr); - for (off64_t off = 0; off < file_size / getpagesize(); off += getpagesize()) { - memset(filebuf.get(), 0x00, getpagesize()); - ASSERT_EQ(pread64(fd, filebuf.get(), getpagesize(), off), getpagesize()); - ASSERT_EQ(memcmp(filebuf.get(), buffer.get(), getpagesize()), 0) - << "Invalid pattern at offset: " << off << " size " << getpagesize(); - } -} - class VerifyBlockWritesF2fs : public ::testing::Test { // 2GB Filesystem and 4k block size by default static constexpr uint64_t block_size = 4096; @@ -335,46 +263,6 @@ class VerifyBlockWritesF2fs : public ::testing::Test { std::string fs_path; }; -TEST_F(VerifyBlockWritesF2fs, CheckWrites) { - EXPECT_EQ(access(fs_path.c_str(), F_OK), 0); - - std::string file_path = mntpoint + "/testfile"; - uint64_t file_size = 100 * 1024 * 1024; - auto buffer = std::unique_ptr<void, decltype(&free)>(calloc(1, getpagesize()), free); - ASSERT_NE(buffer, nullptr); - memset(buffer.get(), 0xa0, getpagesize()); - { - // scoped fiemap writer - FiemapUniquePtr fptr = FiemapWriter::Open(file_path, file_size); - ASSERT_NE(fptr, nullptr); - uint8_t* p = static_cast<uint8_t*>(buffer.get()); - for (off64_t off = 0; off < file_size / getpagesize(); off += getpagesize()) { - ASSERT_TRUE(fptr->Write(off, p, getpagesize())); - } - EXPECT_TRUE(fptr->Flush()); - } - // unmount file system here to make sure we invalidated all page cache and - // remount the filesystem again for verification - ASSERT_EQ(umount(mntpoint.c_str()), 0); - - LoopDevice loop_dev(fs_path); - ASSERT_TRUE(loop_dev.valid()); - ASSERT_EQ(mount(loop_dev.device().c_str(), mntpoint.c_str(), "f2fs", 0, nullptr), 0) - << "failed to mount: " << loop_dev.device() << " on " << mntpoint << ": " - << strerror(errno); - - ::android::base::unique_fd fd(open(file_path.c_str(), O_RDONLY | O_SYNC)); - ASSERT_NE(fd, -1); - auto filebuf = std::unique_ptr<void, decltype(&free)>(calloc(1, getpagesize()), free); - ASSERT_NE(filebuf, nullptr); - for (off64_t off = 0; off < file_size / getpagesize(); off += getpagesize()) { - memset(filebuf.get(), 0x00, getpagesize()); - ASSERT_EQ(pread64(fd, filebuf.get(), getpagesize(), off), getpagesize()); - ASSERT_EQ(memcmp(filebuf.get(), buffer.get(), getpagesize()), 0) - << "Invalid pattern at offset: " << off << " size " << getpagesize(); - } -} - int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); if (argc <= 1) { 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 a0085cfe2..edbae7721 100644 --- a/fs_mgr/libfiemap_writer/include/libfiemap_writer/fiemap_writer.h +++ b/fs_mgr/libfiemap_writer/include/libfiemap_writer/fiemap_writer.h @@ -57,15 +57,6 @@ class FiemapWriter final { // FiemapWriter::Open). static bool HasPinnedExtents(const std::string& file_path); - // Syncs block device writes. - bool Flush() const; - - // Writes the file by using its FIEMAP and performing i/o on the raw block device. - // The return value is success / failure. This will happen in particular if the - // kernel write returns errors, extents are not writeable or more importantly, if the 'size' is - // not aligned to the block device's block size. - bool Write(off64_t off, uint8_t* buffer, uint64_t size); - // The counter part of Write(). It is an error for the offset to be unaligned with // the block device's block size. // In case of error, the contents of buffer MUST be discarded. @@ -93,7 +84,6 @@ class FiemapWriter final { // File descriptors for the file and block device ::android::base::unique_fd file_fd_; - ::android::base::unique_fd bdev_fd_; // Size in bytes of the file this class is writing uint64_t file_size_; @@ -112,9 +102,6 @@ class FiemapWriter final { std::vector<struct fiemap_extent> extents_; FiemapWriter() = default; - - uint64_t WriteExtent(const struct fiemap_extent& ext, uint8_t* buffer, off64_t logical_off, - uint64_t length); }; } // namespace fiemap_writer |