Re: [Gen-art] Gen-art LC review of draft-ietf-nfsv4-rfc3530bis-dot-x-22

[Tom Haynes <[e-mail]>]

Hi Elwyn,

Thanks for examining across both documents.
Tom

> On Nov 12, 2014, at 2:27 PM, Elwyn Davies <[e-mail]> wrote:
> 
> 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-dot-x-22.txt
> Reviewer: Elwyn Davies
> Review Date: 2014-11-12
> IETF LC End Date: 2014-10-06
> IESG Telechat date: 2014-12-04
> 
> Summary:
> The main aims of this review were to
> 1. Check that the extracted .x file actually compiled (it didn't) - fix below.
> 2. To check the text corresponded with draft-ietf-nfsv4-rfc3530bis-33/34
> 
> I found a few minor glitches but otherwise this draft is almost ready.
> 
> Major Issues:
> (well not really major)
> The extracted .x file does not currently compile with rpcgen.
> The cause is the definition of linktext4 (at line 154 of the extracted file) which was just fixed in draft-ietf-nfsv4-rfc3530bis-34 but not in this one.
> s/linktext4/linktext4<>/
> 
> 


I’m pretty sure that was how I fixed it in 34 in the first place. :-)

And checking what would be draft 23, it is fixed there as well.


> Minor Issues:
> 
> The type used for the lease_time attribute in -dot-x-22 is uint32_t, i.e.,
> typedef uint32_t fattr4_lease_time;

> In 3530bis the type shown in Table 2 is nfs_lease4 but there is no mapping from this type to a base integer type in either document.  This needs cleaning up.

Done - thanks for the pointer to fattr4_lease_time- fixed as well now.


> 
> ========================
> 
> Differences between rfc3530-33/34 and rfc35230bus-dot-x-22:
> -----------------------------------------------------------
> 
> The items below need fixing up in 3530bis (apart from possibly the lease_time type) to make them consistent with -dot-x-22.
> 
> - Error codes NFS4ERR_CB_PATH_DOWN and NFS4ERR_SHARE_DENIED are missing from table 5 in rfc3530-33 and there are no corresponding sections in Section 13.1.1 of rfc3530-33.

Done

>  However, they are mentioned in the text and in Tables 6 and 8.  BTW All these tables ought to have titles (assuming they remain after Barry's comments).  

So Table 8 needs a title. Done


> Also the ordering in Table 5 is slightly out of dictionary sort order if anyone cares.

Fixed NFS4ERR_ATTRNOTSUPP.

NFS4_OK is first by convention. Do you want it changed?

I.e., I do care a bit. :-)

> 
> - Required attributes (Table 2 in rfc3530bis):
>  + change attribute type s/b changeid4

Also in 5661

fixed

>  + lease_time attribute the type used in -dot-x is uint32_t -
>    nfs_lease4 used in 3530bis-33 is not defined in dot-x or copied
>    into 3530bis.

fixed

>  + fs_locations attribute type s/b fs_locations4 (not fs_locations)

Hmm, also an issue in RFC 5661

fixed

>  + owner attribute type s/b utf8str_mixed (not utf8<>)
>  + owner_group attribute type s/b utf8str_mixed (not utf8<>)
> 
> 


Both fixed