On Thu, 4 Apr 2013 14:30:12 +0400 Pavel Shilovsky [email protected] wrote:
2013/3/12 Jeff Layton [email protected]:
On Mon, 11 Mar 2013 14:54:34 -0400 Jeff Layton [email protected] wrote:
On Thu, 28 Feb 2013 19:25:32 +0400 Pavel Shilovsky [email protected] wrote:
by passing these flags to NFSv4 open request.
Signed-off-by: Pavel Shilovsky [email protected]
fs/nfs/nfs4xdr.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 26b1439..58ddc74 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -1325,7 +1325,8 @@ static void encode_lookup(struct xdr_stream *xdr, const struct qstr *name, struc encode_string(xdr, name->len, name->name); }
-static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode) +static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode,
int open_flags)
{ __be32 *p;
@@ -1343,7 +1344,22 @@ static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode) default: *p++ = cpu_to_be32(0); }
- *p = cpu_to_be32(0); /* for linux, share_deny = 0 always */
- if (open_flags & O_DENYMAND) {
As Bruce mentioned, I think a mount option to enable this on a per-fs basis would be a better approach than this new O_DENYMAND flag.
switch (open_flags & (O_DENYREAD|O_DENYWRITE)) {
case O_DENYREAD:
*p = cpu_to_be32(NFS4_SHARE_DENY_READ);
break;
case O_DENYWRITE:
*p = cpu_to_be32(NFS4_SHARE_DENY_WRITE);
break;
case O_DENYREAD|O_DENYWRITE:
*p = cpu_to_be32(NFS4_SHARE_DENY_BOTH);
break;
default:
*p = cpu_to_be32(0);
}
- } else
*p = cpu_to_be32(0);
}
static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_openargs *arg) @@ -1354,7 +1370,7 @@ static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_opena
- owner 4 = 32
*/ encode_nfs4_seqid(xdr, arg->seqid);
- encode_share_access(xdr, arg->fmode);
- encode_share_access(xdr, arg->fmode, arg->open_flags); p = reserve_space(xdr, 36); p = xdr_encode_hyper(p, arg->clientid); *p++ = cpu_to_be32(24);
@@ -1491,7 +1507,7 @@ static void encode_open_downgrade(struct xdr_stream *xdr, const struct nfs_close encode_op_hdr(xdr, OP_OPEN_DOWNGRADE, decode_open_downgrade_maxsz, hdr); encode_nfs4_stateid(xdr, arg->stateid); encode_nfs4_seqid(xdr, arg->seqid);
- encode_share_access(xdr, arg->fmode);
- encode_share_access(xdr, arg->fmode, 0);
}
static void
Other than that, this seems reasonable.
Acked-by: Jeff Layton [email protected]
Oh duh...
Please ignore my comment on patch #7 to add a patch for the NFS client. This one does that. That said, there may be a potential problem here that you need to consider.
In the case of a local filesystem you'll want to set deny locks using deny_lock_file(). For a network filesystem like CIFS or NFS though, the server will handle that atomically during the open. You need to ensure that you don't go trying to set LOCK_MAND locks on the file once that's done.
Perhaps you can use a fstype flag to indicate that the filesystem handles this during the open and doesn't need to try and set a flock lock?
Also, we can simply mask off O_DENY* flags in open (and atomic_open) codepath of filesystems that support these flags:
... do open request to the storage ... file->f_flags &= ~(O_DENYREAD | O_DENYWRITE | O_DENYDELETE); ... return to VFS ...
Thoughts?
I'd probably still stick with a FS_* flag for this...
That sort of mechanism would work (for now) but sounds like the sort of subtle behavior that's difficult for filesystem authors to get right. It would also be subject to subtle breakage later.
Also, suppose there are changes in the future that require you to determine this before calling into ->open? Then you'll have to go back and somehow mark the fs anyway...