Question about NFSv4.2 server-side copy implementation

Question about NFSv4.2 server-side copy implementation

[Mora, Jorge]

Hello,
=20
The NFSv4.2 spec (RFC 7862) on section 15.2.3 (COPY description) says the f=
ollowing:
=20
If the source offset or the source offset plus count is greater than
the size of the source file, the operation MUST fail with
NFS4ERR_INVAL.
=20
I can understand failing with NFS4ERR_INVAL when the source offset is beyon=
d the end of the file,
but do you think failing with NFS4ERR_INVAL is too strict when the source o=
ffset plus the count is beyond the end of the file?
What is the rationalization for failing on this specific instance?
Why not return a short copy instead?
Can the COPY return a count less than what it requested (a short copy)?
=20
As of right now, the implementation on the Linux server adheres to the spec=
 but copy_file_range succeeds
when it is called against the local file system, NFSv4.x or NFSv3.
For the local file system, NFSv4.x or NFSv3 copy_file_range falls back to r=
egular copy by
reading from the source file and then writing to the destination file but I=
 do believe the
syscall should be consistent regardless of the underlying file system.
=20
=20
--Jorge

Re: Question about NFSv4.2 server-side copy implementation

[Kinglong Mee]

On 2/15/2017 04:07, Mora, Jorge wrote:
> Hello,
>  
> The NFSv4.2 spec (RFC 7862) on section 15.2.3 (COPY description) says the following:
>  
> If the source offset or the source offset plus count is greater than
> the size of the source file, the operation MUST fail with
> NFS4ERR_INVAL.
>  
> I can understand failing with NFS4ERR_INVAL when the source offset is beyond the end of the file,

Yes, that's right.

> but do you think failing with NFS4ERR_INVAL is too strict when the source offset plus the count is beyond the end of the file?
> What is the rationalization for failing on this specific instance?
> Why not return a short copy instead?

The man-pages of copy_file_range description as,

       EINVAL Requested range extends beyond the end of the  source  file;  or
              the flags argument is not 0.

So, the specific instance you said isn't allowed.

> Can the COPY return a count less than what it requested (a short copy)?

Yes, the return value of copy_file_range is,

RETURN VALUE
       Upon successful completion, copy_file_range() will return the number of
       bytes copied between files.  This could be less than the length  origi‐
       nally requested.

>  
> As of right now, the implementation on the Linux server adheres to the spec but copy_file_range succeeds
> when it is called against the local file system, NFSv4.x or NFSv3.

NFSv3 doesn't support copy_file_range, and only NFSv4.2 supports copy_file_range.

> For the local file system, NFSv4.x or NFSv3 copy_file_range falls back to regular copy by
> reading from the source file and then writing to the destination file but I do believe the
> syscall should be consistent regardless of the underlying file system.
>  
>  
> --Jorge
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: Question about NFSv4.2 server-side copy implementation

[Trond Myklebust]

