[nfsv4] WGLC Review of draft-Ietf-nfsv4-xattrs-03 (part one of two)

[David Noveck <[e-mail]>]

*Review Structure*

This review is presented in multiple parts to get around size limits for
emails on the working group list.

This is the first part and includes:

   - *General Comments*
   -
*Comments by Section (through Section 4) *

*General Comments*

*Overall Evaluation*

Overall, the document is in good shape.

The only major technical issue concerns the extension of acemask4.  This
issue is initially introduced in *7.  Protocol Extensions* (In the next
review section).  There is a more detailed discussion of the issue and
options for addressing it in *Acemask4 Extensions* below.

*Acemask4 Extensions*

Changing the structure of an existing attribute poses difficulties in that
there may be clients and servers who do not understand the extension.  In
this case, there is not a lot of  worry about interoperability.  It is
likely that very few clients are aware of the bits in the acemask4 or might
react badly if they saw a bit they didn't recognize.

Nevertheless we do have clients that were written to conform to RFC5661 and
RFC5862 which assumes that no other ace mask bits are present and a server
sending these new bits has no way of knowing whether it is talking to a
client that is aware of xattr support or not, when an acl attribute is
interrogated.  Conceivably a client copying a file to file to a new server
could present that acl to a new NFSv4.x server and get back NFS4ERR_INVAL
because it includes an undefined mask bit.   That it is not a practical
concern but I am worried that if we make an exception here because we
know/believe this is not a significant issue, it will be hard to get the
IESG to accept a straightforward extension model, and I'd really like to
avoid sticking around in minor-version-land much longer.

If we retain these extensions and do not distinguish acl versions using the
minor version number, the following are possible ways to ascertain that a
particular client interrogating an acl attribute is or is not prepared to
receive previously unknown acemask4 bits:

   - Defining new attributes (e.g. acl2, sacl2, dacl2) defined as including
   the new acemask4 biits.
   - Defining a new bit in eia_flag (e.g. EXCHGID4_FLAG_SUPP_XATTR_ACEMASK)
   to allow a client, at EXCHANGE_ID time, to indicate that he is aware of the
   new acemask4 bits.
   - Requiring that the the server only return the new bits after the
   client indicates, by interrogating xattr_support, that it is aware of the
   new feature.


*Comments by Section (through Section 4)*

*Abstract*

There are some wording changes needed to reflect the progress already
made.  If this document is approved, and I believe it should be, then it
will no longer be a "proposal" and the text needs to reflect that.

In the first sentence, suggest:

   - Replacing "proposes" by "describes".
   - Replacing "usIng NFSv4" by "by NFSv4 clients".

In the second sentence, suggest replacing "An xattr is" by "Xattrs are
provided by".

In the last sentence, suggest

   - Replacing "proposed" by "provided".
   - Replacing ", and" by "with that support consisting of".
   - Deleting "are provided" at the end of the sentence.

*1. Introduction*

In the second sentence of the second paragraph, suggest replacing "do not
need to be" by "are not".

In the third sentence of the second paragraph, suggest replacing "various
flavors of" by "facilities to access and modify".

With regard to the last sentence of the second paragraph, I'm unclear
whether the statement is what is really intended.  Would it be more
accurate to replace "in" by "together with"?

The third paragraph doesn't take account of the fact that xattrs are in the
exactly same position as regards API standardization.  I am going to
suggest the following as a possible replacement:

Extended attributes have not previously been included within NFSv4.
One issue that must be addressed as part of including them is that, as
with named attributes, some aspects of their handling are not
precisely defined and they are not formally documented by any standard
(such as POSIX).  Nevertheless, it appears that xattrs are widely
deployed and that their support in modern disk-based file systems is
nearly universal.

In the first sentence of the fourth paragraph, suggest replacing "clear" by
"existing".

In the first sentence of the sixth paragraph, suggest adding a comma after
"attributes".

Also in the first sentence of the sixth paragraph, suggest looking at the
reference to RFC7530.  Since RFC5661 defines the entire protocol. including
named attributes, perhaps that is a better document to reference.

In the second sentence of the sixth paragraph, suggest replacing "proposes"
by "describes".

*2.  Current and Potential Uses of Extended Attributes*

Unfortunately, the way this is now written undercuts the necessary
message, that this is a file system feature that NFSv4, as a remote
file access protocol, needs to support.  In part this is because
"potential uses" comes across as "here's some great things we could do
if this neat new feature were provided", which inevitably generates
skepticism.  Also, the only actual uses are those not supported by
NFS, since NFS doesn't support this yet.  As a result, the most
significant paragraph appears last.  It appears to me that you have
"buried the lead".

I'm going to suggest dividing this section into two:


   - *Functional Gaps Due to Current Lack of NFSv4 Extended Attribute Support*
   - *Potential Uses of Extended Attributes*

I'm not currently sure about:

   - Whether these should be top-level sections or subsections within Section 2.
   - Exactly how all of the existing paragraphs (except the last)
would be affected.

In any case, I suggest beginning the new section/subsection as follows:

In addition to the prospect of data loss discussed above (in Section
1), arising from use of xattrs on local file systems, application use
of xattrs poses further difficulties given the current lack of xattr
support within NFSv4.  As a result, certain applications may not be
supported by NFS or may be supported in an unsatisfactory way.  Some
examples are discussed below.

Baloo, the file indexing and search framework for KDE, has moved to
storing metadata such as tags, ratings and comments, in file system
xattrs instead of a custom database for simplicity.  Starting with KDE
Plasma 5.1, NFS is no longer supported due to its lack of xattr
support [KDE].

Swift, the OpenStack distributed object store, uses xattrs to store an
object's metadata along with all the data together in one file.
Swift-on-File [Swift] transfers the responsibility of maintaining
object durability and availability to the underlying file system.
Today, this requires a native file system client to mount the volumes,
disallowing use of NFS. Xattr support in NFSv4 would correct this
situation, allowing the storing and consuming data from multiple
storage systems, including remote ones.  This would enable the
migration of data between different backend storage systems.

*4.  Namespaces*

In the first sentence of third paragraph, suggest replacing "provides"
by "provides support".

In the third sentence of third paragraph, "xattrs" needs to be capitalized.

In the last sentence of third paragraph, suggest:


   - Replacing "is a need" by "were to be a need".
   - Replacing "attributes" by "one or more attributes".
   - Replacing "to be act" by "to act".
   - Replacing "need to be specified" by "would be specified".
   - Replacing "provided" by "provided for".
   - Replacing the reference to RFC7530 by one to RFC5661.