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? 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.
Attachment:
signature.asc
Description: PGP signature
