On Wed, Jun 30, 2021 at 2:05 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Wed, 2021-06-30 at 08:52 +0800, Xiubo Li wrote:
> > On 6/29/21 11:38 PM, Jeff Layton wrote:
> > > On Tue, 2021-06-29 at 12:42 +0800, xiubli@xxxxxxxxxx wrote:
> > > > From: Xiubo Li <xiubli@xxxxxxxxxx>
> > > >
> > > > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
> > > > ---
> > > > fs/ceph/mds_client.h | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> > > > index 79d5b8ed62bf..b18eded84ede 100644
> > > > --- a/fs/ceph/mds_client.h
> > > > +++ b/fs/ceph/mds_client.h
> > > > @@ -27,7 +27,9 @@ enum ceph_feature_type {
> > > > CEPHFS_FEATURE_RECLAIM_CLIENT,
> > > > CEPHFS_FEATURE_LAZY_CAP_WANTED,
> > > > CEPHFS_FEATURE_MULTI_RECONNECT,
> > > > + CEPHFS_FEATURE_NAUTILUS,
> > > > CEPHFS_FEATURE_DELEG_INO,
> > > > + CEPHFS_FEATURE_OCTOPUS,
> > > > CEPHFS_FEATURE_METRIC_COLLECT,
> > > >
> > > > CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT,
> > > > @@ -43,7 +45,9 @@ enum ceph_feature_type {
> > > > CEPHFS_FEATURE_REPLY_ENCODING, \
> > > > CEPHFS_FEATURE_LAZY_CAP_WANTED, \
> > > > CEPHFS_FEATURE_MULTI_RECONNECT, \
> > > > + CEPHFS_FEATURE_NAUTILUS, \
> > > > CEPHFS_FEATURE_DELEG_INO, \
> > > > + CEPHFS_FEATURE_OCTOPUS, \
> > > > CEPHFS_FEATURE_METRIC_COLLECT, \
> > > > \
> > > > CEPHFS_FEATURE_MAX, \
> > > Do we need this? I thought we had decided to deprecate the whole concept
> > > of release-based feature flags.
> >
> > This was inconsistent with the MDS side, that means if the MDS only
> > support CEPHFS_FEATURE_DELEG_INO at most in lower version of ceph
> > cluster, then the kclients will try to send the metric messages to
> > MDSes, which could crash the MDS daemons.
> >
> > For the ceph version feature flags they are redundant since we can check
> > this from the con's, since pacific the MDS code stopped updating it. I
> > assume we should deprecate it since Pacific ?
> >
>
> I believe so. Basically the version-based features aren't terribly
> useful. Mostly we want to check feature flags for specific features
> themselves. Since there are no other occurrences of
> CEPHFS_FEATURE_NAUTILUS or CEPHFS_FEATURE_OCTOPUS symbols, it's probably
> best not to define them at all.
Not only that but this patch as is would break CEPHFS_FEATURE_DELEG_INO
and CEPHFS_FEATURE_METRIC_COLLECT checks in the kernel because their bit
numbers would change...
Thanks,
Ilya
