Re: [Gen-art] Gen-art LC review of draft-ietf-nfsv4-rfc3530bis-33

[Tom Haynes <[e-mail]>]

> On Dec 1, 2014, at 6:41 AM, Elwyn Davies <[e-mail]> wrote:
> 
> Hi, Tom and David.
> 
> Sorry to have been dilatory in running through the v35 that Tom sent me.
> 
> This looks pretty good to me and has sorted out almost everything that I flagged up in the review. In particular s7.8 is much improved.
> 
> The one problem that I have left is the issue of directory contents being changed under the feet of a multi-part readdir request.  The last mail on this subject from 14 November said:
>> 
>> 
>>> On Nov 14, 2014, at 3:05 PM, Tom Haynes <[e-mail]> wrote:
>>> 
> 
> <<snip>>
> 
>>>>>> On Nov 11, 2014, at 12:48 PM, Elwyn Davies <[e-mail]
>>>>>> <mailto:[e-mail]>> wrote:
>>>>>> 
>>> 
>>>>> 
>>>>>> 
>>>>>> s15.26.4/s15.26.5: What happens if the directory has been modified
>>>>>> (entries added or deleted) between calls of READDIR intended to
>>>>>> retrieve segments of the directory information?  Related to this,
>>>>>> presumably no ordering of entries is implied in the returned
>>>>>> information - it would be desirable to state this.
>>>>> 
>>>>> 
>>>>> This is where the cookieverf is supposed to come in.
>>>>> 
>>>>> In most implementations, the cookie needs to be persistent, which indeed
>>>>> means an ordering of the information.
>>>> 
>>>> AFAICS the text doesn't tie the cookieverf to the change attribute of the
> >>> directory.  Presumably this is what is intended - it would certainly mean
> 
> that changes would be picked up and READDIR would have to start from the beginning again.
>>>>> 
>>> 
>>> Asking an expert…
>> 
>> From Trond:
>> 
>> As far as I know, the cookieverf is about validating the cookies
>> themselves. There is no requirement in any of the descriptions in
>> RFC1813, RFC3530 or RFC5661 that states that the contents of the
>> directory need to change when the cookieverf does or vice-versa.
>> In theory, the server could throw out the cookies and regenerate them
>> on reboot, for instance.
>> 
> 
> My view (remains) that if the cookie is not tied to the change attribute of the directory, this makes life difficult for both the server (what does it send if the data has been changed and how does it manage maintaining checkpoints -- where a readdir finished --, in particular if the checkpoint was on an entry that has been deleted), and the client.
> 
> Thoughts?
> 
> 
> I also found a some minor typos (mostly finger trouble in the new bits) and there are a couple of nits outstanding (see below).
> 
> Are you intending to publish a -35 before Thursday?
> 
> Regards,
> Elwyn
> 
> Nits:
> =====
> s2.1: nfs_lease4 definition still missing.


Fixed

> 
> s8.5, last para: I am not sure that the jargon 'trunking relation' is widely understood. [I don't think I have ever come across it.]


This is RFC 5661 jargon.

New text:

   If a single location entry designates multiple server IP addresses,
   the client should choose a single one to use.  When two server
   addresses are designated by a single location entry and they
   correspond to different servers, this normally indicates some sort of
   misconfiguration, and so the client should avoid using such location
   entries when alternatives are available.  When they are not, clients
   should pick one of IP addresses and use it, without using others that
   are not directed to the same server.


> 
> s19.4.1, para 2:
> >    For the purposes of open delegation, READs and WRITEs done without an
> >    OPEN are treated as the functional equivalents of a corresponding
> >    type of OPEN.  This refers to the READs and WRITEs that use the
> >    anonymous stateid.  Therefore, READs or WRITEs with an anonymous
> >    stateid done by another client will force the server to recall a
> >    OPEN_DELEGATE_WRITE delegation.  A WRITE with an anonymous stateid
> >    done by another client will force a recall of OPEN_DELEGATE_READ
> >    delegations.
> 
> Did you mean to take out the 'all 1's case' which was an alternative in v33?


Dave, was this your editorial change?


> 
> s9.10, para 1:
>> to use a stateid of all 0's or all 1's
> Ought to be changed to "anonymous" and "READ bypass”

I kinda get the first change:

      Even if the
      client intends to use an anonymous stateid, it must
      still obtain the filehandle for the regular file with the OPEN
      operation so the appropriate share semantics can be applied.

I don’t follow the second request...

> 
> s9.11, last para; s15.18.5, 5th from last para; s15.20.4, para 1: Reference s9.1.3 about incrementing seqid. (Three places all told.)

Done

> 
> Editorial:
> ==========
> 
> s5.4, 2nd bullet labelled '1.': s/common to all object within given file system./common to all objects within the given file system./
> [Very nitty: Could use a different label type for the second set of bullets.]
> 
> s6.2: s/RECOMMENED/RECOMMENDED/

done

> 
> s8.8, para 1: s/an zero-length string/a zero-length string/ [Missed it last time].

done

> 
> s9: s/has a secondary purpose/have a secondary purpose/

done

> 
> New s9.1.3 looks good -- except for a bit of finger trouble
> s9.1.3, para 4: s/cuurent/current/
> s9.1.3, para 5: s/raesonably/reasonably/
> s9.1.3, para 6: s/wraparoundby/wraparound by/, s/gretater/greater/
> 
> s9.1.6, para 2: s/consiting/consisting/
> 
> s9.1.7, para 1: s/Subseuent/Subsequent/


done

> 
> s10.3.1, first bullet: s/(such as an OPENs specifying DENY=NONE)/(such as for [or at or on] OPENs specifying DENY=NONE)/

at

> 
> s12.4, 1st bullet: s/has a non-UT8 encoded/has a non-UTF8 encoded/

done

>                               ^^^
> s15.2.5, para 2: s/in which operation that/in which operations that/

ok

> 
> s15.11.5, para 5: s/the server may return the error NFS4ERR_NOTSUPP./the server MAY return the error NFS4ERR_NOTSUPP./ (I think)

ok

> 
> s15.18.6, last para: s/tfor the same client/for the same client/,
>            s/i size and change attribute/size and change attributes/
>              ^^^                     ^^^
> 

ok


> s15.26.4/s15.26.5:  Not convinced about discussion on directory info changing under the feet of readdir.


I think Dave is responding to this...

> 
> s17, para 7: s/match wth the previous/match with the previous/
>                     ^^^
> 
> 


Caught some whch as well. :-)


> 
> On 18/11/14 15:17, Tom Haynes wrote:
>> 
>>> On Nov 18, 2014, at 1:20 AM, Elwyn Davies <[e-mail]> wrote:
>>> 
>>> Hi.
>>> 
>>> Could you send me a snapshot of the updated version?  I am getting a bit lost on the results of the various changes.
>>> 
>>> Cheers,
>>> Elwyn
>>> 
>>> 
>> 
>> 
>> Hi,
>> 
>> I was wondering about that. :-)
>> 
>> Later,
>> Tom
>> 
>> 
>> 
>> 
>> 
>>