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

Commit d2b2e21f authored by Sandeep Patil's avatar Sandeep Patil
Browse files

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: default avatarSandeep Patil <sspatil@google.com>
parent dbce5856

fs_mgr/libfiemap_writer/fiemap_writer.cpp

+17 −12
Original line number Diff line number Diff line
@@ -303,7 +303,7 @@ static bool PinFile(int file_fd, const std::string& file_path, uint32_t fs_type) 


    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 @@ static bool PinFile(int file_fd, const std::string& file_path, uint32_t fs_type) 
}



#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 @@ static bool PinFileStatus(int file_fd, const std::string& file_path, uint32_t fs 
#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 @@ static bool PinFileStatus(int file_fd, const std::string& file_path, uint32_t fs 
        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 @@ FiemapUniquePtr FiemapWriter::Open(const std::string& file_path, uint64_t file_s 
    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 @@ FiemapUniquePtr FiemapWriter::Open(const std::string& file_path, uint64_t file_s 
    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 FiemapWriter::Open(const std::string& file_path, uint64_t file_s 
    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 @@ bool FiemapWriter::Write(off64_t off, uint8_t* buffer, uint64_t size) { 
                   << " 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;

    }

fs_mgr/libfiemap_writer/fiemap_writer_test.cpp

+25 −15
Original line number Diff line number Diff line
@@ -40,11 +40,22 @@ using namespace android::fiemap_writer; 
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 @@ TEST(FiemapWriter, CreateImpossiblyLargeFile) { 
    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 @@ TEST(FiemapWriter, CreateUnalignedFile) { 
    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 @@ TEST(FiemapWriter, CheckFilePath) { 
    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 @@ TEST(FiemapWriter, CheckFileSizeActual) { 
    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 @@ TEST_F(VerifyBlockWritesF2fs, CheckWrites) { 


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;

        }