| Message ID | 20170606194812.4248-1-steved@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> On Jun 6, 2017, at 3:48 PM, Steve Dickson <steved@redhat.com> wrote: > > When the nfs4 filesystem specified, the default minor > version should be used not v4.0. Isn't the default specified in the mountnfs config file? > Signed-off-by: Steve Dickson <steved@redhat.com> > --- > utils/mount/stropts.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c > index c0266e5..57efb26 100644 > --- a/utils/mount/stropts.c > +++ b/utils/mount/stropts.c > @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi) > > if (strncmp(mi->type, "nfs4", 4) == 0) { > mi->version.major = 4; > - mi->version.v_mode = V_GENERAL; > + mi->version.minor = 2; > + mi->version.v_mode = V_SPECIFIC; > } > /* > * Before 2.6.32, the kernel NFS client didn't > -- > 2.9.4 > > -- > 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 -- Chuck Lever -- 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 06/06/2017 03:52 PM, Chuck Lever wrote: > >> On Jun 6, 2017, at 3:48 PM, Steve Dickson <steved@redhat.com> wrote: >> >> When the nfs4 filesystem specified, the default minor >> version should be used not v4.0. > > Isn't the default specified in the mountnfs config file? It can be... But in general its not... So the question is when the nfs4 file system (aka -t nfs4) what minor version does that mean? v4.0 which was the default in the past or the new default v4.2? steved. > > >> Signed-off-by: Steve Dickson <steved@redhat.com> >> --- >> utils/mount/stropts.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c >> index c0266e5..57efb26 100644 >> --- a/utils/mount/stropts.c >> +++ b/utils/mount/stropts.c >> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi) >> >> if (strncmp(mi->type, "nfs4", 4) == 0) { >> mi->version.major = 4; >> - mi->version.v_mode = V_GENERAL; >> + mi->version.minor = 2; >> + mi->version.v_mode = V_SPECIFIC; >> } >> /* >> * Before 2.6.32, the kernel NFS client didn't >> -- >> 2.9.4 >> >> -- >> 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 > > -- > Chuck Lever > > > -- 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 Jun 6, 2017, at 5:02 PM, Steve Dickson <SteveD@RedHat.com> wrote: > > > > On 06/06/2017 03:52 PM, Chuck Lever wrote: >> >>> On Jun 6, 2017, at 3:48 PM, Steve Dickson <steved@redhat.com> wrote: >>> >>> When the nfs4 filesystem specified, the default minor >>> version should be used not v4.0. >> >> Isn't the default specified in the mountnfs config file? > It can be... But in general its not... > > So the question is when the nfs4 file system (aka -t nfs4) > what minor version does that mean? v4.0 which was the > default in the past or the new default v4.2? IMO the admin (and the distributor) should be allowed to decide which minorversion is the default for vers=4 or "-t nfs4". NFSv4.0 is a safe default for now, so I'm just arguing for taking a little more time to make this change to minor version 2 using the config file instead of hard coding it. No strong opinion, I just think it makes better sense to handle default minor version the way many other default behaviors are configured. > steved. >> >> >>> Signed-off-by: Steve Dickson <steved@redhat.com> >>> --- >>> utils/mount/stropts.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c >>> index c0266e5..57efb26 100644 >>> --- a/utils/mount/stropts.c >>> +++ b/utils/mount/stropts.c >>> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi) >>> >>> if (strncmp(mi->type, "nfs4", 4) == 0) { >>> mi->version.major = 4; >>> - mi->version.v_mode = V_GENERAL; >>> + mi->version.minor = 2; >>> + mi->version.v_mode = V_SPECIFIC; >>> } >>> /* >>> * Before 2.6.32, the kernel NFS client didn't >>> -- >>> 2.9.4 >>> >>> -- >>> 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 >> >> -- >> Chuck Lever >> >> >> -- Chuck Lever -- 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 Tue, Jun 06 2017, Steve Dickson wrote: > When the nfs4 filesystem specified, the default minor > version should be used not v4.0. > > Signed-off-by: Steve Dickson <steved@redhat.com> > --- > utils/mount/stropts.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c > index c0266e5..57efb26 100644 > --- a/utils/mount/stropts.c > +++ b/utils/mount/stropts.c > @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi) > > if (strncmp(mi->type, "nfs4", 4) == 0) { > mi->version.major = 4; > - mi->version.v_mode = V_GENERAL; > + mi->version.minor = 2; > + mi->version.v_mode = V_SPECIFIC; I think this is wrong. By setting the mode to SPECIFIC, you are saying that if the server doesn't support v4.2, then fail the mount. That cannot be right. Given that (currently) v_mode is not V_SPECIFIC, nfs_set_version() will go on and call nfs_default_version(), which will use the default value from the config file - just as Chuck suggests. If there is no default in the config file .... nfs_default_version() will do nothing. So version.minor will probably remain at zero. So setting > + mi->version.minor = 2; (where 2 is the maximum supported version) is probably correct. Setting > + mi->version.v_mode = V_SPECIFIC; is unnecessary and wrong. If there an easy way to find out the maximum minor version that the kernel supports? We should really default version.minor to that. Once we get up to v4.20, it'll seem odd to try to mount 4.20, 4.19, 4.18,... until something succeeds.... Thanks, NeilBrown > } > /* > * Before 2.6.32, the kernel NFS client didn't > -- > 2.9.4 > > -- > 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 06/06/2017 10:48 PM, NeilBrown wrote: > On Tue, Jun 06 2017, Steve Dickson wrote: > >> When the nfs4 filesystem specified, the default minor >> version should be used not v4.0. >> >> Signed-off-by: Steve Dickson <steved@redhat.com> >> --- >> utils/mount/stropts.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c >> index c0266e5..57efb26 100644 >> --- a/utils/mount/stropts.c >> +++ b/utils/mount/stropts.c >> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi) >> >> if (strncmp(mi->type, "nfs4", 4) == 0) { >> mi->version.major = 4; >> - mi->version.v_mode = V_GENERAL; >> + mi->version.minor = 2; >> + mi->version.v_mode = V_SPECIFIC; > > I think this is wrong. > > By setting the mode to SPECIFIC, you are saying that if the server > doesn't support v4.2, then fail the mount. That cannot be right. Ok... I though since nfs4 was being specify it should be V_SPECIFIC but I do see your point about negotiating down. It turns out setting vers4.2=n in /etc/nfs.conf breaks all v4 mounts... So I think we have issues in that area ATM... ;-) > > Given that (currently) v_mode is not V_SPECIFIC, nfs_set_version() > will go on and call nfs_default_version(), which will use the default > value from the config file - just as Chuck suggests. > > If there is no default in the config file .... nfs_default_version() > will do nothing. So version.minor will probably remain at zero. > So setting >> + mi->version.minor = 2; > > (where 2 is the maximum supported version) is probably correct. > Setting >> + mi->version.v_mode = V_SPECIFIC; > is unnecessary and wrong. > > If there an easy way to find out the maximum minor version that the > kernel supports? We should really default version.minor to that. > Once we get up to v4.20, it'll seem odd to try to mount 4.20, 4.19, > 4.18,... until something succeeds.... Maybe it should be something like the server does under /proc/fs/nfsd/version steved. > > Thanks, > NeilBrown > >> } >> /* >> * Before 2.6.32, the kernel NFS client didn't >> -- >> 2.9.4 >> >> -- >> 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 -- 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 06/08/2017 11:36 AM, Steve Dickson wrote: > > > On 06/06/2017 10:48 PM, NeilBrown wrote: >> On Tue, Jun 06 2017, Steve Dickson wrote: >> >>> When the nfs4 filesystem specified, the default minor >>> version should be used not v4.0. >>> >>> Signed-off-by: Steve Dickson <steved@redhat.com> >>> --- >>> utils/mount/stropts.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c >>> index c0266e5..57efb26 100644 >>> --- a/utils/mount/stropts.c >>> +++ b/utils/mount/stropts.c >>> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi) >>> >>> if (strncmp(mi->type, "nfs4", 4) == 0) { >>> mi->version.major = 4; >>> - mi->version.v_mode = V_GENERAL; >>> + mi->version.minor = 2; >>> + mi->version.v_mode = V_SPECIFIC; >> >> I think this is wrong. >> >> By setting the mode to SPECIFIC, you are saying that if the server >> doesn't support v4.2, then fail the mount. That cannot be right. > Ok... I though since nfs4 was being specify it should be > V_SPECIFIC but I do see your point about negotiating down. > > It turns out setting vers4.2=n in /etc/nfs.conf breaks > all v4 mounts... So I think we have issues in that area > ATM... ;-) > >> >> Given that (currently) v_mode is not V_SPECIFIC, nfs_set_version() >> will go on and call nfs_default_version(), which will use the default >> value from the config file - just as Chuck suggests. >> >> If there is no default in the config file .... nfs_default_version() >> will do nothing. So version.minor will probably remain at zero. >> So setting >>> + mi->version.minor = 2; >> >> (where 2 is the maximum supported version) is probably correct. It is because in nfs_default_version() there is this chunk of code if (mi->version.v_mode == V_GENERAL) { if (config_default_vers.v_mode != V_DEFAULT && mi->version.major == config_default_vers.major) mi->version.minor = config_default_vers.minor; return; } that ends up not setting the minor version at all which is the reason the minor version is zero. steved. >> Setting >>> + mi->version.v_mode = V_SPECIFIC; >> is unnecessary and wrong. >> >> If there an easy way to find out the maximum minor version that the >> kernel supports? We should really default version.minor to that. >> Once we get up to v4.20, it'll seem odd to try to mount 4.20, 4.19, >> 4.18,... until something succeeds.... > Maybe it should be something like the server does under /proc/fs/nfsd/version > > steved. > >> >> Thanks, >> NeilBrown >> >>> } >>> /* >>> * Before 2.6.32, the kernel NFS client didn't >>> -- >>> 2.9.4 >>> >>> -- >>> 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 > -- > 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 > -- 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/utils/mount/stropts.c b/utils/mount/stropts.c index c0266e5..57efb26 100644 --- a/utils/mount/stropts.c +++ b/utils/mount/stropts.c @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi) if (strncmp(mi->type, "nfs4", 4) == 0) { mi->version.major = 4; - mi->version.v_mode = V_GENERAL; + mi->version.minor = 2; + mi->version.v_mode = V_SPECIFIC; } /* * Before 2.6.32, the kernel NFS client didn't
When the nfs4 filesystem specified, the default minor version should be used not v4.0. Signed-off-by: Steve Dickson <steved@redhat.com> --- utils/mount/stropts.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)