Revert D4813356: [wdt][PR] File permission transfer

Summary: This reverts commit 588487f73d4a100ad0f8bd79272a65eb991aaea6

Differential Revision: D4813356

fbshipit-source-id: ec993c8a1f3f2db4fdc820807c8500359f6055c2

ByteSource.h

@@ -44,8 +44,6 @@ struct SourceMetaData {
   FileAllocationStatus allocationStatus{NOT_EXISTS};
   /// if there is a size mismatch, this is the previous sequence id
   int64_t prevSeqId{0};
-  /// file permission.
-  int32_t permission{0};
   /// If true, files are read using O_DIRECT or F_NOCACHE
   bool directReads{false};
   /// File descriptor. If this is not -1, then wdt uses this to read

CMakeLists.txt

@@ -22,7 +22,7 @@ cmake_minimum_required(VERSION 3.2)
 # There is no C per se in WDT but if you use CXX only here many checks fail
 # Version is Major.Minor.YYMMDDX for up to 10 releases per day (X from 0 to 9)
 # Minor currently is also the protocol version - has to match with Protocol.cpp
-project("WDT" LANGUAGES C CXX VERSION 1.31.1703310)
+project("WDT" LANGUAGES C CXX VERSION 1.30.1704260)
 
 # On MacOS this requires the latest (master) CMake (and/or CMake 3.1.1/3.2)
 # WDT itself works fine with C++11 (gcc 4.8 for instance) but more recent folly

Protocol.cpp

@@ -34,7 +34,6 @@ const int Protocol::DELETE_CMD_VERSION = 26;
 const int Protocol::VARINT_CHANGE = 27;
 const int Protocol::HEART_BEAT_VERSION = 29;
 const int Protocol::PERIODIC_ENCRYPTION_IV_CHANGE_VERSION = 30;
-const int Protocol::PRESERVE_PERMISSION = 31;
 
 /* All methods of Protocol class are static (functions) */
 
@@ -150,9 +149,6 @@ bool Protocol::encodeHeader(int senderProtocolVersion, char *dest, int64_t &off,
             encodeVarI64C(dest, umax, off, blockDetails.dataSize) &&
             encodeVarI64C(dest, umax, off, blockDetails.offset) &&
             encodeVarI64C(dest, umax, off, blockDetails.fileSize);
-  if (ok && senderProtocolVersion >= PRESERVE_PERMISSION) {
-    ok = encodeVarI64C(dest, umax, off, blockDetails.permission);
-  }
   if (ok && senderProtocolVersion >= HEADER_FLAG_AND_PREV_SEQ_ID_VERSION) {
     uint8_t flags = blockDetails.allocationStatus;
     if (off >= max) {
@@ -183,9 +179,6 @@ bool Protocol::decodeHeader(int receiverProtocolVersion, char *src,
             decodeInt64C(br, blockDetails.dataSize) &&
             decodeInt64C(br, blockDetails.offset) &&
             decodeInt64C(br, blockDetails.fileSize);
-  if (ok && receiverProtocolVersion >= PRESERVE_PERMISSION) {
-      ok = decodeInt32C(br, blockDetails.permission);
-  }
   if (ok && receiverProtocolVersion >= HEADER_FLAG_AND_PREV_SEQ_ID_VERSION) {
     if (br.empty()) {
       WLOG(ERROR) << "Invalid (too short) input len " << max << " at offset "

Protocol.h

@@ -218,8 +218,6 @@ struct BlockDetails {
   FileAllocationStatus allocationStatus{NOT_EXISTS};
   /// seq-id of previous transfer, only valid if there is a size mismatch
   int64_t prevSeqId{0};
-  /// file permission
-  int32_t permission{0644};
 };
 
 /// structure representing settings cmd
@@ -274,8 +272,6 @@ class Protocol {
   static const int HEART_BEAT_VERSION;
   /// version from which wdt started to change encryption iv periodically
   static const int PERIODIC_ENCRYPTION_IV_CHANGE_VERSION;
-  /// version from which file permission is preserved during transfer
-  static const int PRESERVE_PERMISSION;
 
   /// Both version, magic number and command byte
   enum CMD_MAGIC {
@@ -304,10 +300,10 @@ class Protocol {
 
   /// Max size of sender or receiver id
   static constexpr int64_t kMaxTransferIdLength = 50;
-  /// 1 byte for cmd, 2 bytes for file-name length, Max size of filename, 5
-  /// variants(seq-id, data-size, offset, file-size, permission), 1 byte for
-  /// flag, 10 bytes prev seq-id
-  static constexpr int64_t kMaxHeader = 1 + 2 + PATH_MAX + 5 * 10 + 2 + 1 + 10;
+  /// 1 byte for cmd, 2 bytes for file-name length, Max size of filename, 4
+  /// variants(seq-id, data-size, offset, file-size), 1 byte for flag, 10 bytes
+  /// prev seq-id
+  static constexpr int64_t kMaxHeader = 1 + 2 + PATH_MAX + 4 * 10 + 1 + 10;
   /// min number of bytes that must be send to unblock receiver
   static constexpr int64_t kMinBufLength = 256;
   /// max size of done command encoding(1 byte for cmd, 1 for status, 10 for

SenderThread.cpp

@@ -324,7 +324,6 @@ TransferStats SenderThread::sendOneByteSource(
   blockDetails.dataSize = expectedSize;
   blockDetails.allocationStatus = metadata.allocationStatus;
   blockDetails.prevSeqId = metadata.prevSeqId;
-  blockDetails.permission = metadata.permission;
   Protocol::encodeHeader(wdtParent_->getProtocolVersion(), headerBuf, off,
                          Protocol::kMaxHeader, blockDetails);
   int16_t littleEndianOff = folly::Endian::little((int16_t)off);

WdtConfig.h

@@ -10,10 +10,10 @@
 #include <fcntl.h>
 
 #define WDT_VERSION_MAJOR 1
-#define WDT_VERSION_MINOR 31
-#define WDT_VERSION_BUILD 1703310
+#define WDT_VERSION_MINOR 30
+#define WDT_VERSION_BUILD 1704260
 // Add -fbcode to version str
-#define WDT_VERSION_STR "1.31.1703310-fbcode"
+#define WDT_VERSION_STR "1.30.1704260-fbcode"
 // Tie minor and proto version
 #define WDT_PROTOCOL_VERSION WDT_VERSION_MINOR
 

WdtTransferRequest.h

@@ -35,11 +35,6 @@ struct WdtFileInfo {
   /// Whether read should be done using o_direct. If fd is set, this flag will
   /// be set automatically to match the fd open mode
   bool directReads{false};
-  /// File permission.
-  int32_t permission;
-  /// Constructor for file info with name, size, odirect request and permission
-  WdtFileInfo(const std::string& name,
-              int64_t size, bool directReads, int32_t perm);
   /// Constructor for file info with name, size and odirect request
   WdtFileInfo(const std::string& name, int64_t size, bool directReads);
   /**

test/ProtocolTest.cpp

@@ -25,22 +25,20 @@ void testHeader() {
   bd.dataSize = 3;
   bd.offset = 4;
   bd.fileSize = 10;
-  bd.permission = 2;
   bd.allocationStatus = EXISTS_CORRECT_SIZE;
 
   char buf[128];
   int64_t off = 0;
-  Protocol::encodeHeader(Protocol::PRESERVE_PERMISSION, buf, off, sizeof(buf),
-                         bd);
+  Protocol::encodeHeader(Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf,
+                         off, sizeof(buf), bd);
   EXPECT_EQ(off,
-            bd.fileName.size() + 1 + 1 + 1 + 1 + 1 + 1 +
-                1);  // 1 byte variant for seqId, len, size, offset, filesize
-                     // and permission
+            bd.fileName.size() + 1 + 1 + 1 + 1 + 1 +
+                1);  // 1 byte variant for seqId, len, size, offset and filesize
   BlockDetails nbd;
   int64_t noff = 0;
   bool success =
-      Protocol::decodeHeader(Protocol::PRESERVE_PERMISSION, buf, noff,
-                             sizeof(buf), nbd);
+      Protocol::decodeHeader(Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf,
+                             noff, sizeof(buf), nbd);
   EXPECT_TRUE(success);
   EXPECT_EQ(noff, off);
   EXPECT_EQ(nbd.fileName, bd.fileName);
@@ -49,18 +47,17 @@ void testHeader() {
   EXPECT_EQ(nbd.offset, bd.offset);
   EXPECT_EQ(nbd.dataSize, bd.dataSize);
   EXPECT_EQ(nbd.allocationStatus, bd.allocationStatus);
-  EXPECT_EQ(nbd.permission, bd.permission);
   noff = 0;
   // exact size:
   success = Protocol::decodeHeader(
-      Protocol::PRESERVE_PERMISSION, buf, noff, off, nbd);
+      Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf, noff, off, nbd);
   EXPECT_TRUE(success);
 
   WLOG(INFO) << "error tests, expect errors";
   // too short
   noff = 0;
   success = Protocol::decodeHeader(
-      Protocol::PRESERVE_PERMISSION, buf, noff, off - 1, nbd);
+      Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf, noff, off - 1, nbd);
   EXPECT_FALSE(success);
 
   // Long size:
@@ -69,35 +66,33 @@ void testHeader() {
   bd.seqId = 5;
   bd.offset = 3;
   bd.fileSize = 128;
-  bd.permission = 511; // 777
   bd.allocationStatus = EXISTS_TOO_SMALL;
   bd.prevSeqId = 10;
 
   off = 0;
-  Protocol::encodeHeader(Protocol::PRESERVE_PERMISSION, buf, off, sizeof(buf),
-                         bd);
+  Protocol::encodeHeader(Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf,
+                         off, sizeof(buf), bd);
   EXPECT_EQ(off,
-            bd.fileName.size() + 1 + 1 + 6 + 1 + 2 + 1 + 1 +
-                2);  // 1 byte variant for id len and size
+            bd.fileName.size() + 1 + 1 + 6 + 1 + 2 + 1 +
+                1);  // 1 byte variant for id len and size
   noff = 0;
   success =
-      Protocol::decodeHeader(Protocol::PRESERVE_PERMISSION, buf, noff,
-                             sizeof(buf), nbd);
+      Protocol::decodeHeader(Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf,
+                             noff, sizeof(buf), nbd);
   EXPECT_TRUE(success);
   EXPECT_EQ(noff, off);
   EXPECT_EQ(nbd.fileName, bd.fileName);
   EXPECT_EQ(nbd.seqId, bd.seqId);
   EXPECT_EQ(nbd.fileSize, bd.fileSize);
   EXPECT_EQ(nbd.offset, bd.offset);
   EXPECT_EQ(nbd.dataSize, bd.dataSize);
-  EXPECT_EQ(nbd.permission, bd.permission);
   EXPECT_EQ(nbd.allocationStatus, bd.allocationStatus);
   EXPECT_EQ(nbd.prevSeqId, bd.prevSeqId);
   WLOG(INFO) << "got size of " << nbd.dataSize;
   // too short for size encoding:
   noff = 0;
   success = Protocol::decodeHeader(
-      Protocol::PRESERVE_PERMISSION, buf, noff, off - 2, nbd);
+      Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf, noff, off - 2, nbd);
   EXPECT_FALSE(success);
 }
 

test/wdt_e2e_simple_test.sh

@@ -137,13 +137,7 @@ if [ $DO_VERIFY -eq 1 ] ; then
     echo "Should be no diff"
     (cd $DIR; diff -u src.md5s dst.md5s)
     STATUS=$?
-    if [ $STATUS -eq 0 ] ; then
-      (cd $DIR/src ; ( find . -type f -printf %f%m\\n | sort ) > ../src.perm )
-      (cd $DIR/dst ; ( find . -type f -printf %f%m\\n | sort ) > ../dst.perm )
 
-      (cd $DIR; diff -u src.perm dst.perm)
-      STATUS=$?
-    fi
 
   if [ $WDT_TEST_SYMLINKS -eq 1 ]; then
     echo "Verifying for run with follow_symlinks"
@@ -156,9 +150,9 @@ if [ $DO_VERIFY -eq 1 ] ; then
         > ../src_symlinks.md5s )
     (cd $DIR/dst_symlinks ; ( find . -type f -print0 | xargs -0 $MD5SUM \
         | sort ) > ../dst_symlinks.md5s )
+
     echo "Should be no diff"
     (cd $DIR; diff -u src_symlinks.md5s dst_symlinks.md5s)
-
     SYMLINK_STATUS=$?
     if [ $STATUS -eq 0 ] ; then
       STATUS=$SYMLINK_STATUS

util/DirectorySourceQueue.cpp

@@ -33,12 +33,6 @@ WdtFileInfo::WdtFileInfo(const string &name, int64_t size, bool doDirectReads)
     : fileName(name), fileSize(size), directReads(doDirectReads) {
 }
 
-WdtFileInfo::WdtFileInfo(const string &name,
-                         int64_t size, bool doDirectReads, int32_t perm)
-    : WdtFileInfo(name, size, doDirectReads){
-  permission = perm;
-}
-
 WdtFileInfo::WdtFileInfo(int fd, int64_t size, const string &name)
     : WdtFileInfo(name, size, false) {
   this->fd = fd;
@@ -242,11 +236,6 @@ string DirectorySourceQueue::resolvePath(const string &path) {
   return result;
 }
 
-int getPermission(int mode) {
-  // set-user-ID bit | set-group-ID bit | sticky bit | owner | group | others
-  return mode & (S_ISUID | S_ISGID | S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO);
-}
-
 bool DirectorySourceQueue::explore() {
   WLOG(INFO) << "Exploring root dir " << rootDir_
              << " include_pattern : " << includePattern_
@@ -371,9 +360,7 @@ bool DirectorySourceQueue::explore() {
               !std::regex_match(newRelativePath, includeRegex)) {
             continue;
           }
-          const int perm = getPermission(fileStat.st_mode);
-          WdtFileInfo fileInfo(newRelativePath,
-                               fileStat.st_size, directReads_, perm);
+          WdtFileInfo fileInfo(newRelativePath, fileStat.st_size, directReads_);
           createIntoQueue(newFullPath, fileInfo);
           continue;
         }
@@ -453,7 +440,6 @@ void DirectorySourceQueue::createIntoQueue(const string &fullPath,
   metadata->fd = fileInfo.fd;
   metadata->directReads = fileInfo.directReads;
   metadata->size = fileInfo.fileSize;
-  metadata->permission = fileInfo.permission;
   if ((openFilesDuringDiscovery_ != 0) && (metadata->fd < 0)) {
     metadata->fd =
         FileUtil::openForRead(*threadCtx_, fullPath, metadata->directReads);
@@ -571,15 +557,14 @@ bool DirectorySourceQueue::enqueueFiles() {
       return false;
     }
     string fullPath = rootDir_ + info.fileName;
-    struct stat fileStat;
-    if (stat(fullPath.c_str(), &fileStat) != 0) {
-      WPLOG(ERROR) << "stat failed on path " << fullPath;
-      return false;
-    }
     if (info.fileSize < 0) {
+      struct stat fileStat;
+      if (stat(fullPath.c_str(), &fileStat) != 0) {
+        WPLOG(ERROR) << "stat failed on path " << fullPath;
+        return false;
+      }
       info.fileSize = fileStat.st_size;
     }
-    info.permission = getPermission(fileStat.st_mode);
     createIntoQueue(fullPath, info);
   }
   return true;

util/FileCreator.cpp

@@ -58,8 +58,7 @@ int FileCreator::openAndSetSize(ThreadCtx &threadCtx,
   const bool doCreate = (blockDetails->allocationStatus == NOT_EXISTS);
   const bool isTooLarge = (blockDetails->allocationStatus == EXISTS_TOO_LARGE);
   if (doCreate) {
-    fd = createFile(threadCtx,
-                    blockDetails->fileName, blockDetails->permission);
+    fd = createFile(threadCtx, blockDetails->fileName);
   } else {
     fd = openExistingFile(threadCtx, blockDetails->fileName);
   }
@@ -198,8 +197,7 @@ int FileCreator::openExistingFile(ThreadCtx &threadCtx,
   return res;
 }
 
-int FileCreator::createFile(ThreadCtx &threadCtx,
-                            const string &relPathStr, int32_t permission) {
+int FileCreator::createFile(ThreadCtx &threadCtx, const string &relPathStr) {
   CHECK(!relPathStr.empty());
   CHECK(relPathStr[0] != '/');
   CHECK(relPathStr.back() != '/');
@@ -257,7 +255,7 @@ int FileCreator::createFile(ThreadCtx &threadCtx,
   int res;
   {
     PerfStatCollector statCollector(threadCtx, PerfStatReport::FILE_OPEN);
-    res = open(path.c_str(), openFlags, permission);
+    res = open(path.c_str(), openFlags, 0644);
   }
   if (res < 0) {
     if (dir.empty()) {
@@ -278,7 +276,7 @@ int FileCreator::createFile(ThreadCtx &threadCtx,
     }
     {
       PerfStatCollector statCollector(threadCtx, PerfStatReport::FILE_OPEN);
-      res = open(path.c_str(), openFlags, permission);
+      res = open(path.c_str(), openFlags, 0644);
     }
     if (res < 0) {
       WPLOG(ERROR) << "failed creating file " << path;

util/FileCreator.h

@@ -96,20 +96,19 @@ class FileCreator {
   int openAndSetSize(ThreadCtx &threadCtx, BlockDetails const *blockDetails);
 
   /**
-   * Create a file and open for writing with the given permission, recursively
-   * create subdirs. Subdirs are only created once due to createdDirs_ cache,
-   * but if an open fails where we assumed the directory already exists based on
-   * cache, we try creating the dir and open again before failing. Will not
-   * overwrite existing files unless overwrite option is set.
+   * Create a file and open for writing, recursively create subdirs.
+   * Subdirs are only created once due to createdDirs_ cache, but
+   * if an open fails where we assumed the directory already exists
+   * based on cache, we try creating the dir and open again before
+   * failing. Will not overwrite existing files unless overwrite option
+   * is set.
    *
    * @param threadCtx     context of the calling thread
    * @param relPath       path relative to root dir
-   * @param permission    permission to use for the file
    *
    * @return          file descriptor or -1 on error
    */
-  int createFile(ThreadCtx &threadCtx,
-                 const std::string &relPath, int32_t permission);
+  int createFile(ThreadCtx &threadCtx, const std::string &relPath);
   /**
    * Open existing file
    */