Subject: Review of draft-ietf-nfsv4-pnfs-block-10

From: Christian Vogt <[email-masked]>
Date: Thu, 4 Dec 2008 14:42:10 +0200

I was asked by the IESG to review the "pNFS Block/Volume Layout"
specification [draft-ietf-nfsv4-pnfs-block-10], and I am sharing my
review on this list.  The specification is overall in good shape and
should proceed for publication soon.  I do suggest addressing the
following comments, though, before forwarding the document to the RFC
Editor.

Technical:

- Section 2.2.1 ("Volume Identification") specifies two methods for
   identifying a position on a disk: by positive offset starting at the
   beginning of the disk, and by negative offset starting at the end of
   the disk.  The second method is limited to implementations where
   server and client have the same understanding of the disk size. This
   method could be generalized:  If you defined an offset, positive or
   negative, to be in the context of the disk size as seen by the
   client, then you may potentially also support implementations where
   server and clients have a different understanding of the disk size.
   The authors may consider this.

- Section 2.4 ("Crash Recovery Issues") specifies recovery procedures
   that a client could initiate following a server crash.  These
   procedures apply in one specific condition, which is defined at the
   beginning of the section ("When the server crashes while the client
   holds a writable layout, and the client has written data to blocks
   covered by the layout, and the blocks are still in the
   PNFS_BLOCK_INVALID_DATA state, [then]...").  The section should also
   consider other conditions.  It may be sufficient to explain why only
   the described condition requires recovery.

- Section 2.4 ("Crash Recovery Issues") does not explain how a client
   detects a server crash.  The section should briefly explain this. It
   may be sufficient to mention that crash detection is specified in a
   related document, or that it is implementation-specific.

Editorial:

- The specification uses undefined acronyms in a couple of places,
   including in the title.  Those should be spelled out when mentioned
   the first time.  Search for "pNFS", "SAN", "XDR", "LUN" to find the
   relevant places in the specification.

Best regards,
- Christian

_______________________________________________
Ietf mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/ietf

---

From: "Tschofenig, Hannes (NSN - FI/Espoo)" <[email-masked]>
Date: Thu, 4 Dec 2008 15:14:36 +0200

Hi Christian, 

since draft-ietf-nfsv4-pnfs-block-10 is an extension for block and
volume-based layout of
http://www.ietf.org/internet-drafts/draft-ietf-nfsv4-minorversion1-26.tx
t 
and more information about recovery can be found in that NFSv4.1
document. 

Ciao
Hannes

>-----Original Message-----
>From: [email-masked] [mailto:[email-masked]] On 
>Behalf Of ext Christian Vogt
>Sent: 04 December, 2008 14:42
>To: IETF Discussion Mailing List
>Cc: [email-masked]; [email-masked]; 
>[email-masked]; [email-masked]; [email-masked]
>Subject: Review of draft-ietf-nfsv4-pnfs-block-10
>
>specification [draft-ietf-nfsv4-pnfs-block-10], and I am 
>sharing my review on this list.  The specification is overall 
>in good shape and should proceed for publication soon.  I do 
>suggest addressing the following comments, though, before 
>forwarding the document to the RFC Editor.
>
>Technical:
>

---

From: [email-masked]
Date: Fri, 5 Dec 2008 21:31:06 -0500

Christian,

Thank you for doing this review.

>    The authors may consider this.

I would prefer not to do this.  It's fairly easy to make a mistake in
this environment that has the client looking in the wrong place for
the volume identification, risking a false positive, and unpleasant
results.  Beyond this, removing the requirement that server and client
have the same understanding of the disk size may create situations
in which different clients have different ideas of the disk size; that
seems like needless complexity, so all clients need to have the same
understanding of disk size, and adding the server to that understanding
is a small and reasonable step in pursuit of robustness.

As Hannes has already pointed out, there's quite a bit of crash
detection and recovery material in the main NFSv4.1 draft, including
descriptions of how to recover layouts in general (layouts are among
the resources that a client can reclaim during a grace period after
server restart).  Hence this draft only adds material specific to
the block/volume layout.  We'll add a sentence or two in order to
explain this and point to the material in the main NFSv4.1 draft.

We'll expand the first uses of the four acronyms.

Thanks,
--David
----------------------------------------------------
David L. Black, Distinguished Engineer
EMC Corporation, 176 South St., Hopkinton, MA  01748
+1 (508) 293-7953             FAX: +1 (508) 293-7786
[email-masked]        Mobile: +1 (978) 394-7754
----------------------------------------------------

> From: Christian Vogt [mailto:[email-masked]] 
> Sent: Thursday, December 04, 2008 7:42 AM
> Cc: Black, David; fridella, stephen; [email-masked]; 
> [email-masked]; [email-masked]

---

From: Christian Vogt <[email-masked]>
Date: Mon, 8 Dec 2008 21:55:17 +0200

Hi David -

> is a small and reasonable step in pursuit of robustness.

I don't have a strong opinion on whether you add full support for
negative disk offsets despite potential implementation difficulties, or
whether you prohibit such support in order to avoid implementation bugs.
Personally, I would go for the former option -- and add text that warns
about related implementation challenges.  But I won't complain if you
choose the latter option, of course.

> explain this and point to the material in the main NFSv4.1 draft.

Right, a reference to the document that contains the necessary  
background
information is all that's necessary here.

> We'll expand the first uses of the four acronyms.

Perfect.

- Christian