| Message ID | 1375036598-31090-2-git-send-email-steved@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jul 28, 2013 at 02:36:38PM -0400, Steve Dickson wrote: > Signed-off-by: Steve Dickson <steved@redhat.com> I agree that we want to be able to turn on 4.2 support with -V, but this does more than that: > - if (minorvers41) > - off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1", > - minorvers41 > 0 ? '+' : '-'); > + for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) { > + if (NFSCTL_VERISSET(minorvers, n)) > + off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n); > + else > + off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n); > + } > for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) { > if (NFSCTL_VERISSET(ctlbits, n)) > off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n); Previously minorvers41 could be set to: 1: turn on 4.1 -1: turn off 4.1 0: keep kernel default (user didn't specify) This patch removes the "0" case, which I liked. --b. -- 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, 2013-07-30 at 11:59 -0400, J. Bruce Fields wrote: > On Sun, Jul 28, 2013 at 02:36:38PM -0400, Steve Dickson wrote: > > Signed-off-by: Steve Dickson <steved@redhat.com> > > I agree that we want to be able to turn on 4.2 support with -V, but this > does more than that: > > > - if (minorvers41) > > - off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1", > > - minorvers41 > 0 ? '+' : '-'); > > + for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) { > > + if (NFSCTL_VERISSET(minorvers, n)) > > + off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n); > > + else > > + off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n); > > + } > > for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) { > > if (NFSCTL_VERISSET(ctlbits, n)) > > off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n); > > Previously minorvers41 could be set to: > > 1: turn on 4.1 > -1: turn off 4.1 > 0: keep kernel default (user didn't specify) > > This patch removes the "0" case, which I liked. Why should NFSv4 minor versions be treated any differently from major versions here? We don't have a 'kernel default' for NFSv2 or v3 or v4.0. -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@netapp.com www.netapp.com
On Tue, Jul 30, 2013 at 04:09:06PM +0000, Myklebust, Trond wrote: > On Tue, 2013-07-30 at 11:59 -0400, J. Bruce Fields wrote: > > On Sun, Jul 28, 2013 at 02:36:38PM -0400, Steve Dickson wrote: > > > Signed-off-by: Steve Dickson <steved@redhat.com> > > > > I agree that we want to be able to turn on 4.2 support with -V, but this > > does more than that: > > > > > - if (minorvers41) > > > - off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1", > > > - minorvers41 > 0 ? '+' : '-'); > > > + for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) { > > > + if (NFSCTL_VERISSET(minorvers, n)) > > > + off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n); > > > + else > > > + off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n); > > > + } > > > for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) { > > > if (NFSCTL_VERISSET(ctlbits, n)) > > > off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n); > > > > Previously minorvers41 could be set to: > > > > 1: turn on 4.1 > > -1: turn off 4.1 > > 0: keep kernel default (user didn't specify) > > > > This patch removes the "0" case, which I liked. > > Why should NFSv4 minor versions be treated any differently from major > versions here? We don't have a 'kernel default' for NFSv2 or v3 or v4.0. I think "run the highest minorversion currently considered sufficiently mature" is a useful option. I don't know why that wasn't done for earlier versions. --b. -- 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 28/07/13 14:36, Steve Dickson wrote: > Signed-off-by: Steve Dickson <steved@redhat.com> > --- > support/include/nfs/nfs.h | 4 ++++ > utils/nfsd/nfsd.c | 12 ++++++------ > utils/nfsd/nfssvc.c | 11 +++++++---- > 3 files changed, 17 insertions(+), 10 deletions(-) Committed... steved. > > diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h > index 5fd86f6..38db5b5 100644 > --- a/support/include/nfs/nfs.h > +++ b/support/include/nfs/nfs.h > @@ -15,6 +15,10 @@ > #define NFSD_MINVERS 2 > #define NFSD_MAXVERS 4 > > +#define NFS4_MINMINOR 1 > +#define NFS4_MAXMINOR 2 > +#define NFS4_VERDEFAULT 0x1 /* minor verion 1 */ > + > struct nfs_fh_len { > int fh_size; > u_int8_t fh_handle[NFS3_FHSIZE]; > diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c > index df48392..6db92f0 100644 > --- a/utils/nfsd/nfsd.c > +++ b/utils/nfsd/nfsd.c > @@ -99,7 +99,7 @@ main(int argc, char **argv) > char *p, *progname, *port; > char *haddr = NULL; > int socket_up = 0; > - int minorvers41 = 0; /* nfsv4 minor version */ > + int minorvers = NFS4_VERDEFAULT; /* nfsv4 minor version */ > unsigned int versbits = NFSCTL_VERDEFAULT; > unsigned int protobits = NFSCTL_ALLBITS; > unsigned int proto4 = 0; > @@ -160,11 +160,11 @@ main(int argc, char **argv) > case 4: > if (*p == '.') { > int i = atoi(p+1); > - if (i != 1) { > + if (i > 2) { > fprintf(stderr, "%s: unsupported minor version\n", optarg); > exit(1); > } > - minorvers41 = -1; > + NFSCTL_VERUNSET(minorvers, i); > break; > } > case 3: > @@ -181,11 +181,11 @@ main(int argc, char **argv) > case 4: > if (*p == '.') { > int i = atoi(p+1); > - if (i != 1) { > + if (i > 2) { > fprintf(stderr, "%s: unsupported minor version\n", optarg); > exit(1); > } > - minorvers41 = 1; > + NFSCTL_VERSET(minorvers, i); > break; > } > case 3: > @@ -282,7 +282,7 @@ main(int argc, char **argv) > * registered with rpcbind. Note that on older kernels w/o the right > * interfaces, these are a no-op. > */ > - nfssvc_setvers(versbits, minorvers41); > + nfssvc_setvers(versbits, minorvers); > > error = nfssvc_set_sockets(AF_INET, proto4, haddr, port); > if (!error) > diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c > index 683008e..8b85846 100644 > --- a/utils/nfsd/nfssvc.c > +++ b/utils/nfsd/nfssvc.c > @@ -269,7 +269,7 @@ nfssvc_set_sockets(const int family, const unsigned int protobits, > } > > void > -nfssvc_setvers(unsigned int ctlbits, int minorvers41) > +nfssvc_setvers(unsigned int ctlbits, int minorvers) > { > int fd, n, off; > char *ptr; > @@ -280,9 +280,12 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers41) > if (fd < 0) > return; > > - if (minorvers41) > - off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1", > - minorvers41 > 0 ? '+' : '-'); > + for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) { > + if (NFSCTL_VERISSET(minorvers, n)) > + off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n); > + else > + off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n); > + } > for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) { > if (NFSCTL_VERISSET(ctlbits, n)) > off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n); > -- 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/support/include/nfs/nfs.h b/support/include/nfs/nfs.h index 5fd86f6..38db5b5 100644 --- a/support/include/nfs/nfs.h +++ b/support/include/nfs/nfs.h @@ -15,6 +15,10 @@ #define NFSD_MINVERS 2 #define NFSD_MAXVERS 4 +#define NFS4_MINMINOR 1 +#define NFS4_MAXMINOR 2 +#define NFS4_VERDEFAULT 0x1 /* minor verion 1 */ + struct nfs_fh_len { int fh_size; u_int8_t fh_handle[NFS3_FHSIZE]; diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c index df48392..6db92f0 100644 --- a/utils/nfsd/nfsd.c +++ b/utils/nfsd/nfsd.c @@ -99,7 +99,7 @@ main(int argc, char **argv) char *p, *progname, *port; char *haddr = NULL; int socket_up = 0; - int minorvers41 = 0; /* nfsv4 minor version */ + int minorvers = NFS4_VERDEFAULT; /* nfsv4 minor version */ unsigned int versbits = NFSCTL_VERDEFAULT; unsigned int protobits = NFSCTL_ALLBITS; unsigned int proto4 = 0; @@ -160,11 +160,11 @@ main(int argc, char **argv) case 4: if (*p == '.') { int i = atoi(p+1); - if (i != 1) { + if (i > 2) { fprintf(stderr, "%s: unsupported minor version\n", optarg); exit(1); } - minorvers41 = -1; + NFSCTL_VERUNSET(minorvers, i); break; } case 3: @@ -181,11 +181,11 @@ main(int argc, char **argv) case 4: if (*p == '.') { int i = atoi(p+1); - if (i != 1) { + if (i > 2) { fprintf(stderr, "%s: unsupported minor version\n", optarg); exit(1); } - minorvers41 = 1; + NFSCTL_VERSET(minorvers, i); break; } case 3: @@ -282,7 +282,7 @@ main(int argc, char **argv) * registered with rpcbind. Note that on older kernels w/o the right * interfaces, these are a no-op. */ - nfssvc_setvers(versbits, minorvers41); + nfssvc_setvers(versbits, minorvers); error = nfssvc_set_sockets(AF_INET, proto4, haddr, port); if (!error) diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c index 683008e..8b85846 100644 --- a/utils/nfsd/nfssvc.c +++ b/utils/nfsd/nfssvc.c @@ -269,7 +269,7 @@ nfssvc_set_sockets(const int family, const unsigned int protobits, } void -nfssvc_setvers(unsigned int ctlbits, int minorvers41) +nfssvc_setvers(unsigned int ctlbits, int minorvers) { int fd, n, off; char *ptr; @@ -280,9 +280,12 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers41) if (fd < 0) return; - if (minorvers41) - off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1", - minorvers41 > 0 ? '+' : '-'); + for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) { + if (NFSCTL_VERISSET(minorvers, n)) + off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n); + else + off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n); + } for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) { if (NFSCTL_VERISSET(ctlbits, n)) off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
Signed-off-by: Steve Dickson <steved@redhat.com> --- support/include/nfs/nfs.h | 4 ++++ utils/nfsd/nfsd.c | 12 ++++++------ utils/nfsd/nfssvc.c | 11 +++++++---- 3 files changed, 17 insertions(+), 10 deletions(-)