Ok, I think I know what to do. I will update patches later today or tomorrw.
Tigran.
On Tue, Dec 6, 2011 at 3:32 PM, Benny Halevy <bhalevy@xxxxxxxxxx> wrote:
> On 2011-12-06 15:40, J. Bruce Fields wrote:
>> On Tue, Dec 06, 2011 at 02:31:14PM +0100, Tigran Mkrtchyan wrote:
>>> On Tue, Dec 6, 2011 at 12:26 PM, Benny Halevy <bhalevy@xxxxxxxxxx> wrote:
>>>> On 2011-12-06 04:08, J. Bruce Fields wrote:
>>>>> On Sun, Dec 04, 2011 at 02:42:11PM +0200, Benny Halevy wrote:
>>>>>> On 2011-12-04 14:03, tigran.mkrtchyan@xxxxxxx wrote:
>>>>>>> From: Tigran Mkrtchyan <tigran.mkrtchyan@xxxxxxx>
>>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@xxxxxxx>
>>>>>>> ---
>>>>>>> fs/nfsd/nfs4proc.c | 6 ++++++
>>>>>>> fs/nfsd/nfs4state.c | 26 ++++++++++++++++++++------
>>>>>>> 2 files changed, 26 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>>>>>> index fa38336..535aed2 100644
>>>>>>> --- a/fs/nfsd/nfs4proc.c
>>>>>>> +++ b/fs/nfsd/nfs4proc.c
>>>>>>> @@ -400,6 +400,12 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>>>>>> */
>>>>>>> status = nfsd4_process_open2(rqstp, &cstate->current_fh, open);
>>>>>>> WARN_ON(status && open->op_created);
>>>>>>> +
>>>>>>> + if(status)
>>>>>>> + goto out;
>>>>>>> +
>>>>>>> + /* set current state id */
>>>>>>> + memcpy(&cstate->current_stateid, &open->op_stateid, sizeof(stateid_t));
>>>>>
>>>>> That comment is a bit redundant.
>>>>>
>>>>>> Since this should be done for all stateid-returning operations
>>>>>> I think that a cleaner approach could be to mark those as such in
>>>>>> nfsd4_ops by providing a per-op function to return the operation's
>>>>>> stateid. You can then call this method from nfsd4_proc_compound()
>>>>>> after the call to nfsd4_encode_operation() and when status == 0.
>>>>>
>>>>> So the choice is between
>>>>>
>>>>> + memcpy(&cstate->current_stateid, &open->op_stateid,
>>>>> sizeof(stateid_t));
>>>>>
>>>>> and
>>>>>
>>>>> + static void get_open_stateid(stateid_t *s)
>>>>> + {
>>>>> + memcpy(s, open->op_stateid);
>>>>> + }
>>>>> +
>>>>> + [OP_OPEN] = {
>>>>> + ...
>>>>> + .op_get_stateid = get_open_stateid,
>>>>> + ...
>>>>> + }
>>>>>
>>>>> ?
>>>>>
>>>>> I'm not so sure.
>>>>
>>>> The point is to copy the result stateid into the current_stateid
>>>> in a centralized place: nfsd4_proc_compound() and do that for all
>>>> stateid-modifying operations.
>>>
>>> Sounds good, nevertheless all operations call differently resulting
>>> stateid which make it's not possible to use a unified schema.
>
> That was the point of introducing a per-op method to get the value.
>
>>
>> Could we just remove all (or most) of those different fields and replace
>> them by cstate->current_stateid?
>
> Hmm, that will require changing the corresponding xdr result encoding
> routines to use the new field and all other use sites, but I don't think
> of a good reason why this won't work.
>
> Benny
>
>>
>> --b.
>> --
>> 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
