Re: [nfsv4] New Version Notification for draft-bhalevy-nfs-obj-00.txt

Benny Halevy <bhalevy@tonian.com> Sun, 14 October 2012 11:24 UTC

Return-Path: <bhalevy@tonian.com>
X-Original-To: nfsv4@ietfa.amsl.com
Delivered-To: nfsv4@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 5727521F84DC for <nfsv4@ietfa.amsl.com>; Sun, 14 Oct 2012 04:24:28 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.585
X-Spam-Level:
X-Spam-Status: No, score=-0.585 tagged_above=-999 required=5 tests=[BAYES_40=-0.185, J_CHICKENPOX_44=0.6, RCVD_IN_DNSWL_LOW=-1]
Received: from mail.ietf.org ([64.170.98.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id B0GvaK+mgLS0 for <nfsv4@ietfa.amsl.com>; Sun, 14 Oct 2012 04:24:26 -0700 (PDT)
Received: from mail-we0-f172.google.com (mail-we0-f172.google.com [74.125.82.172]) by ietfa.amsl.com (Postfix) with ESMTP id 8BF7321F8458 for <nfsv4@ietf.org>; Sun, 14 Oct 2012 04:24:25 -0700 (PDT)
Received: by mail-we0-f172.google.com with SMTP id u46so2802944wey.31 for <nfsv4@ietf.org>; Sun, 14 Oct 2012 04:24:24 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding :x-gm-message-state; bh=a2mLpInXG+H3clesDzc/gpQeJ2e5Os5a/BB08lix4mY=; b=kGTns5sYkwwwCuUyuAhAGN1PMsbL6Ur+CzLYueASwzDfAPOytlsy2q4EWOtcp3KLjH cX2xEqSi/nPVVeWQOTDxtfzmhmDebWvG79dB+3gW3Sy2gGxnzjdVdJgfIcwhGj+c6f7D bDNbUW1MzgdfJ3F0SDN8YqBVN+8LMuE0J+TNGv2Zbu76EeaLir23xXgLjbtSYjgzJodP o4Ez0CQx/ausO5kb5h601lARvPbl88ONBph5eY7pR5IkoFTgV+x03uEoUTWm55y06eQM /bjpcFgvfqRzUWZUT1L9KWv9Pai48r6Vq7uqNMChiYGpTq4JGMsK9gI7qMOCCrwYOw42 ghhA==
Received: by 10.180.74.33 with SMTP id q1mr17057020wiv.4.1350213864566; Sun, 14 Oct 2012 04:24:24 -0700 (PDT)
Received: from bhalevy-lt.il.tonian.com (bzq-79-183-215-244.red.bezeqint.net. [79.183.215.244]) by mx.google.com with ESMTPS id eq2sm9265272wib.1.2012.10.14.04.24.21 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 14 Oct 2012 04:24:23 -0700 (PDT)
Message-ID: <507AA06F.6000005@tonian.com>
Date: Sun, 14 Oct 2012 13:22:23 +0200
From: Benny Halevy <bhalevy@tonian.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1
MIME-Version: 1.0
To: "Welch, Brent" <welch@panasas.com>
References: <20120831125212.1375.39854.idtracker@ietfa.amsl.com> <5040B986.6010809@tonian.com> <D54C745FA96F75489B58FCBD8956AE8F09896FBA@SACEXCMBX04-PRD.hq.netapp.com> <5076BA8E.5060507@tonian.com> <DA636AC7BF798243A4007D3AA0B3D3A775FEAC89@seabiscuit.int.panasas.com>
In-Reply-To: <DA636AC7BF798243A4007D3AA0B3D3A775FEAC89@seabiscuit.int.panasas.com>
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 7bit
X-Gm-Message-State: ALoCoQlSjqrMEPjaOfWCpKPp4BJpqBdY7HOVMwBfNqZEQUuRdfbQg4/DMaUtgb07zm+bs/COMDL7
Cc: "Haynes, Tom" <Tom.Haynes@netapp.com>, nfsv4 list <nfsv4@ietf.org>
Subject: Re: [nfsv4] New Version Notification for draft-bhalevy-nfs-obj-00.txt
X-BeenThere: nfsv4@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: NFSv4 Working Group <nfsv4.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/nfsv4>
List-Post: <mailto:nfsv4@ietf.org>
List-Help: <mailto:nfsv4-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 14 Oct 2012 11:24:28 -0000

On 2012-10-11 20:04, Welch, Brent wrote:
> You have a few questions in-line for me below.  A couple are minor that we'll try to address in
> our upcoming 5664bis draft we are trying to complete (like the use of hyphens and the (*).
> 
> The one substantial question you raise about exposing the format of the capability is a good
> question.  The capability is really opaque to the client.  It is generated by the MDS and passed
> along to the OSD, which verifies and enforces it.

Therefore I'd be reluctant to require the client to verify its version.

> Of course an "NFS Object" would have something
> completely different in the capability.

We propose a discriminated union that would provide a T-10 OSD capability only
for OSD objects, so not to reuse the present structure members in a hackish way.
The MDS provides a RPC auth structure instead for NFS objects.

> 
> I have one suggestion for Benny.  I think new layout type should be
> LAYOUT4_NFS_OBJECTS rather than LAYOUT4_OBJECTS_V2" 
> This captures your intent of using an NFS server as an object store,
> and I don't want it to be construed as replacing LAYOUT4_OSD2_OBJECTS,
> which is using T10 OSDv2 objects as the object store.
> 

No problem.

> Yours would be a new RFC, not a replacement for 5664

A new independent RFC for sure, and not obsoleting RFC5664, especially
since I propose wire format changes.

> 
> Also, as you can tell from the mailing list discussion with Trond, Boaz, etc., the new RFC needs to have
> a lot more background a motivation that sets it in context of the other layouts and RFCs.

True.

Benny

> 
>  --
> Brent
> 
> 
> -----Original Message-----
> From: Benny Halevy [mailto:bhalevy@tonian.com] 
> Sent: Thursday, October 11, 2012 5:25 AM
> To: Haynes, Tom
> Cc: nfsv4 list; Welch, Brent
> Subject: Re: [nfsv4] New Version Notification for draft-bhalevy-nfs-obj-00.txt
> 
> On 2012-10-08 00:38, Haynes, Tom wrote:
>> Benny,
>>
>> Editorial, "you" means the document. :->
> 
> Sure :)  Thanks for your thorough review!
> 
>>
>> Thanks,
>> Tom
>>
>> On Aug 31, 2012, at 8:17 AM, Benny Halevy <bhalevy@tonian.com <mailto:bhalevy@tonian.com>> wrote:
>>
>>> Folks,
>>>
>>> I've submitted a proposed extension to the pnfs objects layout
>>> that introduces the use of NFS filers as object storage data servers.
>>>
>>> The following features are proposed as well:
>>> * OSD Multi-path: The OSD address may provide an array of network addresses (ooa_netaddrs).
>>> * I/O Statistics: The client may report I/O stats on LAYOUTRETURN
>>>
>>> The motivation for adding support for NFS comes from:
>>> 1. The desire to use an ubiquitous, standard protocol to access the data servers.
>>> The T10 OSD protocol, although standard, lacks wide adoption, while NFS and in particular
>>> NFSv3 is very popular and widely available.
>>>
>>> 2. Encourage best-of-breed solution.
>>> In contrast to the files layout, the object layout does not require a proprietary
>>> back-end protocol, hence the proposed layout allows one to mix and match a metadata
>>> server and data servers from different vendors.
>>> A simple security control method is proposed to achieve that:
>>> The metadata server controls the file ownership and permissions of the objects
>>> stored on data servers and the client is handed a corresponding RPC credential
>>> on LAYOUTGET a-la OSD capabilities.  Outstanding credentials are unilaterally
>>> revoked by the MDS by modifying the objects user or group owner.
>>>
>>> Benny
>>>
>>
>> 1) You need to put the above into the Introduction. I.e., if you have to set the stage in an
>> email, then you also need to in the document.
> 
> OK
> 
>>
>> I also find it very strange that there is no mention of RFC 5664 in this document. Considering
>> a great deal of this document is based off of that document, there should be a reference.
> 
> Will do.
> 
>>
>> Is the intent that this obsoletes RFC 5664? Given that LAYOUT4_OSD2_OBJECTS is
>> not OBSOLETED, my guess is not.
> 
> Correct.  The intent is to extend RFC5664 not to obsolete it.
> 
>>
>>
>> 2) Even with the stage set, what are the use cases?
> 
> The use cases go from clustering filers for HPC, e.g. scaling capacity and
> bandwidth, through Big Data - e.g. exporting Hadoop clusters over pNFS,
> to NAS virtualization.
> 
>>
>> Why is the version 1 of the objects layout insufficient?
>>
> 
> a. It cannot aggregate existing NFS filers.
> b. The OSD protocol is not widely supported and its future development has practically stopped.
> 
>> 3) Provide a reference
>>
>> In pNFS,
>>
>> Also, you haven't defined the term.
> 
> OK
> 
>>
>> 4) Period at the end:
>>
>>    This document describes the layouts used with object-based
>>    storage devices (OSDs) that are accessed according to the OSD storage
>>    protocol standard (ANSI INCITS 400-2004 [1]) or the NFS protocol
>>    (RFC1813 [14], RFC3530 [15], RFC5661 [2])
> 
> OK
> 
>>
>>
>> 5)
>>
>> The Object Storage protocol
>>
>> How is this related to the OSD storage protocol?
>>
>> Pretty sure they are the same, but why are you using different terms?
> 
> I'm trying to distinguish a generic Object Storage protocol/model from the
> specific T10-OSD model, which becomes one embodiment of the model.
> 
>>
>> 6) 
>>
>>    The Object Storage protocol specifies
>>    several operations on objects, including READ, WRITE, FLUSH, GET
>>    ATTRIBUTES, SET ATTRIBUTES, CREATE, and DELETE.  However, using the
>>    object-based layout the client only uses the READ, WRITE, GET
>>    ATTRIBUTES, and FLUSH commands, or in the NFS case, the READ, WRITE,
>>    GETATTR, and COMMIT operations.  The other commands are only used by
>>    the pNFS server.
>>
>>
>> The way this is written, it sound like the pNFS server is used even in the non-NFS case.
> 
> Will clarify.
> 
>>
>> BTW - which pNFS server? MDS or DS?
> 
> The operations are to the data server, which, if NFSv4.1 is in use, will be used
> as a vanilla NFSv4.1 server with no pNFS requirements (neither MDS nor DS).
> 
>>
>> 7) Need a comma:
>>
>> With NFS filers used for object storage devices the object's owner,
>>
>>
>> After devices...
> 
> OK
> 
>>
>> 8) Mismatch between this:
>>
>>    Note that the XDR code contained in this document depends on types
>>    from the NFSv4.1 nfs4_prot.x file ([5]).
>>
>>
>> and this
>>
>>    /// %#include <nfs4_prot.x>
> 
> RFC5662 seems to generate nfs4_prot.x.
> What mismatch are you referring to?
> 
>>
>>
>> 9) Is this always true:
>>
>>    Creation and
>>    management of partitions is outside the scope of this document, and
>>    is a facility provided by the object-based storage file system.
>>
>>
>> While such acts are outside the scope, is there a standard in place which defines the requirement for
>> such filesystems?
>>
>> BTW - what are the requirements for NFS devices?
> 
> The MDS doesn't have to partition the DS and doing so is implementation specific
> so there's no requirement from the storage device to support any particular standard.
> (though SMI-S comes into mind)
> 
>>
>> 10) Used before definition:
>>
>>    However, it MUST
>>    refer to a device identifed as an NFS device, represented as
>>    oda_obj_type equal to PNFS_OBJ_NFS.
>>
>>
>> So PNFS_OBJ_NFS is in Section 3.3. I do not see a oda_obj_type defined.
>>
>> Okay, it is here:
>>
>> /// union pnfs_obj_deviceaddr4 switch (pnfs_obj_type4 oda_obj_type) {
>>
>>
>> I think it is cleaner to use pnfs_obj_type4 here and also in Section 3.1:
>>
>>    The device MUST by identifed as an
>>    OSD device, represented as oda_obj_type equal to PNFS_OBJ_OSD_V1 or
>>    PNFS_OBJ_OSD_V2.
>>
> 
> True.  Will fix.
> 
>>
>> BTW - these are again used before defined. I would suggest that you rearrange your sections such that
>> 3.3 is 3.1.
> 
> OK. I'll look into this.
> 
>>
>> 10.1)
>>
>>    The NFS equivalent of pnfs_obj_osd_objid4 identifies the object using
>>    a NFS filehandle (See RFC1813 [14], RFC3530 [15], or RFC5661 [2]).
>>
>>
>> The implication here is that NFSv3, NFSv4, and NFSv4.1 filehandles are valid to use in this proposed
>> extension. 
>>
>> Given that the only valid operations are:
>>
>>    An "object" is a container for data and attributes, and files are
>>    stored in one or more objects.  The Object Storage protocol specifies
>>    several operations on objects, including READ, WRITE, FLUSH, GET
>>    ATTRIBUTES, SET ATTRIBUTES, CREATE, and DELETE.  However, using the
>>    object-based layout the client only uses the READ, WRITE, GET
>>    ATTRIBUTES, and FLUSH commands, or in the NFS case, the READ, WRITE,
>>    GETATTR, and COMMIT operations.  The other commands are only used by
>>    the pNFS server.
>>
>>
>> then OPEN is not allowed. Since the "pNFS server" must perforce do this for the client, then how does
>> the pNFS server present the stateid needed for NFSv4 and NFSv4.1 operations?
>>
>>    struct READ4args {
>>            /* CURRENT_FH: file */
>>            stateid4        stateid;
>>            offset4         offset;
>>            count4          count;
>>    };
> 
> 
> Oops, this is a leftover from an earlier internal draft.
> OPEN needs to be supported, otherwise the client will have to revert to using
> only special stateless stateids.
> 
>>
>>
>>
>> 11)  Field names do not follow the NFSv4.1 standard of being derived from the structure name:
>>
>>    /// struct pnfs_obj_nfs_objid4 {
>>    ///     deviceid4       nid_device_id;
>>    ///     opaque          nid_fhandle<>;
>>    /// };
>>    ///
>>
>>
>> pono_fhandle would be consistent.
> 
> OK.
> 
>>
>> 12) Rogue period:
> 
> What do you think need changing?
> 
>>
>>    The second generation OSD protocol (SNIA T10/1729-D [16]). has
>>    additional proposed features to support more robust error recovery,
>>    snapshots, and byte-range capabilities.
>>
>>
>> 13) 
>>
>>    pnfs_obj_type4 is used to indicate the object storage protocol type
>>    and version or whether an object is missing (i.e., unavailable).
>>    Some of the object-based layout- supported RAID algorithms encode
>>
>>
>> Why the '-' after 'layout'?
> 
> English, I guess (Ask Brent :)
> 
>>
>> If this was C code, I'd say that the precedence rules were being violated.
> 
> :)
> 
>>
>> How about:
>>
>>    pnfs_obj_type4 is used to indicate whether the object was
>>
>>    missing (i.e., unavailable) or the object storage protocol type
>>
>>    and version.
>>    Some of the object-based layout- supported RAID algorithms encode
> 
> Looks good.
> 
>>
>>
>> And if you have to explain "missing", perhaps a better enum would be PNFS_OBJ_UNAVAILABLE.
> 
> I'd rather leave the RFC5664 terminology.
> 
>>
>> 14) Is this needed?
>>
>>    (This information can also be deduced by looking inside the
>>    capability type at the format field, which is the first byte.  The
>>    format value is 0x1 for an OSD v1 capability.  However, it seems most
>>    robust to call out the version explicitly.)
>>
>>
>> Where is the format field? What is the capability type?
>>
>> What is it for a OSD v2 capability?
> 
> The capability data structure and enumerated values are specified in the T-10 OSD spec.
> 
>>
>> I'd ditch this altogether, but if you need to say anything, just mention while it can be
>> derived from OSD, this just explicitly states it.
>>
> 
> I tend to agree since the capability is opaque to the client which
> does not have to be able to parse and verify it.
> 
> Brent, what do you think?
> 
>> 15) How can I tell which type of object id I am looking at:
>>
>>    /// struct pnfs_obj_osd_cred4 {
>>    ///     pnfs_obj_osd_objid4         ooc_object_id;
>>    ///     pnfs_obj_osd_cap_key_sec4   ooc_cap_key_sec;
>>    ///     opaque                      ooc_capability_key<>;
>>    ///     opaque                      ooc_capability<>;
>>    /// };
>>    ///
>>    /// struct pnfs_obj_nfs_cred4 {
>>    ///     pnfs_obj_nfs_objid4         onc_object_id;
>>    ///     opaque_auth                 onc_auth;
>>    /// };
>>  
>>
>> Both ooc_object_id and onc_object_id appear to be the same type of object based
>> on the name. But they are vastly different.
> 
> The former is typed ..._osd_objid4 and the latter ..._nfs_objid4
> 
> Am I missing anything?
> 
>>
>> 16)  Redundant:
>>
>>    "oc_obj_type" represents the object storage
>>    device protocol type and version, or whether that component is
>>    unavailable.
>>
>>
>> Point them to Section 3.3.
> 
> OK
> 
>>
>> 17) 3.4 is very hard to follow.
>>
>> I would suggest sub-sections:
>>
>> Missing
>> Object
>> NFS
> 
> OK, that makes sense.
> 
>>
>> You have Object and NFS covered, but no coverage of what is to go into oc_missing_obj_id.
>>
>> I'd also split the chunk of XDR at the start of 3.4 into these subsections as needed.
>>
>> You are hurt by embedding the XDR directly into the document. The flow needed to
>> compile is different from the flow needed to present.
>>
> 
> Yup. A short introductory paragraph should help a human reader of the document.
> 
>> 17) Used before definition:
>>
>>    When oc_obj_type indicates PNFS_OBJ_OSD_V1 or
>>    PNFS_OBJ_OSD_V2, the "ooc_object_id" field identifies the component
>>    object, the "ooc_capability" and "ooc_capability_key" fields, along
>>
>>    with the "ooa_systemid" from the pnfs_obj_deviceaddr4, provide the
>>    OSD security credentials needed to access that object. 
>>
>>
>> What is a pnfs_obj_deviceaddr4? See Section 4.2...
> 
> Thanks
> 
>>
>> Also defined in Section 4 before use.
>>
>> Going back to my point about flow, I'm having a difficult time at this point seeing how
>> given a pnfs_obj_comp4, do I get the pnfs_obj_deviceaddr4?
>>
> 
> The deviceid4 is embedded in both pnfs_obj_{nfs,osd}_objid4.
> In theory it can be extracted into pnfs_obj_comp4 but it is needed
> in the *objid4 structure where it is referenced elsewhere (pnfs_obj_ioerr4).
> 
>> Also, this sentence confuses me:
>>
>>    The pnfs_obj_comp4 union is used to identify each component
>>    comprising the file.
>>
>>
>> What is a component here? Will I have an array of pnfs_obj_comp4s?
> 
> Yes, olo_components<> is an array.
> 
>>
>> Yes, you have used component here without defining it. I think you need a prior section
>> which paints the big picture.
> 
> OK.
> 
>>
>> 18) Confusing:
>>
>>    These appear
>>    in the pnfs_obj_deviceaddr4 type below under the "ooa_systemid" and
>>    "oda_osdname" fields.
>>
>>
>> below under
>>
>> I'd say 
>>
>>    These appear
>>    in the pnfs_obj_deviceaddr4 type presented in Section 4.2
>>
>>    as the "ooa_systemid" and "oda_osdname" fields.
>>
>>
> 
> OK
> 
>> 19) By the way, Section 4 provides the overview I am asking for in 17 above for this section...
>>
> 
> I'll see if it makes sense to reverse the order.
> 
>> 20)  Get rid of comma aster "as a SCSI Name, or"
>>
> 
> Sure
> 
>> 21) What is OBJ_TARGET_ANON used for?
> 
> As said at the bottom of 4.2.1:
>    The OBJ_TARGET_ANON pnfs_osd_targetid_type4 MAY be used for providing
>    no target identification.  In this case, only the OSD System ID, and
>    optionally the provided network address, are used to locate the
>    device.
> 
> I'll mention it briefly in 4.1 too.
> 
>>
>> 22) Why the change in naming scheme for the enum?
>>
>>    /// enum pnfs_osd_targetid_type4 {
>>    ///     OBJ_TARGET_ANON             = 1,
>>    ///     OBJ_TARGET_SCSI_NAME        = 2,
>>    ///     OBJ_TARGET_SCSI_DEVICE_ID   = 3
>>    /// };
>>
>>
>> As per the other uses you present, this should be PNFS_OBJ_*.
> 
> No problem.  This was carried verbatim from RFC5664.
> 
>>
>> 23) Formatting is off:
>>
>>   The specification for an object device address is as follows:
>>
>> Halevy                    Expires March 4, 2013                [Page 12]
>> Internet-Draft                pnfs objects                   August 2012
>>
>> /// union pnfs_obj_targetid4 switch (pnfs_osd_targetid_type4 oti_type) {
>>
>>
>> Break the long lines up rather than changing the start column.
> 
> xml2rfc did this for me (us :)... I'll try breaking the long lines
> and hope that the resulting .x will compile.
> 
>>
>> 24) While NetApp appreciates the free publicity, what is a filer here? :->
> 
> Sure.  Will s/filer/Data Server/
>>
>>    The PNFS_OBJ_NFS pnfs_obj_type4 is used to identify NFS filer
>>    devices.  In this case, ona_version and ona_minorversion represent
>>    the NFS protocol version to be used to access the NFS filer.  Either
>>    the "ona_netaddrs" or "ona_fqdn" fields are used to locate the
>>    device.
>>
>>
>> Are there any restrictions as to what version of NFS is to be used here?
>>
> 
> Yes.  >= 3.  Needs to be specified as such...
> 
>> 24.1) 
>>
>>    "ona_path" MUST be set by the server to an exported path on the
>>    device.  The path provided MUST exist and be accessible to the
>>    client.  If the path does not exist, the client MUST ignore this
>>    device information and any layouts referring to the respective
>>    deviceid until valid device information is acquired.
>>
>>
> 
> As discussed with Bruce Fields, this requirement will be changed to SHOULD.
> 
>> Why isn't this a fatal error to the application?
> 
> ona_path is visible only to the pNFS client, not to the application.
> 
>>
>> What gets returned to the application? Is it supposed to wait forever?
>>
>> How does the client inform the MDS that there is an issue?
>>
> 
> It doesn't, it will not use pNFS for any layouts referring to this device.
> 
>> How does the control protocol from the MDS to the DS interact?
>>
> 
> A MDS/DS control protocol is outside the scope of this document.
> 
>> Finally, I'm a NFS server administrator. 
>>
>> a) What do I expect to see if I access this path? Data files? How are they named? How do I fsck this
>> path? 
> 
> There are going to be a bunch of objects which content is opaque to the user.
> The internal organization of the Data Server namespace is implementation specific
> so it is up to the (filesystem running on the) MDS.
> 
>>
>> b) Am I allowed to let other clients access this path via protocols other than NFSv4.1?
>>
> 
> Better not allow WRITE access.
> READ access might be useful for data protection.
> 
>> c) What about with another layout?
> 
> NFSv4.1 allows that.
> 
>>
>>
>> 25) NFSv4.1 does not define layouttype4 as such:
>>
>> The layout4 type is defined in the NFSv4.1 [2] as follows:
>>
>>    enum layouttype4 {
>>        LAYOUT4_NFSV4_1_FILES   = 1,
>>        LAYOUT4_OSD2_OBJECTS    = 2,
>>        LAYOUT4_BLOCK_VOLUME    = 3,
>>        LAYOUT4_OBJECTS_V2      = 0x08010004       /* Tentatively */
>>    };
>>
>>
>> Just mention you want to add LAYOUT_OBJECTS_V2
> 
> OK.
> 
>> By the way, why the change
>> in format from OSD2_OBJECTS? Are these no longer OSD objects? :->
> 
> Not T-10 OSD anymore.
> 
>>
>> I see you have an IANA Considerations in Section 14. You need to make a reference to it
>> here.
> 
> OK
> 
>> And that section is wrong. You are asking for them to assign you a globally unique
>> value. As it is, your current value is private.
> 
> True.
> 
>>
>> BTW, 5661 defines this enum as:
>>
>>    enum layouttype4 {
>>            LAYOUT4_NFSV4_1_FILES   = 0x1,
>>            LAYOUT4_OSD2_OBJECTS    = 0x2,
>>            LAYOUT4_BLOCK_VOLUME    = 0x3
>>    };
>>
>> You have left off the 0x for the first 3.
> 
> Oops, will fix.
> 
>>
>> 26)
>>
>>
>>    "odm_num_comps" is the number of component objects the file is
>>    striped over.  The server MAY grow the file by adding more components
>>    to the stripe while clients hold valid layouts until the file has
>>    reached its final stripe width.  The file length in this case MUST be
>>    limited to the number of bytes in a full stripe.
>>
>>
>> Such an innocent paragraph and so many questions.
>>
>> This is a radical departure from pNFS, which assumes that a layout has a fixed number of components.
>> How is the change communicated from the server (btw, I assume the MDS is meant here) to the client?
>> Is the layout recalled?
> 
> If needed.  Since the layout is limited to the byte range, adding components may not necessarily
> require layout recall if it does not effect the mapping of the bytes in the byte range
> that was handed out.
> 
>>
>> What happens if the client tries to write with a different view of the number of components?
>>
> 
> Again, for the given byte range the layout is fine.
> Adding components dynamically is applicable within the first stripe.
> Once the layout wraps around the stripe (or group) width is set.
> 
>> Are you planning on  using byte ranges for the layout or is the file restriped when this addition of
>> components occurs?
> 
> Planning on using byte ranges.
> 
>>
>> 27) What happens if it is not?
>>
>>    The "odm_group_width" and "odm_group_depth" parameters allow a nested
>>    striping pattern (see Section 5.3.2 for details).  If there is no
>>    nesting, then odm_group_width and odm_group_depth MUST be zero.  The
>>    size of the components array MUST be a multiple of odm_group_width.
>>
>>
>> So if there is no nesting, then the size of the components array must be zero?
> 
> Duh. Will fix.
> 
>>
>> If there is nesting, then what error is returned if the size of the components array is not a multiple of
>> the odm_group_width?
> 
> returned by whom?
> The server MUST provide a valid layout otherwise the client is free to ditch it and revert
> to plain ol' NFSv4.1
> 
>>
>> Also, what error for:
>>
>>    The "odm_mirror_cnt" is used to replicate a file by replicating its
>>    component objects.  If there is no mirroring, then odm_mirror_cnt
>>    MUST be 0.  If odm_mirror_cnt is greater than zero, then the size of
>>    the component array MUST be a multiple of (odm_mirror_cnt+1).
> 
> ditto
> 
>>
>>
>>
>> 28) What error is returned here?
>>
>>    The data
>>    placement algorithm that maps file data onto component objects
>>    assumes that each component object occurs exactly once in the array
>>    of components.  Therefore, component objects MUST appear in the
>>    olo_components array only once.
> 
> ditto
> 
>>
>>
>> 29) We don't use structure notation:
>>
>>    the olo_components array is
>>    equal to olo_map.odm_num_comps. 
>>
>>
>> How about odm_num_comps in olo_map?
> 
> Sounds good.  Using unique prefix for structure members helps here.
> 
>>
>>
>> 30) olo_comps_index
>>
>>
>>    The components array may represent
>>    all objects comprising the file, in which case "olo_comps_index" is
>>    set to zero and the number of entries in the olo_components array is
>>    equal to olo_map.odm_num_comps.  The server MAY return fewer
>>    components than odm_num_comps, provided that the returned components
>>    are sufficient to access any byte in the layout's data range (e.g., a
>>    sub-stripe of "odm_group_width" components).  In this case,
>>    olo_comps_index represents the position of the returned components
>>    array within the full array of components that comprise the file.
>>
>>
>> What is olo_comps_index supposed to do?
>>
>>
>> An assumption here is that if it is a sub-stripe, it can only occur with components on the upper range of
>>
>> the array? I.e., you define the start point, but not the end point. They must be adjacent, etc.
> 
> It is the start point and yes, the components are adjacent in the array.
> 
>>
>>
>> What determines sufficient?
> 
> All bytes in the layout's byte range must map to the provided components.
> or as it is said here: "sufficient to access any byte in the layout's data range"
> It seems clear enough to me, apparently it is not :)
> 
>> And how is an error communicated back to the MDS if this is not sufficient?
> 
> No error, see above.
> 
>>
>>
>> Is this to make up for the fact that odm_num_comps can grow?
> 
> Partly, and part for returning sub-stripes.
> 
>>
>>
>>
>> 31) What client?
>>
>>
>>    The client uses the file
>>    size to decide if it should fill holes with zeros or return a short
>>    read.
> 
> The NFS client.
> 
>>
>>
>> What are holes?
> 
> Byte ranges which map to unallocated areas on the Data Servers.
> I.e. they have no component object existing or to short objects where
> the hole's byte range maps beyond the component object EOF (as explained
> later in the paragraph)
> 
>    Striping patterns can cause cases where component objects are
>    shorter than other components because a hole happens to correspond to
>    the last part of the component object.
> 
>>
>> How does the client return short reads? I.e., I expect a server to return short reads.
> 
> This is client implementation specific.  A NFS client implementation might
> return a status representing fewer bytes than asked by the application when
> issuing a posix compliant "read" call to the operating system.
> 
>>
>> 32) Redundant:
>>
>>    The odm_mirror_cnt is used to replicate a file by replicating its
>>    component objects.  If there is no mirroring, then odm_mirror_cnt
>>    MUST be 0.  If odm_mirror_cnt is greater than zero, then the size of
>>    the olo_components array MUST be a multiple of (odm_mirror_cnt+1).
>>
>>
>> Already provided in Section 5.1.
> 
> OK. Will provide a XREF instead.
> 
>>
>> 33) Another (see Section 3.5) of use of RAID before definition:
>>
>>    Note that mirroring can be defined over any RAID algorithm and
>>    striping pattern (either simple or nested).
> 
> Seems like this also suggests a general introduction section
> before diving into the XDR section.
> 
>>
>>
>> 34) 5.4 RAID Alogorithms.
>>
>>    Note: The term "RAID" (Redundant Array of Independent
>>    Disks) is used in this document to represent an array of component
>>    objects that store data for an individual file.
>>
>>
>> Really needs to be defined much earlier. This is central to what you are trying to define.
> 
> OK
> 
>>
>> 35) IANA or a version bump?
>>
>>    Thus, if one of these schemes is to be used in the future, a distinct
>>    value must be added to pnfs_obj_raid_algorithm4 for it.
> 
> Good catch.
> 
>>
>>
>> 36) What is (*)
>>
>>    In this protocol, the OSD returns the capacity consumed by a
>>    write (*),
> 
> Good question.  Carried from RFC5664, but I'm not sure what the original intent was.
> Brent, can you help?
> 
>>
>>
>> 37) pnfs_obj_layoutupdate4 is used before defined
>>
>>    Object-Based pNFS clients are not allowed to modify the layout.
>>    Therefore, the information passed in pnfs_obj_layoutupdate4 is used
>>    only to update the file's attributes. 
> 
> I assume the reader can and will use text search to find the definition
> to forward usage cases.  Does RFC5661 follow the no-use-before-definition
> rule by the way?
> 
>>
>>
>> 38) Used before defined and no reference target:
>>
>>    If the lora_layout_type layout type is LAYOUT4_OBJECTS_V2, then the
>>    lrf_body opaque value is defined by the pnfs_obj_layoutreturn4 type.
> 
> Which exactly?
> 
>>
>>
>> 39) Which paragraph 4?
>>
>> 13.5.  Security Considerations over NFS
>>
>>    When NFS is used for storage protocol, obviously the T10 security
>>    mechanism cannot be implemented.  Instead, the server uses the NFS
>>    owner and group identifiers, in combination with the RPC credentials
>>    provided with the layout (as described in Paragraph 4), to simulate
> 
> Hmm, I use <xref target="RPC Credentials Usage" /> which
> is defined as <t anchor="RPC Credentials Usage">.
> I guess having a section for it would work better.
> 
>>
>>
>> 40) Yes, you need them to assign a value for LAYOUT4_OBJECTS_v2...
>>
>> 14.  IANA Considerations
>>
>>    As described in NFSv4.1 [2], new layout type numbers have been
>>    assigned by IANA.  This document defines the protocol associated with
>>    the existing layout type number, LAYOUT4_OBJECTS_V2, and it requires
>>    no further actions for IANA.
>>
>>
>> This is a cut and paste error from RFC5664.
> 
> True.  Will fix.
> 
>>
>> 41) And another cut and paste error.
>>
>> Appendix A.  Acknowledgments
>>
>>    Todd Pisek was a co-editor of the initial versions of this document.
>>    Daniel E. Messinger, Pete Wyckoff, Mike Eisler, Sean P. Turner, Brian
>>    E. Carpenter, Jari Arkko, David Black, and Jason Glasgow reviewed and
>>    commented on this document.
> 
> Yes. Already fixed in my repository.
> 
>>
>>
>> 42) Fencing
>>
>> 12.  Client Fencing
>>
>>    In cases where clients are uncommunicative and their lease has
>>    expired or when clients fail to return recalled layouts within a
>>    lease period at the least (see "Recalling a Layout"[2]), the server
>>    MAY revoke client layouts and/or device address mappings and reassign
>>    these resources to other clients.  To avoid data corruption, the
>>    metadata server MUST fence off the revoked clients from the
>>    respective objects as described in Section 13.4.
>>
>>
>> Should be Sections 13.4 and 13.5.
> 
> OK
> 
>>
>> 43)
>>
>> 13.5.  Security Considerations over NFS
>>
>>    When NFS is used for storage protocol, obviously the T10 security
>>    mechanism cannot be implemented.  Instead, the server uses the NFS
>>    owner and group identifiers, in combination with the RPC credentials
>>    provided with the layout (as described in Paragraph 4), to simulate
>>    the OSD CAPKEY security model.  The file's mode or ACL are set to
>>    provide the file's owner READ and WRITE permissions, to the file's
>>    group READ only permissions, and no permissions to others.
>>    Respecively, the client is provided with the respective credentials
>>    to provided READ/WRITE or READ only access for the respective layout
>>    lo_iomode.
>>
>>    Fencing off a client over NFS is achieved by modifying the respective
>>    files' ownership attributes.  This will implictly revoke the
>>    outstanding credentials and will require the client to ask the server
>>    for new layouts.
>>
>>
>> Fencing in terms of pNFS occurs on the DS and not the MDS. I.e., the MDS has revoked the stateid/layout and access,
>> so how does the DS do this on compounds which have left the client before the revocation from the MDS
>> reach the client?
> 
> In this case fencing is done by manipulating the object (file on DS) ownership or ACL.
> 
>>
>> For file-based layouts, the control protocol is used for the MDS to inform the DS to fence.
>>
>> So with this proposal here, if the client ignores the ownership attributes, then it can still write to the NFS
>> device.
> 
> If the device will let it do that and ignores the RPC auth credentials the client provides.
> 
>> It has no means to prevent this - it has no knowledge of either the layout or stateids.
> 
> There is no intention to follow the file-based layout security model, but rather to propose
> a simpler model that could use "loosely coupled" Data Server using no proprietary back-end
> protocol at the basic level.  Such a back-end protocol can be developed as an enhancement.
> 
> Benny
>