[nfsv4] Re: question about delegated timestamps (delstid draft)

[Thomas Haynes <[e-mail]>]

> On Sep 10, 2024, at 9:57 AM, Jeff Layton <[e-mail]> wrote:
> 
> On Tue, 2024-09-10 at 09:53 -0700, Thomas Haynes wrote:
>> 
>> 
>>> On Sep 10, 2024, at 9:22 AM, Jeff Layton <[e-mail]> wrote:
>>> 
>>> On Tue, 2024-09-10 at 09:02 -0700, Thomas Haynes wrote:
>>>> 
>>>>> On Aug 29, 2024, at 5:42 AM, Jeff Layton <[e-mail]> wrote:
>>>>> 
>>>>> On Wed, 2024-08-28 at 15:34 -0700, Thomas Haynes wrote:
>>>>>> 
>>>>>>> On Aug 28, 2024, at 10:13 AM, Jeff Layton <[e-mail]> wrote:
>>>>>>> 
>>>>>>> The draft says mentions this:
>>>>>>> 
>>>>>>> Further, when it gets a SETATTR in the same compound as the
>>>>>>> DELEGRETURN, then it MUST accept those fattr4_time_deleg_access
>>>>>>> attribute and fattr4_time_deleg_modify attribute changes and derive
>>>>>>> the change time or reject the changes with NFS4ERR_DELAY (see
>>>>>>> Section 15.1.1.3 of [RFC8881]).
>>>>>>> 
>>>>>>> Presumably, the SETATTR will precede the DELEGRETURN in the compound,
>>>>>>> in which case (at least under Linux kernel server) we will not have
>>>>>>> vetted the DELEGRETURN stateid yet.
>>>>>>> 
>>>>>>> The simplest fix might be to just have it accept SETATTR for the
>>>>>>> delegated atime and mtime iff the client is the owner of the write
>>>>>>> delegation.
>>>>>>> -- 
>>>>>>> Jeff Layton <[e-mail]>
>>>>>> 
>>>>>> 
>>>>>> Looking at the HS implementation, we do just that, we accept it if the client is the owner of the WRITE delegation.
>>>>>> 
>>>>>> I.e., we do not insist that there is a DELEGRETURN.
>>>>> 
>>>>> Thanks for confirming it. Maybe we should update the draft along those
>>>> 
>>>>> lines too? Have it say that the SETATTR that sends these new attrs must
>>>>> contain a WRITE delegation stateid, and drop the language about having
>>>>> a DELEGRETURN in the compound?
>>>>> 
>>>>> -- 
>>>>> Jeff Layton <[e-mail]>
>>>> 
>>>> 
>>>> Hey Jeff,
>>>> 
>>>> Work interfered, so just now getting back to this.
>>>> 
>>>> The next paragraph in the draft states:
>>>> 
>>>>   These new attributes are invalid to be used with GETATTR, VERIFY, and
>>>>   NVERIFY and can only be used with CB_GETATTR and SETATTR by a client
>>>>   holding an appropriate delegation.  The SETATTR SHOULD either be in a
>>>>   separate compound before the one containing the DELEGRETURN or when
>>>>    in the same compound, as an operation before the DELEGRETURN.
>>>>   Failure to properly sequence the operations may lead to race
>>>>   conditions.
>>>> 
>>>> Why can’t it send a SETATTR in the same compound as the DELEGRETURN?
>>>> 
>>> 
>>> Oh, it absolutely can. When I read the paragraph, I took it as if the
>>> server had to accept those attrs if there was a DELEGRETURN in the same
>>> compound.
>>> 
>>> Maybe I'm just being pedantic, but I don't think we want to allow read
>>> delegation holders to do this, just because they happen to be returning
>>> the deleg in the same compound.
>>> 
>>>> I guess I would rewrite the paragraph you quote as simply:
>>>> 
>>>>   Further, when it gets a SETATTR with those attributes being set, then it MUST accept those fattr4_time_deleg_access
>>>>   attribute and fattr4_time_deleg_modify attribute changes and derive
>>>>   the change time or reject the changes with NFS4ERR_DELAY (see
>>>>   Section 15.1.1.3 of [RFC8881]).
>>>> 
>>> 
>>> I think you just want to make it clear that the only client that can
>>> set these attrs is the one holding the write delegation.
>> 
>> 
>> 
>> It already says:
>> 
>>    These new attributes are invalid to be used with GETATTR, VERIFY, and
>>    NVERIFY and can only be used with CB_GETATTR and SETATTR by a client
>>    holding an appropriate delegation. 
>> 
>> This might be the case that the information is too spread out and that I am too close to the draft.
>> 
>> 
> 
> Or I just didn't read closely enough...
> 
> 
> Fair enough! I think that covers it after all. Sorry for the noise!

No, thanks for the question!


> -- 
> Jeff Layton <[e-mail] <mailto:[e-mail]>>