Adar Dembo has posted comments on this change.

Change subject: fs: ensure FS_IOC_FIEMAP can be used on LBM systems
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6584/1//COMMIT_MSG
Commit Message:

PS1, Line 13: ecryptfs
> nit: eCryptfs
Done


http://gerrit.cloudera.org:8080/#/c/6584/1/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

PS1, Line 100: Status CheckFsFeatures(Env* env, const string& path) {
             :   // Arbitrary constants.
             :   static uint64_t kFileSize = 4096 * 4;
             :   static uint64_t kHoleOffset = 4096;
             :   static uint64_t kHoleSize = 8192;
             :   static uint64_t kPunchedFileSize = kFileSize - kHoleSize;
             : 
             :   // Open the test file.
             :   string filename = JoinPathSegments(path, 
"hole_punch_test_file");
             :   unique_ptr<RWFile> file;
             :   RWFileOptions opts;
             :   RETURN_NOT_OK(env->NewRWFile(opts, filename, &file));
             : 
             :   // The file has been created; delete it on exit no matter what 
happens.
             :   ScopedFileDeleter file_deleter(env, filename);
             : 
             :   // Preallocate it, making sure the file's size is what we'd 
expect.
             :   uint64_t sz;
             :   RETURN_NOT_OK_PREPEND(file->PreAllocate(
             :       0, kFileSize, RWFile::CHANGE_FILE_SIZE), "could not 
preallocate file");
             :   RETURN_NOT_OK(env->GetFileSizeOnDisk(filename, &sz));
             :   if (sz != kFileSize) {
             :     return Status::IOError(Substitute(
             :         "unexpected pre-punch file size for $0: expected $1 but 
got $2",
             :         filename, kFileSize, sz));
             :   }
             : 
             :   // Punch the hole, testing the file's size again.
             :   RETURN_NOT_OK_PREPEND(file->PunchHole(kHoleOffset, kHoleSize),
             :                         "could not punch hole in file");
             :   RETURN_NOT_OK(env->GetFileSizeOnDisk(filename, &sz));
             :   if (sz != kPunchedFileSize) {
             :     return Status::IOError(Substitute(
             :         "unexpected post-punch file size for $0: expected $1 but 
got $2",
             :         filename, kPunchedFileSize, sz));
             :   }
             : 
             :   // Make sure we can get at the file's extents.
             :   RWFile::ExtentMap extents;
             :   RETURN_NOT_OK_PREPEND(file->GetExtentMap(&extents),
             :                         "could not get file's extent map");
             :   return Status::OK();
             : }
> are there cases where people might have data already written to filesystems
That's a good question, and I don't fully know the answer. When I wrote the 
patch, I tested ext4 on el6.6 and on Ubuntu 16.04. Both supported hole punching 
as well as FS_IOC_FIEMAP.

Since you asked, I also tested ext4 and xfs on Ubuntu 14.04, SLES 12 and el6.4. 
Same result.

That's a pretty comprehensive list. What do you think?


http://gerrit.cloudera.org:8080/#/c/6584/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

Line 1306: Status LogBlockManager::Open(FsReport* report) {
> warning: default arguments on virtual or override methods are prohibited [g
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/6584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie027a264473ae5f905f8ad50aa45e810e046a135
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to