Re: nfs generic/373 failure after "fs: allow cross-vfsmount reflink/dedupe"

["J. Bruce Fields" <bfields@fieldses.org>]

On Wed, Mar 02, 2022 at 07:50:01PM -0500, J. Bruce Fields wrote:
> On Wed, Mar 02, 2022 at 07:29:34PM -0500, Josef Bacik wrote:
> > On Wed, Mar 02, 2022 at 07:07:35PM -0500, J. Bruce Fields wrote:
> > > Sorry, took me a minute to understand, myself:
> > > 
> > > It's actually only the client behavior that changed.  Previously the
> > > client would reject an attempt to clone across filesystems, so the
> > > server never saw such a request.  After this patch, the client will go
> > > ahead and send the CLONE.  (Which, come to think of it, is probably the
> > > right thing for the client to do.)
> > > 
> > > So the server's probably always had a bug, and this just uncovered it.
> > > 
> > > I'd be curious what the consequences are.  And where the check should be
> > > (above or below vfs_clone_file_range()?).
> > > 
> > 
> > This is where I'm confused, this really shouldn't succeed
> > 
> > loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,
> >                            struct file *file_out, loff_t pos_out,
> >                            loff_t len, unsigned int remap_flags)
> > {
> >         loff_t ret;
> > 
> >         WARN_ON_ONCE(remap_flags & REMAP_FILE_DEDUP);
> > 
> >         if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb)
> >                 return -EXDEV;
> > 
> > 
> > loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,
> >                             struct file *file_out, loff_t pos_out,
> >                             loff_t len, unsigned int remap_flags)
> > {
> >         loff_t ret;
> > 
> >         file_start_write(file_out);
> >         ret = do_clone_file_range(file_in, pos_in, file_out, pos_out, len,
> >                                   remap_flags);
> > 
> > And even if we get past here, I imagine XFS would freak out because it can't
> > find the extents (unless you're getting lucky and everything is lining up?).
> > I'm super confused...
> 
> Bah, I see what you mean.  Maybe there's something wrong with my setup.
> I'll try some more stuff and report back....

Sorry for the noise, you're right, generic/373 is just being dumb.

I assumed it was mounting different exports for some reason.  But in
fact it's just doing a bind mount and then "cp --reflink=always" between
two mounts of the same filesystem.  Previously that got rejected out of
hand, now the client sends a CLONE and the server handles it.

Which is an improvement.  So it's only generic/373 that needs fixing.

--b.