On Wed, 08 Aug 2018, Scott Mayhew wrote:
> On Wed, 08 Aug 2018, Trond Myklebust wrote:
>
> > On Wed, 2018-08-08 at 07:36 -0400, Scott Mayhew wrote:
> > > According to commit d3635ff07e8 ("nfsd: fix configuration of
> > > supported
> > > minor versions"), it should be possible to use either '4.0' or '4' to
> > > enable or disable minor version 0.
> > >
> > > Currently, writing '+4' to /proc/fs/nfsd/versions has no effect
> > > unless
> > > no minor versions are enabled. That leaves rpc.nfsd without an easy
> > > way to re-enable v4.0, since that's what it does when invoked with
> > > '-V 4.0'.
> > >
> > > Fixes: d3635ff07e8 ("nfsd: fix configuration of supported minor
> > > versions")
> > > Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx>
> > > ---
> > > fs/nfsd/nfsctl.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > > index d107b4426f7e..b3f9f9233953 100644
> > > --- a/fs/nfsd/nfsctl.c
> > > +++ b/fs/nfsd/nfsctl.c
> > > @@ -614,6 +614,14 @@ static ssize_t __write_versions(struct file
> > > *file, char *buf, size_t size)
> > > minor = 0;
> > > while (nfsd_minorversion(minor,
> > > cmd) >= 0)
> > > minor++;
> > > + } else if (cmd == NFSD_SET) {
> > > + /*
> > > + * We have +4 but there are
> > > already some minors
> > > + * enabled. We must ensure 4.0
> > > gets enabled,
> > > + * since it could be a request
> > > from rpc.nfsd.
> > > + */
> > > + if (nfsd_minorversion(0, cmd) <
> > > 0)
> > > + return -EINVAL;
> > > }
> > > break;
> > > default:
> >
> > This breaks the intention behind that patch, which was to ensure that
> > +4 must _not_ automatically enable 4.0...
> >
> > Instead, you use +/-4.0 to enable/disable 4.0, just like you use +/-4.1
> > to enable/disable 4.1, +/-4.2 to enable/disable 4.2, etc, etc...
> >
> > The special value +/-4 exists in order allow you to disable/enable v4
> > altogether. i.e. '-4' acts as a mask to turn off v4.x for all values of
> > x. '+4' will enable v4.x for those values of x which have been
> > explicitly set.
>
> Okay I guess I'm confused by this wording from the commit message in
> d3635ff07e8:
>
> Allow the user to use either '4.0' or '4' to enable or disable minor
> version 0. Other minor versions are still enabled or disabled using the
> '4.x' format.
Okay I see that that commit message actually came from Bruce and is
different than the original commit message... but in your original cover
letter (https://marc.info/?l=linux-nfs&m=148780658904187&w=2) you stated
that '+4' should enable 4.0:
"""
2) Allow the user to use either '4.0' or '4' in order to enable or disable
minor version 0. Other minor versions remain enabled/disabled using the
'4.x' format.
"""
I thought the point of your patches were to allow the user to enable 4.x
(for x > 0) without enabling 4.0. My patch doesn't change that. All it
does is allow '+4' to enable 4.0, which is what both Bruce's commit
message and your cover letter says it should do.
current HEAD:
[root@fedora27_1236568e ~]# echo '-4' >/proc/fs/nfsd/versions
[root@fedora27_1236568e ~]# cat /proc/fs/nfsd/versions
-2 +3 -4 -4.0 -4.1 -4.2
[root@fedora27_1236568e ~]# echo '+4.1' >/proc/fs/nfsd/versions
[root@fedora27_1236568e ~]# !cat
cat /proc/fs/nfsd/versions
-2 +3 +4 -4.0 +4.1 -4.2
[root@fedora27_1236568e ~]# echo '+4' >/proc/fs/nfsd/versions
[root@fedora27_1236568e ~]# !cat
cat /proc/fs/nfsd/versions
-2 +3 +4 -4.0 +4.1 -4.2
with my patch:
[root@fedora27_8686dfcb ~]# echo '-4' >/proc/fs/nfsd/versions
[root@fedora27_8686dfcb ~]# cat /proc/fs/nfsd/versions
-2 +3 -4 -4.0 -4.1 -4.2
[root@fedora27_8686dfcb ~]# echo '+4.1' >/proc/fs/nfsd/versions
[root@fedora27_8686dfcb ~]# !cat
cat /proc/fs/nfsd/versions
-2 +3 +4 -4.0 +4.1 -4.2 <----- 4.0 is still disabled
[root@fedora27_8686dfcb ~]# echo '+4' >/proc/fs/nfsd/versions
[root@fedora27_8686dfcb ~]# !cat
cat /proc/fs/nfsd/versions
-2 +3 +4 +4.1 -4.2 <----- 4.0 is enabled
-Scott
>
> Should rpc.nfsd be changed then? The way it is now, nfssvc_print_vers() will
> write '+4' if you run 'rpc.nfsd -V4.0'...
>
> >
> > --
> > Trond Myklebust
> > CTO, Hammerspace Inc
> > 4300 El Camino Real, Suite 105
> > Los Altos, CA 94022
> > www.hammer.space
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
