| Message ID | 20181026201057.36899-13-olga.kornievskaia@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | client-side support for "inter" SSC copy | expand |
On Fri, Oct 26, 2018 at 04:10:57PM -0400, Olga Kornievskaia wrote: > From: Olga Kornievskaia <kolga@netapp.com> > > Add a check to disallow cross file systems copy offload, both > files are expected to be of NFS4.2+ type. > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> Reviewed-by: Matthew Wilcox <willy@infradead.org>
On Fri, 2018-10-26 at 16:10 -0400, Olga Kornievskaia wrote: > From: Olga Kornievskaia <kolga@netapp.com> > > Add a check to disallow cross file systems copy offload, both > files are expected to be of NFS4.2+ type. > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > --- > fs/nfs/nfs4file.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c > index 2f31f30..344e9dd 100644 > --- a/fs/nfs/nfs4file.c > +++ b/fs/nfs/nfs4file.c > @@ -139,8 +139,14 @@ static ssize_t nfs4_copy_file_range(struct file *file_in, loff_t pos_in, > nfs4_stateid *cnrs = NULL; > ssize_t ret; > > - if (file_in->f_inode->i_sb != file_out->f_inode->i_sb) > + if (file_in->f_op != &nfs4_file_operations) > return -EXDEV; > + else { nit: you don't really need the "else" here since the previous block returns > + struct nfs_client *c_in = > + (NFS_SERVER(file_inode(file_in)))->nfs_client; > + if (c_in->cl_minorversion < 2) > + return -EXDEV; > + } > > if (file_inode(file_in) == file_inode(file_out)) > return -EINVAL; Looks fine though. Reviewed-by: Jeff Layton <jlayton@kernel.org>
On Sat, Oct 27, 2018 at 07:08:11AM -0400, Jeff Layton wrote: > > > > - if (file_in->f_inode->i_sb != file_out->f_inode->i_sb) > > + if (file_in->f_op != &nfs4_file_operations) > > return -EXDEV; > > + else { > > nit: you don't really need the "else" here since the previous block > returns > > > + struct nfs_client *c_in = > > + (NFS_SERVER(file_inode(file_in)))->nfs_client; > > + if (c_in->cl_minorversion < 2) > > + return -EXDEV; > > + } Yeah, but if you don't have the else, then you need to declare the c_in at the beginning of the function instead of in the new block. Mind you, if you do that then: c_in = NFS_SERVER(file_inode(file_in))->nfs_client; fits on one line, so it does look a bit neater.
On Sat, Oct 27, 2018 at 9:26 AM Matthew Wilcox <willy@infradead.org> wrote: > > On Sat, Oct 27, 2018 at 07:08:11AM -0400, Jeff Layton wrote: > > > > > > - if (file_in->f_inode->i_sb != file_out->f_inode->i_sb) > > > + if (file_in->f_op != &nfs4_file_operations) > > > return -EXDEV; > > > + else { > > > > nit: you don't really need the "else" here since the previous block > > returns > > > > > + struct nfs_client *c_in = > > > + (NFS_SERVER(file_inode(file_in)))->nfs_client; > > > + if (c_in->cl_minorversion < 2) > > > + return -EXDEV; > > > + } > > Yeah, but if you don't have the else, then you need to declare the c_in > at the beginning of the function instead of in the new block. Mind you, > if you do that then: > > c_in = NFS_SERVER(file_inode(file_in))->nfs_client; > > fits on one line, so it does look a bit neater. My thoughts for the "else" was to be able to get the nfs_client but yes I could declare for the whole function and assign after the first "if". I'll change it.
diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index 2f31f30..344e9dd 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -139,8 +139,14 @@ static ssize_t nfs4_copy_file_range(struct file *file_in, loff_t pos_in, nfs4_stateid *cnrs = NULL; ssize_t ret; - if (file_in->f_inode->i_sb != file_out->f_inode->i_sb) + if (file_in->f_op != &nfs4_file_operations) return -EXDEV; + else { + struct nfs_client *c_in = + (NFS_SERVER(file_inode(file_in)))->nfs_client; + if (c_in->cl_minorversion < 2) + return -EXDEV; + } if (file_inode(file_in) == file_inode(file_out)) return -EINVAL;