ceph: add support for getvxattr op
Subsystems (2)
v6 cover ceph: add support for getvxattr op
Changes to v6:
* only auth mds session is tested for presence of getvxattr feature
* removed function to test feature cluster-wide
Milind Changire (1):
ceph: add getvxattr op
fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ fs/ceph/mds_client.c | 27 ++++++++++++++++++- fs/ceph/mds_client.h | 12 ++++++++- fs/ceph/strings.c | 1 + fs/ceph/super.h | 1 + fs/ceph/xattr.c | 17 ++++++++++++ include/linux/ceph/ceph_fs.h | 1 + 7 files changed, 108 insertions(+), 2 deletions(-)
Signature · 1 line
2.31.1
PATCH v6 1/1 ceph: add getvxattr op
Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems.
Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go.
As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future.
URL: https://tracker.ceph.com/issues/51062
--- fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ fs/ceph/mds_client.c | 27 ++++++++++++++++++- fs/ceph/mds_client.h | 12 ++++++++- fs/ceph/strings.c | 1 + fs/ceph/super.h | 1 + fs/ceph/xattr.c | 17 ++++++++++++ include/linux/ceph/ceph_fs.h | 1 + 7 files changed, 108 insertions(+), 2 deletions(-)
fs/ceph/inode.c +51 -0
@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,return err; } +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value, +size_t size) +{ +struct ceph_fs_client *fsc =ceph_sb_to_client (inode->i_sb); +struct ceph_mds_client *mdsc = fsc->mdsc; +struct ceph_mds_request *req; +int mode =USE_AUTH_MDS ; +int err; +char *xattr_value; +size_t xattr_value_len; + + req =ceph_mdsc_create_request (mdsc,CEPH_MDS_OP_GETVXATTR , mode); +if (IS_ERR (req)) { + err = -ENOMEM ; + goto out; + } + + req->r_path2 =kstrdup (name,GFP_NOFS ); +if (!req->r_path2) { + err = -ENOMEM ; + goto put; + } + +ihold (inode); + req->r_inode = inode; + err =ceph_mdsc_do_request (mdsc,NULL , req); +if (err <0 ) + goto put; + + xattr_value = req->r_reply_info.xattr_info.xattr_value; + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; + +dout ("do_getvxattr xattr_value_len:%zu, size:%zu\n" , xattr_value_len, size); + + err = (int )xattr_value_len; +if (size ==0 ) + goto put; + +if (xattr_value_len > size) { + err = -ERANGE ; + goto put; + } + +memcpy (value, xattr_value, xattr_value_len); +put: +ceph_mdsc_put_request (req); +out: +dout ("do_getvxattr result=%d\n" , err); +return err; +} +/* * Check inode permissions. We verify we have a valid value for
fs/ceph/mds_client.c +26 -1
@@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end,return -EIO ; } +static int parse_reply_info_getvxattr (void **p,void *end, +struct ceph_mds_reply_info_parsed *info, +u64 features) +{ +u8 struct_v, struct_compat; +u32 struct_len; +u32 value_len; + +ceph_decode_8_safe (p, end, struct_v, bad); +ceph_decode_8_safe (p, end, struct_compat, bad); +ceph_decode_32_safe (p, end, struct_len, bad); +ceph_decode_32_safe (p, end, value_len, bad); + +if (value_len == end - *p) { + info->xattr_info.xattr_value = *p; + info->xattr_info.xattr_value_len = end - *p; + *p = end; +return info->xattr_info.xattr_value_len; + } +bad: +return -EIO ; +} +/* * parse extra results */
@@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end,return parse_reply_info_readdir (p, end, info, features); else if (op == CEPH_MDS_OP_CREATE) return parse_reply_info_create(p, end, info, features, s); + else if (op == CEPH_MDS_OP_GETVXATTR) + return parse_reply_info_getvxattr(p, end, info, features); else return -EIO; }
@@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,if (p != end) goto bad; -return 0 ; +return err; bad: err = -EIO ;
fs/ceph/mds_client.h +11 -1
@@ -29,8 +29,10 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT ,CEPHFS_FEATURE_DELEG_INO ,CEPHFS_FEATURE_METRIC_COLLECT , +CEPHFS_FEATURE_ALTERNATE_NAME , +CEPHFS_FEATURE_GETVXATTR , - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR, };/*
@@ -45,6 +47,8 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT , \CEPHFS_FEATURE_DELEG_INO , \CEPHFS_FEATURE_METRIC_COLLECT , \ +CEPHFS_FEATURE_ALTERNATE_NAME , \ +CEPHFS_FEATURE_GETVXATTR , \ \ CEPHFS_FEATURE_MAX, \ }
@@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry {loff_t offset; }; +struct ceph_mds_reply_xattr { +char *xattr_value; +size_t xattr_value_len; +}; +/* * parsed info about an mds reply, including information about * either: 1) the target inode and/or its parent directory and dentry,
@@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed {char *dname;u32 dname_len;struct ceph_mds_reply_lease *dlease; +struct ceph_mds_reply_xattr xattr_info;/* extra */ union {
fs/ceph/strings.c +1 -0
@@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op)case CEPH_MDS_OP_LOOKUPINO :return "lookupino" ;case CEPH_MDS_OP_LOOKUPNAME :return "lookupname" ;case CEPH_MDS_OP_GETATTR :return "getattr" ; +case CEPH_MDS_OP_GETVXATTR :return "getvxattr" ;case CEPH_MDS_OP_SETXATTR :return "setxattr" ;case CEPH_MDS_OP_SETATTR :return "setattr" ;case CEPH_MDS_OP_RMXATTR :return "rmxattr" ;
fs/ceph/super.h +1 -0
@@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode)/* xattr.c */ int __ceph_setxattr (struct inode *,const char *,const void *,size_t ,int ); +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value,size_t size);ssize_t __ceph_getxattr (struct inode *,const char *,void *,size_t );extern ssize_t ceph_listxattr (struct dentry *,char *,size_t );extern struct ceph_buffer *__ceph_build_xattrs_blob (struct ceph_inode_info *ci);
fs/ceph/xattr.c +17 -0
@@ -924,6 +924,7 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,struct ceph_inode_info *ci =ceph_inode (inode);struct ceph_inode_xattr *xattr;struct ceph_vxattr *vxattr =NULL ; +struct ceph_mds_session *session =NULL ;int req_mask;ssize_t err;
@@ -945,6 +946,22 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, err = -ERANGE ; }return err; + }else { + err = -ENODATA ; +spin_lock (&ci->i_ceph_lock); +if (strncmp (name,XATTR_CEPH_PREFIX ,XATTR_CEPH_PREFIX_LEN )) + goto out; +/* check if the auth mds supports the getvxattr feature */ + session = ci->i_auth_cap->session; +if (!session) + goto out; + +if (test_bit (CEPHFS_FEATURE_GETVXATTR , &session->s_features)) { +spin_unlock (&ci->i_ceph_lock); + err =ceph_do_getvxattr (inode, name, value, size); +spin_lock (&ci->i_ceph_lock); + } + goto out; } req_mask =__get_request_mask (inode);
include/linux/ceph/ceph_fs.h +1 -0
@@ -328,6 +328,7 @@ enum {CEPH_MDS_OP_LOOKUPPARENT =0x00103 ,CEPH_MDS_OP_LOOKUPINO =0x00104 ,CEPH_MDS_OP_LOOKUPNAME =0x00105 , +CEPH_MDS_OP_GETVXATTR =0x00106 ,CEPH_MDS_OP_SETXATTR =0x01105 , CEPH_MDS_OP_RMXATTR = 0x01106,
Signature · 1 line
2.31.1
Re: [PATCH v6 1/1] ceph: add getvxattr op
Quoted 134 lines · Milind Changire
Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems.Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go.
As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future.URL: https://tracker.ceph.com/issues/51062
Signed-off-by Milind Changire <[email protected]>--- fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ fs/ceph/mds_client.c | 27 ++++++++++++++++++- fs/ceph/mds_client.h | 12 ++++++++- fs/ceph/strings.c | 1 + fs/ceph/super.h | 1 + fs/ceph/xattr.c | 17 ++++++++++++ include/linux/ceph/ceph_fs.h | 1 + 7 files changed, 108 insertions(+), 2 deletions(-)fs/ceph/inode.c +51
-0@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,return err; } +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value, +size_t size) +{ +struct ceph_fs_client *fsc =ceph_sb_to_client (inode->i_sb); +struct ceph_mds_client *mdsc = fsc->mdsc; +struct ceph_mds_request *req; +int mode =USE_AUTH_MDS ; +int err; +char *xattr_value; +size_t xattr_value_len; + + req =ceph_mdsc_create_request (mdsc,CEPH_MDS_OP_GETVXATTR , mode); +if (IS_ERR (req)) { + err = -ENOMEM ; + goto out; + } + + req->r_path2 =kstrdup (name,GFP_NOFS ); +if (!req->r_path2) { + err = -ENOMEM ; + goto put; + } + +ihold (inode); + req->r_inode = inode; + err =ceph_mdsc_do_request (mdsc,NULL , req); +if (err <0 ) + goto put; + + xattr_value = req->r_reply_info.xattr_info.xattr_value; + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; + +dout ("do_getvxattr xattr_value_len:%zu, size:%zu\n" , xattr_value_len, size); + + err = (int )xattr_value_len; +if (size ==0 ) + goto put; + +if (xattr_value_len > size) { + err = -ERANGE ; + goto put; + } + +memcpy (value, xattr_value, xattr_value_len); +put: +ceph_mdsc_put_request (req); +out: +dout ("do_getvxattr result=%d\n" , err); +return err; +} +/* * Check inode permissions. We verify we have a valid value for fs/ceph/mds_client.c +25
-0@@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end,return -EIO ; } +static int parse_reply_info_getvxattr (void **p,void *end, +struct ceph_mds_reply_info_parsed *info, +u64 features) +{ +u8 struct_v, struct_compat; +u32 struct_len; +u32 value_len; + +ceph_decode_8_safe (p, end, struct_v, bad); +ceph_decode_8_safe (p, end, struct_compat, bad); +ceph_decode_32_safe (p, end, struct_len, bad); +ceph_decode_32_safe (p, end, value_len, bad); + +if (value_len == end - *p) { + info->xattr_info.xattr_value = *p; + info->xattr_info.xattr_value_len = end - *p; + *p = end; +return info->xattr_info.xattr_value_len; + } +bad: +return -EIO ; +} +/* * parse extra results */ @@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end,return parse_reply_info_readdir (p, end, info, features); else if (op == CEPH_MDS_OP_CREATE) return parse_reply_info_create(p, end, info, features, s); + else if (op == CEPH_MDS_OP_GETVXATTR) + return parse_reply_info_getvxattr(p, end, info, features); else return -EIO; }
fs/ceph/mds_client.c static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg, · line 644
Quoted 6 lines
fs/ceph/mds_client.c +1
-1@@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,if (p != end) goto bad; -return 0 ; +return err;
Why this delta? It looks like this function could now return err uninitialized with it.
Quoted 84 lines
fs/ceph/mds_client.c +0
-0bad: err = -EIO ;fs/ceph/mds_client.h +11
-1@@ -29,8 +29,10 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT ,CEPHFS_FEATURE_DELEG_INO ,CEPHFS_FEATURE_METRIC_COLLECT , +CEPHFS_FEATURE_ALTERNATE_NAME , +CEPHFS_FEATURE_GETVXATTR , - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR, };/* @@ -45,6 +47,8 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT , \CEPHFS_FEATURE_DELEG_INO , \CEPHFS_FEATURE_METRIC_COLLECT , \ +CEPHFS_FEATURE_ALTERNATE_NAME , \ +CEPHFS_FEATURE_GETVXATTR , \ \ CEPHFS_FEATURE_MAX, \ }@@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry {loff_t offset; }; +struct ceph_mds_reply_xattr { +char *xattr_value; +size_t xattr_value_len; +}; +/* * parsed info about an mds reply, including information about * either: 1) the target inode and/or its parent directory and dentry, @@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed {char *dname;u32 dname_len;struct ceph_mds_reply_lease *dlease; +struct ceph_mds_reply_xattr xattr_info;/* extra */ union {fs/ceph/strings.c +1
-0@@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op)case CEPH_MDS_OP_LOOKUPINO :return "lookupino" ;case CEPH_MDS_OP_LOOKUPNAME :return "lookupname" ;case CEPH_MDS_OP_GETATTR :return "getattr" ; +case CEPH_MDS_OP_GETVXATTR :return "getvxattr" ;case CEPH_MDS_OP_SETXATTR :return "setxattr" ;case CEPH_MDS_OP_SETATTR :return "setattr" ;case CEPH_MDS_OP_RMXATTR :return "rmxattr" ;fs/ceph/super.h +1
-0@@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode)/* xattr.c */ int __ceph_setxattr (struct inode *,const char *,const void *,size_t ,int ); +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value,size_t size);ssize_t __ceph_getxattr (struct inode *,const char *,void *,size_t );extern ssize_t ceph_listxattr (struct dentry *,char *,size_t );extern struct ceph_buffer *__ceph_build_xattrs_blob (struct ceph_inode_info *ci);fs/ceph/xattr.c +1
-0@@ -924,6 +924,7 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,struct ceph_inode_info *ci =ceph_inode (inode);struct ceph_inode_xattr *xattr;struct ceph_vxattr *vxattr =NULL ; +struct ceph_mds_session *session =NULL ;int req_mask;ssize_t err;
fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 953
Quoted 8 lines
fs/ceph/xattr.c +5
-0@@ -945,6 +946,22 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, err = -ERANGE ; }return err; + }else { + err = -ENODATA ; +spin_lock (&ci->i_ceph_lock); +if (strncmp (name,XATTR_CEPH_PREFIX ,XATTR_CEPH_PREFIX_LEN )) + goto out;
This looks much better. One thing I'd probably change is to make this do this strncmp first, before calling ceph_match_vxattr. That way we can avoid walking the vxattr list unless the thing starts with "ceph.".
fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 955
Quoted 2 lines
fs/ceph/xattr.c +2
-0+/* check if the auth mds supports the getvxattr feature */ + session = ci->i_auth_cap->session;
i_auth_cap can be NULL, so this may end up being a NULL pointer dereference. You probably also need to be holding the i_ceph_lock when you check it, since that's what protects it.
But, beyond that, the logic here is problematic. This is going to just fail with -ENODATA if we don't happen to have auth caps. That seems wrong. I think if you don't have auth caps, then you should probably walk the sessions array and see if at least one MDS supports GETVXATTR. If it does, then go ahead and do the call.
Quoted 24 lines
fs/ceph/xattr.c +9
-0+if (!session) + goto out; + +if (test_bit (CEPHFS_FEATURE_GETVXATTR , &session->s_features)) { +spin_unlock (&ci->i_ceph_lock); + err =ceph_do_getvxattr (inode, name, value, size); +spin_lock (&ci->i_ceph_lock); + } + goto out; } req_mask =__get_request_mask (inode);include/linux/ceph/ceph_fs.h +1
-0@@ -328,6 +328,7 @@ enum {CEPH_MDS_OP_LOOKUPPARENT =0x00103 ,CEPH_MDS_OP_LOOKUPINO =0x00104 ,CEPH_MDS_OP_LOOKUPNAME =0x00105 , +CEPH_MDS_OP_GETVXATTR =0x00106 ,CEPH_MDS_OP_SETXATTR =0x01105 , CEPH_MDS_OP_RMXATTR = 0x01106,
Signature · 1 line
Jeff Layton <[email protected]>
Re: [PATCH v6 1/1] ceph: add getvxattr op
Quoted 137 lines · Jeff Layton
Quoted 135 lines · Milind Changire
Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems.Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go.
As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future.URL: https://tracker.ceph.com/issues/51062
Signed-off-by Milind Changire <[email protected]>--- fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ fs/ceph/mds_client.c | 27 ++++++++++++++++++- fs/ceph/mds_client.h | 12 ++++++++- fs/ceph/strings.c | 1 + fs/ceph/super.h | 1 + fs/ceph/xattr.c | 17 ++++++++++++ include/linux/ceph/ceph_fs.h | 1 + 7 files changed, 108 insertions(+), 2 deletions(-)fs/ceph/inode.c +51
-0@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,return err; } +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value, +size_t size) +{ +struct ceph_fs_client *fsc =ceph_sb_to_client (inode->i_sb); +struct ceph_mds_client *mdsc = fsc->mdsc; +struct ceph_mds_request *req; +int mode =USE_AUTH_MDS ; +int err; +char *xattr_value; +size_t xattr_value_len; + + req =ceph_mdsc_create_request (mdsc,CEPH_MDS_OP_GETVXATTR , mode); +if (IS_ERR (req)) { + err = -ENOMEM ; + goto out; + } + + req->r_path2 =kstrdup (name,GFP_NOFS ); +if (!req->r_path2) { + err = -ENOMEM ; + goto put; + } + +ihold (inode); + req->r_inode = inode; + err =ceph_mdsc_do_request (mdsc,NULL , req); +if (err <0 ) + goto put; + + xattr_value = req->r_reply_info.xattr_info.xattr_value; + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; + +dout ("do_getvxattr xattr_value_len:%zu, size:%zu\n" , xattr_value_len, size); + + err = (int )xattr_value_len; +if (size ==0 ) + goto put; + +if (xattr_value_len > size) { + err = -ERANGE ; + goto put; + } + +memcpy (value, xattr_value, xattr_value_len); +put: +ceph_mdsc_put_request (req); +out: +dout ("do_getvxattr result=%d\n" , err); +return err; +} +/* * Check inode permissions. We verify we have a valid value for fs/ceph/mds_client.c +25
-0@@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end,return -EIO ; } +static int parse_reply_info_getvxattr (void **p,void *end, +struct ceph_mds_reply_info_parsed *info, +u64 features) +{ +u8 struct_v, struct_compat; +u32 struct_len; +u32 value_len; + +ceph_decode_8_safe (p, end, struct_v, bad); +ceph_decode_8_safe (p, end, struct_compat, bad); +ceph_decode_32_safe (p, end, struct_len, bad); +ceph_decode_32_safe (p, end, value_len, bad); + +if (value_len == end - *p) { + info->xattr_info.xattr_value = *p; + info->xattr_info.xattr_value_len = end - *p; + *p = end; +return info->xattr_info.xattr_value_len; + } +bad: +return -EIO ; +} +/* * parse extra results */ @@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end,return parse_reply_info_readdir (p, end, info, features); else if (op == CEPH_MDS_OP_CREATE) return parse_reply_info_create(p, end, info, features, s); + else if (op == CEPH_MDS_OP_GETVXATTR) + return parse_reply_info_getvxattr(p, end, info, features); else return -EIO; }@@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,
Quoted 8 lines
fs/ceph/mds_client.c static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg, · line 644
Quoted 5 lines
fs/ceph/mds_client.c +1
-1if (p != end) goto bad; -return 0 ; +return err;Why this delta? It looks like this function could now return err uninitialized with it.
delta undone
this was something left behind while testing to understand the code
Quoted 84 lines
Quoted 84 lines
fs/ceph/mds_client.c +0
-0bad: err = -EIO ;fs/ceph/mds_client.h +11
-1@@ -29,8 +29,10 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT ,CEPHFS_FEATURE_DELEG_INO ,CEPHFS_FEATURE_METRIC_COLLECT , +CEPHFS_FEATURE_ALTERNATE_NAME , +CEPHFS_FEATURE_GETVXATTR , - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR, };/* @@ -45,6 +47,8 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT , \CEPHFS_FEATURE_DELEG_INO , \CEPHFS_FEATURE_METRIC_COLLECT , \ +CEPHFS_FEATURE_ALTERNATE_NAME , \ +CEPHFS_FEATURE_GETVXATTR , \ \ CEPHFS_FEATURE_MAX, \ }@@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry {loff_t offset; }; +struct ceph_mds_reply_xattr { +char *xattr_value; +size_t xattr_value_len; +}; +/* * parsed info about an mds reply, including information about * either: 1) the target inode and/or its parent directory and dentry, @@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed {char *dname;u32 dname_len;struct ceph_mds_reply_lease *dlease; +struct ceph_mds_reply_xattr xattr_info;/* extra */ union {fs/ceph/strings.c +1
-0@@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op)case CEPH_MDS_OP_LOOKUPINO :return "lookupino" ;case CEPH_MDS_OP_LOOKUPNAME :return "lookupname" ;case CEPH_MDS_OP_GETATTR :return "getattr" ; +case CEPH_MDS_OP_GETVXATTR :return "getvxattr" ;case CEPH_MDS_OP_SETXATTR :return "setxattr" ;case CEPH_MDS_OP_SETATTR :return "setattr" ;case CEPH_MDS_OP_RMXATTR :return "rmxattr" ;fs/ceph/super.h +1
-0@@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode)/* xattr.c */ int __ceph_setxattr (struct inode *,const char *,const void *,size_t ,int ); +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value,size_t size);ssize_t __ceph_getxattr (struct inode *,const char *,void *,size_t );extern ssize_t ceph_listxattr (struct dentry *,char *,size_t );extern struct ceph_buffer *__ceph_build_xattrs_blob (struct ceph_inode_info *ci);fs/ceph/xattr.c +1
-0@@ -924,6 +924,7 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,struct ceph_inode_info *ci =ceph_inode (inode);struct ceph_inode_xattr *xattr;struct ceph_vxattr *vxattr =NULL ; +struct ceph_mds_session *session =NULL ;int req_mask;ssize_t err;
Quoted 13 lines
fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 953
Quoted 9 lines
fs/ceph/xattr.c +5
-0@@ -945,6 +946,22 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, err = -ERANGE ; }return err; + }else { + err = -ENODATA ; +spin_lock (&ci->i_ceph_lock); +if (strncmp (name,XATTR_CEPH_PREFIX ,XATTR_CEPH_PREFIX_LEN )) + goto out;This looks much better. One thing I'd probably change is to make this do this strncmp first, before calling
ceph_match_vxattr. That way we can avoid walking the vxattr list unless the thing starts with "ceph.".
done ... check v7 patch
Quoted 9 lines
fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 955
Quoted 3 lines
fs/ceph/xattr.c +2
-0+/* check if the auth mds supports the getvxattr feature */ + session = ci->i_auth_cap->session;
i_auth_capcan be NULL, so this may end up being a NULL pointer dereference. You probably also need to be holding thei_ceph_lockwhen you check it, since that's what protects it.
Quoted 5 lines
But, beyond that, the logic here is problematic. This is going to just fail with
-ENODATAif we don't happen to have auth caps. That seems wrong. I think if you don't have auth caps, then you should probably walk the sessions array and see if at least one MDS supports GETVXATTR. If it does, then go ahead and do the call.
done ... check v7 patch
Quoted 28 lines
Quoted 18 lines
fs/ceph/xattr.c +9
-0+if (!session) + goto out; + +if (test_bit (CEPHFS_FEATURE_GETVXATTR , &session->s_features)) { +spin_unlock (&ci->i_ceph_lock); + err =ceph_do_getvxattr (inode, name, value, size); +spin_lock (&ci->i_ceph_lock); + } + goto out; } req_mask =__get_request_mask (inode);include/linux/ceph/ceph_fs.h +0
-0include/linux/ceph/ceph_fs.h enum { · line 334
Quoted 7 lines
include/linux/ceph/ceph_fs.h +1
-0@@ -328,6 +328,7 @@ enum {CEPH_MDS_OP_LOOKUPPARENT =0x00103 ,CEPH_MDS_OP_LOOKUPINO =0x00104 ,CEPH_MDS_OP_LOOKUPNAME =0x00105 , +CEPH_MDS_OP_GETVXATTR =0x00106 ,CEPH_MDS_OP_SETXATTR =0x01105 , CEPH_MDS_OP_RMXATTR = 0x01106,--
Jeff Layton <[email protected]>
Re: [PATCH v6 1/1] ceph: add getvxattr op
In reply to Milind ChangireQuoted 104 lines · Milind Changire
Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems.Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go.
As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future.URL: https://tracker.ceph.com/issues/51062
Signed-off-by Milind Changire <[email protected]>--- fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ fs/ceph/mds_client.c | 27 ++++++++++++++++++- fs/ceph/mds_client.h | 12 ++++++++- fs/ceph/strings.c | 1 + fs/ceph/super.h | 1 + fs/ceph/xattr.c | 17 ++++++++++++ include/linux/ceph/ceph_fs.h | 1 + 7 files changed, 108 insertions(+), 2 deletions(-)fs/ceph/inode.c +51
-0@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,return err; } +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value, +size_t size) +{ +struct ceph_fs_client *fsc =ceph_sb_to_client (inode->i_sb); +struct ceph_mds_client *mdsc = fsc->mdsc; +struct ceph_mds_request *req; +int mode =USE_AUTH_MDS ; +int err; +char *xattr_value; +size_t xattr_value_len; + + req =ceph_mdsc_create_request (mdsc,CEPH_MDS_OP_GETVXATTR , mode); +if (IS_ERR (req)) { + err = -ENOMEM ; + goto out; + } + + req->r_path2 =kstrdup (name,GFP_NOFS ); +if (!req->r_path2) { + err = -ENOMEM ; + goto put; + } + +ihold (inode); + req->r_inode = inode; + err =ceph_mdsc_do_request (mdsc,NULL , req); +if (err <0 ) + goto put; + + xattr_value = req->r_reply_info.xattr_info.xattr_value; + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; + +dout ("do_getvxattr xattr_value_len:%zu, size:%zu\n" , xattr_value_len, size); + + err = (int )xattr_value_len; +if (size ==0 ) + goto put; + +if (xattr_value_len > size) { + err = -ERANGE ; + goto put; + } + +memcpy (value, xattr_value, xattr_value_len); +put: +ceph_mdsc_put_request (req); +out: +dout ("do_getvxattr result=%d\n" , err); +return err; +} +/* * Check inode permissions. We verify we have a valid value for fs/ceph/mds_client.c +6
-0@@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end,return -EIO ; } +static int parse_reply_info_getvxattr (void **p,void *end, +struct ceph_mds_reply_info_parsed *info, +u64 features) +{ +u8 struct_v, struct_compat; +u32 struct_len;
fs/ceph/mds_client.c static int parse_reply_info_create(void **p, void *end, · line 573
Quoted 10 lines
fs/ceph/mds_client.c +10
-0+u32 value_len; + +ceph_decode_8_safe (p, end, struct_v, bad); +ceph_decode_8_safe (p, end, struct_compat, bad); +ceph_decode_32_safe (p, end, struct_len, bad); +ceph_decode_32_safe (p, end, value_len, bad); + +if (value_len == end - *p) { + info->xattr_info.xattr_value = *p; + info->xattr_info.xattr_value_len = end - *p;
Maybe this could be simplified by 's/end - *p/value_len/'
fs/ceph/mds_client.c static int parse_reply_info_create(void **p, void *end, · line 575
Quoted 2 lines
fs/ceph/mds_client.c +2
-0+ *p = end; +return info->xattr_info.xattr_value_len;
And also here just return 'value_len' instead.
Quoted 100 lines
fs/ceph/mds_client.c +8
-1+ } +bad: +return -EIO ; +} +/* * parse extra results */ @@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end,return parse_reply_info_readdir (p, end, info, features); else if (op == CEPH_MDS_OP_CREATE) return parse_reply_info_create(p, end, info, features, s); + else if (op == CEPH_MDS_OP_GETVXATTR) + return parse_reply_info_getvxattr(p, end, info, features); else return -EIO; }@@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,if (p != end) goto bad; -return 0 ; +return err; bad: err = -EIO ;fs/ceph/mds_client.h +11
-1@@ -29,8 +29,10 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT ,CEPHFS_FEATURE_DELEG_INO ,CEPHFS_FEATURE_METRIC_COLLECT , +CEPHFS_FEATURE_ALTERNATE_NAME , +CEPHFS_FEATURE_GETVXATTR , - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR, };/* @@ -45,6 +47,8 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT , \CEPHFS_FEATURE_DELEG_INO , \CEPHFS_FEATURE_METRIC_COLLECT , \ +CEPHFS_FEATURE_ALTERNATE_NAME , \ +CEPHFS_FEATURE_GETVXATTR , \ \ CEPHFS_FEATURE_MAX, \ }@@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry {loff_t offset; }; +struct ceph_mds_reply_xattr { +char *xattr_value; +size_t xattr_value_len; +}; +/* * parsed info about an mds reply, including information about * either: 1) the target inode and/or its parent directory and dentry, @@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed {char *dname;u32 dname_len;struct ceph_mds_reply_lease *dlease; +struct ceph_mds_reply_xattr xattr_info;/* extra */ union {fs/ceph/strings.c +1
-0@@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op)case CEPH_MDS_OP_LOOKUPINO :return "lookupino" ;case CEPH_MDS_OP_LOOKUPNAME :return "lookupname" ;case CEPH_MDS_OP_GETATTR :return "getattr" ; +case CEPH_MDS_OP_GETVXATTR :return "getvxattr" ;case CEPH_MDS_OP_SETXATTR :return "setxattr" ;case CEPH_MDS_OP_SETATTR :return "setattr" ;case CEPH_MDS_OP_RMXATTR :return "rmxattr" ;fs/ceph/super.h +1
-0@@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode)/* xattr.c */ int __ceph_setxattr (struct inode *,const char *,const void *,size_t ,int ); +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value,size_t size);ssize_t __ceph_getxattr (struct inode *,const char *,void *,size_t );extern ssize_t ceph_listxattr (struct dentry *,char *,size_t );extern struct ceph_buffer *__ceph_build_xattrs_blob (struct ceph_inode_info *ci);fs/ceph/xattr.c +0
-0
fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 927
Quoted 4 lines
fs/ceph/xattr.c +1
-0@@ -924,6 +924,7 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,struct ceph_inode_info *ci =ceph_inode (inode);struct ceph_inode_xattr *xattr;struct ceph_vxattr *vxattr =NULL ; +struct ceph_mds_session *session =NULL ;
No need to init this.
fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 953
Quoted 12 lines
fs/ceph/xattr.c +5
-0int req_mask;ssize_t err;@@ -945,6 +946,22 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, err = -ERANGE ; }return err; + }else { + err = -ENODATA ; +spin_lock (&ci->i_ceph_lock); +if (strncmp (name,XATTR_CEPH_PREFIX ,XATTR_CEPH_PREFIX_LEN )) + goto out;
No need to get the 'i_ceph_lock' for the 'strncmp()' here, and could just return '-EINVAL' directly instead of goto out ?
fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 957
Quoted 4 lines
fs/ceph/xattr.c +4
-0+/* check if the auth mds supports the getvxattr feature */ + session = ci->i_auth_cap->session; +if (!session) + goto out;
In which case will the session be NULL ? If the i_auth_cap exists it will always set the 'session'.
Quoted 22 lines
fs/ceph/xattr.c +7
-0+ +if (test_bit (CEPHFS_FEATURE_GETVXATTR , &session->s_features)) { +spin_unlock (&ci->i_ceph_lock); + err =ceph_do_getvxattr (inode, name, value, size); +spin_lock (&ci->i_ceph_lock); + } + goto out; } req_mask =__get_request_mask (inode);include/linux/ceph/ceph_fs.h +1
-0@@ -328,6 +328,7 @@ enum {CEPH_MDS_OP_LOOKUPPARENT =0x00103 ,CEPH_MDS_OP_LOOKUPINO =0x00104 ,CEPH_MDS_OP_LOOKUPNAME =0x00105 , +CEPH_MDS_OP_GETVXATTR =0x00106 ,CEPH_MDS_OP_SETXATTR =0x01105 , CEPH_MDS_OP_RMXATTR = 0x01106,
Re: [PATCH v6 1/1] ceph: add getvxattr op
Quoted 111 lines · Xiubo Li
Quoted 108 lines · Milind Changire
Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems.Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go.
As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future.URL: https://tracker.ceph.com/issues/51062
Signed-off-by Milind Changire <[email protected]>--- fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ fs/ceph/mds_client.c | 27 ++++++++++++++++++- fs/ceph/mds_client.h | 12 ++++++++- fs/ceph/strings.c | 1 + fs/ceph/super.h | 1 + fs/ceph/xattr.c | 17 ++++++++++++ include/linux/ceph/ceph_fs.h | 1 + 7 files changed, 108 insertions(+), 2 deletions(-)fs/ceph/inode.c +51
-0@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,return err; } +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value, +size_t size) +{ +struct ceph_fs_client *fsc =ceph_sb_to_client (inode->i_sb); +struct ceph_mds_client *mdsc = fsc->mdsc; +struct ceph_mds_request *req; +int mode =USE_AUTH_MDS ; +int err; +char *xattr_value; +size_t xattr_value_len; + + req =ceph_mdsc_create_request (mdsc,CEPH_MDS_OP_GETVXATTR , mode); +if (IS_ERR (req)) { + err = -ENOMEM ; + goto out; + } + + req->r_path2 =kstrdup (name,GFP_NOFS ); +if (!req->r_path2) { + err = -ENOMEM ; + goto put; + } + +ihold (inode); + req->r_inode = inode; + err =ceph_mdsc_do_request (mdsc,NULL , req); +if (err <0 ) + goto put; + + xattr_value = req->r_reply_info.xattr_info.xattr_value; + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; + +dout ("do_getvxattr xattr_value_len:%zu, size:%zu\n" , xattr_value_len, size); + + err = (int )xattr_value_len; +if (size ==0 ) + goto put; + +if (xattr_value_len > size) { + err = -ERANGE ; + goto put; + } + +memcpy (value, xattr_value, xattr_value_len); +put: +ceph_mdsc_put_request (req); +out: +dout ("do_getvxattr result=%d\n" , err); +return err; +} +/* * Check inode permissions. We verify we have a valid value for fs/ceph/mds_client.c +10
-0@@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end,return -EIO ; } +static int parse_reply_info_getvxattr (void **p,void *end, +struct ceph_mds_reply_info_parsed *info, +u64 features) +{ +u8 struct_v, struct_compat; +u32 struct_len; +u32 value_len; + +ceph_decode_8_safe (p, end, struct_v, bad); +ceph_decode_8_safe (p, end, struct_compat, bad);
Quoted 8 lines
fs/ceph/mds_client.c static int parse_reply_info_create(void **p, void *end, · line 573
Quoted 6 lines
fs/ceph/mds_client.c +6
-0+ceph_decode_32_safe (p, end, struct_len, bad); +ceph_decode_32_safe (p, end, value_len, bad); + +if (value_len == end - *p) { + info->xattr_info.xattr_value = *p; + info->xattr_info.xattr_value_len = end - *p;Maybe this could be simplified by 's/end - *p/value_len/'
done ... check v7 patch
Quoted 6 lines
fs/ceph/mds_client.c static int parse_reply_info_create(void **p, void *end, · line 575
Quoted 4 lines
fs/ceph/mds_client.c +2
-0+ *p = end; +return info->xattr_info.xattr_value_len;And also here just return '
value_len' instead.
done ... check v7 patch
Quoted 100 lines
Quoted 100 lines
fs/ceph/mds_client.c +8
-1+ } +bad: +return -EIO ; +} +/* * parse extra results */ @@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end,return parse_reply_info_readdir (p, end, info, features); else if (op == CEPH_MDS_OP_CREATE) return parse_reply_info_create(p, end, info, features, s); + else if (op == CEPH_MDS_OP_GETVXATTR) + return parse_reply_info_getvxattr(p, end, info, features); else return -EIO; }@@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,if (p != end) goto bad; -return 0 ; +return err; bad: err = -EIO ;fs/ceph/mds_client.h +11
-1@@ -29,8 +29,10 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT ,CEPHFS_FEATURE_DELEG_INO ,CEPHFS_FEATURE_METRIC_COLLECT , +CEPHFS_FEATURE_ALTERNATE_NAME , +CEPHFS_FEATURE_GETVXATTR , - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR, };/* @@ -45,6 +47,8 @@ enum ceph_feature_type {CEPHFS_FEATURE_MULTI_RECONNECT , \CEPHFS_FEATURE_DELEG_INO , \CEPHFS_FEATURE_METRIC_COLLECT , \ +CEPHFS_FEATURE_ALTERNATE_NAME , \ +CEPHFS_FEATURE_GETVXATTR , \ \ CEPHFS_FEATURE_MAX, \ }@@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry {loff_t offset; }; +struct ceph_mds_reply_xattr { +char *xattr_value; +size_t xattr_value_len; +}; +/* * parsed info about an mds reply, including information about * either: 1) the target inode and/or its parent directory and dentry, @@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed {char *dname;u32 dname_len;struct ceph_mds_reply_lease *dlease; +struct ceph_mds_reply_xattr xattr_info;/* extra */ union {fs/ceph/strings.c +1
-0@@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op)case CEPH_MDS_OP_LOOKUPINO :return "lookupino" ;case CEPH_MDS_OP_LOOKUPNAME :return "lookupname" ;case CEPH_MDS_OP_GETATTR :return "getattr" ; +case CEPH_MDS_OP_GETVXATTR :return "getvxattr" ;case CEPH_MDS_OP_SETXATTR :return "setxattr" ;case CEPH_MDS_OP_SETATTR :return "setattr" ;case CEPH_MDS_OP_RMXATTR :return "rmxattr" ;fs/ceph/super.h +1
-0@@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode)/* xattr.c */ int __ceph_setxattr (struct inode *,const char *,const void *,size_t ,int ); +int ceph_do_getvxattr (struct inode *inode,const char *name,void *value,size_t size);ssize_t __ceph_getxattr (struct inode *,const char *,void *,size_t );extern ssize_t ceph_listxattr (struct dentry *,char *,size_t );extern struct ceph_buffer *__ceph_build_xattrs_blob (struct ceph_inode_info *ci);fs/ceph/xattr.c +0
-1--- a/fs/ceph/xattr.c
Quoted 8 lines
fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 927
Quoted 6 lines
+++
b/fs/ceph/xattr.cfs/ceph/xattr.c +1
-0@@ -924,6 +924,7 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,struct ceph_inode_info *ci =ceph_inode (inode);struct ceph_inode_xattr *xattr;struct ceph_vxattr *vxattr =NULL ; +struct ceph_mds_session *session =NULL ;No need to init this.
done ... check v7 patch
Quoted 19 lines
Quoted 6 lines
fs/ceph/xattr.c +0
-0int req_mask;ssize_t err;fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 953
Quoted 8 lines
fs/ceph/xattr.c +5
-0@@ -945,6 +946,22 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, err = -ERANGE ; }return err; + }else { + err = -ENODATA ; +spin_lock (&ci->i_ceph_lock); +if (strncmp (name,XATTR_CEPH_PREFIX ,XATTR_CEPH_PREFIX_LEN )) + goto out;No need to get the '
i_ceph_lock' for the 'strncmp()' here, and could just return '-EINVAL' directly instead of goto out ?
Quoted 7 lines
fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 957
Quoted 4 lines
fs/ceph/xattr.c +4
-0+/* check if the auth mds supports the getvxattr feature */ + session = ci->i_auth_cap->session; +if (!session) + goto out;In which case will the session be NULL ? If the
i_auth_capexists it will always set the 'session'.
done ... code moved to ceph_vet_session()
Quoted 25 lines
Quoted 24 lines
fs/ceph/xattr.c +7
-0+ +if (test_bit (CEPHFS_FEATURE_GETVXATTR , &session->s_features)) { +spin_unlock (&ci->i_ceph_lock); + err =ceph_do_getvxattr (inode, name, value, size); +spin_lock (&ci->i_ceph_lock); + } + goto out; } req_mask =__get_request_mask (inode);include/linux/ceph/ceph_fs.h +1
-0@@ -328,6 +328,7 @@ enum {CEPH_MDS_OP_LOOKUPPARENT =0x00103 ,CEPH_MDS_OP_LOOKUPINO =0x00104 ,CEPH_MDS_OP_LOOKUPNAME =0x00105 , +CEPH_MDS_OP_GETVXATTR =0x00106 ,CEPH_MDS_OP_SETXATTR =0x01105 , CEPH_MDS_OP_RMXATTR = 0x01106,