Re: [nfsv4] Server-side copy question

[Trond Myklebust <[e-mail]>]

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
>>
>>
>>
>