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

From: Elwyn Davies <[email-masked]>
Date: Tue, 11 Nov 2014 20:48:37 +0000

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-nfsv4-rfc3530bis-33.txt
Reviewer: Elwyn Davies
Review Date: 2014-11-06
IETF LC End Date: 2014-10-06
IESG Telechat date: 2014-12-04

Summary:
Much more ready than last time I reviewed this draft!  There are a few 
issues at the borderline between editorial and minor plus quite a lot of 
editorial nits.

Major Issues:

Minor Issues:
s8.8.1:  I suspect that this section should have been deleted when s7.7 
was deleted between v26 and v27.  The references to various server class 
attributes (handle class, change class, etc) are orphaned with no 
definitions of these items

-----------------------------------------

Handling of wrap around of seqid4 variables:
============================================
The specification of how variables/fields of type seqid4 are incremented 
and compared, dealing with wrap around is very messy.
Currently there are two separate descriptions of handling wrap around 
when a seqid4 variable is incremented through NFS4_UINT32_MAX (which 
turns out to be the all ones binary pattern.  The descriptions are 
slightly different but not incompatible.
The following text appears in s9.1.6
>    Since the sequence number is represented with an unsigned 32-bit
>    integer, the arithmetic involved with the sequence number is mod
>    2^32.  Note that when the seqid wraps, it SHOULD bypass zero and use
>    one as the next seqid value.  For an example of modulo arithmetic
>    involving sequence numbers see [RFC0793].
and s9.1.3.2 has:
>    This pattern continues until the seqid is incremented past
>    NFS4_UINT32_MAX, and one (not zero) SHOULD be the next seqid value.
>    The purpose of the incrementing of the seqid is to allow the server
>    to communicate to the client the order in which operations that
>    modified locking state associated with a stateid have been processed.
>
>    In making comparisons between seqids, both by the client in
>    determining the order of operations and by the server in determining
>    whether the NFS4ERR_OLD_STATEID is to be returned, the possibility of
>    the seqid being swapped around past the NFS4_UINT32_MAX value needs
>    to be taken into account.
>
The definition of the seqid4 type in s2.1 doesn't mention this sort of 
constraint.

Incrementing of seqid4 type fields is mentioned in a number of sections 
along with special values (including 0):
s9.1.3.1, bullet 1 (also bullet 2):
>                 Such stateids are subject
>       to change (with consequent incrementing of the stateid's seqid) in
>       response to OPENs that result in upgrade and OPEN_DOWNGRADE
>       operations.

s9.1.3.3: Refers to special values of seqid (all zeroes, all ones)

s9.1.3.4, two bullets before last bullet have:
>    o  If the "seqid" field is not zero, and it is greater than the
>       current sequence value corresponding the current "other" field,
>       return NFS4ERR_BAD_STATEID.draft-ietf-nfsv4-rfc3530bis
>
>    o  If the "seqid" field is less than the current sequence value
>       corresponding the current "other" field, return
>       NFS4ERR_OLD_STATEID.
>

s9.11, last para:
>    The "seqid" value in the returned stateid
>    MUST be incremented, even in situations in which there is no change
>    to the access and deny bits for the file.

s15.8.5, para 3 before last:
>    In this case, the
>    stateid returned as an "other" field that matches that of the
>    previous open while the "seqid" field is incremented to reflect the
>    change status due to the new open.
>
It would be much cleaner if the texts from s9.1.3.2 and s9.1.6 were 
merged, probably into a section just after 2.1 (say s2.1.1) together 
draft-ietf-nfsv4-rfc3530biswith mention of the special values.  If this 
is done, s2.1.1 can then be referenced wherever a seqid is mentioned as 
being incremented.

Also, I cannot see why the current texts use SHOULD.  I am pretty 
certain that skipping 0 and going to 1 from NFS4_UINT32_MAX is a MUST. 
If it isn't a MUST, what alternative scheme will make the wrap around 
work without both client and server knowing what scheme is being used? 
This is at least partly implied by the definition of NFS4ERR_BAD_SEQID 
in s13.1.8.2 which indicates that both ends have a common idea of what 
the next expected number ought to be..

-----------------------------------

change_info4:  Wrap around in before and after fields.
======================================================
I assume that it is deemed that using a 64 bit number as changeid4 
should obviate the need for dealing with wrap around.  However I don't 
think there is any advice on starting values for the change values - 
some strategies (e.g., random values) could result in change indicators 
starting uncomfortably close to the maximum value and running up against 
wrap around.  A brief note on this would be desirable.

-----------------------------------

s10.7: Informing the server that a file is being memory mapped.
Is there a mechanism? If not should there be (e.g., a flag on OPEN)?

-----------------------------------

s15.2.3, paras 4 and 7: The last sentence in para 7 that implies 
NFS4ERR_MINOR_VERSION_MISMATCH takes priority over all others appears to 
be incompatible with the second XDR decoding strategy in para 4.  If 
decoding of the operation array is interleaved with performing the 
requested operations then it seems possible that the COMPOUND may be 
terminated by an earlier error before the offending operation 2 is 
reached, so that it would never be decoded.  Further, I don't understand 
why operation 2 is singled out for special discussion here, other than 
maybe to say that it is available for future use.  How does it differ 
from any other value that is not supported by the minorversion 
implemented by the server?  Why should the base version be singled out 
for special treatment?

Nits/editorial comments:
========================

Requirements Language note (after Abstract): The terms REQUIRED and 
RECOMMENDED are overloaded in this document, in that they are used both 
in the RFC 2119 sense and as adjectives that characterize attributes. It 
would be a good idea to avoid confusion by providing an extension of the 
usual form of words in this section, such as:
NEW:
    The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
    "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
    document are to be interpreted as described in RFC 2119 [RFC2119]
    except where "REQUIRED" and "RECOMMENDED" are used as qualifiers
    to distinguish classes of attributes as described in Sections
    1.3.5.2 and 5.

General: There are a small number of instances where the form '<name> 
recommended attribute' and similar is used.  It would be clearer to use 
'RECOMMENDED attribute in these cases:
- s15.17.5, next to last para.
- s15.37.5, next to last para.
- s2.2.7

s1.3.3.2: Worth reinforcing that attribute names are (presumably) 
encoded in utf8 as they are just fileobject names.

s1.3.4:  A pointer to s9.9 for an explanation 'share semantics' would be 
very useful.  Copying the first para of s9.9 or something similar to the 
terminology section would also help.

s1.4, Definition of 'Lease', para 2:
OLD:
       All leases granted by a server have the same fixed interval.  Note
       that the fixed interval was chosen to alleviate the expense a
       server would have in maintaining state about variable length
       leases across server failures.
NEW:
       All leases granted by a server have the same fixed duration.  Note
       that the fixed interval duration was chosen to alleviate the
       expense a
       server would have in maintaining state relating to variable length
       leases across server failures.

s1,4, Definition of Stateid:  Definitions of open-owner and lock-owner 
would be helpful.  The terms are used a few times before they are 
defined in s9.

s2: Useful to add a reminder that the size constants and definitive 
definitions can be found in [RFCNFSv4XDR].

s2.1:  Should this have typedef opaque linktext4<>; instead of
        typedef opaque linktext4; ?

s2.2.5: s/mandatory/REQUIRED/

s2.2.10:
OLD:
    The r_netid and
    r_addr fields respectively contain a netid and uaddr.  The netid and
    uaddr concepts are defined in [RFC5665].  The netid and uaddr formats
    for TCP over IPv4 and TCP over IPv6 are defined in [RFC5665],
NEW:
    The r_netid and r_addr fields respectively contain a network id and
    universal address.  The network id  and universal address concepts
    together with formats for them with TCP over IPv4 and TCP over IPv6
    are defined in [RFC5665],

s3.3: The terms 'policy boundary' and 'crossing policy boundaries' are 
used in subsequent sections but not explicitly defined.  I assume that 
they refer to the client accessing fileobjects in areas accessed via the 
different entry points referred to in s3.3.  Assuming I am right 
introducing the 'policy boundary' term in s3.3 would be helpful.

s3.3.3, para 1: s/the security flavor the original SETCLIENTID/the 
security flavor of the original SETCLIENTID/

s4.2.3, para 2: s/mandatory/REQUIRED/

s5.3:  I think it would be worth being explicit as to whether hard links 
can be created between named attribute fileobjects either (1) for named 
attributes of the same fileobject and/or (2) for two different 
fileobjects on the same file system.  Is it also the case that if the 
basic file system supports hard links (link_support == TRUE) then it 
will necessarily support hard links between named attributes?  As I 
understand it, the named attribute system in NFSv4 was modelled on 
Solaris v9+ and the extended attribute system in Linux would probably 
not support hard links (I haven't tried them (yet)) even if the Solaris 
version does.... Later... I see from s13.1.4.14 (NFS4ERR_XDEV) that hard 
links between different named attribute directories are NOT allowed.  It 
would be good to make this explicit here in s5.3.  This still leaves the 
question of whether hard links are allowed in a single named attribute 
directory.  I can't see a lot of use for this capability but it should 
be explicitly allowed or disallowed as appropriate.

s5.6: There is no definition of nfs_lease4 in either this draft or the 
xdr draft.

s5.8.2.14: Presumably this is hard links: s/links/hard links/;  I assume 
that the number of symlinks is essentially unconstrained.

s5.10, last para: it would be a good idea to add a note to the RFC 
Editor to ensure that the statements about the Unicode version will be 
true on publication!

s5.10, last para: Is it conceivable that a client would need to know 
what version of Unicode a server was using ?  If so should there be a 
way for the client to find out what version of Unicode is in use by the 
server?

s6.1:
>       *  Setting only the mode attribute should provide reasonable
>          security.  For example, setting a mode of 000 should be enough
>          to ensure that future opens for read or write by any principal
>          fail, regardless of a previously existing or inherited ACL.
I think this may only apply to non-privileged principals - root can 
still read/write *ix files with mode 0.  Tie in with last para of s6.3.1.1.

s6.2.1, next to last para: I suppose SMB really ought to have a reference.

????? s6.2.1.3, last para:  The "optional" and "RECOMMENDED" should be 
in the same case - I think this should be both upper case RFC 2119 
requirements but it is arguable they should both be lower.

s6.4.3:  It is (still) not clear (at least, to me) how inheritance is 
supposed to apply to named attribute directories.  The named attribute 
directory has the file object to which it applies as its 'parent' rather 
than an ordinary directory.  Presumably inheritance would apply from the 
named attribute directory to the named attributes created in it - but 
are ACLs always supported on named attributes and the named attribute 
directory if they are suported on ordinary files and directories?

s7.8, para 1:  I am unable to parse the 3rd sentence in this para:
>    However, with the support of multiple security mechanisms
>    and the ability to negotiate the appropriate use of these mechanisms,
>    the server is unable to properly determine if a client will be able
>    to authenticate itself.

s8: s/OPTIONAL/optional/ - using multi-server namespaces  is a user 
choice not a protocol requirement.

s8.5:
>    If a single location entry designates multiple server IP addresses,
>    the client cannot assume that these addresses are multiple paths to
>    the same server.  In most cases, they will be, but the client MUST
>    verify that before acting on that assumption.
Is there some means in NFSv4 to do this?  I couldn't see that there was. 
  I guess one could use Reverse DNS.  If an out-of-band verification is 
needed, this should be made explicit.

s8.8: Copying the structure definitions from ss2.2.6/2.2.7 is 
unnecessary and could be better handled with refs.

s8.8.1, first and second bullets:
[This comment is redundant if s8.8.1 gets deleted as suggested in Minor 
Issues above.]
>    o  All listed file system instances should be considered as of the
>       same handle class if and only if the current fh_expire_type
>       attribute does not include the FH4_VOL_MIGRATION bit.
What if fh_expire_type has FH_VOLATILE_ANY set?  This is defined in 
s4.2.3 to imply FH4_VOL_MIGRATION making the 'setting of 
FH4_VOL_MIGRATION redundant'. The same issue applies to the second bullet.

s9.1.1, 2nd para after 2nd set if bullets (near the bottom of p98):
s/the client for purpose/the client for the purpose/

s9.1.1, last para: forward refs to the sections defining SETCLIENTID and 
SETCLIENTID_CONFIRM would be helpful.

s9.1.3.1:s/of guarantee/of a guarantee/

s9.1.5, para 2 and elsewhere in s9; also s10.4.1:
> If no state is established by the client, either
>    byte-range lock or share reservation, a stateid of all bits 0 is
>    used.
Does this differ from the 'anonymous stateid' defined in s9.1.3.3?  If 
not it would be better to refer to it by this title.  There are a number 
of other places in s9 where the expressions like 'stateid of all 0s/1s' 
are used.  It would be more consistent to use the 'anonymous' and 'READ 
bypass' titles set up in s9.1.3.3 in all cases.

s9.1.9, para 3:
>    In any event, the server is able to safely release state-owner state
>    (in the sense that retransmitted requests will not be erroneously
>    acted upon) when the state-owner no currently being utilized by the
                           ^^^^^^^^^^^^^^^^^^^^^^^^
>    client (i.e., there are no open files associated with an open-owner
>    and no lock stateids associated with a lock-owner).
Maybe 'state-owner is not currently'?

s9.1.9, para 3:
OLD:
> However, the period it chooses to hold
>    this state is implementation specific.

NEW:
> However, the period for which it chooses to hold
>    this state is implementation specific.

s9.1.10, para 1:
s/the sequence number may have any value./the sequence number may have 
any valid (i.e., non-zero) value./

s9.1.10, last para:
> thus can ensure that
>    confirmation will not be required.
s/ensure/be confident/

s9.2 (2 places),s10.5,s13.1.8.8: s/lock owner/lock-owner/ (total of 4 
instances)  - might be others at ends of lines that I missed.

s8.6 (4 places), s9.6.1: s/open owner/open-owner/ (total of 5 instances)

s9.6.3.1, 2nd bullet: s?IO?I/O?

s9.6.3.1, 3rd bullet: s/haled as courtesy/haled as a courtesy/

s9.8, para 5:
>  The client may accomplish this task by
>    issuing an I/O request, either a pending I/O or a zero-length read,
>    specifying the stateid associated with the lock in question.
How do you issue a "pending I/O request"??

s9.14.4, title: s/Lease_time/lease_time/

s10.2.1, para 9:
>    Note that the requirement stated above is not meant to imply that
>    when the client is no longer obliged, as required above, to retain
>    delegation information, that it should necessarily dispose of it.
>    Some specific cases are:
I think this para is referring to the server rather than the client: 
s/client/server/.

s10.2.1, para 26 (half way down p141):
> Whereas, for locks and share reservations, freeing of
>    locks will occur immediately upon the appearance of a conflicting
>    request, for delegations, the server may institute period during
>    which conflicting requests are held off.
s/server may institute period/server MAY institute a period/

s10.3.1, 1st bullet: s/The implementer is cautioned in this 
approach./The implementer is cautioned against this approach./

s10.4.3, next to last para: s/break down/breaks down/

s10.7, para 2: s/access remotely/accessed remotely/

s11: For the avoidance of confusion, it would be wise to note that 
REQUIRED and RECOMMENDED are referring to the RFC 2119 usage rather than 
the description of attributes in this section.

s11, item 2; s15.4, next to last para: The implication of the illegal 
operation test in s15.4 is that there should be no gaps in the allocated 
operation number sequence.  The operation numbers for minor version X 
must start immediately after those for version (X-1) and be in a 
continuous group.

s11, item 11; s15.4: If for some arcane reason a server supports minor 
version X (X >= 2) but does not support minor version Y (0 < Y < X) as 
allowed by the SHOULD in s11, should the server return NFS4ERR_NOTSUPP 
or an XDR decode error.  I would say that the SHOULD could lead to some 
considerable confusion for clients without a way to find out what 
versions a server implements.

s11, item 12: Some clarification of the term 'infrastructural' is needed.

s12.4: Has the dreaded BOM ever been encountered in the UTF-8 strings 
created by NFSv5 servers or clients?
    Therefore, the mask returned enumerating which access rights can be
    determined will have the ACCESS4_DELETE value set to 0.

s12.4, 1st bullet: s/non-UT8-name/non-UTF-8 encoded name/

s12.5, last para:@ s/without modifying name/without modifying the name/ 
or maybe "the names".

s12.6: The reference to RFC 5890 needs to be one para earlier to go with 
the first occurrence of U-label and should be added for A-label.

s13.1.10.1: s/client id/clientid/

s15.2.5:
>    The client SHOULD NOT construct a COMPOUND which mixes operations for
>    different client IDs.
Is this advisory on the grounds that it makes recovery difficult - in 
which case s/SHOULD NOT/should not/ - or something that might result in 
the server throwing an error - in which case please point to some 
discussion of why this is not legal. Later... this is also considered in 
s15.18.6, last para.  So there is one set of circumstances when it will 
screw up - but there are others where it is not problematical.  Perhaps 
this can be explained more clearly.

s15.3.5:
>    Therefore, the mask returned enumerating which access rights can be
>    determined will have the ACCESS4_DELETE value set to 0.
s/determined/supported/

s15.11:  Should this section talk about maxlink and the return of
NFS4ERR_MLINK?

s15.16.4: LOOKUPP is said to return the parent directory of a named 
attribute directory. Arguably a named attribute directory doesn't have a 
parent... so does LOOKUPP return the directory in which the object to 
'owning'  the named attribute directory resides?  Or does it return the 
owning object?  Please clarify in the text.

s15.18.2/s15.18.6/s15.21.2/s15.21.4, share_access and share_deny 
arguments: Presumably the shorthand DENY/READ/WRITE/BOTH values referred 
to are the relevant constants defined in s9.9?  If so please add a 
reference (and maybe use the full names in s15).  Otherwise please 
define the values.

s15.18.6, para 9:
> If the component provided to OPEN resolves to something other than a
>    regular file
Worth pointing out this includes named attribute files.

s15.23.4:
OLD:
    The public filehandle represents the concepts embodied in [RFC2054],
    [RFC2055], [RFC2224].  The intent for NFSv4 is that the public
    filehandle (represented by the PUTPUBFH operation) be used as a
    method of providing WebNFS server compatibility with NFSv2 and NFSv3.
NEW:
    The public filehandle concept was introduced with WebNFS described
    in [RFC2054], [RFC2055], and [RFC2224].  The intent for NFSv4 is
    that the public filehandle (provided by the PUTPUBFH operation) be
    used as a method of providing compatibility with the WebNFS server
    extension of NFSv2 and NFSv3.

s15.23.5, last para: s/public filehandle a method/public filehandle as a 
method/

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.

s15.29.4:
>    If the oldname refers to a named attribute and the saved and current
>    filehandles refer to different file system objects, the server will
>    return NFS4ERR_XDEV just as if the saved and current filehandles
>    represented directories on different file systems.
Presumably "filehandles refer to different file system objects" means 
the filehandles refer to the named attribute directories of different 
file system objects.  If so then add "the named attribute directories of".

s15.30.5, 1st bullet: s/client id/client ID/

s15.35.5, 1st bullet: s/client id/client ID/

s17, para 6: s/the client migrate its traffic/the client to migrate its 
traffic/

s17, para 7: s/is checked against and match/is checked against and 
matches with/

s17, para 8: s/see 5.9 and 12/see Sections 5.9 and 12/

s18.1, para 4: I don't believe you can have both policies of FCFS and 
Specification Required at the same time, and then with Expert Review for 
updates.  The proposal sounds like FCFS with a requirement that a 
description of format and intended usage is provided.

s19.2/s5.6: I guess it was intended...  s/ISEG_errata/IESG_ERRATA/

s19: RFC 2279 (UTF-8) is referenced as normative with RFC 3629 which 
obsoletes RFC 2279 as informative.  I think that references to RFC 2279 
should be replaced by RFC 3629 and this one made normative.

s19.2: Arguably RFC 5226 (IANA considerations) is normative.

---

From: Tom Haynes <[email-masked]>
Date: Wed, 12 Nov 2014 10:24:24 -0800

Hi Elwyn,

Thanks for the comprehensive review (again).

I’m probably 2/3rds done with the easy changes and I expect to reply within the next two days.

Tom

> On Nov 11, 2014, at 12:48 PM, Elwyn Davies <[email-masked]> wrote:
> 
> Much more ready than last time I reviewed this draft!  There are a few issues at the borderline between editorial and minor plus quite a lot of editorial nits.
> 
> Major Issues:
> 
> Minor Issues:
> s8.8.1:  I suspect that this section should have been deleted when s7.7 was deleted between v26 and v27.  The references to various server class attributes (handle class, change class, etc) are orphaned with no definitions of these items
> 
> The specification of how variables/fields of type seqid4 are incremented and compared, dealing with wrap around is very messy.
> Currently there are two separate descriptions of handling wrap around when a seqid4 variable is incremented through NFS4_UINT32_MAX (which turns out to be the all ones binary pattern.  The descriptions are slightly different but not incompatible.
> The definition of the seqid4 type in s2.1 doesn't mention this sort of constraint.
> 
> Incrementing of seqid4 type fields is mentioned in a number of sections along with special values (including 0):
> It would be much cleaner if the texts from s9.1.3.2 and s9.1.6 were merged, probably into a section just after 2.1 (say s2.1.1) together draft-ietf-nfsv4-rfc3530biswith mention of the special values.  If this is done, s2.1.1 can then be referenced wherever a seqid is mentioned as being incremented.
> 
> Also, I cannot see why the current texts use SHOULD.  I am pretty certain that skipping 0 and going to 1 from NFS4_UINT32_MAX is a MUST. If it isn't a MUST, what alternative scheme will make the wrap around work without both client and server knowing what scheme is being used? This is at least partly implied by the definition of NFS4ERR_BAD_SEQID in s13.1.8.2 which indicates that both ends have a common idea of what the next expected number ought to be..
> 
> I assume that it is deemed that using a 64 bit number as changeid4 should obviate the need for dealing with wrap around.  However I don't think there is any advice on starting values for the change values - some strategies (e.g., random values) could result in change indicators starting uncomfortably close to the maximum value and running up against wrap around.  A brief note on this would be desirable.
> 
> s15.2.3, paras 4 and 7: The last sentence in para 7 that implies NFS4ERR_MINOR_VERSION_MISMATCH takes priority over all others appears to be incompatible with the second XDR decoding strategy in para 4.  If decoding of the operation array is interleaved with performing the requested operations then it seems possible that the COMPOUND may be terminated by an earlier error before the offending operation 2 is reached, so that it would never be decoded.  Further, I don't understand why operation 2 is singled out for special discussion here, other than maybe to say that it is available for future use.  How does it differ from any other value that is not supported by the minorversion implemented by the server?  Why should the base version be singled out for special treatment?
> 
> Nits/editorial comments:
> ========================
> 
> Requirements Language note (after Abstract): The terms REQUIRED and RECOMMENDED are overloaded in this document, in that they are used both in the RFC 2119 sense and as adjectives that characterize attributes. It would be a good idea to avoid confusion by providing an extension of the usual form of words in this section, such as:
> NEW:
> General: There are a small number of instances where the form '<name> recommended attribute' and similar is used.  It would be clearer to use 'RECOMMENDED attribute in these cases:
> s1.3.3.2: Worth reinforcing that attribute names are (presumably) encoded in utf8 as they are just fileobject names.
> 
> s1.3.4:  A pointer to s9.9 for an explanation 'share semantics' would be very useful.  Copying the first para of s9.9 or something similar to the terminology section would also help.
> 
> s1,4, Definition of Stateid:  Definitions of open-owner and lock-owner would be helpful.  The terms are used a few times before they are defined in s9.
> 
> s2: Useful to add a reminder that the size constants and definitive definitions can be found in [RFCNFSv4XDR].
> 
> s3.3: The terms 'policy boundary' and 'crossing policy boundaries' are used in subsequent sections but not explicitly defined.  I assume that they refer to the client accessing fileobjects in areas accessed via the different entry points referred to in s3.3.  Assuming I am right introducing the 'policy boundary' term in s3.3 would be helpful.
> 
> s3.3.3, para 1: s/the security flavor the original SETCLIENTID/the security flavor of the original SETCLIENTID/
> 
> s5.3:  I think it would be worth being explicit as to whether hard links can be created between named attribute fileobjects either (1) for named attributes of the same fileobject and/or (2) for two different fileobjects on the same file system.  Is it also the case that if the basic file system supports hard links (link_support == TRUE) then it will necessarily support hard links between named attributes?  As I understand it, the named attribute system in NFSv4 was modelled on Solaris v9+ and the extended attribute system in Linux would probably not support hard links (I haven't tried them (yet)) even if the Solaris version does.... Later... I see from s13.1.4.14 (NFS4ERR_XDEV) that hard links between different named attribute directories are NOT allowed.  It would be good to make this explicit here in s5.3.  This still leaves the question of whether hard links are allowed in a single named attribute directory.  I can't see a lot of use for this capability but it should be explicitly allowed or disallowed as appropriate.
> 
> s5.6: There is no definition of nfs_lease4 in either this draft or the xdr draft.
> 
> s5.8.2.14: Presumably this is hard links: s/links/hard links/;  I assume that the number of symlinks is essentially unconstrained.
> 
> s5.10, last para: it would be a good idea to add a note to the RFC Editor to ensure that the statements about the Unicode version will be true on publication!
> 
> s5.10, last para: Is it conceivable that a client would need to know what version of Unicode a server was using ?  If so should there be a way for the client to find out what version of Unicode is in use by the server?
> 
> s6.1:
> I think this may only apply to non-privileged principals - root can still read/write *ix files with mode 0.  Tie in with last para of s6.3.1.1.
> 
> ????? s6.2.1.3, last para:  The "optional" and "RECOMMENDED" should be in the same case - I think this should be both upper case RFC 2119 requirements but it is arguable they should both be lower.
> 
> s6.4.3:  It is (still) not clear (at least, to me) how inheritance is supposed to apply to named attribute directories.  The named attribute directory has the file object to which it applies as its 'parent' rather than an ordinary directory.  Presumably inheritance would apply from the named attribute directory to the named attributes created in it - but are ACLs always supported on named attributes and the named attribute directory if they are suported on ordinary files and directories?
> 
> s8: s/OPTIONAL/optional/ - using multi-server namespaces  is a user choice not a protocol requirement.
> 
> s8.5:
> Is there some means in NFSv4 to do this?  I couldn't see that there was.  I guess one could use Reverse DNS.  If an out-of-band verification is needed, this should be made explicit.
> 
> s8.8: Copying the structure definitions from ss2.2.6/2.2.7 is unnecessary and could be better handled with refs.
> 
> [This comment is redundant if s8.8.1 gets deleted as suggested in Minor Issues above.]
> What if fh_expire_type has FH_VOLATILE_ANY set?  This is defined in s4.2.3 to imply FH4_VOL_MIGRATION making the 'setting of FH4_VOL_MIGRATION redundant'. The same issue applies to the second bullet.
> 
> s9.1.1, last para: forward refs to the sections defining SETCLIENTID and SETCLIENTID_CONFIRM would be helpful.
> 
> Does this differ from the 'anonymous stateid' defined in s9.1.3.3?  If not it would be better to refer to it by this title.  There are a number of other places in s9 where the expressions like 'stateid of all 0s/1s' are used.  It would be more consistent to use the 'anonymous' and 'READ bypass' titles set up in s9.1.3.3 in all cases.
> 
> s9.1.9, para 3:
> s/the sequence number may have any value./the sequence number may have any valid (i.e., non-zero) value./
> 
> s9.1.10, last para:
>> thus can ensure that
> s9.2 (2 places),s10.5,s13.1.8.8: s/lock owner/lock-owner/ (total of 4 instances)  - might be others at ends of lines that I missed.
> 
> I think this para is referring to the server rather than the client: s/client/server/.
> 
> s10.3.1, 1st bullet: s/The implementer is cautioned in this approach./The implementer is cautioned against this approach./
> 
> s11: For the avoidance of confusion, it would be wise to note that REQUIRED and RECOMMENDED are referring to the RFC 2119 usage rather than the description of attributes in this section.
> 
> s11, item 2; s15.4, next to last para: The implication of the illegal operation test in s15.4 is that there should be no gaps in the allocated operation number sequence.  The operation numbers for minor version X must start immediately after those for version (X-1) and be in a continuous group.
> 
> s11, item 11; s15.4: If for some arcane reason a server supports minor version X (X >= 2) but does not support minor version Y (0 < Y < X) as allowed by the SHOULD in s11, should the server return NFS4ERR_NOTSUPP or an XDR decode error.  I would say that the SHOULD could lead to some considerable confusion for clients without a way to find out what versions a server implements.
> 
> s12.4: Has the dreaded BOM ever been encountered in the UTF-8 strings created by NFSv5 servers or clients?
> s12.5, last para:@ s/without modifying name/without modifying the name/ or maybe "the names".
> 
> s12.6: The reference to RFC 5890 needs to be one para earlier to go with the first occurrence of U-label and should be added for A-label.
> 
> Is this advisory on the grounds that it makes recovery difficult - in which case s/SHOULD NOT/should not/ - or something that might result in the server throwing an error - in which case please point to some discussion of why this is not legal. Later... this is also considered in s15.18.6, last para.  So there is one set of circumstances when it will screw up - but there are others where it is not problematical.  Perhaps this can be explained more clearly.
> 
> 
> 
> s15.3.5:
> s15.16.4: LOOKUPP is said to return the parent directory of a named attribute directory. Arguably a named attribute directory doesn't have a parent... so does LOOKUPP return the directory in which the object to 'owning'  the named attribute directory resides?  Or does it return the owning object?  Please clarify in the text.
> 
> s15.18.2/s15.18.6/s15.21.2/s15.21.4, share_access and share_deny arguments: Presumably the shorthand DENY/READ/WRITE/BOTH values referred to are the relevant constants defined in s9.9?  If so please add a reference (and maybe use the full names in s15).  Otherwise please define the values.
> 
> s15.18.6, para 9:
> s15.23.5, last para: s/public filehandle a method/public filehandle as a method/
> 
> 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.
> 
> s15.29.4:
> Presumably "filehandles refer to different file system objects" means the filehandles refer to the named attribute directories of different file system objects.  If so then add "the named attribute directories of".
> 
> s17, para 6: s/the client migrate its traffic/the client to migrate its traffic/
> 
> s17, para 7: s/is checked against and match/is checked against and matches with/
> 
> s18.1, para 4: I don't believe you can have both policies of FCFS and Specification Required at the same time, and then with Expert Review for updates.  The proposal sounds like FCFS with a requirement that a description of format and intended usage is provided.
> 
> s19: RFC 2279 (UTF-8) is referenced as normative with RFC 3629 which obsoletes RFC 2279 as informative.  I think that references to RFC 2279 should be replaced by RFC 3629 and this one made normative.
>

---

From: Elwyn Davies <[email-masked]>
Date: Wed, 12 Nov 2014 19:54:37 +0000

Hi, Tom.

Excellent!

I have been doing a compare of 3530bis and 3530bis-dot-x and this has 
thrown up a couple of minor infelicities.  I'll send an email in a 
couple of hours.

/E

On 12/11/14 18:24, Tom Haynes wrote:
>> s5.3:  I think it would be worth being explicit as to whether hard links can be created between named attribute fileobjects either (1) for named attributes of the same fileobject and/or (2) for two different fileobjects on the same file system.  Is it also the case that if the basic file system supports hard links (link_support == TRUE) then it will necessarily support hard links between named attributes?  As I understand it, the named attribute system in NFSv4 was modelled on Solaris v9+ and the extended attribute system in Linux would probably not support hard links (I haven't tried them (yet)) even if the Solaris version does.... Later... I see from s13.1.4.14 (NFS4ERR_XDEV) that hard links between different named attribute directories are NOT allowed.  It would be good to make this explicit here in s5.3.  This still leaves the question of whether hard links are allowed in a single named attribute directory.  I can't see a lot of use for this capability but it should be explici
 t
ly allowed or disallowed as appropriate.

---

From: Elwyn Davies <[email-masked]>
Date: Thu, 13 Nov 2014 20:27:41 +0000

Hi, Tom.

Some comments inline - I have deleted the various things that are agreed.

Regards,
E;lwyn

On 12/11/14 22:11, Tom Haynes wrote:
> Hi Elwyn,
>
> Before I forget, thanks for the review.
.. and your prompt response. :-)
>
> I have replied inline for the changes I have made or have a question on.
>
> Tom
>
>> On Nov 11, 2014, at 12:48 PM, Elwyn Davies <[email-masked]
>> <mailto:[email-masked]>> wrote:
>>
>> issues at the borderline between editorial and minor plus quite a lot
>> of editorial nits.
>>
>> Major Issues:
>>
>> Minor Issues:
>> s8.8.1:  I suspect that this section should have been deleted when
>> s7.7 was deleted between v26 and v27.  The references to various
>> server class attributes (handle class, change class, etc) are orphaned
>> with no definitions of these items
>
> I’m fine with deleting this section.
>
OK
> David will reply with to the seqid4 issues.
OK
>> The specification of how variables/fields of type seqid4 are
>> incremented and compared, dealing with wrap around is very messy.
>> Incrementing of seqid4 type fields is mentioned in a number of
>> sections along with special values (including 0):
>> draft-ietf-nfsv4-rfc3530biswith mention of the special values.  If
>> this is done, s2.1.1 can then be referenced wherever a seqid is
>> mentioned as being incremented.
>>
>> some strategies (e.g., random values) could result in change
>> indicators starting uncomfortably close to the maximum value and
>> running up against wrap around.  A brief note on this would be desirable.
(see s10.4.3 which talks about adding 1 to a change_info4 value.)
>> —————————————————
> <— to here —>
> I stumbled on this as well, until I noticed this:
>
>     o  If there is an application on the server that has memory mapped a
>        file that a client is also accessing, the client may not be able
>        to get a consistent value of the change attribute to determine
>        whether its cache is stale or not.
>
> I.e., the server only knows about locally memory mapped files.

Perhaps better expressed as the server *doesn't* know about locally 
memory mapped files if they are being accessed behind the server's back 
through the underlying file system.  There would be some very limited 
point to having a mechanism (flag on open, as suggested) that allowed a 
client to inform the server that a file was being memory mapped, but 
this would of course require OS support but could give a way of 
overcoming the issue recorded at the end of bullet 3 in s10.7:
>       Depending on the
>       nature of the client's memory management system, this weak
>       obligation may not be possible.  A client MAY return stale
>       information in CB_GETATTR whenever the file is memory mapped.

Probably not worth worrying about.
>> NFS4ERR_MINOR_VERSION_MISMATCH takes priority over all others appears
>> to be incompatible with the second XDR decoding strategy in para 4.
>>  If decoding of the operation array is interleaved with performing the
>> reached, so that it would never be decoded.  Further, I don't
>> understand why operation 2 is singled out for special discussion here,
>> other than maybe to say that it is available for future use.
>
> I think it is just because it is reserved for future use with minor
> versioning.
>
> I.e., other operations having values might also be bad, but if they do,
> we can assume it is because they are not involved with minor versioning.
>
>
>> How does it differ from any other value that is not supported by the
>> minorversion implemented by the server?  Why should the base version
>> be singled out for special treatment?
Having checked out what is signed off in RFC 5661 for v4.1 and proposed 
for v4.2 in the current draft, and given the demise of most of s11, 
wouldn't it be easier just to make operation 2 reserved for ever?  I 
can't see that a distinguished op 2 is ever that important.

> I’m not sure that strictly follows - see S 12.8 for example.
>
> Indeed, it is conceivable that in different parts of the name space of
> the server, different character sets are possible (because of different
> underlying file systems). That has the corollary that moving a
> filesystem object may result in an error because the target rejects the
> name.
>
>

> Agreed:
>          <t hangText="Lock-Owner:">
>            Each byte-range lock is associated with a specific lock-owner
>            and an open-owner.  The lock-owner consists of a
>            Client ID and an opaque owner string.
>            The client presents this to the server to establish
>            the ownership of the byte-range lock as needed.
>          </t>
>
>          <t hangText="Open-Owner:">
>            Each open file is associated with a specific open-owner,
>            which consists of a Client ID and an opaque owner string.
>            The client presents this to the server to establish
>            the ownership of the open as needed.
>          </t>
>
That looks good.

>> they refer to the client accessing fileobjects in areas accessed via
>> the different entry points referred to in s3.3.  Assuming I am right
> The document strives very hard not to discuss export mechanisms:
>
> kitty:linux loghyr$ more /etc/exports
> / *(rw)
> /home/loghyr *(rw)
> /pynfs *(rw,no_root_squash,no_all_squash,insecure)
> /public *(rw,no_root_squash,no_all_squash,insecure)
>
> Each one of these define a “policy boundary” in the namespace.
>
> OLD:
>
> In general, the client will not have to use
>     the SECINFO operation except during initial communication with the
>     server or when the client crosses policy boundaries at the server.
>     It is possible that the server's policies change during the client's
>     interaction therefore forcing the client to negotiate a new security
>     triple.
>
>
> NEW:
>
> In general, the client will not have to use
>     the SECINFO operation except during initial communication with the
>     server or when the client encounters a new security policy as the
>     client navigates the name space.  Either condition will force the
>     client to negotiate a new security triple.
>
I think that will address the issue.

>>
>> s5.3:  I think it would be worth being explicit as to whether hard
>> links can be created between named attribute fileobjects either (1)
>> for named attributes of the same fileobject and/or (2) for two
>> different fileobjects on the same file system.  Is it also the case
>> that if the basic file system supports hard links (link_support ==
>> TRUE) then it will necessarily support hard links between named
>> attributes?  As I understand it, the named attribute system in NFSv4
>> was modelled on Solaris v9+ and the extended attribute system in Linux
>> would probably not support hard links (I haven't tried them (yet))
>> even if the Solaris version does.... Later... I see from s13.1.4.14
>> (NFS4ERR_XDEV) that hard links between different named attribute
>> directories are NOT allowed.  It would be good to make this explicit
>> here in s5.3.  This still leaves the question of whether hard links
>> are allowed in a single named attribute directory.  I can't see a lot
>> of use for this capability but it should be explicitly allowed or
>> disallowed as appropriate.
>
> This is not addressed in 3530, but it is in 5661 (section 18.9.3), which
> is equivalent to 15.29.5 in 3530bis:
>
> Shepler, et al.              Standards Track                  [Page 425]
> ^L
> RFC 5661                         NFSv4.1                    January 2010
>
>
>     On some servers, "." and ".." are illegal values for newname and the
>     error NFS4ERR_BADNAME will be returned if they are specified.
>
>     When the current filehandle designates a named attribute directory
>     and the object to be linked (the saved filehandle) is not a named
>     attribute for the same object, the error NFS4ERR_XDEV MUST be
>     returned.  When the saved filehandle designates a named attribute and
>     the current filehandle is not the appropriate named attribute
>     directory, the error NFS4ERR_XDEV MUST also be returned.
>
> I.e., it implies that if the current filehandle is the appropriate named
> attribute
> directory, the LINK is okay.
>
> We could either explicitly state in Section 5.3 that hard links are
> legal here or
> change the LINK IMPLEMENTATION Section to follow that of 5661.
>
> I am partial to syncing the two sections. Any concern?
No.  That seems entirely sensible.
> So nfs_lease4 is in RFC3530, but again with no corresponding XDR.
>
> 5661 has it fixed.
>
> Okay, the xdr draft now has a fix for it. Great catch!
Great!

>> what version of Unicode a server was using ?
>
> No
>
>>  If so should there be a way for the client to find out what version
>> of Unicode is in use by the server?
>
> <Shudder />

<Cowards /> ;-) Alright, I'll believe you!
> Which is why this is a “should” and not a “SHOULD” or a “MUST”?
>
>>
>> directory has the file object to which it applies as its 'parent'
>> rather than an ordinary directory.  Presumably inheritance would apply
>> from the named attribute directory to the named attributes created in
>> it - but are ACLs always supported on named attributes and the named
>> attribute directory if they are suported on ordinary files and
>> directories?
>
>
> David??
>
>>
> And the authors of 5661 seem to agree. It is also S7.8 in that document.
>
> What the sentence means is that since the server has no way to determine
> how the client will authenticate in the presence of multiple security
> mechanisms,
> it should as 5661 states:
>
>     Because NFSv4 clients possess the ability to change the security
>     mechanisms used, after determining what is allowed, by using SECINFO
>     and SECINFO_NONAME, the server SHOULD NOT present a different view of
>     the namespace based on the security mechanism being used by a client.
>     Instead, it should present a consistent view and return
>     NFS4ERR_WRONGSEC if an attempt is made to access data with an
>     inappropriate security mechanism.
>
> I’d be willing to pull Section 7.8 from 5661 back to 3530bis - besides using
> security policy as a term, it appears easier to read. :-)

I agree.  Notes for the 5661 version
- Remove ref to SECINFO_NONAME from para 1
- s/the entire the pseudo file system/the entire pseudo file system/ in 
next to last para.
>> Is there some means in NFSv4 to do this?  I couldn't see that there
>> was.  I guess one could use Reverse DNS.  If an out-of-band
>> verification is needed, this should be made explicit.
>
> The client could access each IP address and examine the resulting
> ClientID from each.
>
> But even then, I think that is not sufficient.  David?
>

>> not it would be better to refer to it by this title.  There are a
>> number of other places in s9 where the expressions like 'stateid of
>> all 0s/1s' are used.  It would be more consistent to use the
>> 'anonymous' and 'READ bypass' titles set up in s9.1.3.3 in all cases.
>
>
> David...
>
>>
> also s/haled/held/

... and there I was thinking you had used a nice archaic form... :-)
>> How do you issue a "pending I/O request”??
>
>
> I think the intent is one that was pending or if none were pending, fake
> it with a zero-length read.
Perhaps, NEW:
     The client may accomplish this task by
     issuing an I/O request; if there is no relevant I/O pending,
     a zero-length read specifying the stateid associated with the lock
     in question can be synthesised to trigger the renewal.

> Dave, can you look at this one?
>
> http://www.ietf.org/mail-archive/web/nfsv4/current/msg10668.html
>

> David?
>> which case s/SHOULD NOT/should not/ - or something that might result
>> in the server throwing an error - in which case please point to some
>> discussion of why this is not legal. Later... this is also considered
>> in s15.18.6, last para.  So there is one set of circumstances when it
>> will screw up - but there are others where it is not problematical.
>>  Perhaps this can be explained more clearly.
>>
>
>
> Are these not the same?
Yes, the requirement on the client is indeed the same.  The ordering of 
the text in s15.2.5 seemed to indicate that changing the Client ID 
might make recovery from resource availability problems more difficult 
but s15.18.6  makes it clear this isn't the reason.  What triggered my 
comment was that the (earlier) s15.2.5 version has no explanation of why 
it is problematical.  A pointer to s15.18.6 in s15.2.5 would help 
alleviate the problem but is not a total solution.
>
> Further, the client SHOULD
>     NOT construct a COMPOUND which mixes operations for different client
>     IDs.
>
>
> David, are there migration scenarios where the client might be
> presenting different client IDs?

Effectively OPEN with a successful OPEN_DELEGATE_WRITE creates transient 
state in the server for the duration of the enclosing COMPOUND 
procedure, essentially contravening the statement in the last para of s4.3:
>  Note that the COMPOUND procedure does not provide atomicity.

The first question is are there any other circumstances that generate 
transient state in a similar way or is this the only example?

Later... after some considerable thinking about what is going on here.

Presumably the basic problem here is that if the the client is single 
threaded or otherwise would get its knickers twisted if it issued a 
GETATTR for a file on which it held a write delegation thus having its 
internal state for the file locked when the CB_GETATTR arrived.  This is 
clearly potentially deadlock time!

Given that the client has the write delegation, it shouldn't need to 
call GETATTR except in the one initial circumstance documented.

It seems to me that the server could ALWAYS suppress the CB_GETATTR if 
the clientid is set for the client that owns the write delegation, and 
the client could be instructed that it MUST NOT call GETATTR on a file 
for which it has a write delegation other than in the one special 
circumstance (or maybe unless it knows the clientid is set correctly).

Is my analysis right?  If so I think this area can be made clearer (and 
possibly more robust).

> It could, is it necessary? No.
>
>
>>
>> attribute directory. Arguably a named attribute directory doesn't have
>> a parent... so does LOOKUPP return the directory in which the object
>> to 'owning'  the named attribute directory resides?  Or does it return
>> the owning object?  Please clarify in the text.
>
>
> Good question.
>
>  From reading the text, it should return the parent of the directory in
> which the object to 'owning'  the named attribute directory resides.
>
> I’ve asked on list.
>
> Ahh, 5661 to the rescue:
>
>     If the current filehandle is a named attribute directory that is
>     associated with a file system object via OPENATTR (i.e., not a sub-
>     directory of a named attribute directory), LOOKUPP SHOULD return the
>     filehandle of the associated file system object.
>
> I have taken that paragraph back into 3530bis,
Fine.

>> arguments: Presumably the shorthand DENY/READ/WRITE/BOTH values
>> referred to are the relevant constants defined in s9.9?  If so please
>> add a reference (and maybe use the full names in s15).  Otherwise
>> please define the values.
>
> Full names it is - btw - I agree, easier to read, but harder to map back
> to the real values.
> And I did provide a link in the “one” which used the terms heavily.

OK.

> Do you mean:
>     regular file (or a named attribute)
Yes.

>> (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.
> I’ll have to track down what is happening here, might be with the newer
> xml2rfc:
>
>      string comparison - see Sections
>      <xref target="sec:fattr:owner_group" format="counter" /> and
>      <xref target="sec:i18n" format="counter" /> for further
>
> Hmm, this is generating correctly in draft 34.
Right. .. and that is what I would expect to see in xml2rfc!

> I’ll make sure to track this with every new draft version.
Fine.
>> Specification Required at the same time, and then with Expert Review
>> for updates.  The proposal sounds like FCFS with a requirement that a
> Already fixed in version 34.
>
> Amazing how it got through 3 different prior reviews (3530, 5661, and
> first 3530bis). :-)
:-))))