T24gV2VkLCAyMDE3LTAyLTE1IGF0IDE4OjM2ICswODAwLCBLaW5nbG9uZyBNZWUgd3JvdGU6DQo+
IE9uIDIvMTUvMjAxNyAwNDowNywgTW9yYSwgSm9yZ2Ugd3JvdGU6DQo+ID4gSGVsbG8sDQo+ID4g
wqANCj4gPiBUaGUgTkZTdjQuMiBzcGVjIChSRkMgNzg2Mikgb24gc2VjdGlvbiAxNS4yLjMgKENP
UFkgZGVzY3JpcHRpb24pDQo+ID4gc2F5cyB0aGUgZm9sbG93aW5nOg0KPiA+IMKgDQo+ID4gSWYg
dGhlIHNvdXJjZSBvZmZzZXQgb3IgdGhlIHNvdXJjZSBvZmZzZXQgcGx1cyBjb3VudCBpcyBncmVh
dGVyDQo+ID4gdGhhbg0KPiA+IHRoZSBzaXplIG9mIHRoZSBzb3VyY2UgZmlsZSwgdGhlIG9wZXJh
dGlvbiBNVVNUIGZhaWwgd2l0aA0KPiA+IE5GUzRFUlJfSU5WQUwuDQo+ID4gwqANCj4gPiBJIGNh
biB1bmRlcnN0YW5kIGZhaWxpbmcgd2l0aCBORlM0RVJSX0lOVkFMIHdoZW4gdGhlIHNvdXJjZSBv
ZmZzZXQNCj4gPiBpcyBiZXlvbmQgdGhlIGVuZCBvZiB0aGUgZmlsZSwNCj4gDQo+IFllcywgdGhh
dCdzIHJpZ2h0Lg0KPiANCj4gPiBidXQgZG8geW91IHRoaW5rIGZhaWxpbmcgd2l0aCBORlM0RVJS
X0lOVkFMIGlzIHRvbyBzdHJpY3Qgd2hlbiB0aGUNCj4gPiBzb3VyY2Ugb2Zmc2V0IHBsdXMgdGhl
IGNvdW50IGlzIGJleW9uZCB0aGUgZW5kIG9mIHRoZSBmaWxlPw0KPiA+IFdoYXQgaXMgdGhlIHJh
dGlvbmFsaXphdGlvbiBmb3IgZmFpbGluZyBvbiB0aGlzIHNwZWNpZmljIGluc3RhbmNlPw0KPiA+
IFdoeSBub3QgcmV0dXJuIGEgc2hvcnQgY29weSBpbnN0ZWFkPw0KPiANCj4gVGhlIG1hbi1wYWdl
cyBvZiBjb3B5X2ZpbGVfcmFuZ2UgZGVzY3JpcHRpb24gYXMsDQo+IA0KPiDCoMKgwqDCoMKgwqDC
oEVJTlZBTCBSZXF1ZXN0ZWQgcmFuZ2UgZXh0ZW5kcyBiZXlvbmQgdGhlIGVuZCBvZg0KPiB0aGXC
oMKgc291cmNlwqDCoGZpbGU7wqDCoG9yDQo+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB0
aGUgZmxhZ3MgYXJndW1lbnQgaXMgbm90IDAuDQo+IA0KPiBTbywgdGhlIHNwZWNpZmljIGluc3Rh
bmNlIHlvdSBzYWlkIGlzbid0IGFsbG93ZWQuDQo+IA0KPiA+IENhbiB0aGUgQ09QWSByZXR1cm4g
YSBjb3VudCBsZXNzIHRoYW4gd2hhdCBpdCByZXF1ZXN0ZWQgKGEgc2hvcnQNCj4gPiBjb3B5KT8N
Cj4gDQo+IFllcywgdGhlIHJldHVybiB2YWx1ZSBvZiBjb3B5X2ZpbGVfcmFuZ2UgaXMsDQo+IA0K
PiBSRVRVUk4gVkFMVUUNCj4gwqDCoMKgwqDCoMKgwqBVcG9uIHN1Y2Nlc3NmdWwgY29tcGxldGlv
biwgY29weV9maWxlX3JhbmdlKCkgd2lsbCByZXR1cm4gdGhlDQo+IG51bWJlciBvZg0KPiDCoMKg
wqDCoMKgwqDCoGJ5dGVzIGNvcGllZCBiZXR3ZWVuIGZpbGVzLsKgwqBUaGlzIGNvdWxkIGJlIGxl
c3MgdGhhbiB0aGUNCj4gbGVuZ3RowqDCoG9yaWdp4oCQDQo+IMKgwqDCoMKgwqDCoMKgbmFsbHkg
cmVxdWVzdGVkLg0KPiANCg0KSXQncyBhIHZhbGlkIHF1ZXN0aW9uLCB0aG91Z2guIE5vdCBsZWFz
dCBiZWNhdXNlIGNvcHlfZmlsZV9yYW5nZSgpIGlzDQpub3QgZ3VhcmFudGVlZCB0byBiZSBhdG9t
aWMsIG1lYW5pbmcgdGhhdCB5b3UgY291bGQgaGF2ZSByYWNlcyB3aGVyZQ0KdGhlIGV4ZWN1dGlv
biBvZiB0aGUgY29weV9maWxlX3JhbmdlKCkgaXMgc3RhcnRlZCwgYnV0IHRoZSBmaWxlIGlzDQp0
cnVuY2F0ZWQgd2hlbiB0aGUgY29weSBpcyBvbmx5IGhhbGYtY29tcGxldGUuDQoNCkpvcmdlLCBJ
IHN1Z2dlc3QgcmFpc2luZyB0aGUgcXVlc3Rpb24gb24gdGhlIElFVEYgbWFpbGluZyBsaXN0LiBU
aGlzDQptaWdodCBiZSBhIGNhbmRpZGF0ZSBmb3IgYSBORlN2NC4yIHByb3RvY29sIGVycmF0YS4N
Cg0KQ2hlZXJzDQogIFRyb25kDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVu
dCBtYWludGFpbmVyLCBQcmltYXJ5RGF0YQ0KdHJvbmQubXlrbGVidXN0QHByaW1hcnlkYXRhLmNv
bQ0K

