Reviewer: Carsten Bormann Review result: Ready with Issues (Insert ARTART review boilerplate here.) I have been using NFS and XDR since the early 1980s, but I am not an expert on NFSv4.2 or pNFS. I did not find a github repository for the draft, so I'll provide comments in a more traditional/tedious form. (Please use the venue/"about this document" facilities of document generation to make the repo easy to find for reviewers.) Any page numbers below are those in the plaintext form of the I-D. ## Major There is an editors' note that appear to be unanswered questions: §2 (p5): // Can it go into LAYOUTRETURN? Please answer. §3.4.2 says: The reason to provide these two attributes is in case of NFS4ERR_ACCESS, the metadata server can compare what it expects the values of the uid and gid of the data file to be versus the actual values. It can then repair the permissions as needed or modify the expected values it has cached. To someone not familiar with the underlying protocols, this appears to be very weak advice on what the metadata server should be doing here. In particular, is such a "repair" already covered by the Security Considerations in [RFC7862] (which is all the Security Considerations in this document amount to)? Also, it says: The mapping of NFSv3 to NFSv4 attributes shown in Table 1 also details which attributes the LAYOUT_WCC SHOULD be providing to the metadata server, [...] Does it? I cannot find that information. Or is the intended meaning that *all* the attributes shown in Table 1 SHOULD be provided? What is the limit of the SHOULD (i.e., under what specific circumstances can that be overridden)? (I note that there is a lower-case "should" in Section 3.4.1 (p6); please check that this is the intended [non-2119] meaning.) ## Minor The definitions (1.1) should probably mention any other sources of definitions that this document relies on (e.g., for "layout"). Section 3.6 could hint at how the optionality is handled (e.g., does the client stop sending the operation when it gets a specific error from the server?). (The answer may be obvious with the level of knowledge about NFSv4.2 actually required by this document, please ignore this observation then.) Section 3.7: But the positional correspondence between the elements is not sufficient to determine the attributes to update. [...] In either case, the combination of ffdsw_deviceid, ffdsw_stateid, and ffdsw_fh_vers will uniquely identify the attributes to be updated. Is this really about the specific attributes to update/to be updated or is it actually about identifying the information object (mirror?) in which the attributes are to be updated? One would expect Section 4 to be boilerplate now; does this differ in any way from similar sections in previous documents? ## Nits Nits are given as a pair of old and new lines, if possible. Abstract (p1) It does not provide a mechanism for the data server to update the metadata server of changes to the data part of the file. The client Abstract(p1) allow the client to update the metadata server to changes on the data Which one should it be -- update "of" or update "to"? §1.1 (p3): ease of use Section 2.1 of [RFC8434] (enable directly linking the section reference on this citation) (This is a recurring comment to a couple dozen other places in the document as well.) §2 (p4): Typo Because there is no contol protocol (see [RFC8434]) possible with all Because there is no control protocol (see [RFC8434]) possible with all §2 (p5): consistency Is it "flexible files layout" (plural, used here) or "Flexible File Layout" (singular, as in abstract/introduction)? §2 (p5): typo model, the metatdate server MAY make such calls anyway in order to model, the metadata server MAY make such calls anyway in order to §3.4.1 (p6): typo data files. While The client can send a LAYOUT_WCC at any time, data files. While the client can send a LAYOUT_WCC at any time, §3.4.2 (p7): typo metadata server, Both the uid and gid are stringified into their metadata server. Both the uid and gid are stringified into their §3.4.2 (p7): Table 1 could use a caption §3.5 (p9): Table 2 could use a caption, maybe moving the stray text line Valid Error Returns for LAYOUT_WCC Is there anything gained from presenting this list in an essentially single-cell table? Maybe it could be formatted to resemble Table 12 (Valid Error Returns for Each Protocol Operation) in Section 15.2 of [RFC8881]? §4 (p10)): Typo shell script to produce the machine readable XDR description of the shell script to produce the machine-readable XDR description of the §4 (p11): Typo code snippets belong in their respective areas of the that XDR. code snippets belong in their respective areas of that XDR. §7.1 (p11): missing link This reference seems to be hand-built and is providing an unusual citation of the draft name (which is also missing a link): [delstid] Haynes, T. and T. Myklebust, "Extending the Opening of Files in NFSv4.2", draft-ietf-nfsv4-delstid-02.xml (Work In Progress), February 2023. Please use the references from bib.ietf.org instead. ## Sorry, could not resist §4 (p10): (would be even easier with sed -E) grep '^ *///' $* | sed 's?^ */// ??' | sed 's?^ *///$??' sed -n 's,^ *///,,p' "$@" | sed 's/^ //' -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
