Re: [nfsv4] Server-side copy question

["J. Bruce Fields" <[e-mail]>]

On Thu, Feb 23, 2017 at 08:00:09AM -0500, David Noveck wrote:
> > The real problem is not so much that the size can change between a client
> check and COPY. It is that, unlike CLONE, the COPY operation itself is not
> required to be
> > atomic by the protocol.
> 
> And while the protocol could define it as atomic, there isn't a lot of
> point in doing that.  There are not many servers that could implement it
> that way, and a non-atomic copy that avoids a lot of OTW traffic is better
> than a non-atomic copy that wastes network bandwidth.  In any case, if you
> don't want the file changed while it is copied, you can open it with
> DENY=WRITE before doing the COPY.
> 
> Let's get back to the issue of the erroneous specification of COPY
> semantics in RFC 7862.
> 
> Let's call the behavior currently specified as a MUST in ETC 7862,
> returning INVAL if the area to be copied spans the file's EOF, Behavior A.
> 
> The more sensible behavior, allowing the copy to proceed, terminating the
> range to be copied at the EOF, will be called behavior B.
> 
> I don't have any doubt that RFC 7862 is, as Bruce says, "just wrong" and
> that behavior B is the right choice.  The question is what to do about the
> issue.

I think what we really want is to change the MUST to something weaker
(MAY?).

Maybe it doesn't work, but I the intention was to allow COPY to be
implemented by either a copy-like or clone-like backend.

> Apart from the question of any possible errata one might or might not
> submit, the important point is that servers that manifest behavior A
> actually exist and will continue to exist for some time.
> 
> In light of this fact, clients might need to adapt to the situation that
> such servers exist.  Whether they do so, depends on how serious they
> consider the issue that Jorge has brought to our attention.  I can see some
> clients ignoring this issue since copying a range, as opposed to a whole
> file, is an unusual case.
> 
> Despite that, it is reasonable for a client to deal with the possibility of
> behavior-A servers by checking for a copy ending point past EOF and
> shortening the range to be copied.  Behavior-B servers would not be
> affected.
> 
> With regard to Bruce's concerns about potential bad consequences resulting
> from changes in EOF after the check and before the copy, I haven't been
> able to come up with a case where anything bad happens. You can't make the
> check-copy combination atomic, because copy itself isn't atomic.
> 
> With regard to Trond's suggestion to use VERIFY, it does narrow the
> window but the fundamental problem of the non-atomicity of COPY remains.
> Also, I'd be reluctant to implement things this way because it is hard to
> figure out what to do if the VERIFY fails.  You may need to retry for a
> while and then try to do the copy using READs and WRITEs.  You wind up with
> a big bunch of code that is almost never tested.  I wouldn't want to go
> there.
> 
> So this brings up back to the errata question.  The goal of an errata would
> be to prevent more behavior-A servers from being written, since we can't be
> sure that any that exist are fixed/decommissioned.

So, my only worry would be to avoid clients that are unprepared to deal
with a short read.  Thankfully I think short reads are already allowed
for other reasons, so I think the risk of a major interoperability
problem here is low?

--b.

