[copyRangeMethod] invalid cross-device link

[da2x]

Syncthing version 1.19.1 on Fedora Linux 35. Any recent Linux kernel should behave the same.

How to reproduce

1. You need two disk partitions or separate disks. Both need to be formatted as either XFS or BtrFS. Cloned extents must be enabled (default for disks formatted in the last four years or so.)
2. Create a folder on each disk device.
3. Add both folders to Syncthing.
4. Open advanced settings for both folders and set copyRangeMethod to ioctl.
5. Share the folders with another device.
6. Drop a file into one of the folders on the other device.
7. Wait for it to sync.
8. Drop the exact same file into the other folder on the other device.

Actual

Syncthing will fail to write the file to the second directory. It tries to share the extents between devices and fail with syncing: dst write: invalid cross-device link. This is the expected result for sharing extends between devices. However, it’s not the expected results for Syncthing!

Expected

Syncthing should indeed try to reuse the extents. However, it should also detect the error and handle it by duplication/copying the bytes instead of reusing extents. (So, the expected behavior is for Syncthing to behave like cp --reflink=auto (best effort or fallback to copy) whereas Syncthing currently behaves more like cp --reflink=always which fails in the same way in this situation.

[AudriusButkevicius]

Sounds like a Henny Youngman problem to me.

It's not the default setting specifically for this reason, and the user is expected to configure it as needed. You can use "all" if you want fallback behaviours.

[imsodin]

Would it make sense to always fall back to a "normal" copy? One might want to use ioctl and not all the others, as they don't apply for all the scenarios where it works (there may be 2 folders on the same device) - however syncthing should still work if there happens to be a copy between devices. Any downsides?

[AudriusButkevicius]

Not sure what you mean it does not apply.

It tries all of them, so something must stick, the other cases are for explicitly "use this and only this".

If you want something different then it should be a different mode.

[imsodin]

What I meant is the scenario where you know that e.g. only ioctl might work, but none of the others ever do. Then I'd see why you'd expect Syncthing to try ioctl, and if it doesn't work try standard. Basically all the CopyRangeMethods would then be ...WithFallback. Or put differently: What's the point/use of the "and only this" behaviour - testing maybe?

[AudriusButkevicius]

The point of only this behaviour is when you want to make sure we're not creating needless copies of the same data. Having a fallback prevents that.

Whats the downside calling methods that are not supported and fail immediately?

[imsodin]

Whats the downside calling methods that are not supported and fail immediately?

Probably none. I just believe that no fallback isn't the obvious/intuitive behaviour, and I can see a user e.g. knowing that the ioctl one works, just picking that instead of all (because safer (no unknown methods potentially run) and possibly performant (probably not, but I could imagine the thought coming up)). Anyway, I propose adding a section to the docs to point out this behaviour: #8271

[AudriusButkevicius]

Its the always vs auto behaviour, where auto is "all" in our case, and always is ioctl/whatever mode.

[calmh]

I think the bug here, if there is a bug, is that the documentation mentions all last and it's unlikely to get chosen by the user (while it is usually the right choice). Maybe it should be called efficient and be the the first choice mentioned after standard.

[da2x]

I use ioctl because copy_file_range causes the kernel to freeze the system in certain conditions. I’ve never had any problems with the ioctl calls.

Can I set the config option to ioctl, standard to get the desired fallback behavior? It’s unclear from the documentation.

[imsodin]

You can't do that currently.

Should we add a copyRangeWithFallback boolean parameter, which when true takes the chosen method and combines it with standard?

[AudriusButkevicius]

We already have an enum for modes, if we want more modes, we should just add more enums.