| Message ID | 20110815212539.GA32181@fieldses.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 15, 2011 at 05:25:39PM -0400, J. Bruce Fields wrote: > On Mon, Aug 15, 2011 at 09:03:30AM -0700, Myklebust, Trond wrote: > > > -----Original Message----- > > > From: J. Bruce Fields [mailto:bfields@fieldses.org] > > > How is the client supposed to handle opens of device special files? > > On > > > a 3.1-rc1-based client to a linux server over v4.0 I'm seeing it try > > an > > > OPEN call and failing when it gets an INVAL return. > > > > > > This looks like bogus client behavior (OPEN should fail on such > > files), > > > unless the server has the error return wrong and the client's using an > > > OPEN error to recover. > > > > > > If I first stat the device and then open it then it works as expected > > > (the client does an open of the local device). > > > > > > I'm a bit annoyed at myself as I have a feeling we've discussed this > > > before but I can't find a reference now. > > > > Hmm... NFS4ERR_INVAL means 'invalid argument', which is not the case > > here; all the arguments that the client is passing to the server are > > valid, however the file cannot be opened because the pathname resolves > > on the server to a file of the wrong type. The long description of NFS4ERR_INVAL from rfc 3530: "Invalid argument or unsupported argument for an operation. Two examples are attempting a READLINK on an object other than a symbolic link or specifying a value for an enum field that is not defined in the protocol (e.g., nfs_ftype4)." So the text does recommend NFS4ERR_INVAL for the case where the operation doesn't make sense for the type of object given. (In the readlink example there's no pathname resolution, though). EINVAL seems to be used in the kernel for some such cases as well (e.g. attempt to truncate a device special file). > > I can't see any other error definition that is "obviously correct", but > > it looks to me as if NFS4ERR_SYMLINK might be the closest thing. One > > reason is the dot-x file defines NFS4ERR_SYMLINK as meaning "should be > > file/directory". And this seems more a coincidence due to vagueness resulting from the need for a single-line comment; the longer description is: "NFS4ERR_SYMLINK The current filehandle provided for a LOOKUP is not a directory but a symbolic link. Also used if the final component of the OPEN path is a symbolic link." > > The other reason is that NFS4ERR_SYMLINK should > > _always_ trigger the correct behaviour on a client: a fresh lookup of > > the component. > > Confirmed the fix; I'll apply. Applying anyway for compatibility with existing clients, but this all seems a little weird. --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 Mon, Aug 15, 2011 at 06:23:36PM -0400, J. Bruce Fields wrote: > On Mon, Aug 15, 2011 at 05:25:39PM -0400, J. Bruce Fields wrote: > > On Mon, Aug 15, 2011 at 09:03:30AM -0700, Myklebust, Trond wrote: > > > > -----Original Message----- > > > > From: J. Bruce Fields [mailto:bfields@fieldses.org] > > > > How is the client supposed to handle opens of device special files? > > > On > > > > a 3.1-rc1-based client to a linux server over v4.0 I'm seeing it try > > > an > > > > OPEN call and failing when it gets an INVAL return. > > > > > > > > This looks like bogus client behavior (OPEN should fail on such > > > files), > > > > unless the server has the error return wrong and the client's using an > > > > OPEN error to recover. > > > > > > > > If I first stat the device and then open it then it works as expected > > > > (the client does an open of the local device). > > > > > > > > I'm a bit annoyed at myself as I have a feeling we've discussed this > > > > before but I can't find a reference now. > > > > > > Hmm... NFS4ERR_INVAL means 'invalid argument', which is not the case > > > here; all the arguments that the client is passing to the server are > > > valid, however the file cannot be opened because the pathname resolves > > > on the server to a file of the wrong type. > > The long description of NFS4ERR_INVAL from rfc 3530: > > "Invalid argument or unsupported argument for an operation. Two > examples are attempting a READLINK on an object other than a > symbolic link or specifying a value for an enum field that is > not defined in the protocol (e.g., nfs_ftype4)." > > So the text does recommend NFS4ERR_INVAL for the case where the > operation doesn't make sense for the type of object given. (In the > readlink example there's no pathname resolution, though). > > EINVAL seems to be used in the kernel for some such cases as well (e.g. > attempt to truncate a device special file). > > > > I can't see any other error definition that is "obviously correct", but > > > it looks to me as if NFS4ERR_SYMLINK might be the closest thing. One > > > reason is the dot-x file defines NFS4ERR_SYMLINK as meaning "should be > > > file/directory". > > And this seems more a coincidence due to vagueness resulting from the > need for a single-line comment; the longer description is: > > "NFS4ERR_SYMLINK The current filehandle provided for a LOOKUP is > not a directory but a symbolic link. Also used > if the final component of the OPEN path is a > symbolic link." > > > > The other reason is that NFS4ERR_SYMLINK should > > > _always_ trigger the correct behaviour on a client: a fresh lookup of > > > the component. > > > > Confirmed the fix; I'll apply. > > Applying anyway for compatibility with existing clients, but this all > seems a little weird. And the change also makes a bunch of pynfs tests fail, complaining that various operations against incompatible types should have returned INVAL and not SYMLINK. Not that I'm convinced pynfs is correct--at the very least it should have accepted a range of errors for those tests, I think--but anyone else that ran pynfs against their server may have assumed it pynfs was correct in these cases.... --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 Mon, Aug 15, 2011 at 05:25:39PM -0400, J. Bruce Fields wrote:
> Confirmed the fix; I'll apply.
Also found some opportunity for cleanups while I was there, as follows.
--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
> -----Original Message----- > From: J. Bruce Fields [mailto:bfields@fieldses.org] > Sent: Monday, August 15, 2011 6:24 PM > To: Myklebust, Trond > Cc: linux-nfs@vger.kernel.org; steved@redhat.com; nfsv4@ietf.org > Subject: Re: open() of device special files > > On Mon, Aug 15, 2011 at 05:25:39PM -0400, J. Bruce Fields wrote: > > On Mon, Aug 15, 2011 at 09:03:30AM -0700, Myklebust, Trond wrote: > > > > -----Original Message----- > > > > From: J. Bruce Fields [mailto:bfields@fieldses.org] > > > > How is the client supposed to handle opens of device special > files? > > > On > > > > a 3.1-rc1-based client to a linux server over v4.0 I'm seeing it > try > > > an > > > > OPEN call and failing when it gets an INVAL return. > > > > > > > > This looks like bogus client behavior (OPEN should fail on such > > > files), > > > > unless the server has the error return wrong and the client's > using an > > > > OPEN error to recover. > > > > > > > > If I first stat the device and then open it then it works as > expected > > > > (the client does an open of the local device). > > > > > > > > I'm a bit annoyed at myself as I have a feeling we've discussed > this > > > > before but I can't find a reference now. > > > > > > Hmm... NFS4ERR_INVAL means 'invalid argument', which is not the > case > > > here; all the arguments that the client is passing to the server > are > > > valid, however the file cannot be opened because the pathname > resolves > > > on the server to a file of the wrong type. > > The long description of NFS4ERR_INVAL from rfc 3530: > > "Invalid argument or unsupported argument for an operation. Two > examples are attempting a READLINK on an object other than a > symbolic link or specifying a value for an enum field that is > not defined in the protocol (e.g., nfs_ftype4)." > > So the text does recommend NFS4ERR_INVAL for the case where the > operation doesn't make sense for the type of object given. (In the > readlink example there's no pathname resolution, though). Right. In the case of readlink, you pass a file handle, which points to an object for which you _can_ check the type before attempting the readlink operation. OPEN (and LOOKUP, RENAME, REMOVE,...) takes a filename, which could point to anything. There is no way for the client to know a priori the type of the object being pointed to. > EINVAL seems to be used in the kernel for some such cases as well (e.g. > attempt to truncate a device special file). My reading of the manpages and the posix spec is that truncate() will return EINVAL if and only if the length argument is wrong. Ftruncate() will in addition return EINVAL if the file descriptor points to a non-regular file (the assumption presumably being that you should have tried to fstat() first). IOW: The criterion that they apply is that "if the user should have known better" then EINVAL is correct. If there is no way for the user to be sure a priori, then it is not. So, for instance, rmdir() will not return EINVAL if you try to apply it to a file. > > > I can't see any other error definition that is "obviously correct", > but > > > it looks to me as if NFS4ERR_SYMLINK might be the closest thing. > One > > > reason is the dot-x file defines NFS4ERR_SYMLINK as meaning "should > be > > > file/directory". > > And this seems more a coincidence due to vagueness resulting from the > need for a single-line comment; the longer description is: > > "NFS4ERR_SYMLINK The current filehandle provided for a LOOKUP is > not a directory but a symbolic link. Also used > if the final component of the OPEN path is a > symbolic link." > > > > The other reason is that NFS4ERR_SYMLINK should > > > _always_ trigger the correct behaviour on a client: a fresh lookup > of > > > the component. > > > > Confirmed the fix; I'll apply. > > Applying anyway for compatibility with existing clients, but this all > seems a little weird. Agreed, but it appears to be a case that has been missed when we defined OPEN. Trond -- 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
> -----Original Message----- > From: J. Bruce Fields [mailto:bfields@fieldses.org] > Sent: Monday, August 15, 2011 6:27 PM > To: Myklebust, Trond > Cc: linux-nfs@vger.kernel.org; steved@redhat.com; nfsv4@ietf.org > Subject: Re: open() of device special files > > On Mon, Aug 15, 2011 at 06:23:36PM -0400, J. Bruce Fields wrote: > > On Mon, Aug 15, 2011 at 05:25:39PM -0400, J. Bruce Fields wrote: > > > On Mon, Aug 15, 2011 at 09:03:30AM -0700, Myklebust, Trond wrote: > > > > > -----Original Message----- > > > > > From: J. Bruce Fields [mailto:bfields@fieldses.org] > > > > > How is the client supposed to handle opens of device special > files? > > > > On > > > > > a 3.1-rc1-based client to a linux server over v4.0 I'm seeing > it try > > > > an > > > > > OPEN call and failing when it gets an INVAL return. > > > > > > > > > > This looks like bogus client behavior (OPEN should fail on such > > > > files), > > > > > unless the server has the error return wrong and the client's > using an > > > > > OPEN error to recover. > > > > > > > > > > If I first stat the device and then open it then it works as > expected > > > > > (the client does an open of the local device). > > > > > > > > > > I'm a bit annoyed at myself as I have a feeling we've discussed > this > > > > > before but I can't find a reference now. > > > > > > > > Hmm... NFS4ERR_INVAL means 'invalid argument', which is not the > case > > > > here; all the arguments that the client is passing to the server > are > > > > valid, however the file cannot be opened because the pathname > resolves > > > > on the server to a file of the wrong type. > > > > The long description of NFS4ERR_INVAL from rfc 3530: > > > > "Invalid argument or unsupported argument for an operation. Two > > examples are attempting a READLINK on an object other than a > > symbolic link or specifying a value for an enum field that is > > not defined in the protocol (e.g., nfs_ftype4)." > > > > So the text does recommend NFS4ERR_INVAL for the case where the > > operation doesn't make sense for the type of object given. (In the > > readlink example there's no pathname resolution, though). > > > > EINVAL seems to be used in the kernel for some such cases as well > (e.g. > > attempt to truncate a device special file). > > > > > > I can't see any other error definition that is "obviously > correct", but > > > > it looks to me as if NFS4ERR_SYMLINK might be the closest thing. > One > > > > reason is the dot-x file defines NFS4ERR_SYMLINK as meaning > "should be > > > > file/directory". > > > > And this seems more a coincidence due to vagueness resulting from the > > need for a single-line comment; the longer description is: > > > > "NFS4ERR_SYMLINK The current filehandle provided for a LOOKUP is > > not a directory but a symbolic link. Also used > > if the final component of the OPEN path is a > > symbolic link." > > > > > > The other reason is that NFS4ERR_SYMLINK should > > > > _always_ trigger the correct behaviour on a client: a fresh > lookup of > > > > the component. > > > > > > Confirmed the fix; I'll apply. > > > > Applying anyway for compatibility with existing clients, but this all > > seems a little weird. > > And the change also makes a bunch of pynfs tests fail, complaining that > various operations against incompatible types should have returned > INVAL > and not SYMLINK. > > Not that I'm convinced pynfs is correct--at the very least it should > have accepted a range of errors for those tests, I think--but anyone > else that ran pynfs against their server may have assumed it pynfs was > correct in these cases.... Pynfs is not an authoritative source for anything. Trond -- 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 Mon, Aug 15, 2011 at 10:03:49PM -0700, Myklebust, Trond wrote: > > -----Original Message----- > > From: J. Bruce Fields [mailto:bfields@fieldses.org] > > The long description of NFS4ERR_INVAL from rfc 3530: > > > > "Invalid argument or unsupported argument for an operation. Two > > examples are attempting a READLINK on an object other than a > > symbolic link or specifying a value for an enum field that is > > not defined in the protocol (e.g., nfs_ftype4)." > > > > So the text does recommend NFS4ERR_INVAL for the case where the > > operation doesn't make sense for the type of object given. (In the > > readlink example there's no pathname resolution, though). > > Right. In the case of readlink, you pass a file handle, which points to > an object for which you _can_ check the type before attempting the > readlink operation. > > OPEN (and LOOKUP, RENAME, REMOVE,...) takes a filename, which could > point to anything. There is no way for the client to know a priori the > type of the object being pointed to. > > > EINVAL seems to be used in the kernel for some such cases as well > (e.g. > > attempt to truncate a device special file). > > My reading of the manpages and the posix spec is that truncate() will > return EINVAL if and only if the length argument is wrong. Ftruncate() > will in addition return EINVAL if the file descriptor points to a > non-regular file (the assumption presumably being that you should have > tried to fstat() first). > > IOW: The criterion that they apply is that "if the user should have > known better" then EINVAL is correct. If there is no way for the user to > be sure a priori, then it is not. So, for instance, rmdir() will not > return EINVAL if you try to apply it to a file. That's perfectly logical, but, taking the example of (f)truncate: - truncate(2) as far as I can tell doesn't specify behavior for truncate in this case, only for ftruncate. - http://pubs.opengroup.org/onlinepubs/009695399/ says behavior is undefined for ftruncate, and says nothing for truncate. - linux actually returns EINVAL in both cases: $ sudo mknod tmp c 1 3 -m 666 $ make trunc-vs-ftrunc cc trunc-vs-ftrunc.c -o trunc-vs-ftrunc $ ./trunc-vs-ftrunc tmp trunc-vs-ftrunc: truncate: Invalid argument trunc-vs-ftrunc: ftruncate: Invalid argument But I don't really know why I'm arguing here--I don't disagree. You have a reasonable way to define EINVAL. This just appears to be easy to get wrong.... --b. trunc-vs-ftrunc.c: #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #include <unistd.h> #include <err.h> int main(int argc, char *argv[]) { int fd, ret; if (argc != 2) errx(1, "usage: %s path\n", argv[0]); ret = truncate(argv[1], 0); if (ret) warn("truncate"); fd = open(argv[1], O_WRONLY); if (fd < 0) err(1, "open"); ret = ftruncate(fd, 0); if (ret) warn("ftruncate"); } -- 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 08/15/2011 05:25 PM, J. Bruce Fields wrote: > On Mon, Aug 15, 2011 at 09:03:30AM -0700, Myklebust, Trond wrote: >>> -----Original Message----- >>> From: J. Bruce Fields [mailto:bfields@fieldses.org] >>> How is the client supposed to handle opens of device special files? >> On >>> a 3.1-rc1-based client to a linux server over v4.0 I'm seeing it try >> an >>> OPEN call and failing when it gets an INVAL return. >>> >>> This looks like bogus client behavior (OPEN should fail on such >> files), >>> unless the server has the error return wrong and the client's using an >>> OPEN error to recover. >>> >>> If I first stat the device and then open it then it works as expected >>> (the client does an open of the local device). >>> >>> I'm a bit annoyed at myself as I have a feeling we've discussed this >>> before but I can't find a reference now. >> >> Hmm... NFS4ERR_INVAL means 'invalid argument', which is not the case >> here; all the arguments that the client is passing to the server are >> valid, however the file cannot be opened because the pathname resolves >> on the server to a file of the wrong type. >> >> I can't see any other error definition that is "obviously correct", but >> it looks to me as if NFS4ERR_SYMLINK might be the closest thing. One >> reason is the dot-x file defines NFS4ERR_SYMLINK as meaning "should be >> file/directory". The other reason is that NFS4ERR_SYMLINK should >> _always_ trigger the correct behaviour on a client: a fresh lookup of >> the component. > > Confirmed the fix; I'll apply. Confirmed! The following open now works with all NFS versions: int main (void) { int fd; char buf[20]; size_t nbytes; ssize_t bytes_read; nbytes = sizeof(buf); fd = open("/mnt/dev/urandom", O_RDONLY|O_NOCTTY|O_NONBLOCK); if (fd < 0) { perror("open"); exit(1); } bytes_read = read(fd, buf, nbytes); return 0; } Thanks! steved. > > But that's tricky--we should be document it for other server > implementers if it's the right thing to do (working group list cc'd). > > --b. > > commit 3d83c016da8652f30dcac372772b67d68f33bfbd > Author: J. Bruce Fields <bfields@redhat.com> > Date: Mon Aug 15 16:55:02 2011 -0400 > > nfsd4: return nfserr_symlink on v4 OPEN of non-regular file > > Without this, an attempt to open a device special file without first > stat'ing it will fail. > > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c > index 90c6aa6..cc8eb8d 100644 > --- a/fs/nfsd/nfsfh.c > +++ b/fs/nfsd/nfsfh.c > @@ -69,6 +69,17 @@ nfsd_mode_check(struct svc_rqst *rqstp, umode_t mode, int type) > return nfserr_notdir; > else if ((mode & S_IFMT) == S_IFDIR) > return nfserr_isdir; > + /* > + * err_symlink is our catch-all error in the v4 case; this > + * looks odd, but: > + * - the comment next to ERR_SYMLINK in file is > + * "should be file/directory" > + * - we happen to know this will cause the linux v4 > + * client to do the right thing on attempts to open > + * something other than a regular file: > + */ > + else if (rqstp->rq_vers == 4) > + return nfserr_symlink; > else > return nfserr_inval; > } -- 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/nfsfh.c b/fs/nfsd/nfsfh.c index 90c6aa6..cc8eb8d 100644 --- a/fs/nfsd/nfsfh.c +++ b/fs/nfsd/nfsfh.c @@ -69,6 +69,17 @@ nfsd_mode_check(struct svc_rqst *rqstp, umode_t mode, int type) return nfserr_notdir; else if ((mode & S_IFMT) == S_IFDIR) return nfserr_isdir; + /* + * err_symlink is our catch-all error in the v4 case; this + * looks odd, but: + * - the comment next to ERR_SYMLINK in file is + * "should be file/directory" + * - we happen to know this will cause the linux v4 + * client to do the right thing on attempts to open + * something other than a regular file: + */ + else if (rqstp->rq_vers == 4) + return nfserr_symlink; else return nfserr_inval; }