Re: [nfsv4] Review of draft-haynes-nfsv4-delstid-00

[Trond Myklebust <[e-mail]>]

Hi Dave,

While read delegations may not allow the client to cache byte range locks,
they do allow it to cache OPENs provided that the application only asks for
a share lock of OPEN4_SHARE_DENY_NONE or OPEN4_SHARE_DENY_WRITE, in
combination with OPEN4_SHARE_ACCESS_READ. See for instance:
https://tools.ietf.org/html/rfc5661#section-10.4

Concerning your comments around the offline bit. I do agree that an open
mode requesting that the server fail if the file is offline would be useful
for a number of cases (e.g. when dealing with backup applications). One
main use case for the offline bit is to support existing desktop file
browsers that want to highlight files that are offline to ensure the user
does not try to open them.

Concerning the use of bitmaps in open-support, the problem is that the OPEN
flags are currently an unholy mixture of flags and bit fields (in the C
language sense). For instance, the OPEN4_SHARE_ACCESS_WANT_* are mostly a
bit field taking values between 0x0 and 0xFF, and so we require that
extensions for new share want modes will be adding values starting at
0x06... Furthermore, not all servers currently support all the existing
modes (e.g. OPEN4_SHARE_ACCESS_WANT_CANCEL), so we should require that
clients be able to detect this.
The same argument is true of the claim type: nobody currently implements
CLAIM_DELEGATE_PREV or CLAIM_DELEG_PREV_FH, so any future client that wants
to figure out if the server supports those modes cannot do so. Assuming
that just because the server implements CLAIM_FH, then it must implement
all the modes with values < CLAIM_FH is patently false.

Concerning your question around the time attribute proxying, the reason why
we should add new attributes, is because the behaviour of SETATTR using
these attributes differs from the behaviour when setting mtime and atime
directly. In particular, the behaviour of the ctime is very different. In
order to be able to emulate POSIX semantics on the client, we do not want
the server to change the ctime at all when we set the atime when returning
the delegation. If we're setting the mtime, we want the server to update
the ctime if and only if the new mtime is more recent than the existing
value for ctime.
This behaviour should only be allowed if the client holds the required
delegation type and uses these attributes. Otherwise, standard semantics
apply when setting the mtime and atime using the existing
time_access_set/time_modify_set attributes.

Cheers
   Trond
On Mon, 21 May 2018 at 13:10, David Noveck <[e-mail]> wrote:

> I have read the document and believe all of the extensions suggested in
this document are worthy of serious consideration by the working group,
but, as we are unsure which of these the working group will choose to
pusue, I don't think we can be sure of the structure of the working group
document(s) that will be pursued.  I hope we will get a better sense of
that at IETF 102.

> There are a number of specific issues of presentation that I want to call
your attention to.   As these made it rather difficult to understand the
various individual extensions proposed, I suggest that you take note of
these in preparing further I-D versions.  Some of these might also be
useful in preparing your presentation for IETF 102.

> There was a lot of uncetainty about features involving delegations that
results from confusing text that mentions "delegations" but in fact only
refers to write delegations.  Note that read delegations do not give the
client the ability to open the file on the client without reference to the
server but much of the text assumes that returning any delegation will make
it unnecessary to return an open stateid.
> It is very confusing that section 2 includes two of your extensions
without clearly distinguishing between them.  Adding to the confusion is
the fact that the motivation of the open support attribute is dependent on
the added flags bits which appear later.  I suggest you give each of the
proposed extensions its own section, including a statement of the
motivation for it.  Note that section 4.1 is a good explanation of the need
for the time-attribute proxying feature but it is hidden and appears after
the feature is laid out.  The place for the motivation of a feature is near
the beginning of the description that feature.   After all, you know why
the feature is needed but you can't expect the reader to have the same
insight, and it is unrealistic to assume he is going to assimilate the
protocol details and then correlate them with text explaiing th motivation
that appears later.
> The material in sections 5 and 5.1 is distracting.   You need to look at
RFCs 8275 and 8276 as a model for dealing with XDR extension issues, rather
than the pNFS RFCs.   I don't see the need for a consolidated XDR file such
as is appropriate to a pNFS mapping type or a separate licensing section.

> With regard to the individual exensions, I have, in some cases, issues
with how you have chosen to do the feature while agreeing that these cover
important issues that it is worthwhile to address:

> With regard to the offline file support, I don't agree with the
assumption that the server will necessarily have to bring the file online
in order to open it.  Also, requiring him to do a GETATTR in one compound
and then decide whether to open the file adds to complexity.  Given that
you are extending open, you should consider bits that indicate that an
offline file is not to be opened, or that bringing the file online be
deferred until the first IO operation.  I still think the attribute is
useful but don't think it should be the only  piece of offline file support
specified.
> With regard to the open-support attribute. I like the general idea.  It
is certainly better than forcing the client to try various bits and deal
with NOTSUPP errors.  However, I think that basing this on bitmaps adds
unneded complexity, in that you need separate bit number definitions for
each existing flag and have the task of adding a bit nmber whenever you
extend the protocol by adding a flag.  It would be simpler for this
attribute to parallel the current definition of open and use flags where
OPEN uses flags.  Also, definition of bit numbers for combinations of flags
(e.g. _NONE, _BOTH) probably will become unworkable if we add more bits to
share and deny such as DELETE.
> With regard to the time-attribute-proxying feature, I don't understand
why new attributes are needed rather than basing this on  setting the
attributes that are being proxied.  One option that should be considered is
extending the union time_how to provide for this case. I also have problems
with requirements that SETATTR's be done the same in COMPOUND as the
DELEGRETURN.  I'm not sure why this is needed.  If it is really necessary
that setting of these attribute needs to be tightly bound to the delegation
return, an extended delegation return operation should be considered.   If
you do use SETATTRs, then the stateid would have to be that of the
delegation.  Given what section 18.30 of RFC5661 currently says about the
stateid in SETATTR, the very limited discussion of the stateid in that
section would have to be extended.   The best way to do that would for the
eventual RFC to include a new section essentially replacing section 18.30
of RFC5661.


> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4