Re: [nfsv4] Server-side copy question

[Olga Kornievskaia <[e-mail]>]

On Tue, Feb 28, 2017 at 12:32 PM, J. Bruce Fields <[e-mail]> wrote:
> On Tue, Feb 28, 2017 at 05:04:27PM +0000, Adamson, Andy wrote:
>> I also thought that bytes copied is only included in the success case – but as Olga pointed out, not true. The coa_status of NFS4_OK has the coa_resok4->wr_count showing the bytes copied, and the default (e.g. failure cases) has the coa_bytes_copied.
>>
>> So the server can send a CB_OFFLOAD with an error (such as the –EINVAL in the example) and send the bytes copied in coa_bytes_copied.
>>
>>    union offload_info4 switch (nfsstat4 coa_status) {
>>    case NFS4_OK:
>>            write_response4 coa_resok4;
>>    default:
>>            length4         coa_bytes_copied;
>>    };
>
> Gah, I'm sorry, Olga said CB_OFFLOAD but for some reason I was looking
> at COPY.
>
> I have no idea what to think of that.  I don't see why it should be
> different from COPY.

I'd like to resurrect this thread...

This discussion was initiated to allow for the server to do a partial
copy instead of failing the copy with EINVAL when copy is requested
beyond the end of the file.

However, the problem I'm running into is that because CB_OFFLOAD only
returns "coa_bytes_copied" and does not include stable_how and
verifier. So even if client were to then send a COMMIT for the bytes
that were copied, it has no verifier to match from the COMMIT to
anything.

Should the client then ignore the lack of the matching verifier and
just assume the data was committed for the partial bytes?


>
> --b.
>
>>
>> -->Andy
>>
>> From: nfsv4 <[e-mail]> on behalf of David Noveck <[e-mail]>
>> Date: Tuesday, February 28, 2017 at 11:52 AM
>> To: "J. Bruce Fields" <[e-mail]>
>> Cc: "[e-mail]" <[e-mail]>
>> Subject: Re: [nfsv4] Server-side copy question
>>
>> The bytes copied is only included in the success case, but .... The text talks about the bytes copied being returned in the failure case :-(
>>
>> On Feb 28, 2017 11:44 AM, "J. Bruce Fields" <[e-mail]<mailto:[e-mail]>> wrote:
>> On Tue, Feb 28, 2017 at 11:18:22AM -0500, Olga Kornievskaia wrote:
>> > On Fri, Feb 24, 2017 at 5:21 PM, J. Bruce Fields <[e-mail]<mailto:[e-mail]>> wrote:
>> > > On Thu, Feb 23, 2017 at 04:46:33PM -0500, David Noveck wrote:
>> > >> > I think what we really want is to change the MUST to something weaker
>> > >> > (MAY?).
>> > >>
>> > >> I think we should stay away from the question of what we really want,
>> > >> since everybody's preferences are different.
>> > >>
>> > >> Fortunately, we don't need agreement/consensus on that.  Anyone,
>> > >> can submit an errata.  All you need is a suggested corrected text that
>> > >> people will not object strongly to and will regard as an improvement.
>> > >> Given how broken the existing text is, that is a pretty low bar.
>> > >>
>> > >> I have already suggested using MAY so I'm on board.  Trond
>> > >> had some objections but if he can't come up with something better,
>> > >> I don't think he'll object to your approach.
>> > >>
>> > >> > 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.
>> > >>
>> > >> That's fine but the question is going to be about how such a client
>> > >> deals with server's who manifest behavior A, mandated by RFC 7862
>> > >> and allowed by the corrected text we are discussing.
>> > >>
>> > >> > 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?
>> > >>
>> > >> Given that this only occurs in the range case, which is quite rarely used,
>> > >> the chance of a "major" interoperability problem is not low or
>> > >> infinitesimal.
>> > >> It is zero.
>> > >
>> > > The Linux server implementation currently caps the amount of data copied
>> > > at 4MB, returning a short result instead, to prevent arbitrary long
>> > > copies.  (Partly because we didn't want to implement async copy at
>> > > first.)
>> > >
>> > > So an app might do something like:
>> > >
>> > >         offset = 0;
>> > >         while (copied = copy_file_range(fdin, offset, fdout, offset,
>> > >                                                         len, flags) > 0)
>> > >                 offset += copied;
>> > >
>> > > So I expect the range case to be the normal case there even when copying
>> > > the whole file.  But we're also counting on clients and apps to handle
>> > > short copy results.
>> > >
>> > > (Hm, linux's copy_file_range isn't documented to have the "0 == copy to
>> > > end of file convention".  That's a potentially ugly mismatch?  Not sure
>> > > if our API can or should be fixed.)
>> > >
>> > > The possibility that the copy could return either a short read or EINVAL
>> > > make this all kind of annoying.  You might end up having to do something
>> > > like
>> > >
>> > >         offset = 0;
>> > >         while (1) {
>> > >                 copied = copy_file_range(fdin, offset, fdout, offset,
>> > >                                                         len, flags) > 0)
>> > >                 if (copied == -EINVAL) {
>> > >                         /* Uh, I dunno, maybe stat the file for a new
>> > >                          * length and retry? */
>> >
>> > Bruce, just checking in general for returned EINVAL isn't great
>> > because there is also the case when the source offset is beyond the
>> > end of the file. So now we are forcing the client (application) to
>> > distinguish between this case that is not OK and another case where it
>> > might be ok?
>> >
>> > >                 } else if (copied < 0)
>> > >                         /* error out */
>> > >                 else
>> > >                         offset += copied;
>> > >         }
>> > >
>> > > But, anyway, I guess that's doable.
>> >
>> > An an implementor, I'm still a bit confused about the direction to
>> > take especially on the client side. On the server side, I'm concluding
>> > from this discussion that it's beneficial to ignore the error of
>> > copying beyond the end of the file and returning to the client a
>> > partial copy.
>> >
>> > On the client side, I see 3 possibilities that the client can receive
>> > in asynchronous copy case
>> >
>> > say client sent COPY bytes 0-100 but say there are only 50bytes in the file.
>> > server can send back
>> > 1. CB_OFFLOAD status NFS4_OK bytes copied 50.
>> >  -- given that client needs to be prepared for a short read anyway,
>> > should it retry from offset 50 and now offset is beyond the end of the
>> > file and should the whole copy fail?
>>
>> I think that's right, yes.
>>
>> > 2. CB_OFFLOAD status EINVAL bytes copied 50.
>>
>> Fortunately, this isn't possible--the xdr for bytes copied isn't even
>> there in the error case.
>>
>> --b.
>>
>> >  -- should the client application (or the NFS layer) fail since EINVAL
>> > was returned or should it ignore it and return success to the user?
>> > 3. CB_OFFLOAD status EINVAL bytes copied -1 (or 0).
>> >   -- this is the case where the server strictly enforces the current
>> > spec. This is the clearest case, client can't do anything but fail.
>>
>> _______________________________________________
>> nfsv4 mailing list
>> [e-mail]<mailto:[e-mail]>
>> https://www.ietf.org/mailman/listinfo/nfsv4
>
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4