Re: Question about NFSv4.2 server-side copy implementation

[Olga Kornievskaia]

On Wed, Feb 15, 2017 at 5:36 AM, Kinglong Mee <kinglongmee@gmail.com> wrote=
:
> On 2/15/2017 04:07, Mora, Jorge wrote:
>> Hello,
>>
>> The NFSv4.2 spec (RFC 7862) on section 15.2.3 (COPY description) says th=
e following:
>>
>> If the source offset or the source offset plus count is greater than
>> the size of the source file, the operation MUST fail with
>> NFS4ERR_INVAL.
>>
>> I can understand failing with NFS4ERR_INVAL when the source offset is be=
yond the end of the file,
>
> Yes, that's right.
>
>> but do you think failing with NFS4ERR_INVAL is too strict when the sourc=
e offset plus the count is beyond the end of the file?
>> What is the rationalization for failing on this specific instance?
>> Why not return a short copy instead?
>
> The man-pages of copy_file_range description as,
>
>        EINVAL Requested range extends beyond the end of the  source  file=
;  or
>               the flags argument is not 0.
>
> So, the specific instance you said isn't allowed.
>
>> Can the COPY return a count less than what it requested (a short copy)?
>
> Yes, the return value of copy_file_range is,
>
> RETURN VALUE
>        Upon successful completion, copy_file_range() will return the numb=
er of
>        bytes copied between files.  This could be less than the length  o=
rigi=E2=80=90
>        nally requested.
>
>>
>> As of right now, the implementation on the Linux server adheres to the s=
pec but copy_file_range succeeds
>> when it is called against the local file system, NFSv4.x or NFSv3.
>
> NFSv3 doesn't support copy_file_range, and only NFSv4.2 supports copy_fil=
e_range.

copy_file_range() can be called with two file description that are
from the NFSv3 mounts and currently as you point NFSv3 does not
support copy_file_range() vfs_copy_file_range() will fall back into
doing "normal" copy via do_splice_direct() what happens here is that
NFSv3 will not fail with EINVAL and instead will return a partial
copy. Thus as Jorge points out, we have inconsistent results of a copy
being done via an NFS protocol v3 vs 4.2 protocol (if we were to
enforce the rule that the spec says).


>> For the local file system, NFSv4.x or NFSv3 copy_file_range falls back t=
o regular copy by
>> reading from the source file and then writing to the destination file bu=
t I do believe the
>> syscall should be consistent regardless of the underlying file system.
>>
>>
>> --Jorge
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about NFSv4.2 server-side copy implementation

[J. Bruce Fields]

On Tue, Feb 14, 2017 at 08:07:48PM +0000, Mora, Jorge wrote:
> I can understand failing with NFS4ERR_INVAL when the source offset is beyond the end of the file,
> but do you think failing with NFS4ERR_INVAL is too strict when the source offset plus the count is beyond the end of the file?
> What is the rationalization for failing on this specific instance?
> Why not return a short copy instead?
> Can the COPY return a count less than what it requested (a short copy)?
>  
> As of right now, the implementation on the Linux server adheres to the spec

That's weird, do you have network traces showing that, or is it possible
the EINVAL is happening on the client side?