> 
> The simplest and most obvious errata is one that simply switches from
> mandating behavior A to mandating behavior B
> 
> 
> I am worried about the time/difficulty of getting such an errata approved.
> The existing text, by using "MUST" really says two things:
> 
>    1. Servers have to implement behavior A.
>    2. Clients depend on behavior A.
> 
> We know that these two statements are wrong, and, in other situations, the
> editor  (Tom) would quickly issue a correction.
> 
> I don't know the details of the errata process but I'm pretty sure it will
> be nothing like the editor simply considering the matter and making the
> necessary correction.  It will involve one or more cycles of  review,
> inquiry, and reassurance, to make sure that existing clients and servers
> will not be adversely affected.  As a result we will have to tell people:
> 
> 
> a) That all (or most) existing servers will be considered non-compliant
> 
> b) That the clients who have been interacting successfully with behavior-A
> servers will have no problems interacting with behavior-B servers, despite
> statement 2 above.
> 
> Given that, approval should not be considered a sure thing and is not going
> to happen quickly.
> 
> Other possible errata texts have been proposed but they don't really
> address the issue of preventing new Behavior-A servers from being created.
> 
> Given the potential difficulties of the errata process and the fact that
> behavior-A servers will continue to exist for a while no matter what
> happens, I think the focus has to be on client changes to work around
> behavior A, if this issue is important enough to do anything about.
> 
> 
> 
> 
> 
> 
> On Feb 18, 2017 11:23 PM, "Trond Myklebust" <[e-mail]> wrote:
> 
> 
> 
> On 18 February 2017 at 15:57, David Noveck <[e-mail]> wrote:
> 
> > > Could we please make that message to implementors stronger, but saying “the server SHOULD limit the are to be copied to reflect the size of the source file, but it MAY fail the operation with NFS4ERR_INVAL”?
> >
> > I think we can do something in this regard but your text is not in accord with RFC2119, although I recall seeing things like this in other documents.  SHOULD means a client is allowed to do something else under some fairly restrictive conditions while MAY says he can choose to do something simply because he wants to.  While it is reasonable to say you "should do X by may do Y", saying you "SHOULD do X but MAY do Y" is almost always self-contradictory.
> >
> > How about leaving the proposed text and adding the following sentence?
> >
> > The latter is generally preferable since the former might  force clients implementing primitives for copying byte ranges to check the size of the file before issuing the copy, which in turn raises issues because such a check and the copy are done at different times and the size might change between the two operations.
> >
> >
> The real problem is not so much that the size can change between a client
> check and COPY. It is that, unlike CLONE, the COPY operation itself is not
> required to be atomic by the protocol. That means that most implementations
> will have to accept one of either two options:
> 
>    1. The server on which the source file resides must somehow cooperate to
>    prevent truncation of the file while the copy is in progress. Note that
>    since the client MAY be holding a file byte range or share lock, it is
>    impossible for either one of the servers to make use of file locks.
>    2. Accept the inevitability that a truncate on the source may cut the
>    COPY short at any time. i.e. that the TOCTOU race condition lasts for the
>    entire duration of the (synchronous or asynchronous) copy.
> 
> Note also that if the _client_ wants to enforce this EINVAL semantic, it
> can easily throw in a VERIFY operation in order to check the file length
> before attempting the COPY operation. Granted that is also non-atomic, but
> that's about as good as it gets here.
> 
> On Sat, Feb 18, 2017 at 1:29 PM, Trond Myklebust <[e-mail]> wrote:
> >
> >>
> >> So here is my proposed corrected text:
> >>
> >> When the source offset is greater than the size of the source file, the
> >> error
> >>
> >> is reported by failing the operation with NFS4ERR_INVAL.  Otherwise, If
> >>
> >> the source offset plus count is greater than the size of the source
> >> file, the
> >>
> >> server MAY fail the operation with NFS4ERR_INVAL.or limit the area to
> >>
> >> be copied to reflect the size of the source file.
> >>
> >> Could we please make that message to implementors stronger, but saying
> >> “the server SHOULD limit the are to be copied to reflect the size of the
> >> source file, but it MAY fail the operation with NFS4ERR_INVAL”?
> >>
> >> Cheers
> >>   Trond
> >>
> >>
> >>
> >> On Feb 18, 2017, at 07:45, David Noveck <[e-mail]> wrote:
> >>
> >> > > I agree but think that the most expedient way of addressing this
> >> problem is
> >> > > in the Linux client,  It can check against the file size before
> >> issuing the
> >> > > COPY request  to prevent an erroneous INVAL error being returned.
> >>
> >> > That doesn't work, due to races with concurrent modifications to the
> >> > file.
> >>
> >> I'm not sure whether it works or not.  I can't find an obvious breakage
> >> but I can't prove that none exists.
> >>
> >> In any case, we can't consider this an easy fix for the issue.
> >>
> >> > The server itself would seem susceptible to such races: if it implements
> >> > the copy as a read-write loop then blocking concurrent truncates
> >> > probably isn't reasonable, so its only choice is a short copy.  (Too
> >> > late to return INVAL once you've written to the destination....).
> >>
> >> That's an implementation problem that would exist no matter
> >> what we do to the spec text under discussion.
> >>
> >> > I really think the spec's just wrong here.'
> >>
> >> I agree, as does Tom and Jorge as well I suppose.   Nobody has said it
> >> was a good choice.
> >>
> >> The question is what to do to fix it.  This is particularly difficult
> >> given
> >> the misuse of "MUST".  The problem I'm worried about is that
> >> the existing approach is presented as required for interoperability.
> >> We know of cases where that approach has been followed and
> >> the problem is that the existing spec essentially says that clients
> >> might break if servers do something else. :-(
> >>
> >> We know that isn't true but sombody (Spencer D?) is going to have
> >> to convince people in the IESG of that.
> >>
> >> So I think the focus of any correction has to be on the fact that the
> >> "MUST"
> >> is wrong rather than on the fact (also true) that the wrong behavior was
> >> chosen.
> >>
> >> So here is my proposed corrected text:
> >>
> >> When the source offset is greater than the size of the source file, the
> >> error
> >>
> >> is reported by failing the operation with NFS4ERR_INVAL.  Otherwise, If
> >>
> >> the source offset plus count is greater than the size of the source
> >> file, the
> >>
> >> server MAY fail the operation with NFS4ERR_INVAL.or limit the area to
> >>
> >> be copied to reflect the size of the source file.
> >>
> >>
> >>
> >>
> >> On Fri, Feb 17, 2017 at 2:51 PM, J. Bruce Fields <[e-mail]>
> >> wrote:
> >>
> >>> On Thu, Feb 16, 2017 at 08:00:36AM -0500, David Noveck wrote:
> >>> > I agree but think that the most expedient way of addressing this
> >>> problem is
> >>> > in the Linux client,  It can check against the file size before
> >>> issuing the
> >>> > COPY request  to prevent an erroneous INVAL error being returned.
> >>>
> >>> That doesn't work, due to races with concurrent modifications to the
> >>> file.
> >>>
> >>> The server itself would seem succeptible to such races: if it implements
> >>> the copy as a read-write loop then blocking concurrent truncates
> >>> probably isn't reasonable, so its only choice is a short copy.  (Too
> >>> late to return INVAL once you've written to the destination....).
> >>>
> >>> I really think the spec's just wrong here.
> >>>
> >>> --b.
> >>>
> >>
> >> _______________________________________________
> >> nfsv4 mailing list
> >> [e-mail]
> >> https://www.ietf.org/mailman/listinfo/nfsv4
> >>
> >>
> >>
> >