On Wed, Jul 27, 2022 at 01:56:37PM +0800, xiubli@xxxxxxxxxx wrote: > From: Xiubo Li <xiubli@xxxxxxxxxx> > > If the peer MDS doesn't support getvxattr op then just fall back to > use old getattr method to get it. Or for the old MDSs they will crash > when receive an unknown op. > > URL: https://tracker.ceph.com/issues/56529 > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx> > --- > fs/ceph/mds_client.c | 10 ++++++++++ > fs/ceph/mds_client.h | 4 +++- > fs/ceph/xattr.c | 9 ++++++--- > 3 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index 598012ddc401..bfe6d6393eba 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -3255,6 +3255,16 @@ static void __do_request(struct ceph_mds_client *mdsc, > > dout("do_request mds%d session %p state %s\n", mds, session, > ceph_session_state_name(session->s_state)); > + > + /* > + * The old ceph will crash the MDSs when see unknown OPs > + */ > + if (req->r_op == CEPH_MDS_OP_GETVXATTR && > + !test_bit(CEPHFS_FEATURE_OP_GETVXATTR, &session->s_features)) { > + err = -EOPNOTSUPP; > + goto out_session; > + } > + > if (session->s_state != CEPH_MDS_SESSION_OPEN && > session->s_state != CEPH_MDS_SESSION_HUNG) { > /* > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > index e15ee2858fef..0e03efab872a 100644 > --- a/fs/ceph/mds_client.h > +++ b/fs/ceph/mds_client.h > @@ -31,8 +31,9 @@ enum ceph_feature_type { > CEPHFS_FEATURE_METRIC_COLLECT, > CEPHFS_FEATURE_ALTERNATE_NAME, > CEPHFS_FEATURE_NOTIFY_SESSION_STATE, > + CEPHFS_FEATURE_OP_GETVXATTR, > > - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_NOTIFY_SESSION_STATE, > + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_OP_GETVXATTR, > }; > > #define CEPHFS_FEATURES_CLIENT_SUPPORTED { \ > @@ -45,6 +46,7 @@ enum ceph_feature_type { > CEPHFS_FEATURE_METRIC_COLLECT, \ > CEPHFS_FEATURE_ALTERNATE_NAME, \ > CEPHFS_FEATURE_NOTIFY_SESSION_STATE, \ > + CEPHFS_FEATURE_OP_GETVXATTR, \ > } > > /* > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c > index b10d459c2326..8f8db621772a 100644 > --- a/fs/ceph/xattr.c > +++ b/fs/ceph/xattr.c > @@ -984,9 +984,12 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, > return err; > } else { > err = ceph_do_getvxattr(inode, name, value, size); > - /* this would happen with a new client and old server combo */ > + /* > + * This would happen with a new client and old server combo, > + * then fall back to use old method to get it > + */ > if (err == -EOPNOTSUPP) > - err = -ENODATA; > + goto handle_non_vxattrs; > return err; Nit: maybe just do: if (err != -EOPNOTSUPP) return err instead of using a 'goto' statement. > } > handle_non_vxattrs: > @@ -996,7 +999,7 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, > dout("getxattr %p name '%s' ver=%lld index_ver=%lld\n", inode, name, > ci->i_xattrs.version, ci->i_xattrs.index_version); > > - if (ci->i_xattrs.version == 0 || > + if (ci->i_xattrs.version == 0 || err == -EOPNOTSUPP || You'll need to initialise 'err' when declaring it. Cheers, -- Luís > !((req_mask & CEPH_CAP_XATTR_SHARED) || > __ceph_caps_issued_mask_metric(ci, CEPH_CAP_XATTR_SHARED, 1))) { > spin_unlock(&ci->i_ceph_lock); > -- > 2.36.0.rc1 >
