On Jun. 15, 2010, 12:19 -0400, Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote:
> 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?
>
Duh. Sorry, I missed that.
Benny
>>>>> + 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
--
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