> Ok, first part is fixed, will also change case.
>

OOPS!  I didn't mean to put ERRATA in caps.  v34 is just fine. Sorry.

---

From: Tom Haynes <[email-masked]>
Date: Fri, 14 Nov 2014 15:05:55 -0800

Hi Elwyn,

Here is my first set of responses - there are still some open items.

Thanks,
Tom

> On Nov 13, 2014, at 12:27 PM, Elwyn Davies <[email-masked] <mailto:[email-masked]>> wrote:
> 
> Hi, Tom.
> 
>>> On Nov 11, 2014, at 12:48 PM, Elwyn Davies <[email-masked] <mailto:[email-masked]>
>>> <mailto:[email-masked] <mailto:[email-masked]>>> wrote:
>>> 
>>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>>.
>>> 

David has made all of the necessary changes to section 9.

I.e., I think he just applied all of your suggested changes.

> Perhaps better expressed as the server *doesn't* know about locally memory mapped files if they are being accessed behind the server's back through the underlying file system.  There would be some very limited point to having a mechanism (flag on open, as suggested) that allowed a client to inform the server that a file was being memory mapped, but this would of course require OS support but could give a way of overcoming the issue recorded at the end of bullet 3 in s10.7:
>>      Depending on the

Agreed - thanks.

> Having checked out what is signed off in RFC 5661 for v4.1 and proposed for v4.2 in the current draft, and given the demise of most of s11, wouldn't it be easier just to make operation 2 reserved for ever?  I can't see that a distinguished op 2 is ever that important.

Need to let this sink in...

Okay, there is no way that we can use this operation for NFSv4.0. The text clearly
states that we MUST return NFS4ERR_OP_ILLEGAL if the minor version is 0.

And while 5661 is worded better for using this, I think that ship has sailed.

So if we squish together the two paragraphs, I think this works:

        Note that operations, 0 (zero), 1 (one), and 2 (two) are not defined
        for the COMPOUND procedure. It is possible that the server receives a request that
        contains an operation that is less than the first legal
        operation (OP_ACCESS) or greater than the last legal operation
        (OP_RELEASE_LOCKOWNER).
        In this case, the server's response will encode the opcode
        OP_ILLEGAL rather than the illegal opcode of the request. The
        status field in the ILLEGAL return results will set to
        NFS4ERR_OP_ILLEGAL.  The COMPOUND procedure's return results
        will also be NFS4ERR_OP_ILLEGAL.

> No.  That seems entirely sensible.

Done.

> - s/the entire the pseudo file system/the entire pseudo file system/ in next to last para.

Done and thanks for the warnings!

>    in question can be synthesised to trigger the renewal.

ok

>> http://www.ietf.org/mail-archive/web/nfsv4/current/msg10668.html <http://www.ietf.org/mail-archive/web/nfsv4/current/msg10668.html>
>> 
> 
> Yes, the requirement on the client is indeed the same.  The ordering of the text in s15.2.5 seemed to indicate that changing the Client ID might make recovery from resource availability problems more difficult but s15.18.6  makes it clear this isn't the reason.  What triggered my comment was that the (earlier) s15.2.5 version has no explanation of why it is problematical.  A pointer to s15.18.6 in s15.2.5 would help alleviate the problem but is not a total solution.

> Effectively OPEN with a successful OPEN_DELEGATE_WRITE creates transient state in the server for the duration of the enclosing COMPOUND procedure, essentially contravening the statement in the last para of s4.3:
> The first question is are there any other circumstances that generate transient state in a similar way or is this the only example?
> 
> Presumably the basic problem here is that if the the client is single threaded or otherwise would get its knickers twisted if it issued a GETATTR for a file on which it held a write delegation thus having its internal state for the file locked when the CB_GETATTR arrived.  This is clearly potentially deadlock time!
> 
> Given that the client has the write delegation, it shouldn't need to call GETATTR except in the one initial circumstance documented.
> 
> It seems to me that the server could ALWAYS suppress the CB_GETATTR if the clientid is set for the client that owns the write delegation, and the client could be instructed that it MUST NOT call GETATTR on a file for which it has a write delegation other than in the one special circumstance (or maybe unless it knows the clientid is set correctly).
> 
> Is my analysis right?  If so I think this area can be made clearer (and possibly more robust).
> 
> 

I think the original authors were very cautious - the use of SHOULD NOT instead of MUST NOT.

David, do you recall why 3530 was originally this way?

(Yes, I am trying to buy some time here. :-)

>>    regular file (or a named attribute)

ok

> 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: Tom Haynes <[email-masked]>
Date: Fri, 14 Nov 2014 15:31:00 -0800

> On Nov 14, 2014, at 3:05 PM, Tom Haynes <[email-masked]> wrote:
> 
> Hi Elwyn,
> 
> 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.