If Copy-on-Write is enabled in a btrfs/xfs mount then create a clone otherwise
fallback to standard copy.
Details
Details
- Reviewers
dfaure thiago ossi - Group Reviewers
Frameworks
Manually copied files and verified with "btrfs filesystem du" command.
All existing unit tests pass.
Diff Detail
Diff Detail
- Repository
- R241 KIO
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 6731 Build 6749: arc lint + arc unit
Comment Actions
I think it would be much simpler if you just tried to do the the FICLONE iotctl in the job, without any prior checking:
- no possibility for races
- less syscals
- less code
| src/core/copyjob.cpp | ||
|---|---|---|
| 742 ↗ | (On Diff #48014) | it is *much* cheaper to compare the st_dev fields from stat for the source file and the destination directory. |
| src/core/kmountpoint.cpp | ||
| 471 ↗ | (On Diff #48014) | This is not correct, xfs may support reflinks, but it is a feature only recently added, and has to be enabled at file system creation time. |
| src/core/kmountpoint.h | ||
| 145 ↗ | (On Diff #48014) | The correct term here is "reflink", not CoW. |
Comment Actions
I am not sure I followed you here. To me ioctl seems to naturally fit in FileProtocol::copy(). Can I ask you to elaborate a bit?
| src/core/copyjob.cpp | ||
|---|---|---|
| 742 ↗ | (On Diff #48014) | I agree but in case of KIO::copy() the destination can be a non-existent file resulting in lstat to fail. However, I did considered using st_dev for source but couldn't find a way to map it to a block device. Maybe we can use st_dev by default and fallback to KMountPoint upon encountering this particular case? |
| src/core/kmountpoint.cpp | ||
| 471 ↗ | (On Diff #48014) | Here by 'support' I meant support in its implementation. Meaning aside, unlike btrfs' "nodatacow" option which we can get using KMountPoint I couldn't find any API to get the "reflink" option set during file system creation time. How do you suggest I proceed from here? |
| src/core/kmountpoint.h | ||
| 145 ↗ | (On Diff #48014) | Some documents I read on btrfs and xfs described copy-on-write as a 'feature'. So I felt the enum should contain 'Supports' followed by the feature name. |
Comment Actions
I meant scrap everything but the changes from file_unix.cpp.
Worst case - the ioctl fails (for whatever reason). That is one additional ioctl/syscall. The probing code using KMountPoint::List::findByPath internally creates a QFileInfo, thus does a stat (syscall) as well.
Best case - the ioctl succeeds. You save the syscall required for the probing.
Comment Actions
Can you update the summary - although CoW and cloning typically come together, they are two different functions (and you can e.g. have CoW disabled for a dir/file, and cloning still works).
Comment Actions
No objection from me, but I don't know those clone APIs, so I can't really review/approve it.
Maybe Thiago or Oswald, if nobody else does.
| src/ioslaves/file/file_unix.cpp | ||
|---|---|---|
| 263 | nitpick - wrong indentation | |
Comment Actions
yes, and an api that is utterly unsuitable for interactive applications. ;)
if you just meant it as a source to steal from, fair enough.