Subject: [nfsv4] I-D Action: draft-ietf-nfsv4-layout-types-07.txt

From: [email-masked]
Date: Tue, 29 Aug 2017 14:01:50 -0700

A New Internet-Draft is available from the on-line Internet-Drafts directories.
This draft is a work item of the Network File System Version 4 WG of the IETF.

        Title           : Requirements for pNFS Layout Types
        Author          : Thomas Haynes
	Filename        : draft-ietf-nfsv4-layout-types-07.txt
	Pages           : 16
	Date            : 2017-08-29

Abstract:
   This document defines the requirements which individual pNFS layout
   types need to meet in order to work within the parallel NFS (pNFS)
   framework as defined in RFC5661.  In so doing, it aims to clearly
   distinguish between requirements for pNFS as a whole and those
   specifically directed to the pNFS File Layout.  The lack of a clear
   separation between the two set of requirements has been troublesome
   for those specifying and evaluating new Layout Types.  In this
   regard, this document effectively updates RFC5661.

The IETF datatracker status page for this draft is:
https://datatracker.ietf.org/doc/draft-ietf-nfsv4-layout-types/

There are also htmlized versions available at:
https://tools.ietf.org/html/draft-ietf-nfsv4-layout-types-07
https://datatracker.ietf.org/doc/html/draft-ietf-nfsv4-layout-types-07

A diff from the previous version is available at:
https://www.ietf.org/rfcdiff?url2=draft-ietf-nfsv4-layout-types-07

Please note that it may take a couple of minutes from the time of submission
until the htmlized version and diff are available at tools.ietf.org.

Internet-Drafts are also available by anonymous FTP at:
ftp://ftp.ietf.org/internet-drafts/

---

From: "Black, David" <[email-masked]>
Date: Tue, 29 Aug 2017 21:42:45 +0000

Reading this late in the process, I noticed a few small things:

-- Section 2.  Definitions

   control protocol:  is the particular mechanism that an implementation
      of a layout type would use to meet the control communication
      requirement for that layout type.  This need not be a protocol as
      normally understood.  In some cases the same protocol may be used
      as a control protocol and data access protocol.

"data access protocol" -> "storage protocol"  as that's the term used elsewhere in this document.

-- Section 3.1 item (2)

It might be helpful to add a sentence or two to state that these layout revocation requirements do not apply to layout recall, where the client's cooperative participation is expected (with layout revocation available as a fallback if the client does not cooperate).

-- Section 3.1 item (4).

   (4)  Interactions between locking and I/O operations MUST obey
        existing semantic restrictions.  In particular, if an I/O
        operation would be invalid when directed at the metadata server,
        it is not to be allowed when performed on the storage device.

This is easy to misread as placing responsibility for not allowing the I/O operation on the storage device.  In some layouts, the client is responsible for enforcing that.  Suggested rephrasing of the last line:

        then the I/O operation MUST also be invalid between the
        client and storage device.   Client rejection of the invalid I/O
        operation is a valid means of enforcing this requirements.

The rest of this concern is then covered by the last two paragraphs in Section 3.1 that allow different layouts to place/divide responsibility for meeting the requirements in different ways.

-- Use of ALL CAPS

This ought to be limited to RFC 2119 keywords, e.g., "REQUIREMENTS" ought to be lower case "requirements" in the titles of Sections 3.1 and 3.2, plus one instance in Section 3.2.

Also in Section 3.2:

   (1)  The metadata server MIGHT use fencing operations to the storage
        devices to enforce layout revocation against the client.

MIGHT -> MAY

-- End of Section 3.3

   o  If the metadata server implements mandatory byte-range locking
      when accessed directly by the client, it must do so when data is
      read or written using the designated storage protocol.

Referent of "it" in the second line is wrong: "it must do so" -> "then the layout type specification must require that this also be done"

Thanks, --David

> -----Original Message-----
> From: nfsv4 [mailto:[email-masked]] On Behalf Of internet-
> [email-masked]
> Sent: Tuesday, August 29, 2017 5:02 PM
> To: [email-masked]
> Cc: [email-masked]
> Subject: [nfsv4] I-D Action: draft-ietf-nfsv4-layout-types-07.txt
> 
> 
> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4

---

From: Thomas Haynes <[email-masked]>
Date: Tue, 29 Aug 2017 22:25:04 +0000

Hi David,

Thanks for the review - only one contention and one major change in my replies.

> On Aug 29, 2017, at 2:42 PM, Black, David <[email-masked]> wrote:
> 

I *thought* I eradicated all of those?

Ah, I got rid of data storage protocol.

Ack

