On Thu, 2018-08-09 at 11:46 -0400, Scott Mayhew wrote:
> 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
This is not what is intended. As I said, the intention was that you
should use '+4.0' to enable 4.0 and '-4.0' to disable it.
The exception is for legacy kernels, which are not typically able to
recognise the '+4.0' or '-4.0', so for those kernels (and those kernels
only), the '+4'/'-4' need to continue to work as before.
>
> -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
> > >
> > >
--
Trond Myklebust
CTO, Hammerspace Inc
4300 El Camino Real, Suite 105
Los Altos, CA 94022
www.hammer.space
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
