Re: [PATCH 1/1] NFSv4.1 fix incorrect return value in copy_file_range

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@xxxxxxxxxxxxxxx
>
>



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux