Re: [Gen-art] Gen-art additional review of draft-ietf-nfsv4-rfc3530bis-25

"Noveck, David" <david.noveck@emc.com> Mon, 19 August 2013 19:56 UTC

Return-Path: <david.noveck@emc.com>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 2F0E611E82BA; Mon, 19 Aug 2013 12:56:52 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.698
X-Spam-Level:
X-Spam-Status: No, score=-2.698 tagged_above=-999 required=5 tests=[AWL=0.100, BAYES_00=-2.599, GB_I_LETTER=-2, HTML_MESSAGE=0.001, J_CHICKENPOX_23=0.6, J_CHICKENPOX_29=0.6, J_CHICKENPOX_46=0.6]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xTx5fIlZnQcS; Mon, 19 Aug 2013 12:56:14 -0700 (PDT)
Received: from mexforward.lss.emc.com (hop-nat-141.emc.com [168.159.213.141]) by ietfa.amsl.com (Postfix) with ESMTP id 9947B21F9A35; Mon, 19 Aug 2013 12:56:11 -0700 (PDT)
Received: from hop04-l1d11-si01.isus.emc.com (HOP04-L1D11-SI01.isus.emc.com [10.254.111.54]) by mexforward.lss.emc.com (Switch-3.4.3/Switch-3.4.3) with ESMTP id r7JJtbhx028496 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 19 Aug 2013 15:55:44 -0400
Received: from mailhub.lss.emc.com (mailhubhoprd03.lss.emc.com [10.254.221.145]) by hop04-l1d11-si01.isus.emc.com (RSA Interceptor); Mon, 19 Aug 2013 15:55:12 -0400
Received: from mxhub27.corp.emc.com (mxhub27.corp.emc.com [10.254.110.183]) by mailhub.lss.emc.com (Switch-3.4.3/Switch-3.4.3) with ESMTP id r7JJtBZK004440; Mon, 19 Aug 2013 15:55:11 -0400
Received: from mx31a.corp.emc.com ([169.254.1.238]) by mxhub27.corp.emc.com ([10.254.110.183]) with mapi; Mon, 19 Aug 2013 15:55:10 -0400
From: "Noveck, David" <david.noveck@emc.com>
To: Thomas Haynes <loghyr@gmail.com>, Elwyn Davies <elwynd@dial.pipex.com>
Date: Mon, 19 Aug 2013 15:55:08 -0400
Thread-Topic: Gen-art additional review of draft-ietf-nfsv4-rfc3530bis-25
Thread-Index: Ac6ay7IZNN/9U+82R5SGYjHbtkd4iwCRQ9Eg
Message-ID: <5DEA8DB993B81040A21CF3CB332489F607B5AAD7D7@MX31A.corp.emc.com>
References: <C5D71833-AD50-47F2-8BB9-2E5FFF03CF2E@gmail.com>
In-Reply-To: <C5D71833-AD50-47F2-8BB9-2E5FFF03CF2E@gmail.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
acceptlanguage: en-US
Content-Type: multipart/alternative; boundary="_000_5DEA8DB993B81040A21CF3CB332489F607B5AAD7D7MX31Acorpemcc_"
MIME-Version: 1.0
X-EMM-MHVC: 1
X-Mailman-Approved-At: Mon, 19 Aug 2013 13:40:38 -0700
Cc: General Area Review Team <gen-art@ietf.org>, "draft-ietf-nfsv4-rfc3530bis.all@tools.ietf.org" <draft-ietf-nfsv4-rfc3530bis.all@tools.ietf.org>, "nfsv4 list (nfsv4@ietf.org)" <nfsv4@ietf.org>
Subject: Re: [Gen-art] Gen-art additional review of draft-ietf-nfsv4-rfc3530bis-25
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/gen-art>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 19 Aug 2013 19:56:52 -0000

I have some comments on the XXX items.

> so... should the client be able to find out which version of Unicode the server is working to?

The problem with this is that the way to do this is a new attribute and that the minor versioning rules as now written preclude adding such a new attribute in v4.0. , giving that v4.1 exists and v4.2 almost exists.  Those rules could be modified but it would be very hard to reach consensus on such a change, so I'd prefer we not open up that can of worms, unless it were really necessary, and not just a nice-to-have.

Also, this needs to be looked at in the context of the new (really old) I18N chapter in -27.   The goal of that is to describe what actual NFSv4.0 servers and clients actually do,  making adding a new attribute that none of them has asked for pretty much out of bounds.

Maybe we can defer this discussion until we consider the new I18N chapter in a later pass of the process.

> s6.3.1.2: Is there any interaction between delegation and client considerations for ACLs? Do
> clients holding delegations have to do any access checks in the client that would normally be
> done in the server if there was no delegation?  I'm not sure

The client doesn't have to do any access checks.  If it doesn't want to do ACL Interpretation it can do
an OPEN(CLAIM_DELEGATE_CUR) and let the server do it.

> - Is there any interaction between byte range locks and share reservations?  e.g., does a client
> holding (say) a byte range write lock but with no DENY state prevent another client taking
> out a DENY_WRITE  share reservation on the same file?

I don't think it does.  I think the intention is that server implementations can grant an open just
knowing about existing share reservations and delegations.


> This needs some tidying up.  One could try using 'data object' as an unambiguous term for the
> union of the actual file (NF4REG) class and the named attribute class.
I agree this needs tidying up.


From: Thomas Haynes [mailto:loghyr@gmail.com]
Sent: Friday, August 16, 2013 5:56 PM
To: Elwyn Davies
Cc: General Area Review Team; draft-ietf-nfsv4-rfc3530bis.all@tools.ietf.org; nfsv4 list (nfsv4@ietf.org)
Subject: Re: Gen-art additional review of draft-ietf-nfsv4-rfc3530bis-25

Hi Elwyn,

Thank you for the detailed review. It has taken so long because even for the
points which I disagreed with, I spent considerable time proving to myself
first your objection and second why I wasn't going to take the suggested change.

Also, the WG needed to meet to agree with what to do with the i18n comments.

I've left XXX or xxx where other people in the WG might want to reply.

I'll be sending out a new version of the drafts by Aug 18th.

Tom

On May 8, 2013, at 3:35 PM, Elwyn Davies <elwynd@dial.pipex.com<mailto:elwynd@dial.pipex.com>> wrote:



I am another 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><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-25
Reviewer: Elwyn Davies
Review Date: 8 May 2013
IETF LC End Date: -
IESG Telechat date: 30 May 2013

Summary:
This is a mammoth document.  The comments run to 650 lines.  Clearly there ae issues in modifying a -bis document given that it has to be pretty much compatible with its progenitor  - but that is now going on 14 years old. I believe that there are a significant number of minor issues to address and parts of the document (despite the overall size) need to provide a bit more explanation of what is going on.  In particular I think that a little bit more rationale over the way in which an NFS server attempts to provide something like a common interface to the two big player filesystem types (*ix and Windows)

Outside of the scope of the document.



[Aside: I am disappointed that it doesn't try to handle versioning file systems like the venerable VAX filing system! ;-) ]  There are some areas where the addition of named attributes hasn't really been properly dealt with AFAICS. One major area that I felt ought to have been better addressed in the -bis was internationalisation.

As agreed, this is being rewritten.

(BTW - this is why I've held off on replying.)



  As mentioned below, I don't understand why additional atrributes couldn't be added to help clients understand what the server is doing with internationalisation - the client has to do some complex acrobatics which seem close to guesswork to work out what is going on in the server.


Which is actually a design mantra when it comes to NFS - we talk about dumb servers and smart clients. We push the burden
to the client to figure out what to do.





  As ever, I am not an expert in NFSv4 (although I used v2 extensively almost 30 years ago... arrrgghhh!) so there may well be comments where I don't understand the problem.

Major issues:

Overall section comments:
s6: AccessControl Attributes
Combining two different access control paradigms (UNIX-clone mode control bitmasks and access control lists) is an uphill struggle and provides a deal of complexity in situations where both, either one or neither may be specified.  The detailed text deals reasonably with the various cases.  There are, in my view, a couple of significant problems:
- The first is essentially editorial:  I would say that rather more than basic understanding of UNIX-like file systems is needed.  An explanation or at least a suitable reference to explain the use of the mode bits in s6.2.2, especially the first three (SUID, SGID, SVTX) would help, and a clearer linkage to the explanations of owner, group and other sets of users that are distributed in parts of the text.

It is more complex than that unfortunately - we had another document which discussed such issues and it atrophied off.

I'll look at resurrecting it, we actually have confusion in the community as different vendors like to quote different revisions.






- The second is more fundamental:  The intended functionality of inheritance is not explicitly documented.  To come to any sort of understanding of how inheritance is supposed to work you have to read through to the next to the last section (s6.4.3).  I have now read through the section two or three times and I have a model in my mind, but there is no text that would allow me to verify my understanding.  I *think* i have understood that (1) inheritance only applies when you initially create an object rather than being fed through to all 'inferior' objects in the tree if a superior object has a heritable ACE added; (2) inheritance only applies if you don't explicitly give any ACEs when the object is created, and (3) if you give a mode but no ACL then some complex combination of inherited ACL and mode is applied.  An upfront explanation of what is trying to be achieved would have helped a lot.  However, none of this explains what happens with either hard or symbolic links.  It is unclear to me whether inheritance of ACLs is applied when creating a hard or symbolic link to an existing object.

s7, Multi-Server Namespace:
This section is very verbose and contains a number of areas that bring in topics that  are actually forward references without indicating where the information can be found, particularly in relation to various 'change classes'.  Some reordering might help.  The whole concept of change classes seems a little odd, because when they are finally defined (s7.9.1), it appears that servers are rarely in the same class during any of the interesting transitions (s7.9.1 says that in most cases they have to be taken as in different classes).  Overall, this section contains an awful lot of discursive specification relating to servers that may not have any coordination which makes it very difficult both from the specification and the implementation angles to check that everything is working.


We brought this section forward from 5661 in order to provide some help in dealing with the migration issues implementors are having with 3550.

And I really want to thank your review here, I don't think we can make this work at all. I've yanked it.




s8, NFS Server namespace:  This is a important section for overall understanding and I think it would help to have it much earlier, certainly before s7.

We want to maintain as much section compatibility with 3550 as possible.




s9, File Locking and Share Reservations:
Generally very well written, but the treatment of share reservations is very skimpy,
It would also be good to be more 'honest' about the interaction with the semantics of the underlying file system as regards locks as described in s15.12.

s10, Clientside Caching:
Clearly very complex to implement but seems to be well-described.  A bit of earlier explanation of the use of CLAIM_PREVIOUS vs CLAIM_DELEGATION_PREV in client/server reboots would help.

s11, Minor Versions:
No particular issues.

s12, Internationalisation:
A minefield.  There seems to be an awful lot of guess work for clients trying to decide how the sever behaves.  On reflection, I am surprised that several of the things that clients are supposed to deduce (e.g., what normalisation the server does) were not supplied as attributes of the server.  Is there some good reason for this?  I was also surprised that there didn't seem to be a requirement that servers working on a particular filesystem all did the same internationalisation things.  It appears to me that if a filesystem gets migrated the client doesn't have any sort of guarantee that the internationalisation behaviour won't change under its feet - and it has no easy way to check if this is or is not the case because of the lack of attributes.  I have no idea to what extent the internationalisation is implemented in current servers.

s13, Error Codes:
Generally OK but with a few code value inconsistencies.  I haven't checked the cross-consistency of Table 9 and Table 11 or whether the sets of error codes per operation are really right - they are just too voluminous.


They are machine generated from a common source.




s14, NFSv4 Requests:
No problems.

s15/16, Operations:
Generally in good shape apart from some issues with the meaning of 'regular files'.

s17: Security:
I have some doubts about the Kerberos algorithms suggested.  Maybe should be moving on to better algorithms even if this is a bis.


As discussed on the mailing list, we agreed and fixed it.

Again, here is where I would prefer we push this to another document which tracks which Kerberos algorithms should be used.





=============
Minor issues:
S1.1, bullet 7: Are there issues with backwards compatibility after removal of LIPKEY and SPKM-3 completely?

No, not a single vendor implemented these.




s1.5.1: Should NFSv4 be using RPCSEC_GSS v2 (RFC 5403) rather than v1 (RFC 2203)?


It should be "and" and not "rather". v1 is not widely adopted, but nothing precludes its use.

I've updated the instances of references to include both.



s.1.5.3.2, para 4, and s5.3: It would be useful to say upfront in s1.5.3.2 whether the attribute names are fully internationalized or whether this is still an ASCII name.


I don't think anything precludes an internationalized name here since these are in the directory space.



s2.1:  This section contains a number of symbolic constants (chiefly for opaque element sizes) which have not been defined as yet.  It would be highly desirable if these were also defined before this section.

These are in the companion document.

We broke the XDR out starting with RFC 5661 - we've made the same effort
here as readers prefer having the other document to reference.




s2.1, utf8val_RECOMMENDED4:


 String SHOULD be sent UTF-8
What else could it be sent as in this type?


We will revisit with the i18n rewhack.




s2.2/s6.2.1:  The type nfsace4 is used in the protocol: Accordingly it ought to be defined in s2.2 rather than s6.2.1.

Part of the XDR document.



s2.2.3/s5.8.2.34/s5.8.2.40: It would probably be nicer if the union case SET_TO_SERVER_TIME4 were put into the union explicitly.  A note about being able to set the access and modify times to either client or server time would be appropriate in the two items in s5.8.2.  Are there any constraints on the values that can be given when setting from the client.. like not moving the time backwards (or is this explained later?).

We can't change the enum or union at this point.

Given that there can be time skew, the server cannot tell if the client is moving things backward or not.

In this case, the server would assume that the client was correct and allow the change.





s3.1, para 1:


 Using the registered port for NFS services

   means the NFS client will not need to use the RPC binding protocols

   as described in [RFC1833<http://tools.ietf.org/html/rfc1833>]; this will allow NFS to transit firewalls.
I think some explanation of why this will definitively allow NFS to transit firewalls is needed.


In NFSv3, there were several sideband protocols which did not sit on well defined ports. Each
server vendor either picked a port or allowed for them to be configurable. As such, it is somewhat
unwieldily to configure firewalls.

With just using port 2049, it is easy to determine which single port needs to be open.




s3.1.1: Presumably the requirement to eschew silent dropping of 'requests' does not apply if security is being negotiated.  I take it 'requests' should be interpreted as  'NFSv4 requests' and procedures as defined in s14/15 and this requirement should apply only once the secure connection is established.  The word 'established' might help.


Yes, NFSv4 requests and established.

I can add both.





s3.2: (reprise) Should we be using RPCSEC_GSSv2 (RFC 5203) rather than v1?

Same reply as above.



s3.2.1.1: The Kerberos specifications appear to use rather ancient algorithms.  In particular DES, MD5.. should there not be more  modern algorithms?  DES and MD5 get a pretty bad press these days.  RFC 2623 is also a bit of an antique so I can't say its a modern assessment.

Already taken care of.



s3.3.1,security poiicy boundaries etc:


 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.
 The  concept of 'security policy boundary' has not been previously discussed in the doc.  How would the client know it had crossed a security boundary or that the server had changed the policy under its feet? I suppose maybe the next section tells me... but... How does this interact with the no silent drop, mostly no retries requirement in s3.1.1? or have i to read s15.33.

I suspect that some discussion of the interaction of mount points and security policy boundaries may be needed at an earlier stage.  I suspect that mount points might well be security policy boundaries and crossing mount points might interact.  There is nothing in the description of mounted_on_fileid (s5.8.2.19).


I think this was an effort to avoid talking about exports.




s5.6, item lease_time:  I can't find a definition of nfs_lease4 type used for this.

The accompanying XDR document has it.




s5.6, item rdattr_error: Just giving 'enum' as the type isn't too specific!

Agreed, I'll put nfsstat4




s5.8.2.25:


It is understood that this space may be

   consumed by allocations to other files or directories though there is

   a rule as to which other files or directories.
Where do I find the rule?

Yes, confusing. The next section illustrates that this is defined by the server.

I'll make it more explicit here.




s5.10, para 1:


although there are variations that occur additional characters with a

   name including "SMALL" or "CAPITAL" are added in a subsequent version

   of Unicode.
so... should the client be able to find out which version of Unicode the server is working to?

XXX



s6.3.1.2: Is there any interaction between delegation and client considerations for ACLs? Do clients holding delegations have to do any access checks in the client that would normally be done in the server if there was no delegation?  I'm not sure.

XXX



s7.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.
How is the verification achieved?  I don't see any attributes within NFSv4 that would help.

A swear word needs to be inserted here.

It is possible in NFSv4.1, but not NFSv4.0. I've removed this section.




s7.7.6.1, para 3:


   If state has not been transferred transparently because the client ID

   is rejected when presented to the new server, the client should fetch

   the value of lease_time on the new (i.e., destination) server, and

   use it for subsequent locking requests.  However, the server must

   respect a grace period of at least as long as the lease_time on the

   source server, in order to ensure that clients have ample time to

   reclaim their lock before potentially conflicting non-reclaimed locks

   are granted.
How does the destination server know what the lease_time/grace period on the source server was? It doesn't know what server the client was previously connected to in many cases and so can't ask.  Doesn't this amount to saying all servers for a given file system should (MUST) be configured with the same lease time in the non-transparent transparent case?

It doesn't, it makes an assumption that they were the same.




s9.9: This section is a little skimpy.  There are several points that probably ought to be clarified:
- The section explicitly refers to files:  Does it actually apply to all filesystem objects?

No, just regular files.



- An explicit statement that the 'file_state' is maintained for each file(system object)




- The pseudo-code applies to the OPEN case; different pseudo-code (presumably) applies to OPEN_DOWNGRADE.  I *think* I could guess it but it would be good to be explicit.

I'm happy enough without another example here.



- Is there any interaction between byte range locks and share reservations?  e.g., does a client holding (say) a byte range write lock but with no DENY state prevent another client taking out a DENY_WRITE  share reservation on the same file?


XXX


- Does the discussion of placing lock state into stable storage across server reboot also apply to the share reservation file_state?

s9.14:  The text in this section addresses topics also covered in s7.  This does  not seem desirable as there may be discrepancies (see next comment).  It would seem better to merge the text into s7 and possibly provide a reference back to s7 from s9.

s9.14.1:  This section envisages migration of state from one server to another where the client already has state on the other server using the same client ID.  Merging of the states is suggested.  AFAICS this situation is not discussed in s7 which seems a little odd.

We are going to have a new document on migration come out based on this text.

I would prefer to keep this as is and allow those authors to drive that discussion.




s12:  It seems the client has to play '20 questions' with the server to find out how it treats internationalized component names.  Some of it seems little better than guesswork.  I was wondering why some of the server choices are not described using attributes so that the client doesn't have to thrash around in the dark.  Also there doesn't seem to be any requirement for a filesystem to be treated identically after migration. As far as I can see the client can't really rely on the internationalisation treatment not changing after migration.

Again, i18n is going to be addressed.



s18.1:  The NFSv4 Named Attribute Definitions registry has already been created in exactly the form specified here by RFC 5661 (NFSv4.1).  This section should be removed and words noting that the existing registry is common to all minor versions of NFSv4  should be substituted.


I'd prefer to leave this as is because:

     IANA had created a registry called the "NFSv4 Named Attribute Definitions Registry"
     for <xref target="RFC3530" /> and <xref target="RFC5661" />. This section introduces no new
     changes, but it does recap the intent.





==================
Nits/editorial comments:
General: Review the remaining instances of 'file' and 'files' to determine if they should really be 'filesystem objects'.

General: Consistency between 'file system' and 'filesystem'.  'filesystem' is used first in abstract and s1.3 but only File System is defined.  I cannot see whether there is any real distinction between the usages... but  am open to argument! OTOH there is definitely inconsistency: Consider 'fsid' and 'fsid4' - the type definition says fsid4 is for a filesystem (s2.2.5) but s5.8.1.9 says this is about a 'file system'

And finally got to this one: "file system"!





General: The terms 'regular file' and 'named attribute' are not used uniformly.  s5.8.1.2 typifies the confusion.  In the first set of bullets, NF4REG is defined as 'a regular file' and NF4NAMEDATTR is defined as a Named Attribute.  In the last bullet, "is an[sic] regular file" is defined as implying either type NF4REG or NF4NAMEDATTR.
Thus we have the same term used for a set and a superset of the same set (maybe when qualified with 'is a').  Consequently, for example in s15.25.4,  we have "the READ operation reads data from the regular file...".  Presumably this is supposed to cover both NF4REG and NF4NAMEDATTR types but one can't be absolutely sure (to be pedantic it doesn't explicitly say 'is a regular file' so perhaps it should be read as NF4REG only).  OTOH the OPEN operation (s15.18.5) has two separate paragraphs for regular files and named attributes.  This needs some tidying up.  One could try using 'data object' as an unambiguous term for the union of the actual file (NF4REG) class and the named attribute class.


XXX



Abstract: s/owes heritage to/builds on the heritage of/

ok



s1.1: Expand IDNA acronym.

ok



s1.2, bullet 5:s/mounted_on_filed/mounted_on_fileid/

ok


s1.3: Expand acronyms ONCRPC and RPCSEC_GSS (and/or ref to RFC 2203 or 5403).

Fixed ONC RPC.

RPCSEC_GSS is not defined as an acronym



s1.5: s/the reader that is new/the reader who is new/

ok



s1.5.1, para 2: s/provides the client a method/provides the client with a method/


ok


s1.5.3:  A couple of sentences outlining the presentational scheme for filesystem object names (i.e., /a/b/c etc) would be worthwhile even if 'we all know what this means'!  This could include defining 'component name' used later with no introduction.

s1.5.3.1, et seq: Would it be useful to identify 'persistent' and 'volatile' as key words: e.g., 'Persistent' and 'Volatile'.

s1.5.3.2, para 3: For consistency the 'acl' example should have a ref (s6.2.1) earlier in the para.  So s/((Section 6)/(Section 6.2.1)/ and move one sentence earlier.

ok


s1.5.5: There should be a discussion of share reservations in this section.

s1.6, definition of Lease: There should be a definition of 'grace period' (as per s9.6.2) as there are lots of references to it earlier than s9.6.2.

s1.6, definition of Lock:  There should be a definition of share reservation.

s2.1 et seq:  Whilst the terms 'hard link' and 'symbolic link' are pretty commonly known,  they are so fundamental that I think putting them in the definitions would be highly desirable.


I'm okay with this as is.



s2.1, attrlist4:  In line with comment about opaque max sizes, should this have a max size? (Might also apply to bitmap4)

In the XDR



s2.2: It would be useful to have section references to the places where the types are used where these are explicitly mentioned.

s2.2.8 (and s2.1):  I guess that the bit ordering in bitmap4 is implicitly derived from the XDR representation, but it would be useful  to remind people of what this bit ordering actually is.

s2.2.9, atomic element:  With a name like that are there any special handling requirements for this element/structure?

s2.2.10: Note  to self: verify external refs in this section.

s2.2.11, cb_program: It should be explained that the cb_program is an RPC program number (had to ferret in s15.35.4 to understand this).  BTW this would entail an earlier expansion of RPC acronym.

1.1 is the earliest (in newest draft.)

But okay with leaving this as is.




s2.2.12:  It would be good to have a definition of NFS4_OPAQUE_LIMIT before this section (see earlier comment about s2.1) so that all the magic number constants are concentrated in one place.

s2.2.13:  Arrgh! Now we have two definitions of NFS_OPAQUE_LIMIT (fortunately they are the same until they aren't).

s2.2.14: ARRRGGGHHH! THREE defs.

It is now only defined in the XDR document.




s2.2.16, 'other':  And this definition contains the magic number 12 not as a symbolic constant and with no explanation of why 12 is a good value!

No explanation, but provided a name for the magic number.




s3.1, para 2:


   Where an NFSv4 implementation supports operation over the IP network

   protocol, the supported transports between NFS and IP MUST be among

   the IETF-approved congestion control transport protocols, which

   include TCP and SCTP.
I know what is meant, but technically, I don't believe the IETF actually has a list marked 'congestion control transport protocols'.  A better way of phrasing this might be:
   Where an NFSv4 implementation supports operation over the IP network
   protocol, the supported transport layer between NFS and IP MUST be an IETF
   standardised transport protocol that is specified to avoid network
   congestion; such  transports include TCP and SCTP.

ok


[Check whether SCTP is on the IANA well-known acronym list].

Also:
OLD:
to use a different IETF-approved congestion control transport protocol.
NEW:
to use a different IETF standardised transport protocol with appropriate congestion control.

ok



s3.1, para 3: Expand WAN acronym. (I think)

ok



s3.1, para 4:  Where would I find a justification that DCCP or NORM are inadequate?

It must have been debated on the mailing list.

I know this came up again in the context of 3530bis and we determined that they were not adequate.




s3.1, para 5: s/regardless whether/regardless of whether/

ok



s3.1, para 6: A rather throwaway comment about avoiding timer sync.  Not sure yet how many timers, but it would be good to know which ones were critical.  After reding the document I am still not sure.  It might almost be worth adding a section that summarizes the set of timers used.

I think removing the comment altogether is better.


s3.2.1/s3.2.1.1: Check section title capitalization.  (Mostly OK - spotted also s6.4.1/s6.4.1.x - possibly)

ok




s4.2.1: Earlier parts of s4 have been generalised to talk about 'filesystem objects'.  This section still has filehandles referring to 'files'.

ok




s4.2.3, para at top of p33; s4.3, para 4: Ditto talking about 'files'.

In this case, the context establishes that the filehandle is to a file.




s4.3, para 1:


   If possible, the client SHOULD recover from the receipt of an

   NFS4ERR_FHEXPIRED error.
I think this is a 'should' rather than a 'SHOULD'. Its not something that the protocol designer has any control over.

ok



s5 title:  Attributes apply to more than just files.

ok



s5, para 4: s/new  OPENATTR operation/OPENATTR operation introduced in NFSv4/

I think just removing "new" suffices.




s5.1, last sentence: I think the 'must' is a 'MUST' - the server is constrained to send the values if asked (but the client has a choice to ask or not).

Agreed.



s5.2:


A client MAY ask for the

   set of attributes the server supports and SHOULD NOT request

   attributes the server does not support.
This should probably be
   An operation allows the client to determine the set of attributes
   that the server supports; a client should not request attributes the
   server does not support.
Its not something the protocol can do anything about.

What I don't like here is what does the server do if the client requests
an attribute that is neither REQUIRED nor RECOMMENDED?

I.e., say the last supported attribute is X, and a client queries on X+2.

Should the server silently ignore this?

What if two different client implementations use that attribute to mean
different things?

For those reasons, I like the wording as is.



s5.8.1.4:s/may/MAY/

ok



s5.8.2.10, para 2: Not sure what root path has to do with locations.

This specifies how root path can be communicated.



s5.9, para 4:


 Servers that do not provide support for all possible values of the

   owner and owner_group attributes SHOULD return an error

   (NFS4ERR_BADOWNER) when a string is presented that has no

   translation, as the value to be set for a SETATTR of the owner,

   owner_group, or acl attributes.
What might the server do if it doesn't return NFS4ERR_BADOWNER? I would have thought this ought to be  MUST.

They might just accept it as untranslatable and not return an error.

As long as they give it back on the GETATTR, this is valid.




s5.9, para 5:


   The "dns_domain" portion of the owner string is meant to be a DNS

   domain name.  For example, user@example.org<mailto:user@example.org>.  Servers should accept

   as valid a set of users for at least one domain.  A server may treat

   other domains as having no valid translations.  A more general

   service is provided when a server is capable of accepting users for

   multiple domains, or for all domains, subject to security

   constraints.
I suspect that the 'should' and 'may' ought to be 'SHOULD' and 'MAY'.  Arguably the 'should' ought to be a 'MUST' if the server recognises owner and owner_group as attributes. i.e., a mandatory to implement.

ok



s5.9, para 6:


In the absence of such a

   configuration, or as a default, the current DNS domain name should be

   the value used for the "dns_domain".
The term 'current DNS domain' is not very specific. Do you mean the DNS domain in which the host resides?

Yes, tried to make clearer



s5.9, 1st bullet point on page 50: A reference to (probably) RFC 5890 is needed to explain U-labels and other IDN stuff is needed (probably best in s1.6).

Added the reference, even though this may change.


s5.10: Expand UCS acronym (and add ref to ISO.10646-1.1993).

ok



s6.1, bullet 3:  An explanation of what is meant by (acl) inheritance is needed.

I'm okay with inheritance being well understood by the target audience.



s6.1, bullet 4, sub-bullet 1:


      *  Setting only the mode attribute should effectively control the

         traditional UNIX-like permissions of read, write, and execute

         on owner, owner_group, and other.
Does this extend to the interpretation of the x bit as searchability on directory objects as per UNIX?


Yes, the next question is does it need to be explained? I think not.




s6.1, bullet 5:  Does mode setting affect the inherited ACL attributes as well as the those of the object for which the mode is set?

s6.2.1, next to last para: Expand SMB (and/or give reference).

expanded, getting correct reference



s6.2.1.1, sentence above table:


   All four but types are permitted in the acl attribute.
'but' is clearly not the right word but I am not quite sure what this trying to say.

'bit'




s6.2.1.3.1, ACE4_ADD_SUBDIRECTORY:
Why is the RENAME operation affected if the object being renamed is a file rather than a directory?


Good question,



s6.2.1.3.1, ACE4_EXECUTE:
Given that directory operation aliases for ACE4_READ_DATA and ACE4_WRITE_DATA have been defined, it would seem sensible to define an alias for the directory/traverse use case of ACE4_EXECUTE.

s6.2.1.3.1, ACE4_DELETE_CHILD and ACE4_DELETE:
s/for information on/for information on how/

ok



s6.2.1.3.1, ACE4_WRITE_ATTRIBUTES - file times modification:
Can the client find out if the server uses client or server times? I didn't see an attribute that tells me which is allowed on the server - cansettime only allows the client to find out if it can modify the times.  What happens (which error code comes back) if the SETATTR has the wrong sort of time spec (e.g., client time when the server only supports server times)? As previously mentioned, is the server expected to do any sanity checks on supplied client times (maybe not allowing - at least - ordinary users to set times backwards from previous values.)

          Permission to change the times associated with a
          file or directory to an arbitrary value.

Which means to me that no sanity check need be applied.




s6.2.1.4: A few words on the effect of inheritance would help and and clarify the next two comments.  Points that arise from the discussions in s6.2.1.4.1:
- Is there a time factor in ACEs?  The phrase 'newly created' seems to imply that there is.  The document doesn't really explain how it is expected that inheritance will be implemented: It could be seen as copying the current set of ACEs from the parents up the tree at the time an object is created or tracking the ACEs up the tree when a access request has to be verified.
- Is the inheritance fully recursive down a directory tree?  I guess that it is unless the NO_PROPAGATE flag is set.. but what about symbolic links?
- Does the inheritance propagate across mount points?

Define what you mean by mount point. Remember that a mount point is a client
side artifact and not a server one.

If you however consider a server which has a namespace in which one filesystem
is mounted on another and that is exported to the client as a single namespace,
then my expectation is that the inheritance is not propagated across the
filesystems.




Additionally, how does ACE inheritance interact with hard and symbolic links?  Does the inheritance attach to the underlying object or only to the name path?

s6.2.1.4.1, ACE4_FILE_INHERIT_ACE:
Three issues:
- presumably this implies the ACE is (primarily) inherited by files in the directory to which it applies
- does 'any sub-directory' imply that the inheritance applies recursively down the tree?
- does 'any non-directory file' include symbolic links?

s6.2.1.4.1, ACE4_DIRECTORY_INHERIT_ACE:
Three issues:
- Does 'should be added to each *new* directory created' imply that the ACE does not get inherited by sub-directories are already in existence when the ACE is applied? Either way it would be good to be explicit about the effect.
- Does this apply recursively down the tree?
- Would the inheritance apply to symbolic links to directories?

s6.2.2:
It would be worth pointing out that the execute permission bits imply searchability for directory objects.  This isn't mentioned in READDIR... is this something missing?

Those are POSIX semantics.




s6.2.2:
Are there restrictions on the mode bits that can be set on named attributes and their directories?  Wouldn't make a lot of sense to set the x bits on named attributes  (probably), nor the suid and svtx bits.

The mode bits set on a named attribute would be those on the base file.




s6.3.1.2, para 1: s/interpretation the ACL/interpretation of the ACL/

ok



s6.1, last para: Expand DCE/DFS acronyms.

deleted



s6.2: Expand TLS and SPKM acronyms. (IPsec is 'well-known')

deleted, but I did expand TLS in another place.



s7.1:


These attributes specify such file system

   instances by specifying a server address target (either as a DNS name

   representing one or more IP addresses or as a literal IP address)

   together with the path of that file system within the associated

   single-server namespace.
I don't understand 'associated single-server namespace'.  Does this mean the aggregated logical file system or something else?  Having read further I suspect that this at least needs a forward reference to Section 8.

Doesn't the first paragraph of Section 7 introduce the term?




s7.2 and s7.3:  These sections are very repetitive.  They essentially convey the fact that if a file system is absent only GETATTR on the absent system and READDIR on a  referring system will produce useful answers and only a couple of attributes are valid.

s7.6:


   A potential problem exists if a client were to allow an open owner to

   have state on multiple filesystems on server, in that it is unclear

   how the sequence numbers associated with open owners are to be dealt

   with, in the event of transparent state migration.  A client can

   avoid such a situation, if it ensures that any use of an open owner

   is confined to a single filesystem.
This paragraph is not very helpful because the concepts of 'open owner', associated state and sequence numbers have not been introduced up to this point apart from as cryptic references in type definitions.  At worst a forward reference is needed.  It might be better to have a brief introduction to the state mechanism under the NFSv4 basic concepts section (maybe around s1.5.4).

Without this introduction I have no idea under what circumstances the 'open owner' might have state on multiple file systems - and hence what I would have to do to avoid going down this rat hole.

As a whole, this document assumes you have already read this document before you read it.

Changing that is beyond the scope of work I want to undertake.




s7.7, last para:  Would be worthwhile to either substitute 'network order' for 'big endian' or add it as an alternative. (It turns out that s9.14 uses network order for just this requirement - consistency would be good!)

The entirety 7.7 from draft 25 is now gone.



s7.7.1:


   When a single file system may be accessed at multiple locations,

   either because of an indication of file system identity as reported

   by the fs_locations attribute, the client will, depending on specific

   circumstances as discussed below, either:
The first 'either' has no corresponding 'or'.
The second 'either' needs an 'or' at the end of the first bullet.

s7.7.1, 2nd bullet: s/Accesses/Access/

s7.7.2, para 3 (and  para 2):


   When there is co-operation in filehandle assignment, the two file

   systems are reported as being in the same handle classes.
It appears that the term 'handle class' is not defined until s7.9.1 (like fileid class in the s7.7.3 and various other attribute classes).  A forward reference to this section is essential  (or maybe better a reordering of the sections).  It would be worth a note somewhere in this area that a client needs to ensure that it knows the fh_expire_type for any filesystem instance it is accessing  in case it get forcibly transitioned, since it can't find out post-facto from an absent filesystem.

s7.7.2, paras 2 and 3:


   When there is no such cooperation in filehandle assignment, the two

   file systems are reported as being in different handle classes.  In

   this case, all filehandles are assumed to expire as part of the file

   system transition.  Note that this behavior does not depend on

   fh_expire_type attribute and depends on the specification of the

   FH4_VOL_MIGRATION bit.



   When there is co-operation in filehandle assignment, the two file

   systems are reported as being in the same handle classes.  In this

   case, persistent filehandles remain valid after the file system

   transition, while volatile filehandles (excluding those that are only

   volatile due to the FH4_VOL_MIGRATION bit) are subject to expiration

   on the target server.
I do not understand the final parts of these paras where FH4_VOL_MIGRATION is mentioned:
- In para 2, FH4_VOL_MIGRATION is a bit in fh_expire_type, so I fail to understand how the behavior can both be independent of fh_expire_type and dependent on a bit within it at the same time.
- In para 3, the wording appears to imply that filehandles with FH4_VOL_MIGRATION set will not expire. My  understanding of the definition of this bit was that such filehandles would explicitly expire on migration.  Accordingly I would have expected that such filehandles would have been guaranteed to expire on transition  - but maybe this is some difficulty with the meanings of transition and migration?

s7.7.3, last para:


   Note that when consistent fileids do not exist across a transition

   (either because there is no continuity of fileids or because fileid

   is not a supported attribute on one of instances involved), and there

   are no reliable filehandles across a transition event (either because

   there is no filehandle continuity or because the filehandles are

   volatile),
I don't understand how there can be no filehandle continuity other than because the filehandle is volatile.  If a filehandle is not volatile, then it is persistent and s4.2.2 appears to indicate that any persistent filehandle would be valid across a transition.

s7.7.5 and s7.7.6, last paras:  s7.9.1 states that distinct server instances are always in different change classes. Since there doesn't seem to be anyway to check that two instances of a server are actually the same when accessed over different connections, the consistent change class mentioned in the last para of s7.7.5 and the continuity mentioned in the last para of s7.7.6 can never occur in practice and would probably be better omitted.

s7.7.6, para 7: A reference to s9.6.3.4 where the edge conditions are described would be desirable.

s7,7,8:  s7.9.1 says all server instances have different readdir classes, so once again the instances will never be deemed consistent.

^^GONE^^





s7.9:  Copying the fs_location(s)4 structure definitions here creates a double maintenance problem.  Better just to reference ss2.2.6/2.2.7 where they are defined.

Actually, due to the way we structure our document source, this is always in sync.

But I don't like how we have two sections and 1 description here:

2.2.6.  fs_location4

   struct fs_location4 {
           utf8val_REQUIRED4       server<>;
           pathname4               rootpath;
   };

2.2.7.  fs_locations4

   struct fs_locations4 {
           pathname4       fs_root;
           fs_location4    locations<>;
   };

   The fs_location4 and fs_locations4 data types are used for the
   fs_locations recommended attribute which is used for migration and
   replication support.

5661 got rid of these as Structured Data Types.

XXX




s8:  Placing this section much earlier in the document would help understanding, especially of Section 7.

Ouch. Yes, single server namespace before multi-server.



s9:  There is a comment in s15.12.5 (para 2) that exactly how locks behave depends on the underlying semantics of the exported  file system.  It would be worth reproducing that statement here to ensure that this section is interpreted in that context.

I don't see how to tie that into the content of s9. In s9, we are talking about the needs of the
protocol and not the permission checking of s15.12.5.



s9, last para:


To support Win32 share reservations it is necessary to atomically

   OPEN or CREATE files.
Add 'and apply the appropriate locks in the same operation' to the end of this sentence.

ack



s9.1: I think that for the sake of clarity, it would be worth explicitly stating that the client has to hold an open before applying locks to the same file.

You need a FH to apply a LOCK. That means either you got it from a OPEN (in which case it is supposed to be a regular file)
or you got it from a LOOKUP, PUTROOTFH, etc (in which case it may be a regular file or any other filesystem object).




s9.1:  What happens if you try to acquire a byte range lock on a non-file filesystem object?


I would expect to see NFS4ERR_BADHANDLE returned.




s9.1.1, para after struct nfs_client_id4 definition: s/which the server has previously recorded the client/which the server has previously recorded for the client/.  Also copying the structure definition creates a double maintenance problem.  A ref to s2.2.12 would be better.

Ack.

There isn't a maintenance issue and since the text is directly explaining the
fields, I want to avoid a reference.

I.e., here is why the XDR presented in the document is not a maintenance issue:

<pre>
      <t>
        Client identification is encapsulated in the following structure:

        <?rfc include='autogen/type_nfs_client_id4.xml'?>
      </t>
</pre>



s9.1.3, first sentence:  Question: Should the 'including' list also have share reservations or are these included in byte-range locks or opens?

Opens.

In any event, the entire parenthetical clause can go.



s9.1.3.5: Expand acronym I/O - this isn't on the RFC Editors 'well known' list - maybe strangely.

Can you suggest they add it? Pretty well understood acronym.




s9.1.3.2: NFS4_UINT32_MAX should be in the proposed constants section.

Which would mean we have such a section.



s9.1.3.2, para 2 and s9.1.6, para 1: The two paragraphs have conflicting statements about the initial value of seqid for locks (s9.1.3.2 => 1, s9.1.6 => server choice) - not that it really matters much unless clients check that they always get 1!  Note that s9.1.6 does say anything about missing out 0 when wrapping round.

So S9.1.3.2 is correct, thanks for catching this one!





s9.1.5, para 9 (bottom of page 116): s/ (e.g., another open specify denial of READs)/(e.g., another open specifying denial of READs)/


ack


s9.4:  Changing:
OLD:
   The server is not
   required to maintain a list of pending blocked locks as it is used to
   increase fairness and not correct operation.
NEW:
   The server is not
   required to maintain a list of pending blocked locks as it is not used to
   provide correct operation but only to increase fairness.
scans more easily.

Ok



s9.6: If I understand correctly, share reservations with a DENY reservation are essentially locks that cover the whole file and adjust dynamically to cover the entire byte range of the file if it changes. Presumably. therefore, the whole discussion of lock recovery applies both to byte range locks and share reservation implicit locks.  I think it would be worth pointing out that this is the case at the beginning of s9.6 (assuming this is indeed the case - or alternatively explaining how things differ with share reservations).

Share reservations are a subset of locks. Seeing an unadorned usage of locks
should alert the reader that it means both share and byte level locks.




s9.8, para 4: In the last sentence s/may not/MUST NOT/.

OK



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.
I am not sure how you 'issue a pending I/O'.  Does this mean checking whether there is already a pending I/O request associated with this stateid?

What I think it means is that if there is a pending I/O in the client's queue,
it should issue that and if not, then it can do a zero-length read.





s10.2, para 6: s/server times its wait/server times out its wait/


ok


s10.2, para 7: I think the lat two sentences are requiremenst:
next to last sentence: s/may/MAY/
last sentence: s/should not/SHOULD NOT/

ok


s10.2.1, para 9 (last para on p146):  I think that this para is referring to the server not the client: s/client/server/

The paragraph details how a client might be able to determine if the
server is limiting the number of available delegations.



s10.2.1, para 12 (2nd bullet): Good to have a ref to the Courtesy Locks section (s9.6.3.1)

ok



s10.2.1, para 17:  "s special semantic adjustments" => either "special semantic adjustments" or "a special semantic adjustment"

ok




s10.2.1, para 19:  It might be better to explain about the distinction between the CLAIM_xxx attribute to be used after server and client restarts that doesn't come till para 24 currently - the changeover to CLAIM_PREVIOUS in this para isn't obvious without this discussion.


XXX



s10.2.1, last para, sentence 1: s/Is/is/

ok


s10.3, 1st bullet: s/and not change/and not the change attribute/ ... there are a couple of other instances where this might also be helpful.

ok



s10.3, 2nd bullet: s/client OPENS as file/the client OPENS a file/

thanks



s10.3.3:  I need to think a bit more about what mandatory locking implies.

s10.4, para after 2nd set of bullets: should the 'standard stateid' be associated with the open-owner rather than the lock-owner as currently stated.

Yes, very good catch!

I had to go back to the RFC 3530 to clear it up...



s10.4.3: Should there be some discussion of wrap round in the 'change' attribute?

No, there is nothing special about a change value of 0. This is unlike a seqid, which
has semantics on a wrap.



s10.5.1, para 2: s/a complete cached copy  of file/a complete cached copy  of the file/,
s/In other case/In other cases/

ok



s10.7, para 2:
OLD:

(regardless whether the file is local file or is being access remotely)
NEW:

(regardless of whether the file is local file or is being accessed remotely)

ok


s11, item 8, first sentence: s/implement/inplemented/

ok



s11, items 9 and 10: Its probably not that important, but would OPTIONAL <=> REQURED be allowed - i.e., two 'levels' of change rather then the explicit one currently documented.

Yes, but we don't want a change here.



s12.1.1, bullet 2: s/in many case/in many cases/

s12.1.2, para 1: s/important to understand/important to understanding/

s12.1.2: The quoted sections should be attributed to where they come from (including section numbers).

s12.1.2: There seems to be a spurious ACCENT in this para:


         LATIN SMALL LETTER E, COMBINING MACRON, ACCENT, COMBINING ACUTE

         ACCENT (U+0xxx, U+0304, U+0301)
s12.1.2 (Maybe expand IDNA again - its 180 pages since it was last expanded).

s12.2.3, para just after Table 5: s/or it in may/or it may/

s12.3: (maybe expand UCS again since it is 130 pages since previous occurrence).

s12.7, 2nd bullet: s/lintext4/linktext4/;  also the term 'link text' hasn't been properly defined.

s12.7.1.5.1: Need to expand NFC and NFD and provide a reference.

s12.7.1.5.2, para 8, last sentence: s/it should generally mapped/it should generally be mapped/

s12.7.3, bullet 4: Should 'converting a user to group to an internal id' be 'converting a user or group to an internal id'?


All of 12 is rewritten.



s13.1.1.7: s/fit/fitted/

ok


s13.1.2.1 s/file handle/filehandle/

ok



s13.1.2.7: should refer to filesystem object rather than file.

ok




s13.1.4.2: Value of NFS4ERR_DQUOT is 19 here but 69 in table 8

Fied



s13.1.4.3/4/5/7: should refer to filesystem object rather than file.

done




s13.1.4.8: Does this apply to any filesystem object rather than just file or directory?


We could just say filesystem object.

fixed up




s13,1.4.11: Value of NFS4ERR_NXIO is 5 here but 6 in table 8 (and value 5 is already used for NFS4ERR_IO).

ok



s13.1.5.2: Value of NFS4ERR_BAD_STATEID is 10026 here but 10025 in table 8 (and value 10026 is already used for NFS4ERR_BAD_SEQID).

ok



s13.1.5.2, bullet 1: should refer to filesystem object rather than file.

xxx




s13.1.7: s/When the strings are not are of length zero./When the strings are not of length zero./

ok



s13.2-13.4: I didn't check these tables.  Table 11 can be checked automatically if it is thought worthwhile by inverting tables 9 and 10.

These are autogenerated from the same source.

As can be seen, the section titles were not.




s14.2: Might be worth stating that COMPOUND doesn't represent a 'transaction', as is expressed in para 3.

ok



s14.2, para 4:  The operation descriptions explicitly mention what happens to the current filehandle after operations.  Nothing is said about the stability of the saved filehandle.  Can this be assumed to be preserved in all cases whether the operation succeeds or fails?  I guess it would also be useful to explicitly state that there are no guarantees about the contents of the current filehandle after an operation unless explicitly stated (i.e., after most errors you ought to reload the current filehandle).

Yes, if you do not modify these filehandles, then the last one entered is valid. I.e., think of it as a register.



s14.3, para 1: s/UNSTABLE/UNSTABLE4/

ok




s15.2.5, para 2: I am not sure how you *could* mix operations - I was under the impression that the COMPOUND is submitted in the context of a particular client ID and the current/saved filehandles are tied to the client ID/connection. The client ID can't be changed in a single compound statement so this seems a spurious statement.  Presumably if stateid's etc are used on the wrong client ID the server will object. See also s15.18.6.


It is possible to migrate a filesystem and state from one server to another, in which case the client could have a client ID that
was migrated.

The intent here is to not mingle such state in a COMPOUND.



s15.3.4: Presumably the access permissions generally apply to named attributes.  Currently it only talks about files and directories.


   ACCESS determines the access rights that a user, as identified by the
   credentials in the RPC request, has with respect to the file system
   object specified by the current filehandle.

So a named attribute is a file system object.



s15.3.5, para 2, sentence 3: s/which will result/which might result/?

ok



s15.3.5, para 4: I found the following confusing:


   The client should use the effective credentials of the user to build

   the authentication information in the ACCESS request used to

   determine access rights.
I am not sure where there is any authentication info (explicitly) "in the ACCESS request" - the parameter is just the requested access mask.  Where is the auth info?

The entire COMPOUND is inside a RPC layer request. NFS operations use the credentials
from that layer to determine ACCESS.




s15.9.4:
OLD:


The server returns an attribute bitmap that

   indicates the attribute values for which it was able to return,

NEW:


The server returns an attribute bitmap that

   indicates the attributes for which it was able to return values,


ack


s15.9.5, last sentence: s/returned if while a delegation/returned if [or while] a delegation/

ok


s15.11.4:  This section refers explicitly to files.  Is it ever possible to make hard links to:
- directories?
- named attributes?
- device and other special types of 'file'?
It would be good to be explicit.

Going to pass on this one.




s15.12.4, para 2:


 To lock the file from a specific offset

   through the end-of-file (no matter how long the file actually is) use

   a length field with all bits set to 1 (one).
Is the end offset fixed at the value given by the length of the file at the time of applying the lock, or does this byte range vary as the length of the file changes?

It should vary as the length varies.


What happens if the length of the file gets to be less than the start offset, or indeed is currently less than the length of the file?

Someone else can comment, my guess is that POSIX file locking semantics would apply.


Actually that is a more general question: Is any special treatment needed for locks that are outside the current length of the file?  Does the server make any checks on whether a lock is within the current length of the file when applied - or automatically delete locks off the end of a file.. probably not but would be good to be explicit.

s15.14.5, para 1:

In all of

   these cases, allowed by POSIX locking [fcntl<cid:part4.00080701.00010200@dial.pipex.com>] semantics, a client

   receiving this error, should if it desires support for such

   operations, simulate the operation using LOCKU on ranges

   corresponding to locks it actually holds, possibly followed by LOCK

   requests for the sub-ranges not being unlocked.
Shouldn't the client apply the sub-range locks before unlocking?  Otherwise there is a potential race condition and some other client could get in a lock between the unlock and the relock. [After a bit of background reading I understand that this rather silly arrangement is 'the way it is done'

ack


s15.16.4/5:  What happens when you apply LOOKUPP to the 'hidden' named attributes directory?  Do you get NFS4ERR_NOENT or the filehandle of the object that owns the named attributes (inverting OPENATTR)?  s15.16.4 only talks about the root of the server's file tree.

s15.18.6: Do SHARES work on named attributes?  Just checking...

Named attributes are not always implemented.

They would be treated as a normal file as far as I can see.





s15.20.5, para3, sentence 1:  "Servers must not require..." I think this is a 'MUST NOT'.

ok




s15.23.5: Acronm SNEGO needs an expansion and a reference (RFC 2478?)

RFC 4178



s15.25.4, para 4: If the OPEN didn't use locks or share reservations and there is no delegation, is the stateid in the READ request the 'basic' OPEN stateid returned with all OPENs? It doesn't explicitly say this.  (also relevant to WRITE operation - s15.38.4, para 3).

Yes, this is to your earlier question on s10.4




s15.25.5, para 1:
OLD:


 It

   is possible that the server reduce the transfer size and so return a

   short read result.  Server resource exhaustion may also occur in a

   short read.
NEW:


 It

   is possible that the server reduces the transfer size and so returns a

   short read result.  Server resource exhaustion may also result in a

   short read.


ok



s15.28.4: The specification says nothing about the semantics of sub-directory removal.  NFSv3 says that 'some servers won't allow removal of non-empty directories'.  What is the story for NFSv4?  Is there any standardised strategy for dealing with 'orphaned' resources in sub-directories if the server does allow a non-empty sub-directory to be deleted?


My guess is that server implementations fall back on their NFSv3 behavior.




s15.29.4, para 5:  It would be clearer to say that the operation will fail if the saved and current file handles refer to named attribute directories attached to different filesystem objects.

I'd like to keep the pain in place for anyone who reads that sentence.

XXX


s15.30.5, para 1 , last sentence: s/such that is could/such that it could/

ok



s15.33.5, para 2:


The filehandle is either provided by the client (PUTFH,

   PUTPUBFH, PUTROOTFH) or generated as a result of a name to filehandle

   translation (LOOKUP and OPEN).
Aren't the PUTxxx filehandles supplied by the server?

The filehandles are provided by the server, but the operations are provided by the client.




s15.38.4:  Does all the discussion of writing to stable storage apply when writing to a named attribute?

yes.



s15.38.5, para next to next to last:


   Some implementations may return NFS4ERR_NOSPC instead of

   NFS4ERR_DQUOT when a user's quota is exceeded.  In the case that the

   current filehandle is a directory, the server will return

   NFS4ERR_ISDIR.  If the current filehandle is not a regular file or a

   directory, the server will return NFS4ERR_INVAL.
 I am not sure what circumstances the last sentence is referring to.  One interpretation would be that WRITE is not allowed at all for special files.  I guess this is not what is intended - please clarify.

named attribute?