Document: draft-ietf-nfsv4-pnfs-block-09
Reviewer: Ben Campbell
Review Date:  2008-11-04
IETF LC End Date: 2008-11-27

Summary:

This draft is almost ready for publication as a proposed standard. I have a couple of questions that I think should be considered first, and a couple of nits.

Comments:

[Disclaimer]

While it is normal for a Gen-ART reviewer to be inexpert in the subject matter being reviewed, I found myself feeling much more inexpert than usual while reviewing this draft. This is due to the subject matter, not the presentation, which seems to be clear in general.


[substantive]

-- Section 2.3.4, second paragraph, last sentence:

> This policy SHOULD be implemented when
>    storage devices do not provide atomicity for concurrent read/write
>    and write/write operations to the same data.

I wonder why that's not a MUST. It seems like the result of neither providing atomic operations, or implementing this policy, would result in corruption, right? Isn't it safe to say that the client MUST NOT create corruption?

-- Section 3, paragraph 2:

> In environments where the
>    security requirements are such that client-side protection from
>    access to storage outside of the layout is not sufficient pNFS
>    block/volume storage layouts for pNFS SHOULD NOT be used, unless 
> the
>    storage device is able to implement the appropriate access checks,
>    via use of virtualized block addresses, or other means.

Maybe this is my ignorance of the subject talking, but I have to wonder what sort of environment might exist where it is sufficient to allow client-side protection only for this sort of thing. It seems like a malicious client could cause all sorts of mayhem.


[Nits and Editorial]

-- IDNITs reports that Page 1 is too long (59 lines)

-- Section 2.2.2, last paragraph:

> In the case of the server
>    returning NFS4ERR_TOOSMALL the client SHOULD allocate a buffer of 
> at
>    least gdir_mincount_bytes to contain the expected result and retry
>    the GETDEVICEINFO request.
>

Should this be GETDEVICELIST? I don't see a prior reference to an  
initial GETDEVICEINFO request.