On Fri, 2017-03-03 at 09:57 +1100, NeilBrown wrote:
> On Mon, Feb 27 2017, bfields@xxxxxxxxxxxx wrote:
>
> > On Thu, Feb 23, 2017 at 01:00:12AM +0000, Trond Myklebust wrote:
> > > On Thu, 2017-02-23 at 11:13 +1100, NeilBrown wrote:
> > > > On Wed, Feb 22 2017, Trond Myklebust wrote:
> > > >
> > > > > This patch series was intended as a standalone series.
> > > > > However it
> > > > > now
> > > > > aims to fix up a few issues with Neil's initial patch:
> > > > >
> > > > > 1) When the user turns off all minor versions of NFSv4, then
> > > > > that
> > > > > should
> > > > > be equivalent to turning off NFSv4 support, and so when
> > > > > someone
> > > > > tries
> > > > > to mount, we should return RPC_PROG_MISMATCH.
> > > > > 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.
> > > >
> > > > If I understand you and the code correctly, then this change
> > > > means
> > > > that
> > > > if I run
> > > >
> > > > rpc.nfsd -N 4
> > > >
> > > > the nfs server will continue to server v4.1 and v4.2 requests,
> > > > only
> > > > v4.0
> > > > will be disabled (Assuming a kernel which defaults to serving
> > > > v4.0,
> > > > v4.1
> > > > and v4.0).
> > >
> > > Unfortunately not... The problem is that rpc.nfsd has a borken
> > > parser
> > > that refuses the '-N4 -V4.2' syntax, and that throws hissy fit if
> > > you
> > > try to feed it '-N4.0' or '-V4.0'.
> > >
> > > The kernel change needs to be accompanied by a fix to nfs-utils.
> > >
> > > > Is that really what we want?
> > > >
> > > > The text in the man page for rpc.nfsd could be read as allowing
> > > > this
> > > > interpretation, though it isn't explicit.
> > > > I've always used "NFSv4" to mean any or all for 4.0, 4.1, 4.2,
> > > > ...
> > >
> > > We _could_ do that. As I said, that's a question of fixing
> > > rpc.nfsd.
> > >
> > > The question here, though, is what should the kernel do when you
> > > start
> > > poking into /proc/fs/nfsd/versions. I think adding a new field
> > > between
> > > '4' and '4.0' might be more disruptive than changing the meaning
> > > of '4'
> > > to mean '4.0' as there might be other parsers of that pseudofile
> > > out
> > > there today.
> >
> > I'm OK with that, unless Neil sees a problem.
>
> I don't know ... the more I think about it, the less certain I seem
> to
> be.
>
> Prior to Trond's updates for nfs-utils, "rpc.nfsd -N 4" would write
> "+3 -4"
> to the 'versions' file, which will currently disable NFSv4
> completely.
> With this patch it will disable 4.0, but leave 4.1 active.
>
> This behavior is consistent with the documentation, which says:
>
> The current version of rpc.nfsd can support major NFS versions
> 2,3,4
> and the minor versions 4.1 and 4.2.
>
> which suggests that "4.1" is a different thing to "4", not part of
> it.
> And the current nfs-utils doesn't support disabling just 4.0 at all.
> You cannot say "rpc.nfsd -N 4.0".
> So this patch gives more control to the old nfs-utils, by changing
> the
> semantics of a particular operation.
>
> Technically, it is a regression though. It does result in a more
> coherent interface, which is a good thing in the long term.
>
> When you read from "versions" you get the major version and minor
> version listed separately, with a special case that "+4.0" is never
> reported.
> So if just 4.1 is enabled, you might get
> -3 +4 -4.0 +4.1 -4.2
>
> after Trond's change "4" means "4.0" and "4.0" is never displayed,
> so the above case would become
> -3 -4 +4.1 -4.2
>
> which is very different. Probably another regression.
>
> I guess I'm not entirely sure what Trond is trying to fix.
> This bit:
>
> 1) When the user turns off all minor versions of NFSv4, then that
> should
> be equivalent to turning off NFSv4 support, and so when someone
> tries
> to mount, we should return RPC_PROG_MISMATCH.
>
> Makes perfect sense. But I'm not clear on why the format of the
> versions file needs to change.
> Trond - what am I missing?
>
The main issue that worried me was one of predictability. What is
supposed to happen when you type "echo +4"? One thing that I considered
to be a regression, was that previously, I could expect that "echo +4"
would at the very least turn on NFSv4 minor version 0, but with the
change to semantics, it would only do so if nobody had typed "echo
-4.0".
An analogy would be putting 2 light switches in front of a light bulb,
so that unless both switches are on, the bulb will not turn on.
Actually, it is worse than that, because none of the bulbs turn on
until you start up knfsd (so you can argue that there is a third switch
in front of the other two).
Why do we need this many levels of switches in a kernel interface? You
should be able to achieve the same functionality by just turning on and
off the individual minor versions. The right place for designing more
complex interfaces would be userspace, and is exactly what the rpc.nfsd
utility should be taking care of.
Finally, there is the issue that the interface allowed situations where
knfsd was advertising support for NFSv4 via rpcbind, but there were no
minor versions enabled, and so you'd just get a confusing series of
NFS4ERR_MINOR_VERSION_MISMATCH replies when attempting to mount. Why
even advertise support in that case?
> Thanks,
> NeilBrown
>
>
>
> >
> > The changelog on the first patch doesn't seem to describe the v2
> > version
> > very well; using the below cribbed from your cover letter instead.
> >
> > nfsd: fix configuration of supported minor versions
> >
> > When the user turns off all minor versions of NFSv4, that
> > should be
> > equivalent to turning off NFSv4 support, so a mount attempt
> > using NFSv4
> > should get RPC_PROG_MISMATCH, not NFSERR_MINOR_VERS_MISMATCH.
> >
> > 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.
> >
> > --b.
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
