On Thu, 2019-04-11 at 12:52 -0400, Olga Kornievskaia wrote:
> On Thu, Apr 11, 2019 at 12:46 PM Trond Myklebust
> <trondmy@xxxxxxxxxxxxxxx> wrote:
> > On Thu, 2019-04-11 at 12:27 -0400, Olga Kornievskaia wrote:
> > > From: Olga Kornievskaia <kolga@xxxxxxxxxx>
> > >
> > > When VFS calls copy_file_range on a 4.0 mount (and when in and
> > > out
> > > file is the same), we need to return ENOTSUPP instead of EINVAL.
> > > Since no COPY operation is defined in 4.0, then like 3.0, it
> > > should
> > > fallback to doing do_splice_direct().
> > >
> > > Otherwise xfstest generic/075,091,112,263 fail.
> > >
> > > Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
> > > ---
> > > fs/nfs/nfs4file.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> > > index 45b2322..9a222b0 100644
> > > --- a/fs/nfs/nfs4file.c
> > > +++ b/fs/nfs/nfs4file.c
> > > @@ -133,6 +133,12 @@ static ssize_t nfs4_copy_file_range(struct
> > > file
> > > *file_in, loff_t pos_in,
> > > struct file *file_out, loff_t
> > > pos_out,
> > > size_t count, unsigned int
> > > flags)
> > > {
> > > + struct nfs_server *server = NFS_SERVER(file_inode(file_in);
> > > + struct nfs_client *client = server->nfs_client;
> > > +
> > > + if (client->cl_minorversion < 1)
> > > + return -EOPNOTSUPP;
> > > +
> > > if (file_inode(file_in) == file_inode(file_out))
> > > return -EINVAL;
> > > return nfs42_proc_copy(file_in, pos_in, file_out, pos_out,
> > > count);
> >
> > Let's please just move the test for NFS_CAP_COPY from
> > nfs42_proc_copy()
> > into the above function.
>
> But the return order of errors here for different conditions matter.
> For 4.1+ mounts, if in == out, we'd expect EINVAL even if server
> isn't
> capable. But we can't just put the in == out first because for 4.0,
> we
> need to always return EOPNOTSUPP.
>
> > BTW: Why do we return -EINVAL in the file_in == file_out case? Is
> > there
> > any reason why we shouldn't just return EOPNOTSUPP there too in
> > order
> > to let splice() fulfil the operation?
>
> Yes: in == out -> EINVAL is a spec requirement.
>
Returning NFS4ERR_INVAL when the files are identical is a requirement
for the NFSv4.2 server.
On the other hand, the requirement for the Linux file copy operation is
that it should succeed in this situation. So as far as I can tell, we
will continue to fail xfstests until we work around the discrepancy
with the protocol requirement here. The simplest way to do so would
appear to be to return EOPNOTSUPP.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
