ietf
[Top] [All Lists]

RE: [nfsv4] Last Call: <draft-ietf-nfsv4-scsi-layout-06.txt> (Parallel NFS (pNFS) SCSI Layout) to Proposed Standard

2016-07-18 00:51:09

Updated draft is fine with me (if the question was for Spencer S. instead of 
Spencer D).


-----Original Message-----
From: nfsv4 [mailto:nfsv4-bounces(_at_)ietf(_dot_)org] On Behalf Of Christoph 
Hellwig
Sent: Sunday, July 17, 2016 9:35 PM
To: Black, David <david(_dot_)black(_at_)emc(_dot_)com>
Cc: nfsv4-chairs(_at_)ietf(_dot_)org; 
spencerdawkins(_dot_)ietf(_at_)gmail(_dot_)com; ietf(_at_)ietf(_dot_)org; 
nfsv4(_at_)ietf(_dot_)org; draft-ietf-nfsv4-scsi-layout(_at_)ietf(_dot_)org
Subject: Re: [nfsv4] Last Call: <draft-ietf-nfsv4-scsi-layout-06.txt> (Parallel 
NFS (pNFS) SCSI Layout) to Proposed Standard

Hi David,

thanks for the updates.  Let's talk about the detail tomorrow in Berlin,
but unless Spencer disagrees I'll prepare a new draft once the meeting
is over.

On Sun, Jul 10, 2016 at 06:51:40PM +0000, Black, David wrote:
-- Introduction, last paragraph.

Add a sentence saying that there are no other significant differences from 
RFC 5663
(previous sentences indicate use of SCSI for fencing and LAYOUTCOMMIT 
improvements),
e.g., the volume topology (Section 2.3.2) and data structures that describe 
extents
(Section 2.3.3.) are common with RFC 5663.  Those two examples seem important.

Just scrapping the sentence might be best.


-- Section 2.1, 1st paragraph
      " and the SCSI initiators used for the pNFS Metadata Server and clients 
MUST
         support SCSI persistent reservations."
Add a citation of [SPC4] to support that MUST requirement.

Ok.

-- Section 2.1, 2nd paragraph:

   Clients MUST be able to perform I/O to
   the block extents without affecting additional areas of storage
   (especially important for writes); therefore, extents MUST be aligned
   to 512-byte boundaries.

That assumes a 512 byte logical block size, which is generally ok for now, 
but 4k is coming.
At a minimum,  "extents MUST be aligned to logical block size boundaries of 
the logical
units, e.g., 512 bytes."  OTOH, would it be reasonable to just make 4k 
alignment a MUST
now, as there will be storage systems that need 4k alignment and 4k alignment 
generally
works better than 512-byte alignment with existing systems?

I think the right thing is to simply scrap the 512 byte example and just
require extents to be aligned to at least the logical block size.

All implementations of the block and scsi layout known to me align to 4k or
larger, but I see no fundamental reason to forbid 512 byte alignment.


-- Section 2.3.1

   It is similar to the "Identification
   Descriptor Target Descriptor" specified in [SPC4], but limits the
   allowed values to those that uniquely identify a LU.

I suggest just deleting this sentence, as that is now called an 
"Identification CSCD Descriptor" - 
if this sentence is retained, the use of those descriptors in EXTENDED COPY 
would be
important to mention - that seems like a diversion.

Ok.

   2.  The "DESIGNATOR TYPE" MUST be set to one of four values

T10 is now allowing UUIDs to also be used in the working draft of SPC-5, and 
those would be
appropriate here.  Nonetheless, I suggest no change until SPC-5 is completed 
at T10, as this
draft is (properly, IMHO) based on SPC-4.

Yes, I would love to be able to support UUIDs, but I see no way to allow
for that until SPC-5 has been completed.

-- Section 2.4.10.3

   To make sure all I_T nexuses are registered,
   the client SHOULD set the "All Target Ports" (ALL_TG_PT) bit when
   registering the key, or otherwise ensure the registration is
   performed for each initiator port.

It looks like initiator and target registration scopes were conflated here and
need to be separated.  Suggested text:

   To make sure all I_T nexuses are registered,
   the client SHOULD set the "All Target Ports" (ALL_TG_PT) bit when
   registering the key, or otherwise ensure the registration is
   performed for each target port, and MUST perform registration
   for each initiator port.

Thanks, this will need an update.


-- References

Current version of SAM is SAM-5, consider updating SAM-4 reference to SAM-5.

Ok.

_______________________________________________
nfsv4 mailing list
nfsv4(_at_)ietf(_dot_)org
https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fwww.ietf.org%2fmailman%2flistinfo%2fnfsv4&data=01%7c01%7csshepler%40microsoft.com%7c507dc68413fa4645e0f808d3aec4ec9a%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=1v23lbjmin6wj%2bPK%2fKkGaZNEAYpa3A9r50Nf4tunVrY%3d