btrfs_clone_files and bind mounts

btrfs_clone_files and bind mounts

[Hristo Venev]

[-- Attachment #1: Type: text/plain, Size: 93 bytes --]

What is the problem with cloning files between different (vfs)mounts of
the same filesystem?

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: btrfs_clone_files and bind mounts

[Nikolay Borisov]

On 20.02.2018 17:50, Hristo Venev wrote:
> What is the problem with cloning files between different (vfs)mounts of
> the same filesystem?
> 

The "problem" is not really a problem, but rather a well-imposed
restriction:

>From  http://man7.org/linux/man-pages/man2/ioctl_ficlonerange.2.html

"Both files must reside within the same filesystem."

And as a matter of fact this is enforced in the generic
vfs_clone_file_range. Of course if we were to do this across filesystem
then we'd have all the problems associated with not being able to ensure
atomicity of operations.

Re: btrfs_clone_files and bind mounts

[Hristo Venev]

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

On Tue, 2018-02-20 at 18:41 +0200, Nikolay Borisov wrote:
> 
> On 20.02.2018 17:50, Hristo Venev wrote:
> > What is the problem with cloning files between different
> > (vfs)mounts of
> > the same filesystem?
> > 
> 
> The "problem" is not really a problem, but rather a well-imposed
> restriction:
> 
> From  http://man7.org/linux/man-pages/man2/ioctl_ficlonerange.2.html
> 
> "Both files must reside within the same filesystem."
> 
> And as a matter of fact this is enforced in the generic
> vfs_clone_file_range. Of course if we were to do this across
> filesystem
> then we'd have all the problems associated with not being able to
> ensure
> atomicity of operations.

My question was about doing this within the same filesystem. In my case
(and I think it's relatively common), subvolumes of the same filesystem
are mounted separately, and I can't think of a good reason why
FICLONERANGE can't be made to work (it works if the subvolumes are
accessed within the same mount point).

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: btrfs_clone_files and bind mounts

[Nikolay Borisov]

[CCing Amir since he touched precisely the code responsible for this
semantics]

On 26.02.2018 14:01, Hristo Venev wrote:
> On Tue, 2018-02-20 at 18:41 +0200, Nikolay Borisov wrote:
>>
>> On 20.02.2018 17:50, Hristo Venev wrote:
>>> What is the problem with cloning files between different
>>> (vfs)mounts of
>>> the same filesystem?
>>>
>>
>> The "problem" is not really a problem, but rather a well-imposed
>> restriction:
>>
>> From  http://man7.org/linux/man-pages/man2/ioctl_ficlonerange.2.html
>>
>> "Both files must reside within the same filesystem."
>>
>> And as a matter of fact this is enforced in the generic
>> vfs_clone_file_range. Of course if we were to do this across
>> filesystem
>> then we'd have all the problems associated with not being able to
>> ensure
>> atomicity of operations.
> 
> My question was about doing this within the same filesystem. In my case
> (and I think it's relatively common), subvolumes of the same filesystem
> are mounted separately, and I can't think of a good reason why
> FICLONERANGE can't be made to work (it works if the subvolumes are
> accessed within the same mount point).
> 

So Amir,

In 913b86e92e1f ("vfs: allow vfs_clone_file_range() across mount
points") you did relax the requirements of vfs_clone_file_range by
moving the mount point check into ioctl_file_clone. In btrfs we can have
a situation where multiple subvolumes, belonging to the same FS are at
different mountpoint I.e. :

mount /dev/vdc /media/scratch/
echo "test file" >> /media/scratch/foo

btrfs subvolume create /media/scratch/subvol1

btrfs subvolume list /media/scratch/
   ID 257 gen 7 top level 5 path subvol1

mount -osubvolid=257 /dev/vdc /media/subvol-mount/

cp --reflink=always /media/scratch/foo /media/subvol-mount/foo-reflink

cp: failed to clone '/media/subvol-mount/foo-reflink' from
'/media/scratch/foo': Invalid cross-device link


Shouldn't reflinking be allowed in this case? ftrace shows that we are
failing "if (src_file.file->f_path.mnt != dst_file->f_path.mnt)" in
ioctl_file_clone


In your commit you state:

FICLONE/FICLONERANGE ioctls enforce that src and dest files are on
 the same mount.

Whereas the man page says they should be on the same filesystem (not
necessarily on the same mount?)

Re: btrfs_clone_files and bind mounts

[Amir Goldstein]

On Mon, Feb 26, 2018 at 3:01 PM, Nikolay Borisov <nborisov@suse.com> wrote:
> [CCing Amir since he touched precisely the code responsible for this
> semantics]

[CCing fsdevel and NFS list since there is already a long standing attempt
 to relax the copy_file_range() API for cross server copy]

>
> On 26.02.2018 14:01, Hristo Venev wrote:
>> On Tue, 2018-02-20 at 18:41 +0200, Nikolay Borisov wrote:
>>>
>>> On 20.02.2018 17:50, Hristo Venev wrote:
>>>> What is the problem with cloning files between different
>>>> (vfs)mounts of
>>>> the same filesystem?
>>>>
>>>
>>> The "problem" is not really a problem, but rather a well-imposed
>>> restriction:
>>>
>>> From  http://man7.org/linux/man-pages/man2/ioctl_ficlonerange.2.html
>>>
>>> "Both files must reside within the same filesystem."
>>>
>>> And as a matter of fact this is enforced in the generic
>>> vfs_clone_file_range. Of course if we were to do this across
>>> filesystem
>>> then we'd have all the problems associated with not being able to
>>> ensure
>>> atomicity of operations.
>>
>> My question was about doing this within the same filesystem. In my case
>> (and I think it's relatively common), subvolumes of the same filesystem
>> are mounted separately, and I can't think of a good reason why
>> FICLONERANGE can't be made to work (it works if the subvolumes are
>> accessed within the same mount point).
>>
>
> So Amir,
>
> In 913b86e92e1f ("vfs: allow vfs_clone_file_range() across mount
> points") you did relax the requirements of vfs_clone_file_range by
> moving the mount point check into ioctl_file_clone.

[...]
>
>
> Shouldn't reflinking be allowed in this case? ftrace shows that we are
> failing "if (src_file.file->f_path.mnt != dst_file->f_path.mnt)" in
> ioctl_file_clone
>
>
> In your commit you state:
>
> FICLONE/FICLONERANGE ioctls enforce that src and dest files are on
>  the same mount.
>
> Whereas the man page says they should be on the same filesystem (not
> necessarily on the same mount?)

See, all I did was avoid the flames involved with changing application
behavior, because I had no reason to change it.
Not that the man page is what matters, its what applications expect that
matter, but the man page also says this open for interpretation phrase:
EXDEV  dest_fd and src_fd are not on the same *mounted* filesystem.

NFS folks have been trying to change the behavior of copy_file_range()
for over two years now, we little success as far as I can tell:
https://marc.info/?l=linux-fsdevel&m=149971144627690&w=2

Semi-interesting point - it looks like this commit has changed behavior
of clone file range on CIFS:
04b38d601239 vfs: pull btrfs clone API to vfs layer

I don't see that CIFS has the "same mount" restriction before the
generalized VFS API, but btrfs had the restriction in-place and it still exists
in the btrfs implementation btrfs_clone_files().

So I think you will have no way around this other than proposing a new
ioctl FICLONERANGE2 with flags argument to opt-in for the new required
behavior. I you do that please CC linux-api and expect the flames.

Thanks,
Amir.