Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zfs_clone_range should return a more descriptive return value #15148

Merged
merged 1 commit into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion module/os/freebsd/zfs/zfs_vnops_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -6290,7 +6290,7 @@ zfs_freebsd_copy_file_range(struct vop_copy_file_range_args *ap)

error = zfs_clone_range(VTOZ(invp), ap->a_inoffp, VTOZ(outvp),
ap->a_outoffp, &len, ap->a_outcred);
if (error == EXDEV || error == EOPNOTSUPP)
if (error == EXDEV || error == EINVAL || error == EOPNOTSUPP)
goto bad_locked_fallback;
*ap->a_lenp = (size_t)len;
out_locked:
Expand Down
4 changes: 2 additions & 2 deletions module/os/linux/zfs/zpl_file_range.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,15 @@ zpl_copy_file_range(struct file *src_file, loff_t src_off,
* Since Linux 5.3 the filesystem driver is responsible for executing
* an appropriate fallback, and a generic fallback function is provided.
*/
if (ret == -EOPNOTSUPP || ret == -EXDEV)
if (ret == -EOPNOTSUPP || ret == -EINVAL || ret == -EXDEV)
ret = generic_copy_file_range(src_file, src_off, dst_file,
dst_off, len, flags);
#else
/*
* Before Linux 5.3 the filesystem has to return -EOPNOTSUPP to signal
* to the kernel that it should fallback to a content copy.
*/
if (ret == -EXDEV)
if (ret == -EINVAL || ret == -EXDEV)
ret = -EOPNOTSUPP;
#endif /* HAVE_VFS_GENERIC_COPY_FILE_RANGE */

Expand Down
6 changes: 3 additions & 3 deletions module/zfs/brt.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
* size_t len, unsigned int flags);
*
* Even though offsets and length represent bytes, they have to be
* block-aligned or we will return the EXDEV error so the upper layer can
* block-aligned or we will return an error so the upper layer can
* fallback to the generic mechanism that will just copy the data.
* Using copy_file_range(2) will call OS-independent zfs_clone_range() function.
* This function was implemented based on zfs_write(), but instead of writing
Expand All @@ -192,9 +192,9 @@
* Some special cases to consider and how we address them:
* - The block we want to clone may have been created within the same
* transaction group that we are trying to clone. Such block has no BP
* allocated yet, so cannot be immediately cloned. We return EXDEV.
* allocated yet, so cannot be immediately cloned. We return EAGAIN.
* - The block we want to clone may have been modified within the same
* transaction group. We return EXDEV.
* transaction group. We return EAGAIN.
* - A block may be cloned multiple times during one transaction group (that's
* why pending list is actually a tree and not an append-only list - this
* way we can figure out faster if this block is cloned for the first time
Expand Down
13 changes: 7 additions & 6 deletions module/zfs/zfs_vnops.c
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,10 @@ zfs_exit_two(zfsvfs_t *zfsvfs1, zfsvfs_t *zfsvfs2, const char *tag)
*
* On success, the function return the number of bytes copied in *lenp.
* Note, it doesn't return how much bytes are left to be copied.
* On errors which are caused by any file system limitations or
* brt limitations `EINVAL` is returned. In the most cases a user
* requested bad parameters, it could be possible to clone the file but
* some parameters don't match the requirements.
*/
int
zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
Expand Down Expand Up @@ -1171,23 +1175,23 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
* We cannot clone into files with different block size.
*/
if (inblksz != outzp->z_blksz && outzp->z_size > inblksz) {
error = SET_ERROR(EXDEV);
error = SET_ERROR(EINVAL);
goto unlock;
}

/*
* Offsets and len must be at block boundries.
*/
if ((inoff % inblksz) != 0 || (outoff % inblksz) != 0) {
error = SET_ERROR(EXDEV);
error = SET_ERROR(EINVAL);
goto unlock;
}
/*
* Length must be multipe of blksz, except for the end of the file.
*/
if ((len % inblksz) != 0 &&
(len < inzp->z_size - inoff || len < outzp->z_size - outoff)) {
error = SET_ERROR(EXDEV);
error = SET_ERROR(EINVAL);
goto unlock;
}

Expand Down Expand Up @@ -1246,9 +1250,6 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
* in the current transaction group. Return an error,
* so the caller can fallback to just copying the data.
*/
if (error == EAGAIN) {
error = SET_ERROR(EXDEV);
}
break;
}
/*
Expand Down