Re: Question about NFSv4.2 server-side copy implementation

[Olga Kornievskaia]

On Fri, Feb 17, 2017 at 3:02 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Tue, Feb 14, 2017 at 08:07:48PM +0000, Mora, Jorge wrote:
>> I can understand failing with NFS4ERR_INVAL when the source offset is beyond the end of the file,
>> but do you think failing with NFS4ERR_INVAL is too strict when the source offset plus the count is beyond the end of the file?
>> What is the rationalization for failing on this specific instance?
>> Why not return a short copy instead?
>> Can the COPY return a count less than what it requested (a short copy)?
>>
>> As of right now, the implementation on the Linux server adheres to the spec
>
> That's weird, do you have network traces showing that, or is it possible
> the EINVAL is happening on the client side?
>
> From a quick look at the server code I can't see where it would be
> generating that EINVAL, but I haven't tested this case and I could be
> overlooking something....
>

I think the current upstream COPY does not fail with EINVAL. The new
code that Jorge was testing does adhere to the spec and fails with
EINVAL.

Bruce, it looks to me that current upstream CLONE in this situation
will return EINVAL (I see that on the network trace since the client
will first try to do CLONE and if it can't it'll do COPY).

But we are trying to get some consistency in errors.


> --b.
>
>> but copy_file_range succeeds
>>
>> when it is called against the local file system, NFSv4.x or NFSv3.
>> For the local file system, NFSv4.x or NFSv3 copy_file_range falls back to regular copy by
>> reading from the source file and then writing to the destination file but I do believe the
>> syscall should be consistent regardless of the underlying file system.
>>
>>
>> --Jorge
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about NFSv4.2 server-side copy implementation

[J. Bruce Fields]

On Fri, Feb 17, 2017 at 04:10:45PM -0500, Olga Kornievskaia wrote:
> On Fri, Feb 17, 2017 at 3:02 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Tue, Feb 14, 2017 at 08:07:48PM +0000, Mora, Jorge wrote:
> >> I can understand failing with NFS4ERR_INVAL when the source offset is beyond the end of the file,
> >> but do you think failing with NFS4ERR_INVAL is too strict when the source offset plus the count is beyond the end of the file?
> >> What is the rationalization for failing on this specific instance?
> >> Why not return a short copy instead?
> >> Can the COPY return a count less than what it requested (a short copy)?
> >>
> >> As of right now, the implementation on the Linux server adheres to the spec
> >
> > That's weird, do you have network traces showing that, or is it possible
> > the EINVAL is happening on the client side?
> >
> > From a quick look at the server code I can't see where it would be
> > generating that EINVAL, but I haven't tested this case and I could be
> > overlooking something....
> >
> 
> I think the current upstream COPY does not fail with EINVAL. The new
> code that Jorge was testing does adhere to the spec and fails with
> EINVAL.
> 
> Bruce, it looks to me that current upstream CLONE in this situation
> will return EINVAL (I see that on the network trace since the client
> will first try to do CLONE and if it can't it'll do COPY).

OK, I see, in fs/read_write.c:vfs_clone_file_range(),

	if (pos_in + len > i_size_read(inode_in))
		return -EINVAL;

And since a copy can be implemented under the covers as a clone, we can
hit that case in copy too.

(Though vfs_copy_file_range calls ->clone_file_range directly instead of
calling vfs_clone_file_range, so it's up to individual filesystems
whether they EINVAL in that case--the one I checked (btrfs) did, and I
think it makes sense for clone as it's an all-or-nothing operation.)

So maybe we have to allow this EINVAL behavior on copy, but it still
looks wrong to me to require it.

--b.


> 
> But we are trying to get some consistency in errors.
> 
> 
> > --b.
> >
> >> but copy_file_range succeeds
> >>
> >> when it is called against the local file system, NFSv4.x or NFSv3.
> >> For the local file system, NFSv4.x or NFSv3 copy_file_range falls back to regular copy by
> >> reading from the source file and then writing to the destination file but I do believe the
> >> syscall should be consistent regardless of the underlying file system.
> >>
> >>
> >> --Jorge
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html