| Message ID | 20180808113600.11502-1-smayhew@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfsd: ensure that writing '+4' to /proc/fs/nfsd/versions enables minor version 0 | expand |
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@redhat.com> > --- > 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.
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@redhat.com> > > --- > > 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. 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@redhat.com> > > > --- > > > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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:
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@redhat.com> --- fs/nfsd/nfsctl.c | 8 ++++++++ 1 file changed, 8 insertions(+)