> It might be helpful to add a sentence or two to state that these layout revocation requirements do not apply to layout recall, where the client's cooperative participation is expected (with layout…

Currently:

          The metadata
          server typically would revoke a layout whenever a client fails
          to respond to a recall or a client's lease is expired due to
          non-renewal.

Is it too subtle?

And remember we did define both revoking and recalling a layout in Section 2.

> This is easy to misread as placing responsibility for not allowing the I/O operation on the storage device.

I agree with your analysis.

>  In some layouts, the client is responsible for enforcing that.  Suggested rephrasing of the last line:
> 

Going with the format in the third item, how about this:

        For the block and SCSI layout, as the storage device is not able
        to reject the I/O operation, the client is responsible for
        enforcing this requirement.

> This ought to be limited to RFC 2119 keywords, e.g., "REQUIREMENTS" ought to be lower case "requirements" in the titles of Sections 3.1 and 3.2, plus one instance in Section 3.2.

This has already flipped once per review.

Argh, Section 3.3 is not consistent.

I’m going to go with we haven’t been consistent outside of titles (and evidently within) and I find it to read better in lowercase.

Changes made.

>        devices to enforce layout revocation against the client.

ack

> Referent of "it" in the second line is wrong: "it must do so" -> "then the layout type specification must require that this also be done”
> 

Changed.

---

From: "Black, David" <[email-masked]>
Date: Tue, 29 Aug 2017 23:33:26 +0000

Hi Tom,

Following up on this one item, as everything else in your email is fine with me:

> > -- Section 3.1 item (2)
> >
> > It might be helpful to add a sentence or two to state that these layout
> revocation requirements do not apply to layout recall, where the client's
> cooperative participation is expected (with layout revocation available as a
> fallback if the client does not cooperate).
> 
> 
> Currently:
> 
>           The metadata

Yup - I wrote too much in my comment - "requirements do not apply to layout recall" is the only thing that's not obvious.

I'd suggest adding one more sentence as a separate paragraph at the end of item (2):

	In contrast, there is no requirement to restrict access to a file
	on the storage devices when a layout is recalled.

Thanks, --David

> From: Thomas Haynes [mailto:[email-masked]]
> Sent: Tuesday, August 29, 2017 6:25 PM
> To: Black, David <[email-masked]>
> Cc: [email-masked]
> Subject: Re: [nfsv4] I-D Action: draft-ietf-nfsv4-layout-types-07.txt
> 
> Hi David,
> 
> > It might be helpful to add a sentence or two to state that these layout
> revocation requirements do not apply to layout recall, where the client's
> cooperative participation is expected (with layout revocation available as a
> fallback if the client does not cooperate).
> 
> 
> Currently:
> 
>           The metadata
> > The rest of this concern is then covered by the last two paragraphs in
> Section 3.1 that allow different layouts to place/divide responsibility for
> meeting the requirements in different ways.
> >
> > -- Use of ALL CAPS
> >
> > This ought to be limited to RFC 2119 keywords, e.g., "REQUIREMENTS"
> ought to be lower case "requirements" in the titles of Sections 3.1 and 3.2,
> plus one instance in Section 3.2.
> >
> 
> I’m going to go with we haven’t been consistent outside of titles (and
> evidently within) and I find it to read better in lowercase.
> 
> Changes made.
> 
> > Also in Section 3.2:
> >
> > Referent of "it" in the second line is wrong: "it must do so" -> "then the
> layout type specification must require that this also be done”
> >
> 
> Changed.
> 
> 
> 
> > Thanks, --David
> >
> >

---

From: Thomas Haynes <[email-masked]>
Date: Wed, 30 Aug 2017 01:02:43 +0000

> On Aug 29, 2017, at 4:33 PM, Black, David <[email-masked]> wrote:
> 
> Hi Tom,
> 

Okay, that took a little bit of thinking to realize that the key point here was the last word.

I think this is a little bit of domain expertise in that I am assuming the reader is aware
that a recall turns into a revocation goes bad.

I have added the following additional paragraph:

          In contrast, there is no requirement to restrict access to a file
          on the storage devices when a layout is recalled.  It is only
          after the metadata server determines that the client is
          not gracefully returning the layout and starts the revocation
          that this requirement is enforced.

---

From: "Black, David" <[email-masked]>
Date: Wed, 30 Aug 2017 14:35:37 +0000

Looks good, Thanks, --David

> Sent: Tuesday, August 29, 2017 9:03 PM
> >> And remember we did define both revoking and recalling a layout in
> Section
> >> 2.
> >
> > Yup - I wrote too much in my comment - "requirements do not apply to
> layout recall" is the only thing that's not obvious.
> >