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

[Thomas Haynes <[e-mail]>]

> 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.






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