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

curvefs: implement setxattr interface. #1935

Merged
merged 1 commit into from
Oct 13, 2022
Merged

Conversation

SeanHai
Copy link
Contributor

@SeanHai SeanHai commented Sep 23, 2022

Signed-off-by: wanghai01 [email protected]

What problem does this PR solve?

Issue Number: #xxx

Problem Summary:

What is changed and how it works?

What's Changed:

How it Works:

Side effects(Breaking backward compatibility? Performance regression?):

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

std::string xattrValue(value, size);
VLOG(9) << "FuseOpSetXattr"
<< " ino " << ino << " name " << name << " value " << xattrValue
<< " flags " << flags;
int code = ENOTSUP;
int code = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

code only use in if, move it in the if

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

}

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

@SeanHai SeanHai force-pushed the setxattr branch 2 times, most recently from f401834 to 7ffbdfc Compare September 27, 2022 07:25
CURVEFS_ERROR FuseClient::FuseOpSetXattr(fuse_req_t req, fuse_ino_t ino,
const char* name, const char* value,
size_t size, int flags) {
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

} else {
ret = xattrManager_->CalAllLayerSumInfo(&inodeAttr);
}
// get summary info
Copy link
Contributor

Choose a reason for hiding this comment

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

The following conditions are different, please describe the function of this code as a whole.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


::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

@SeanHai
Copy link
Contributor Author

SeanHai commented Oct 10, 2022

recheck

// otherwise only recursive computation all dirs.
if (!enableSumInDir_) {
if (IsOneLayer(name)) {
ret = xattrManager_->CalOneLayerSumInfo(&inodeAttr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe enableSumInDir_ is more suitable as a member of xattrManager_

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

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

@SeanHai SeanHai force-pushed the setxattr branch 3 times, most recently from 37da493 to 66c7e38 Compare October 13, 2022 01:46
@SeanHai SeanHai merged commit fa27f31 into opencurve:master1 Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants