Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File permission transfer #155

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ByteSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ 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
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.30.1703300)
project("WDT" LANGUAGES C CXX VERSION 1.31.1703310)

# 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
Expand Down
2 changes: 1 addition & 1 deletion ErrorCodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
#include <wdt/ErrorCodes.h>
#include <folly/Conv.h>
#include <string.h>
#include <wdt/ErrorCodes.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you revert this change?


DEFINE_int32(wdt_double_precision, 2, "Precision while printing double");

Expand Down
7 changes: 7 additions & 0 deletions Protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ 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) */

Expand Down Expand Up @@ -149,6 +150,9 @@ 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) {
Expand Down Expand Up @@ -179,6 +183,9 @@ 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 "
Expand Down
12 changes: 8 additions & 4 deletions Protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ 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
Expand Down Expand Up @@ -272,6 +274,8 @@ 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 {
Expand Down Expand Up @@ -300,10 +304,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, 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;
/// 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 + 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
Expand Down
2 changes: 1 addition & 1 deletion ReceiverThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
#include <wdt/ReceiverThread.h>
#include <folly/Bits.h>
#include <folly/Checksum.h>
#include <folly/Conv.h>
#include <folly/Memory.h>
#include <folly/ScopeGuard.h>
#include <folly/String.h>
#include <wdt/ReceiverThread.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this one too.

"The associated header file of .cpp files should be included before any other includes. (This helps catch missing header file dependencies in the .h)"

This is the lint error I'm seeing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah... this is what clang gave me... Let me revert it...

#include <wdt/util/FileWriter.h>

namespace facebook {
Expand Down
2 changes: 1 addition & 1 deletion Reporting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
#include <wdt/Reporting.h>
#include <folly/String.h>
#include <wdt/Protocol.h>
#include <wdt/Reporting.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one too

#include <wdt/WdtOptions.h>

#include <algorithm>
Expand Down
3 changes: 2 additions & 1 deletion SenderThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
#include <wdt/SenderThread.h>
#include <folly/Bits.h>
#include <folly/Checksum.h>
#include <folly/Conv.h>
#include <folly/Memory.h>
#include <folly/String.h>
#include <sys/stat.h>
#include <wdt/Sender.h>
#include <wdt/SenderThread.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another one

#include <wdt/util/ClientSocket.h>

namespace facebook {
Expand Down Expand Up @@ -324,6 +324,7 @@ 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);
Expand Down
2 changes: 1 addition & 1 deletion Throttler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
#include <wdt/Throttler.h>
#include <wdt/ErrorCodes.h>
#include <wdt/Throttler.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one too

#include <wdt/WdtOptions.h>

namespace facebook {
Expand Down
6 changes: 3 additions & 3 deletions WdtConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
#include <fcntl.h>

#define WDT_VERSION_MAJOR 1
#define WDT_VERSION_MINOR 30
#define WDT_VERSION_BUILD 1703300
#define WDT_VERSION_MINOR 31
#define WDT_VERSION_BUILD 1703310
// Add -fbcode to version str
#define WDT_VERSION_STR "1.30.1703300-fbcode"
#define WDT_VERSION_STR "1.31.1703310-fbcode"
// Tie minor and proto version
#define WDT_PROTOCOL_VERSION WDT_VERSION_MINOR

Expand Down
2 changes: 1 addition & 1 deletion WdtOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
#include <wdt/WdtOptions.h>
#include <glog/logging.h>
#include <wdt/WdtOptions.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another


namespace facebook {
namespace wdt {
Expand Down
2 changes: 1 addition & 1 deletion WdtTransferRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
#include <wdt/WdtTransferRequest.h>
#include <folly/Conv.h>
#include <folly/Range.h>
#include <wdt/WdtTransferRequest.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one too

#include <ctime>

using namespace std;
Expand Down
5 changes: 5 additions & 0 deletions WdtTransferRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ 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);
/**
Expand Down
1 change: 1 addition & 0 deletions build/travis_osx.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export PATH=/usr/local/opt/openssl/bin:$CMAKE_BIN_DIR:$HOME/bin:$PATH
export LD_LIBRARY_PATH=/usr/local/opt/openssl/lib:$HOME/lib:$LD_LIBRARY_PATH
export OPENSSL_ROOT_DIR=/usr/local/opt/openssl
export CMAKE_PREFIX_PATH=$HOME
export WDT_TEST_PERMISSION=0
openssl version -a
wget https://www.cmake.org/files/v3.6/$CMAKE_BASE.tar.gz
tar xfz $CMAKE_BASE.tar.gz
Expand Down
22 changes: 22 additions & 0 deletions test/FileReaderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ class RandomFile {
int64_t getSize() const {
return fileSize_;
}
int32_t getPermission() {
if (permission_ == -1) {
struct stat fileStat;
if (stat(fileName_.c_str(), &fileStat) != 0) {
WLOG(ERROR) << "Error when calling stat()";
return -1;
}
permission_ = fileStat.st_mode &
(S_ISUID | S_ISGID | S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO);
}
return permission_;
}
const string& getFileName() const {
return fileName_;
}
Expand All @@ -65,6 +77,7 @@ class RandomFile {

private:
int64_t fileSize_{-1};
int32_t permission_{-1};
string fileName_;
SourceMetaData* metaData_{nullptr};
};
Expand Down Expand Up @@ -98,6 +111,10 @@ void testReadSize(int64_t fileSize, ByteSource& byteSource) {
EXPECT_EQ(totalSizeRead, fileSize);
}

void testReadPermission(int32_t permission, ByteSource& byteSource) {
EXPECT_EQ(permission, byteSource.getMetaData().permission);
}

void testFileRead(const WdtOptions& options, int64_t fileSize,
bool directReads) {
RandomFile file(fileSize);
Expand Down Expand Up @@ -158,6 +175,7 @@ TEST(FileByteSource, FILEINFO_ODIRECT) {
ThreadCtx threadCtx(options, true);
auto byteSource = Q.getNextSource(&threadCtx, code);
testReadSize(sizeToRead, *byteSource);
testReadPermission(file.getPermission(), *byteSource);
}

TEST(FileByteSource, MULTIPLEFILES_ODIRECT) {
Expand Down Expand Up @@ -191,6 +209,7 @@ TEST(FileByteSource, MULTIPLEFILES_ODIRECT) {
break;
}
testReadSize(sizeToRead, *byteSource);
testReadPermission(randFiles[fileNumber].getPermission(), *byteSource);
++fileNumber;
}
EXPECT_EQ(fileNumber, numFiles);
Expand Down Expand Up @@ -219,13 +238,16 @@ TEST(FileByteSource, MULTIPLEFILES_REGULAR) {
Q.setFileInfo(files);
Q.buildQueueSynchronously();
ErrorCode code;
int fileNumber = 0;
ThreadCtx threadCtx(options, true);
while (true) {
auto byteSource = Q.getNextSource(&threadCtx, code);
if (!byteSource) {
break;
}
testReadSize(sizeToRead, *byteSource);
testReadPermission(randFiles[fileNumber].getPermission(), *byteSource);
++fileNumber;
}
}
}
Expand Down
43 changes: 23 additions & 20 deletions test/ProtocolTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,21 @@ 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::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf,
off, sizeof(buf), bd);
Protocol::encodeHeader(Protocol::PRESERVE_PERMISSION, buf, off, sizeof(buf),
bd);
EXPECT_EQ(off,
bd.fileName.size() + 1 + 1 + 1 + 1 + 1 +
1); // 1 byte variant for seqId, len, size, offset and filesize
bd.fileName.size() + 1 + 1 + 1 + 1 + 1 + 1 +
1); // 1 byte variant for seqId, len, size, offset, filesize
// and permission
BlockDetails nbd;
int64_t noff = 0;
bool success =
Protocol::decodeHeader(Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf,
noff, sizeof(buf), nbd);
bool success = Protocol::decodeHeader(Protocol::PRESERVE_PERMISSION, buf,
noff, sizeof(buf), nbd);
EXPECT_TRUE(success);
EXPECT_EQ(noff, off);
EXPECT_EQ(nbd.fileName, bd.fileName);
Expand All @@ -47,17 +48,18 @@ 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::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf, noff, off, nbd);
success = Protocol::decodeHeader(Protocol::PRESERVE_PERMISSION, buf, noff,
off, nbd);
EXPECT_TRUE(success);

WLOG(INFO) << "error tests, expect errors";
// too short
noff = 0;
success = Protocol::decodeHeader(
Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf, noff, off - 1, nbd);
success = Protocol::decodeHeader(Protocol::PRESERVE_PERMISSION, buf, noff,
off - 1, nbd);
EXPECT_FALSE(success);

// Long size:
Expand All @@ -66,33 +68,34 @@ 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::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf,
off, sizeof(buf), bd);
Protocol::encodeHeader(Protocol::PRESERVE_PERMISSION, buf, off, sizeof(buf),
bd);
EXPECT_EQ(off,
bd.fileName.size() + 1 + 1 + 6 + 1 + 2 + 1 +
1); // 1 byte variant for id len and size
bd.fileName.size() + 1 + 1 + 6 + 1 + 2 + 1 + 1 +
2); // 1 byte variant for id len and size
noff = 0;
success =
Protocol::decodeHeader(Protocol::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf,
noff, sizeof(buf), nbd);
success = Protocol::decodeHeader(Protocol::PRESERVE_PERMISSION, 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::HEADER_FLAG_AND_PREV_SEQ_ID_VERSION, buf, noff, off - 2, nbd);
success = Protocol::decodeHeader(Protocol::PRESERVE_PERMISSION, buf, noff,
off - 2, nbd);
EXPECT_FALSE(success);
}

Expand Down
Loading