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.

Sign up for GitHub

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

Jump to bottom

curvefs: implement setxattr interface. #1935

Merged
merged 1 commit into from
Oct 13, 2022
+196 −73
Merged

curvefs: implement setxattr interface. #1935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 8 additions & 4 deletions curvefs/src/client/curve_fuse_op.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,17 +322,21 @@ int Warmup(const std::string& name, const std::string& value) {

void FuseOpSetXattr(fuse_req_t req, fuse_ino_t ino, const char* name,
const char* value, size_t size, int flags) {
// only support "curvefs.warmup.op" xattr
std::string xattrValue(value, size);
VLOG(9) << "FuseOpSetXattr"
<< " ino " << ino << " name " << name << " value " << xattrValue
<< " flags " << flags;
int code = ENOTSUP;
if (strcmp(name, curvefs::client::common::kCurveFsWarmupXAttr) == 0) {
// warmup
code = Warmup(name, xattrValue);
int code = Warmup(name, xattrValue);
fuse_reply_err(req, code);
} else {
// set xattr
CURVEFS_ERROR ret = g_ClientInstance->FuseOpSetXattr(req, ino, name,
value, size, flags);
FuseReplyErrByErrCode(req, ret);
}
fuse_reply_err(req, code);

VLOG(9) << "FuseOpSetXattr done";
}

Expand Down
110 changes: 52 additions & 58 deletions curvefs/src/client/fuse_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,7 @@ CURVEFS_ERROR FuseClient::FuseOpInit(void *userdata,
dentryManager_->SetFsId(fsInfo_->fsid());
enableSumInDir_ = fsInfo_->enablesumindir() && !FLAGS_enableCto;
LOG(INFO) << "Mount " << fsName << " on " << mountpoint_.ShortDebugString()
<< " success!"
<< " enableSumInDir = " << enableSumInDir_;
<< " success!" << " enableSumInDir = " << enableSumInDir_;

fsMetric_ = std::make_shared<FSMetric>(fsName);

Expand Down Expand Up @@ -1076,24 +1075,6 @@ CURVEFS_ERROR FuseClient::FuseOpSetAttr(fuse_req_t req, fuse_ino_t ino,
return ret;
}

namespace {

bool IsSummaryInfo(const char *name) {
return std::strstr(name, SUMMARYPREFIX);
}

bool IsOneLayer(const char *name) {
if (std::strcmp(name, XATTRFILES) == 0 ||
std::strcmp(name, XATTRSUBDIRS) == 0 ||
std::strcmp(name, XATTRENTRIES) == 0 ||
std::strcmp(name, XATTRFBYTES) == 0) {
return true;
}
return false;
}

} // namespace

CURVEFS_ERROR FuseClient::FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino,
const char* name, void* value,
size_t size) {
Expand All @@ -1104,49 +1085,24 @@ CURVEFS_ERROR FuseClient::FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino,
}

std::string xValue;
// get summary info
if (IsSummaryInfo(name)) {
InodeAttr inodeAttr;
CURVEFS_ERROR ret = inodeManager_->GetInodeAttr(ino, &inodeAttr);
if (ret != CURVEFS_ERROR::OK) {
LOG(ERROR) << "inodeManager get inodeAttr fail, ret = " << ret
<< ", inodeid = " << ino;
return ret;
}

if (inodeAttr.type() == FsFileType::TYPE_DIRECTORY) {
// not enable record summary info in dir xattr,
// need recursive computation all files
if (!enableSumInDir_) {
if (IsOneLayer(name)) {
ret = xattrManager_->CalOneLayerSumInfo(&inodeAttr);
} else {
ret = xattrManager_->CalAllLayerSumInfo(&inodeAttr);
}
} else {
if (IsOneLayer(name)) {
ret = xattrManager_->FastCalOneLayerSumInfo(&inodeAttr);
} else {
ret = xattrManager_->FastCalAllLayerSumInfo(&inodeAttr);
}
}
InodeAttr inodeAttr;
CURVEFS_ERROR ret = inodeManager_->GetInodeAttr(ino, &inodeAttr);
if (ret != CURVEFS_ERROR::OK) {
LOG(ERROR) << "inodeManager get inodeAttr fail, ret = " << ret
<< ", inodeid = " << ino;
return ret;
}

if (CURVEFS_ERROR::OK != ret) {
return ret;
}
LOG(INFO) << "After calculate summary info:\n"
<< inodeAttr.DebugString();
auto it = inodeAttr.xattr().find(name);
if (it != inodeAttr.xattr().end()) {
xValue = it->second;
}
}
ret = xattrManager_->GetXattr(name, &xValue, &inodeAttr, enableSumInDir_);
if (CURVEFS_ERROR::OK != ret) {
LOG(ERROR) << "xattrManager get xattr failed, name = " << name;
return ret;
}

CURVEFS_ERROR ret = CURVEFS_ERROR::NODATA;
ret = CURVEFS_ERROR::NODATA;
if (xValue.length() > 0) {
if ((size == 0 && xValue.length() <= MAXXATTRLENGTH) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

what does size == 0 mean？
size == 0是什么意思，这时候不会出错吗？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/**

  • Get an extended attribute
  • If size is zero, the size of the value should be sent with
  • fuse_reply_xattr.
  • If the size is non-zero, and the value fits in the buffer, the
  • value should be sent with fuse_reply_buf.
  • If the size is too small for the value, the ERANGE error should
  • be sent.

size ==0 means you should return value's length by fuse_reply_xattr, and this flag used to get the value's length first by kernel and the real size will be set in the next request.

Copy link
Contributor

Choose a reason for hiding this comment

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

if size > MAXXATTRLENGTH && xValue.length() <= MAXXATTRLENGTH
it will return out of range?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

(size >= xValue.length() && size <= MAXXATTRLENGTH)) {
(size >= xValue.length() && xValue.length() <= MAXXATTRLENGTH)) {
memcpy(value, xValue.c_str(), xValue.length());
ret = CURVEFS_ERROR::OK;
} else {
Expand All @@ -1156,6 +1112,44 @@ CURVEFS_ERROR FuseClient::FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino,
return ret;
}

CURVEFS_ERROR FuseClient::FuseOpSetXattr(fuse_req_t req, fuse_ino_t ino,
const char* name, const char* value,
size_t size, int flags) {
VLOG(1) << "FuseOpSetXattr ino: " << ino << ", name: " << name
<< ", value: " << value;
if (option_.disableXattr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

VLOG(1) << "FuseOpSetXattr " << ****;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

return CURVEFS_ERROR::NOTSUPPORT;
}

std::string strname(name);
std::string strvalue(value, size);
if (strname.length() > MAXXATTRLENGTH || size > MAXXATTRLENGTH) {
LOG(ERROR) << "xattr length is too long, name = " << name
<< ", name length = " << strname.length()
<< ", value length = " << size;
return CURVEFS_ERROR::OUT_OF_RANGE;
}

std::shared_ptr<InodeWrapper> inodeWrapper;
CURVEFS_ERROR ret = inodeManager_->GetInode(ino, inodeWrapper);
if (ret != CURVEFS_ERROR::OK) {
LOG(ERROR) << "inodeManager get inode fail, ret = " << ret
<< ", inodeid = " << ino;
return ret;
}

::curve::common::UniqueLock lgGuard = inodeWrapper->GetUniqueLock();
inodeWrapper->SetXattrLocked(strname, strvalue);
ret = inodeWrapper->SyncAttr();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to do sync every time here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The xattr will lost when cache crashed If not update the xattr to metaserver

if (ret != CURVEFS_ERROR::OK) {
LOG(ERROR) << "set xattr fail, ret = " << ret << ", inodeid = " << ino
<< ", name = " << strname << ", value = " << strvalue;
return ret;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

VLOG(1) << "FuseOpSetXattr end" << ****

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

VLOG(1) << "FuseOpSetXattr end";
return CURVEFS_ERROR::OK;
}

CURVEFS_ERROR FuseClient::FuseOpListXattr(fuse_req_t req, fuse_ino_t ino,
char *value, size_t size, size_t *realSize) {
VLOG(1) << "FuseOpListXattr, ino: " << ino << ", size = " << size;
Expand Down
4 changes: 4 additions & 0 deletions curvefs/src/client/fuse_client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ class FuseClient {
const char* name, void* value,
size_t size);

virtual CURVEFS_ERROR FuseOpSetXattr(fuse_req_t req, fuse_ino_t ino,
const char* name, const char* value,
size_t size, int flags);

virtual CURVEFS_ERROR FuseOpListXattr(fuse_req_t req, fuse_ino_t ino,
char *value, size_t size, size_t *realSize);

Expand Down
6 changes: 3 additions & 3 deletions curvefs/src/client/inode_cache_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttrAsync(
std::map<uint64_t, InodeAttr> *attrs) {
NameLockGuard lg(asyncNameLock_, std::to_string(parentId));
std::map<uint64_t, InodeAttr> cachedAttr;
bool ok = iAttrCache_->Get(parentId, &cachedAttr);
bool cache = iAttrCache_->Get(parentId, &cachedAttr);

// get some inode attr in icache
for (auto iter = inodeIds->begin(); iter != inodeIds->end();) {
Expand All @@ -254,7 +254,7 @@ CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttrAsync(
inodeWrapper->GetInodeAttr(&tmpAttr);
attrs->emplace(*iter, tmpAttr);
iter = inodeIds->erase(iter);
} else if (ok && cachedAttr.find(*iter) != cachedAttr.end()) {
} else if (cache && cachedAttr.find(*iter) != cachedAttr.end()) {
attrs->emplace(*iter, cachedAttr[*iter]);
iter = inodeIds->erase(iter);
} else {
Expand Down Expand Up @@ -291,7 +291,7 @@ CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttrAsync(
// wait for all sudrequest finished
cond->Wait();

ok = iAttrCache_->Get(parentId, attrs);
bool ok = iAttrCache_->Get(parentId, attrs);
if (!ok) {
LOG(WARNING) << "get attrs form iAttrCache_ failed.";
}
Expand Down
6 changes: 6 additions & 0 deletions curvefs/src/client/inode_wrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,12 @@ class InodeWrapper : public std::enable_shared_from_this<InodeWrapper> {
return ret;
}

void SetXattrLocked(const std::string &key, const std::string &value) {
(*inode_.mutable_xattr())[key] = value;
(*dirtyAttr_.mutable_xattr()) = inode_.xattr();
dirty_ = true;
}

curve::common::UniqueLock GetUniqueLock() {
return curve::common::UniqueLock(mtx_);
}
Expand Down
52 changes: 52 additions & 0 deletions curvefs/src/client/xattr_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,21 @@ namespace client {
using ::curve::common::StringToUll;
using ::curve::common::Thread;


bool IsSummaryInfo(const char *name) {
return std::strstr(name, SUMMARYPREFIX);
}

bool IsOneLayer(const char *name) {
if (std::strcmp(name, XATTRFILES) == 0 ||
std::strcmp(name, XATTRSUBDIRS) == 0 ||
std::strcmp(name, XATTRENTRIES) == 0 ||
std::strcmp(name, XATTRFBYTES) == 0) {
return true;
}
return false;
}

// if direction is true means '+', false means '-'
bool AddUllStringToFirst(std::string *first, uint64_t second, bool direction) {
uint64_t firstNum = 0;
Expand Down Expand Up @@ -441,6 +456,43 @@ CURVEFS_ERROR XattrManager::FastCalAllLayerSumInfo(InodeAttr *attr) {
return CURVEFS_ERROR::OK;
}

CURVEFS_ERROR XattrManager::GetXattr(const char* name, std::string *value,
InodeAttr *attr, bool enableSumInDir) {
CURVEFS_ERROR ret = CURVEFS_ERROR::OK;
// get summary info if the xattr name is summary type
if (IsSummaryInfo(name) && attr->type() == FsFileType::TYPE_DIRECTORY) {
// if not enable record summary info in dir xattr,
// need recursive computation all files;
// otherwise only recursive computation all dirs.
if (!enableSumInDir) {
if (IsOneLayer(name)) {
ret = CalOneLayerSumInfo(attr);
} else {
ret = CalAllLayerSumInfo(attr);
}
} else {
if (IsOneLayer(name)) {
ret = FastCalOneLayerSumInfo(attr);
} else {
ret = FastCalAllLayerSumInfo(attr);
}
}

if (CURVEFS_ERROR::OK != ret) {
return ret;
}
LOG(INFO) << "After calculate summary info:\n"
<< attr->DebugString();
}

auto it = attr->xattr().find(name);
if (it != attr->xattr().end()) {
*value = it->second;
}
return ret;
}


CURVEFS_ERROR XattrManager::UpdateParentInodeXattr(uint64_t parentId,
const XAttr &xattr, bool direction) {
VLOG(9) << "UpdateParentInodeXattr inodeId = " << parentId
Expand Down
17 changes: 10 additions & 7 deletions curvefs/src/client/xattr_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,8 @@ class XattrManager {
isStop_.store(true);
}

CURVEFS_ERROR CalOneLayerSumInfo(InodeAttr *attr);

CURVEFS_ERROR CalAllLayerSumInfo(InodeAttr *attr);

CURVEFS_ERROR FastCalOneLayerSumInfo(InodeAttr *attr);

CURVEFS_ERROR FastCalAllLayerSumInfo(InodeAttr *attr);
CURVEFS_ERROR GetXattr(const char* name, std::string *value,
InodeAttr *attr, bool enableSumInDir);

CURVEFS_ERROR UpdateParentInodeXattr(uint64_t parentId,
const XAttr &xattr, bool direction);
Expand Down Expand Up @@ -116,6 +111,14 @@ class XattrManager {
Atomic<uint32_t> *inflightNum,
Atomic<bool> *ret);

CURVEFS_ERROR CalOneLayerSumInfo(InodeAttr *attr);

CURVEFS_ERROR CalAllLayerSumInfo(InodeAttr *attr);

CURVEFS_ERROR FastCalOneLayerSumInfo(InodeAttr *attr);

CURVEFS_ERROR FastCalAllLayerSumInfo(InodeAttr *attr);

private:
// inode cache manager
std::shared_ptr<InodeCacheManager> inodeManager_;
Expand Down
54 changes: 53 additions & 1 deletion curvefs/test/client/test_fuse_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ TEST_F(TestFuseVolumeClient, FuseOpInit_when_fs_exist) {

EXPECT_CALL(*blockDeviceClient_, Open(_, _))
.WillOnce(Return(true));

CURVEFS_ERROR ret = client_->FuseOpInit(&mOpts, nullptr);
ASSERT_EQ(CURVEFS_ERROR::OK, ret);

Expand Down Expand Up @@ -3484,5 +3483,58 @@ TEST_F(TestFuseS3Client, FuseOpListXattr) {
ASSERT_EQ(CURVEFS_ERROR::OK, ret);
}

TEST_F(TestFuseS3Client, FuseOpSetXattr_TooLong) {
// in
fuse_req_t req;
fuse_ino_t ino = 1;
const char name[] = "security.selinux";
size_t size = 300;
char value[300];
std::memset(value, 0, 300);

CURVEFS_ERROR ret = client_->FuseOpSetXattr(
req, ino, name, value, size, 0);
ASSERT_EQ(CURVEFS_ERROR::OUT_OF_RANGE, ret);
}

TEST_F(TestFuseS3Client, FuseOpSetXattr) {
// in
fuse_req_t req;
fuse_ino_t ino = 1;
const char name[] = "security.selinux";
size_t size = 100;
char value[100];
std::memset(value, 0, 100);

// get inode failed
EXPECT_CALL(*inodeManager_, GetInode(ino, _))
.WillOnce(Return(CURVEFS_ERROR::INTERNAL));
CURVEFS_ERROR ret = client_->FuseOpSetXattr(
req, ino, name, value, size, 0);
ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret);

// updateInode failed
auto inodeWrapper = std::make_shared<InodeWrapper>(Inode(), metaClient_);
EXPECT_CALL(*inodeManager_, GetInode(ino, _))
.WillOnce(
DoAll(SetArgReferee<1>(inodeWrapper), Return(CURVEFS_ERROR::OK)));
EXPECT_CALL(*metaClient_, UpdateInodeAttrWithOutNlink(_, _, _, _, _))
.WillOnce(Return(MetaStatusCode::NOT_FOUND));
ret = client_->FuseOpSetXattr(
req, ino, name, value, size, 0);
ASSERT_EQ(CURVEFS_ERROR::NOTEXIST, ret);

// success
EXPECT_CALL(*inodeManager_, GetInode(ino, _))
.WillOnce(
DoAll(SetArgReferee<1>(inodeWrapper), Return(CURVEFS_ERROR::OK)));
EXPECT_CALL(*metaClient_, UpdateInodeAttrWithOutNlink(_, _, _, _, _))
.WillOnce(Return(MetaStatusCode::OK));
ret = client_->FuseOpSetXattr(
req, ino, name, value, size, 0);
ASSERT_EQ(CURVEFS_ERROR::OK, ret);
}


} // namespace client
} // namespace curvefs
8 changes: 8 additions & 0 deletions curvefs/test/client/test_inodeWrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,5 +333,13 @@ TEST_F(TestInodeWrapper, TestUpdateInodeAttrIncrementally) {
ASSERT_FALSE(wrapper.dirtyAttr_.has_atime_ns());
}

TEST_F(TestInodeWrapper, TestSetXattr) {
inodeWrapper_->SetXattrLocked("name", "value");
XAttr xattr = inodeWrapper_->GetXattr();
ASSERT_TRUE(xattr.xattrinfos().find("name") != xattr.xattrinfos().end());
ASSERT_EQ((*xattr.mutable_xattrinfos())["name"], "value");
ASSERT_TRUE(inodeWrapper_->IsDirty());
}

} // namespace client
} // namespace curvefs