diff --git a/curvefs/proto/metaserver.proto b/curvefs/proto/metaserver.proto index ecb329b04b..6048e59482 100644 --- a/curvefs/proto/metaserver.proto +++ b/curvefs/proto/metaserver.proto @@ -218,7 +218,7 @@ message Inode { map s3ChunkInfoMap = 18; // TYPE_S3 only, first is chunk index optional uint32 dtime = 19; optional uint32 openmpcount = 20; // openmpcount mount points had the file open - map xattr = 21; + map xattr = 21; repeated uint64 parent = 22; } @@ -314,7 +314,7 @@ message UpdateInodeRequest { map s3ChunkInfoMap = 17; optional uint32 nlink = 18; // field 19 is left for compatibility - map xattr = 20; + map xattr = 20; repeated uint64 parent = 21; map s3ChunkInfoAdd = 22; optional VolumeExtentList volumeExtents = 23; @@ -407,7 +407,7 @@ message InodeAttr { optional uint64 rdev = 16; optional uint32 dtime = 17; optional uint32 openmpcount = 18; - map xattr = 19; + map xattr = 19; repeated uint64 parent = 20; } @@ -438,7 +438,7 @@ message BatchGetXAttrRequest { message XAttr { required uint64 inodeId = 1; required uint32 fsId = 2; - map xAttrInfos = 3; + map xAttrInfos = 3; } message BatchGetXAttrResponse { diff --git a/curvefs/src/client/common/common.h b/curvefs/src/client/common/common.h index eb55d716e0..836367a6eb 100644 --- a/curvefs/src/client/common/common.h +++ b/curvefs/src/client/common/common.h @@ -64,7 +64,8 @@ enum class MetaServerOpType { std::ostream &operator<<(std::ostream &os, MetaServerOpType optype); -const uint32_t MAXXATTRLENGTH = 256; +const uint32_t MAX_XATTR_NAME_LENGTH = 255; +const uint32_t MAX_XATTR_VALUE_LENGTH = 64 * 1024; const char kCurveFsWarmupXAttr[] = "curvefs.warmup.op"; diff --git a/curvefs/src/client/curve_fuse_op.cpp b/curvefs/src/client/curve_fuse_op.cpp index 7abee3d020..a9e5afcefa 100644 --- a/curvefs/src/client/curve_fuse_op.cpp +++ b/curvefs/src/client/curve_fuse_op.cpp @@ -49,7 +49,6 @@ using ::curvefs::client::FuseClient; using ::curvefs::client::FuseS3Client; using ::curvefs::client::FuseVolumeClient; using ::curvefs::client::common::FuseClientOption; -using ::curvefs::client::common::MAXXATTRLENGTH; using ::curvefs::client::rpcclient::MdsClientImpl; using ::curvefs::client::rpcclient::MDSBaseClient; using ::curvefs::client::metric::ClientOpMetric; @@ -344,9 +343,9 @@ void FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino, const char *name, size_t size) { InflightGuard guard(&g_clientOpMetric->opGetXattr.inflightOpNum); LatencyUpdater updater(&g_clientOpMetric->opGetXattr.latency); - char buf[MAXXATTRLENGTH] = {0}; + std::string buf; CURVEFS_ERROR ret = g_ClientInstance->FuseOpGetXattr(req, ino, name, - buf, size); + &buf, size); if (ret != CURVEFS_ERROR::OK) { g_clientOpMetric->opGetXattr.ecount << 1; FuseReplyErrByErrCode(req, ret); @@ -354,9 +353,9 @@ void FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino, const char *name, } if (size == 0) { - fuse_reply_xattr(req, strlen(buf)); + fuse_reply_xattr(req, buf.length()); } else { - fuse_reply_buf(req, buf, strlen(buf)); + fuse_reply_buf(req, buf.data(), buf.length()); } } diff --git a/curvefs/src/client/fuse_client.cpp b/curvefs/src/client/fuse_client.cpp index ac39a3a64a..622b280f8a 100644 --- a/curvefs/src/client/fuse_client.cpp +++ b/curvefs/src/client/fuse_client.cpp @@ -51,7 +51,8 @@ using ::curvefs::common::S3Info; using ::curvefs::common::Volume; using ::curvefs::mds::topology::PartitionTxId; using ::curvefs::mds::FSStatusCode_Name; -using ::curvefs::client::common::MAXXATTRLENGTH; +using ::curvefs::client::common::MAX_XATTR_NAME_LENGTH; +using ::curvefs::client::common::MAX_XATTR_VALUE_LENGTH; using ::curvefs::client::common::FileHandle; #define RETURN_IF_UNSUCCESS(action) \ @@ -1325,7 +1326,7 @@ CURVEFS_ERROR FuseClient::FuseOpSetAttr(fuse_req_t req, fuse_ino_t ino, } CURVEFS_ERROR FuseClient::FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino, - const char* name, void* value, + const char* name, std::string* value, size_t size) { VLOG(9) << "FuseOpGetXattr, ino: " << ino << ", name: " << name << ", size = " << size; @@ -1333,7 +1334,6 @@ CURVEFS_ERROR FuseClient::FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino, return CURVEFS_ERROR::NOTSUPPORT; } - std::string xValue; InodeAttr inodeAttr; CURVEFS_ERROR ret = inodeManager_->GetInodeAttr(ino, &inodeAttr); if (ret != CURVEFS_ERROR::OK) { @@ -1342,17 +1342,20 @@ CURVEFS_ERROR FuseClient::FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino, return ret; } - ret = xattrManager_->GetXattr(name, &xValue, &inodeAttr, enableSumInDir_); + ret = xattrManager_->GetXattr(name, value, &inodeAttr, enableSumInDir_); if (CURVEFS_ERROR::OK != ret) { LOG(ERROR) << "xattrManager get xattr failed, name = " << name; return ret; } ret = CURVEFS_ERROR::NODATA; - if (xValue.length() > 0) { - if ((size == 0 && xValue.length() <= MAXXATTRLENGTH) || - (size >= xValue.length() && xValue.length() <= MAXXATTRLENGTH)) { - memcpy(value, xValue.c_str(), xValue.length()); + if (value->length() > 0) { + if ((size == 0 && value->length() <= MAX_XATTR_VALUE_LENGTH) || + (size >= value->length() && + value->length() <= MAX_XATTR_VALUE_LENGTH)) { + VLOG(1) << "FuseOpGetXattr name = " << name + << ", length = " << value->length() + << ", value = " << *value; ret = CURVEFS_ERROR::OK; } else { ret = CURVEFS_ERROR::OUT_OF_RANGE; @@ -1364,15 +1367,17 @@ CURVEFS_ERROR FuseClient::FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino, 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) { return CURVEFS_ERROR::NOTSUPPORT; } std::string strname(name); std::string strvalue(value, size); - if (strname.length() > MAXXATTRLENGTH || size > MAXXATTRLENGTH) { + VLOG(1) << "FuseOpSetXattr ino: " << ino << ", name: " << name + << ", size = " << size + << ", strvalue: " << strvalue; + if (strname.length() > MAX_XATTR_NAME_LENGTH || + size > MAX_XATTR_VALUE_LENGTH) { LOG(ERROR) << "xattr length is too long, name = " << name << ", name length = " << strname.length() << ", value length = " << size; diff --git a/curvefs/src/client/fuse_client.h b/curvefs/src/client/fuse_client.h index 5c6b6781a4..50ceb9726a 100644 --- a/curvefs/src/client/fuse_client.h +++ b/curvefs/src/client/fuse_client.h @@ -192,7 +192,7 @@ class FuseClient { struct stat* attrOut); virtual CURVEFS_ERROR FuseOpGetXattr(fuse_req_t req, fuse_ino_t ino, - const char* name, void* value, + const char* name, std::string* value, size_t size); virtual CURVEFS_ERROR FuseOpSetXattr(fuse_req_t req, fuse_ino_t ino, diff --git a/curvefs/test/client/test_fuse_s3_client.cpp b/curvefs/test/client/test_fuse_s3_client.cpp index 59bc7120d2..d8f4b83cee 100644 --- a/curvefs/test/client/test_fuse_s3_client.cpp +++ b/curvefs/test/client/test_fuse_s3_client.cpp @@ -1018,11 +1018,9 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotSummaryInfo) { fuse_ino_t ino = 1; const char name[] = "security.selinux"; size_t size = 100; - char value[100]; - std::memset(value, 0, 100); + std::string value; - CURVEFS_ERROR ret = client_->FuseOpGetXattr( - req, ino, name, static_cast(value), size); + CURVEFS_ERROR ret = client_->FuseOpGetXattr(req, ino, name, &value, size); ASSERT_EQ(CURVEFS_ERROR::NODATA, ret); } @@ -1033,8 +1031,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir) { const char rname[] = "curve.dir.rfbytes"; const char name[] = "curve.dir.fbytes"; size_t size = 100; - char value[100]; - std::memset(value, 0, 100); + std::string value; // out uint32_t fsId = 1; @@ -1110,10 +1107,9 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir) { .WillOnce( DoAll(SetArgPointee<1>(attrs1), Return(CURVEFS_ERROR::OK))); - CURVEFS_ERROR ret = client_->FuseOpGetXattr( - req, ino, rname, static_cast(value), size); + CURVEFS_ERROR ret = client_->FuseOpGetXattr(req, ino, rname, &value, size); ASSERT_EQ(CURVEFS_ERROR::OK, ret); - ASSERT_EQ(std::string(value), "4596"); + ASSERT_EQ(value, "4596"); EXPECT_CALL(*inodeManager_, GetInodeAttr(ino, _)) .WillOnce( @@ -1125,10 +1121,9 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir) { .WillOnce( DoAll(SetArgPointee<1>(attrs), Return(CURVEFS_ERROR::OK))); - ret = client_->FuseOpGetXattr( - req, ino, name, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, name, &value, size); ASSERT_EQ(CURVEFS_ERROR::OK, ret); - ASSERT_EQ(std::string(value), "4396"); + ASSERT_EQ(value, "4396"); } TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir_Failed) { @@ -1138,8 +1133,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir_Failed) { const char rname[] = "curve.dir.rfbytes"; const char name[] = "curve.dir.fbytes"; size_t size = 100; - char value[100]; - std::memset(value, 0, 100); + std::string value; // out uint32_t fsId = 1; @@ -1176,8 +1170,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir_Failed) { EXPECT_CALL(*inodeManager_, GetInodeAttr(ino, _)) .WillOnce(DoAll(SetArgPointee<1>(inode), Return(CURVEFS_ERROR::INTERNAL))); - CURVEFS_ERROR ret = client_->FuseOpGetXattr( - req, ino, rname, static_cast(value), size); + CURVEFS_ERROR ret = client_->FuseOpGetXattr(req, ino, rname, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // list dentry failed @@ -1186,8 +1179,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir_Failed) { DoAll(SetArgPointee<1>(inode), Return(CURVEFS_ERROR::OK))); EXPECT_CALL(*dentryManager_, ListDentry(_, _, _, _, _)) .WillOnce(Return(CURVEFS_ERROR::NOTEXIST)); - ret = client_->FuseOpGetXattr( - req, ino, rname, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, rname, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // BatchGetInodeAttr failed @@ -1199,8 +1191,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir_Failed) { DoAll(SetArgPointee<1>(dlist), Return(CURVEFS_ERROR::OK))); EXPECT_CALL(*inodeManager_, BatchGetInodeAttr(_, _)) .WillOnce(Return(CURVEFS_ERROR::INTERNAL)); - ret = client_->FuseOpGetXattr( - req, ino, rname, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, rname, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // AddUllStringToFirst XATTRFILES failed @@ -1213,8 +1204,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir_Failed) { EXPECT_CALL(*inodeManager_, BatchGetInodeAttr(_, _)) .WillOnce( DoAll(SetArgPointee<1>(attrs), Return(CURVEFS_ERROR::OK))); - ret = client_->FuseOpGetXattr( - req, ino, name, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, name, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // AddUllStringToFirst XATTRSUBDIRS failed @@ -1229,8 +1219,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir_Failed) { EXPECT_CALL(*inodeManager_, BatchGetInodeAttr(_, _)) .WillOnce( DoAll(SetArgPointee<1>(attrs), Return(CURVEFS_ERROR::OK))); - ret = client_->FuseOpGetXattr( - req, ino, name, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, name, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // AddUllStringToFirst XATTRENTRIES failed @@ -1245,8 +1234,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir_Failed) { EXPECT_CALL(*inodeManager_, BatchGetInodeAttr(_, _)) .WillOnce( DoAll(SetArgPointee<1>(attrs), Return(CURVEFS_ERROR::OK))); - ret = client_->FuseOpGetXattr( - req, ino, name, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, name, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // AddUllStringToFirst XATTRFBYTES failed @@ -1261,8 +1249,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_NotEnableSumInDir_Failed) { EXPECT_CALL(*inodeManager_, BatchGetInodeAttr(_, _)) .WillOnce( DoAll(SetArgPointee<1>(attrs), Return(CURVEFS_ERROR::OK))); - ret = client_->FuseOpGetXattr( - req, ino, name, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, name, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); } @@ -1273,8 +1260,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir) { fuse_ino_t ino = 1; const char name[] = "curve.dir.rentries"; size_t size = 100; - char value[100]; - std::memset(value, 0, 100); + std::string value; // out uint32_t fsId = 1; @@ -1330,10 +1316,9 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir) { .WillOnce( DoAll(SetArgPointee<1>(xattrs), Return(CURVEFS_ERROR::OK))); - CURVEFS_ERROR ret = client_->FuseOpGetXattr( - req, ino, name, static_cast(value), size); + CURVEFS_ERROR ret = client_->FuseOpGetXattr(req, ino, name, &value, size); ASSERT_EQ(CURVEFS_ERROR::OK, ret); - ASSERT_EQ(std::string(value), "6"); + ASSERT_EQ(value, "6"); } TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir_Failed) { @@ -1344,8 +1329,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir_Failed) { const char name[] = "curve.dir.entries"; const char rname[] = "curve.dir.rentries"; size_t size = 100; - char value[100]; - std::memset(value, 0, 100); + std::string value; // out uint32_t fsId = 1; @@ -1387,16 +1371,14 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir_Failed) { // get inode failed EXPECT_CALL(*inodeManager_, GetInodeAttr(ino, _)) .WillOnce(Return(CURVEFS_ERROR::INTERNAL)); - CURVEFS_ERROR ret = client_->FuseOpGetXattr( - req, ino, name, static_cast(value), size); + CURVEFS_ERROR ret = client_->FuseOpGetXattr(req, ino, name, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // AddUllStringToFirst failed EXPECT_CALL(*inodeManager_, GetInodeAttr(ino, _)) .WillOnce(DoAll(SetArgPointee<1>(inode), Return(CURVEFS_ERROR::OK))); - ret = client_->FuseOpGetXattr( - req, ino, name, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, name, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); inode.mutable_xattr()->find(XATTRFBYTES)->second = "100"; @@ -1407,8 +1389,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir_Failed) { DoAll(SetArgPointee<1>(inode), Return(CURVEFS_ERROR::OK))); EXPECT_CALL(*dentryManager_, ListDentry(_, _, _, _, _)) .WillOnce(Return(CURVEFS_ERROR::NOTEXIST)); - ret = client_->FuseOpGetXattr( - req, ino, rname, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, rname, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // BatchGetInodeAttr failed @@ -1423,8 +1404,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir_Failed) { DoAll(SetArgPointee<1>(emptyDlist), Return(CURVEFS_ERROR::OK))); EXPECT_CALL(*inodeManager_, BatchGetXAttr(_, _)) .WillOnce(Return(CURVEFS_ERROR::INTERNAL)); - ret = client_->FuseOpGetXattr( - req, ino, rname, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, rname, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // AddUllStringToFirst XATTRFILES failed @@ -1441,8 +1421,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir_Failed) { EXPECT_CALL(*inodeManager_, BatchGetXAttr(_, _)) .WillOnce( DoAll(SetArgPointee<1>(xattrs), Return(CURVEFS_ERROR::OK))); - ret = client_->FuseOpGetXattr( - req, ino, rname, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, rname, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // AddUllStringToFirst XATTRSUBDIRS failed @@ -1460,8 +1439,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir_Failed) { EXPECT_CALL(*inodeManager_, BatchGetXAttr(_, _)) .WillOnce( DoAll(SetArgPointee<1>(xattrs), Return(CURVEFS_ERROR::OK))); - ret = client_->FuseOpGetXattr( - req, ino, rname, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, rname, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); // AddUllStringToFirst XATTRENTRIES failed @@ -1479,8 +1457,7 @@ TEST_F(TestFuseS3Client, FuseOpGetXattr_EnableSumInDir_Failed) { EXPECT_CALL(*inodeManager_, BatchGetXAttr(_, _)) .WillOnce( DoAll(SetArgPointee<1>(xattrs), Return(CURVEFS_ERROR::OK))); - ret = client_->FuseOpGetXattr( - req, ino, rname, static_cast(value), size); + ret = client_->FuseOpGetXattr(req, ino, rname, &value, size); ASSERT_EQ(CURVEFS_ERROR::INTERNAL, ret); } @@ -1861,9 +1838,9 @@ TEST_F(TestFuseS3Client, FuseOpSetXattr_TooLong) { 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); + size_t size = 64 * 1024 + 1; + char value[64 * 1024 + 1]; + std::memset(value, 0, size); CURVEFS_ERROR ret = client_->FuseOpSetXattr( req, ino, name, value, size, 0);