Merge changes from topic "fiemap-writer-1" am: 32b695daa7 am: 22e1ae9e22
am: 26ea1fc054
Change-Id: I624f17970f34c2e81123e9fe537ba21d4d6dee3a
diff --git a/fs_mgr/libfiemap_writer/fiemap_writer.cpp b/fs_mgr/libfiemap_writer/fiemap_writer.cpp
index 097e07f..4a0d4b5 100644
--- a/fs_mgr/libfiemap_writer/fiemap_writer.cpp
+++ b/fs_mgr/libfiemap_writer/fiemap_writer.cpp
@@ -165,23 +165,13 @@
return true;
}
-static bool GetBlockDeviceParams(int bdev_fd, const std::string& bdev_path, uint64_t* blocksz,
- uint64_t* bdev_size) {
- // TODO: For some reason, the block device ioctl require the argument to be initialized
- // to zero even if its the out parameter for the given ioctl cmd.
- uint64_t blksz = 0;
- if (ioctl(bdev_fd, BLKBSZGET, &blksz)) {
- PLOG(ERROR) << "Failed to get block size for: " << bdev_path;
- return false;
- }
-
+static bool GetBlockDeviceSize(int bdev_fd, const std::string& bdev_path, uint64_t* bdev_size) {
uint64_t size_in_bytes = 0;
if (ioctl(bdev_fd, BLKGETSIZE64, &size_in_bytes)) {
PLOG(ERROR) << "Failed to get total size for: " << bdev_path;
return false;
}
- *blocksz = blksz;
*bdev_size = size_in_bytes;
return true;
@@ -197,22 +187,20 @@
return sb.st_size;
}
-static bool PerformFileChecks(const std::string& file_path, uint64_t file_size, uint64_t blocksz,
+static bool PerformFileChecks(const std::string& file_path, uint64_t file_size, uint64_t* blocksz,
uint32_t* fs_type) {
- // Check if the size aligned to the block size of the block device.
- // We need this to be true in order to be able to write the file using FIEMAP.
- if (file_size % blocksz) {
- LOG(ERROR) << "File size " << file_size << " is not aligned to block size " << blocksz
- << " for file " << file_path;
- return false;
- }
-
struct statfs64 sfs;
if (statfs64(file_path.c_str(), &sfs)) {
PLOG(ERROR) << "Failed to read file system status at: " << file_path;
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;
+ return false;
+ }
+
// Check if the filesystem is of supported types.
// Only ext4 and f2fs are tested and supported.
if ((sfs.f_type != EXT4_SUPER_MAGIC) && (sfs.f_type != F2FS_SUPER_MAGIC)) {
@@ -226,6 +214,7 @@
return false;
}
+ *blocksz = sfs.f_bsize;
*fs_type = sfs.f_type;
return true;
}
@@ -303,7 +292,7 @@
uint32_t pin_status = 1;
int error = ioctl(file_fd, F2FS_IOC_SET_PIN_FILE, &pin_status);
- if (error) {
+ if (error < 0) {
if ((errno == ENOTTY) || (errno == ENOTSUP)) {
PLOG(ERROR) << "Failed to pin file, not supported by kernel: " << file_path;
} else {
@@ -316,7 +305,7 @@
}
#if 0
-static bool PinFileStatus(int file_fd, const std::string& file_path, uint32_t fs_type) {
+static bool IsFilePinned(int file_fd, const std::string& file_path, uint32_t fs_type) {
if (fs_type == EXT4_SUPER_MAGIC) {
// No pinning necessary for ext4. The blocks, once allocated, are expected
// to be fixed.
@@ -339,9 +328,10 @@
#define F2FS_IOC_GET_PIN_FILE _IOR(F2FS_IOCTL_MAGIC, 14, __u32)
#endif
- uint32_t pin_status;
- int error = ioctl(file_fd, F2FS_IOC_GET_PIN_FILE, &pin_status);
- if (error) {
+ // F2FS_IOC_GET_PIN_FILE returns the number of blocks moved.
+ uint32_t moved_blocks_nr;
+ int error = ioctl(file_fd, F2FS_IOC_GET_PIN_FILE, &moved_blocks_nr);
+ if (error < 0) {
if ((errno == ENOTTY) || (errno == ENOTSUP)) {
PLOG(ERROR) << "Failed to get file pin status, not supported by kernel: " << file_path;
} else {
@@ -350,7 +340,10 @@
return false;
}
- return !!pin_status;
+ if (moved_blocks_nr) {
+ LOG(ERROR) << moved_blocks_nr << " blocks moved in file " << file_path;
+ }
+ return moved_blocks_nr == 0;
}
#endif
@@ -447,7 +440,7 @@
std::string bdev_path;
if (!FileToBlockDevicePath(abs_path, &bdev_path)) {
LOG(ERROR) << "Failed to get block dev path for file: " << file_path;
- cleanup(file_path, create);
+ cleanup(abs_path, create);
return nullptr;
}
@@ -459,9 +452,9 @@
return nullptr;
}
- uint64_t blocksz, bdevsz;
- if (!GetBlockDeviceParams(bdev_fd, bdev_path, &blocksz, &bdevsz)) {
- LOG(ERROR) << "Failed to get block device params for: " << bdev_path;
+ uint64_t bdevsz;
+ if (!GetBlockDeviceSize(bdev_fd, bdev_path, &bdevsz)) {
+ LOG(ERROR) << "Failed to get block device size for : " << bdev_path;
cleanup(file_path, create);
return nullptr;
}
@@ -474,23 +467,24 @@
}
}
+ uint64_t blocksz;
uint32_t fs_type;
- if (!PerformFileChecks(abs_path, file_size, blocksz, &fs_type)) {
+ if (!PerformFileChecks(abs_path, file_size, &blocksz, &fs_type)) {
LOG(ERROR) << "Failed to validate file or file system for file:" << abs_path;
- cleanup(file_path, create);
+ cleanup(abs_path, create);
return nullptr;
}
if (create) {
if (!AllocateFile(file_fd, abs_path, blocksz, file_size)) {
- unlink(abs_path.c_str());
+ cleanup(abs_path, create);
return nullptr;
}
}
// f2fs may move the file blocks around.
if (!PinFile(file_fd, file_path, fs_type)) {
- cleanup(file_path, create);
+ cleanup(abs_path, create);
LOG(ERROR) << "Failed to pin the file in storage";
return nullptr;
}
@@ -499,7 +493,7 @@
FiemapUniquePtr fmap(new FiemapWriter());
if (!ReadFiemap(file_fd, abs_path, &fmap->extents_)) {
LOG(ERROR) << "Failed to read fiemap of file: " << abs_path;
- cleanup(file_path, create);
+ cleanup(abs_path, create);
return nullptr;
}
@@ -543,9 +537,10 @@
<< " for block size " << block_size_;
return false;
}
+
#if 0
// TODO(b/122138114): check why this fails.
- if (!PinFileStatus(file_fd_, file_path_, fs_type_)) {
+ if (!IsFilePinned(file_fd_, file_path_, fs_type_)) {
LOG(ERROR) << "Failed write: file " << file_path_ << " is not pinned";
return false;
}
diff --git a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp
index 6653ada..6dff0e8 100644
--- a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp
+++ b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp
@@ -40,11 +40,22 @@
using unique_fd = android::base::unique_fd;
using LoopDevice = android::dm::LoopDevice;
-std::string testfile = "";
std::string testbdev = "";
uint64_t testfile_size = 536870912; // default of 512MiB
-TEST(FiemapWriter, CreateImpossiblyLargeFile) {
+class FiemapWriterTest : public ::testing::Test {
+ protected:
+ void SetUp() override {
+ const ::testing::TestInfo* tinfo = ::testing::UnitTest::GetInstance()->current_test_info();
+ std::string exec_dir = ::android::base::GetExecutableDirectory();
+ testfile = ::android::base::StringPrintf("%s/testdata/%s", exec_dir.c_str(), tinfo->name());
+ }
+
+ // name of the file we use for testing
+ std::string testfile;
+};
+
+TEST_F(FiemapWriterTest, CreateImpossiblyLargeFile) {
// Try creating a file of size ~100TB but aligned to
// 512 byte to make sure block alignment tests don't
// fail.
@@ -54,7 +65,7 @@
EXPECT_EQ(errno, ENOENT);
}
-TEST(FiemapWriter, CreateUnalignedFile) {
+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.
@@ -64,7 +75,7 @@
EXPECT_EQ(errno, ENOENT);
}
-TEST(FiemapWriter, CheckFilePath) {
+TEST_F(FiemapWriterTest, CheckFilePath) {
FiemapUniquePtr fptr = FiemapWriter::Open(testfile, 4096);
ASSERT_NE(fptr, nullptr);
EXPECT_EQ(fptr->size(), 4096);
@@ -72,20 +83,20 @@
EXPECT_EQ(access(testfile.c_str(), F_OK), 0);
}
-TEST(FiemapWriter, CheckBlockDevicePath) {
+TEST_F(FiemapWriterTest, CheckBlockDevicePath) {
FiemapUniquePtr fptr = FiemapWriter::Open(testfile, 4096);
EXPECT_EQ(fptr->size(), 4096);
EXPECT_EQ(fptr->bdev_path(), testbdev);
}
-TEST(FiemapWriter, CheckFileCreated) {
+TEST_F(FiemapWriterTest, CheckFileCreated) {
FiemapUniquePtr fptr = FiemapWriter::Open(testfile, 32768);
ASSERT_NE(fptr, nullptr);
unique_fd fd(open(testfile.c_str(), O_RDONLY));
EXPECT_GT(fd, -1);
}
-TEST(FiemapWriter, CheckFileSizeActual) {
+TEST_F(FiemapWriterTest, CheckFileSizeActual) {
FiemapUniquePtr fptr = FiemapWriter::Open(testfile, testfile_size);
ASSERT_NE(fptr, nullptr);
@@ -94,13 +105,13 @@
EXPECT_EQ(sb.st_size, testfile_size);
}
-TEST(FiemapWriter, CheckFileExtents) {
+TEST_F(FiemapWriterTest, CheckFileExtents) {
FiemapUniquePtr fptr = FiemapWriter::Open(testfile, testfile_size);
ASSERT_NE(fptr, nullptr);
EXPECT_GT(fptr->extents().size(), 0);
}
-TEST(FiemapWriter, CheckWriteError) {
+TEST_F(FiemapWriterTest, CheckWriteError) {
FiemapUniquePtr fptr = FiemapWriter::Open(testfile, testfile_size);
ASSERT_NE(fptr, nullptr);
@@ -339,18 +350,17 @@
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
- if (argc <= 2) {
+ if (argc <= 1) {
cerr << "Filepath with its bdev path must be provided as follows:" << endl;
- cerr << " $ fiemap_writer_test <path to file> </dev/block/XXXX" << endl;
+ cerr << " $ fiemap_writer_test </dev/block/XXXX" << endl;
cerr << " where, /dev/block/XXX is the block device where the file resides" << endl;
exit(EXIT_FAILURE);
}
::android::base::InitLogging(argv, ::android::base::StderrLogger);
- testfile = argv[1];
- testbdev = argv[2];
- if (argc > 3) {
- testfile_size = strtoull(argv[3], NULL, 0);
+ testbdev = argv[1];
+ if (argc > 2) {
+ testfile_size = strtoull(argv[2], NULL, 0);
if (testfile_size == ULLONG_MAX) {
testfile_size = 512 * 1024 * 1024;
}