Re: [nfsv4] Server-side copy question

["Adamson, Andy" <[e-mail]>]

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

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