Re: [PATCH v2 0/2] Fix up nfsd to enable NFSv4.x without NFSv4.0

["bfields@fieldses.org" <bfields@fieldses.org>]

Sorry for not paying close attention here.  Catching up:

On Mon, Mar 06, 2017 at 03:26:40PM +1100, NeilBrown wrote:
> On Sat, Mar 04 2017, Trond Myklebust wrote:
> 
> > 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".
> 
> I don't think I would consider this as a regression. Prior to
> 
> Commit: e35659f1b03c ("NFSD: correctly range-check v4.x minor version when setting versions.")
> 
> "echo -4.0" would result in an error.  After that patch it will result in
> behaviour that you think is inconsistent.  While that might be a poor
> design choice, I don't think it is a regression because it is not
> (holistically) something that worked before and now works differently.
>
> I agree that "echo +4" should do something sensible and predictable.  I
> would like to suggest that it should enable all "supported" NFSv4.x minor
> versions.  That is consistent with how rpc.nfsd uses it, and makes sense
> to me. "echo -4" should disable all minor versions.
> 
> >
> > 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.
> 
> The "no regressions" rule can often lead to clunky interfaces.  It
> certainly isn't ideal, but sometimes we need to live with it.
> The right place to hide that clunkiness is in rpc.nfsd :-)

That sounds right to me.

> > 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?
> 
> I agree with this.
> If all minor versions are disabled, the major version should be disabled
> as well.  If any minor versions are enabled, the major version must be
> enabled too.

So, if you have a patch that keeps the agreed-on change while reverting
to a (clunkier, but more backwards-compatible) interface, and if you can
do it while we're still early in 4.11, then I'd take that.

--b.