[nfsv4] Re: Éric Vyncke's No Objection on draft-ietf-nfsv4-delstid-05: (with COMMENT)

[Thomas Haynes <[e-mail]>]

> On Aug 19, 2024, at 1:11 AM, Éric Vyncke via Datatracker <[e-mail]> wrote:
> 
> Éric Vyncke has entered the following ballot position for
> draft-ietf-nfsv4-delstid-05: No Objection
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
> for more information about how to handle DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-delstid/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> 
> # Éric Vyncke, INT AD, comments for draft-ietf-nfsv4-delstid-05
> 
> Thank you for the work put into this document.
> 
> Please find below some non-blocking COMMENT points (but replies would be
> appreciated even if only for my own education).
> 
> Special thanks to Christopher Inacio for the shepherd's detailed write-up
> including the WG consensus and the justification of the intended status.
> 
> I hope that this review helps to improve the document,
> 
> Regards,
> 
> -éric

Hi Éric,

Thanks for the review (and every review needs responses!).

Replies in-line

> 
> # COMMENTS (non-blocking)
> 
> ## Abstract
> 
> The text about `Discussion of this draft...` should not be part of the abstract
> but be in its own dedicated section.


In the XML, this is:

  <note removeInRFC="true">

I believe there was a bug report generated as for why this did not appear in all converted versions.


> 
> ## Section 1
> 
> While the abstract contains `This document extends both NFSv4.1 (see RFC8881)
> and NFSv4.2 (see RFC7863)`, this section only has `the revisions in this
> document become an extension of NFSv4.2 [RFC7862]`. I.e., there is a
> discrepancy at first sight.


I fixed this in the review from Gunter.


> 
> Some explanations about `delegation stateids` will be welcome for the reader.
> 

I provided a pointer to what a stateid was and I’m not sure how much further I would have to explain here?


> ## Section 2
> 
> Should there be informative reference (or a warning in section 1.1 to re-use
> terms from RFC xyz) for GETATTR, ... ?

I fixed this in the review from Gunter.




> 
> ## Section 3
> 
> What is `bitmap4`, while I am not a NFS expert, I would expect having some
> terminology defined (see comment to section 2).
> 
> ## Section 3.1
> 
> I am not familiar with XDR, but I wonder why the specifications are split in
> multiple CODE brackets rather than a single one. Also puzzled by the use of
> `///` as I read them as comments. Even section 6 does not explain why there is
> a need for a sentinel.
> 

They are split into multiple CODE brackets because implementors might want to cherry-pick them into existing XDR files.

I.e., they may want to keep all of the attribute types in one section, attribute enums in another, etc.

Having said that, I don’t see this splitting helping that much.

So I’ve merged them.

As for the sentinel, there are cases in other documents where we provide both code snippets for comparison and others for specification.

See https://datatracker.ietf.org/doc/html/rfc8435#section-5 vs https://datatracker.ietf.org/doc/html/rfc8435#section-5.1


> ## Section 5.1
> 
> Suggest to mention NFSv3 in the section title.


Agreed.


> 
> ## Section 6.1
> 
> It does not really hurt repeating the code licensing but it is redundant with
> "Copyright notice"

Already addressed in Roman’s review

> 
> ## Section 8
> 
> As I am not a NFS expert, I find strange that IANA registries are not used in
> this document. But, if this is the authors/WG choice, then let it be.
> 
> 

We could have a registry for the attribute ids, but we simply rely on authors to get this straight.

If we do NFSv4.3, I will strongly consider a registry for attribute ids.



> 
> _______________________________________________
> nfsv4 mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]