fiemap_writer: Fix file pinning / pinning status check code.
The IsFilePinned() function is still not being called because I am still
seeing # blocks moved when I run following test on
aosp_blueline-userdebug.
$ fiemap_writer_test /dev/block/sda21 \
--gtest_filter=FiemapWriterTest.CheckWriteError
Bug: 122138114
Test: fiemap_writer_test /dev/block/sda21
Change-Id: I08d74093a082674d621772d202143d2f32e7c665
Signed-off-by: Sandeep Patil <sspatil@google.com>
diff --git a/fs_mgr/libfiemap_writer/fiemap_writer.cpp b/fs_mgr/libfiemap_writer/fiemap_writer.cpp
index 097e07f..427e16d 100644
--- a/fs_mgr/libfiemap_writer/fiemap_writer.cpp
+++ b/fs_mgr/libfiemap_writer/fiemap_writer.cpp
@@ -303,7 +303,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 +316,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 +339,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 +351,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 +451,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;
}
@@ -477,20 +481,20 @@
uint32_t 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 +503,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 +547,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;
}