On Tue, 2010-06-15 at 12:08 -0400, Benny Halevy wrote: > On Jun. 15, 2010, 11:50 -0400, Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote: > > On Mon, 2010-06-14 at 19:13 -0400, Benny Halevy wrote: > >> On Jun. 14, 2010, 17:51 -0400, Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote: > >>> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> > >>> --- > >>> fs/nfs/nfs4proc.c | 17 ++++++----------- > >>> include/linux/nfs_xdr.h | 6 ++++-- > >>> 2 files changed, 10 insertions(+), 13 deletions(-) > >>> > >>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > >>> index ef613ea..e9913de 100644 > >>> --- a/fs/nfs/nfs4proc.c > >>> +++ b/fs/nfs/nfs4proc.c > >>> @@ -744,19 +744,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct path *path, > >>> p->o_arg.server = server; > >>> p->o_arg.bitmask = server->attr_bitmask; > >>> p->o_arg.claim = NFS4_OPEN_CLAIM_NULL; > >>> - if (flags & O_EXCL) { > >>> - if (nfs4_has_persistent_session(server->nfs_client)) { > >>> - /* GUARDED */ > >>> - p->o_arg.u.attrs = &p->attrs; > >>> - memcpy(&p->attrs, attrs, sizeof(p->attrs)); > >> > >> What about this case? > >> Aren't we losing the attrs? > > > > I'm not sure what you mean. Are you asking what will happen if someone > > sets O_EXCL without setting O_CREAT? We don't call encode_createmode at > > all in that case. Otherwise, see below. > > > > It looks like in the O_CREAT|O_EXCL case with persistent session > the attrs passed in to nfs4_opendata_alloc are not copied to p->attrs > and p->o_arg.u.attrs is not set. > Then later on, in encode_createmode, in the respective path, we still use it: > if (nfs4_has_persistent_session(clp)) { > *p = cpu_to_be32(NFS4_CREATE_GUARDED); > encode_attrs(xdr, arg->u.attrs, arg->server); > } else { > > Benny > > > Cheers > > Trond > > > >> > >>> - } else { /* EXCLUSIVE4_1 */ > >>> - u32 *s = (u32 *) p->o_arg.u.verifier.data; > >>> - s[0] = jiffies; > >>> - s[1] = current->pid; > >>> - } > >>> - } else if (flags & O_CREAT) { > >>> + if (flags & O_CREAT) { > >>> + u32 *s; > >>> + > >>> p->o_arg.u.attrs = &p->attrs; > >>> memcpy(&p->attrs, attrs, sizeof(p->attrs)); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Why isn't this sufficient? > >>> + s = (u32 *) p->o_arg.u.verifier.data; > >>> + s[0] = jiffies; > >>> + s[1] = current->pid; > >>> } > >>> p->c_arg.fh = &p->o_res.fh; > >>> p->c_arg.stateid = &p->o_res.stateid; > >>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h > >>> index 51914d7..a319cb9 100644 > >>> --- a/include/linux/nfs_xdr.h > >>> +++ b/include/linux/nfs_xdr.h > >>> @@ -196,8 +196,10 @@ struct nfs_openargs { > >>> __u64 clientid; > >>> __u64 id; > >>> union { > >>> - struct iattr * attrs; /* UNCHECKED, GUARDED */ > >>> - nfs4_verifier verifier; /* EXCLUSIVE */ > >>> + struct { > >>> + struct iattr * attrs; /* UNCHECKED, GUARDED */ > >>> + nfs4_verifier verifier; /* EXCLUSIVE */ > >>> + }; > >>> nfs4_stateid delegation; /* CLAIM_DELEGATE_CUR */ > >>> fmode_t delegation_type; /* CLAIM_PREVIOUS */ > >>> } u; > >> > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html