zfs_clone_range should return a descriptive error codes

Return the more descriptive error codes instead of `EXDEV` when
the parameters don't match the requirements of the clone function.
Updated the comments in `brt.c` accordingly.
The first three errors are just invalid parameters, which zfs can
not handle.
The fourth error indicates that the block which should be cloned
is created and cloned or modified in the same transaction
group (`txg`).

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Rob Norris <rob.norris@klarasystems.com>
Signed-off-by: Kay Pedersen <mail@mkwg.de>
Closes #15148
This commit is contained in:
oromenahar 2023-08-08 18:37:06 +02:00 committed by GitHub
parent 683edb32b7
commit 019dea0a55
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 13 additions and 12 deletions

View File

@ -6288,7 +6288,7 @@ zfs_freebsd_copy_file_range(struct vop_copy_file_range_args *ap)
error = zfs_clone_range(VTOZ(invp), ap->a_inoffp, VTOZ(outvp), error = zfs_clone_range(VTOZ(invp), ap->a_inoffp, VTOZ(outvp),
ap->a_outoffp, &len, ap->a_outcred); ap->a_outoffp, &len, ap->a_outcred);
if (error == EXDEV || error == EOPNOTSUPP) if (error == EXDEV || error == EINVAL || error == EOPNOTSUPP)
goto bad_locked_fallback; goto bad_locked_fallback;
*ap->a_lenp = (size_t)len; *ap->a_lenp = (size_t)len;
out_locked: out_locked:

View File

@ -103,7 +103,7 @@ zpl_copy_file_range(struct file *src_file, loff_t src_off,
* Since Linux 5.3 the filesystem driver is responsible for executing * Since Linux 5.3 the filesystem driver is responsible for executing
* an appropriate fallback, and a generic fallback function is provided. * 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, ret = generic_copy_file_range(src_file, src_off, dst_file,
dst_off, len, flags); dst_off, len, flags);
#else #else
@ -111,7 +111,7 @@ zpl_copy_file_range(struct file *src_file, loff_t src_off,
* Before Linux 5.3 the filesystem has to return -EOPNOTSUPP to signal * Before Linux 5.3 the filesystem has to return -EOPNOTSUPP to signal
* to the kernel that it should fallback to a content copy. * to the kernel that it should fallback to a content copy.
*/ */
if (ret == -EXDEV) if (ret == -EINVAL || ret == -EXDEV)
ret = -EOPNOTSUPP; ret = -EOPNOTSUPP;
#endif /* HAVE_VFS_GENERIC_COPY_FILE_RANGE */ #endif /* HAVE_VFS_GENERIC_COPY_FILE_RANGE */

View File

@ -174,7 +174,7 @@
* size_t len, unsigned int flags); * size_t len, unsigned int flags);
* *
* Even though offsets and length represent bytes, they have to be * 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. * fallback to the generic mechanism that will just copy the data.
* Using copy_file_range(2) will call OS-independent zfs_clone_range() function. * 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 * This function was implemented based on zfs_write(), but instead of writing
@ -192,9 +192,9 @@
* Some special cases to consider and how we address them: * Some special cases to consider and how we address them:
* - The block we want to clone may have been created within the same * - 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 * 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 * - 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 * - 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 * 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 * way we can figure out faster if this block is cloned for the first time

View File

@ -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. * 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. * 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 int
zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp, zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
@ -1171,7 +1175,7 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
* We cannot clone into files with different block size. * We cannot clone into files with different block size.
*/ */
if (inblksz != outzp->z_blksz && outzp->z_size > inblksz) { if (inblksz != outzp->z_blksz && outzp->z_size > inblksz) {
error = SET_ERROR(EXDEV); error = SET_ERROR(EINVAL);
goto unlock; goto unlock;
} }
@ -1179,7 +1183,7 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
* Offsets and len must be at block boundries. * Offsets and len must be at block boundries.
*/ */
if ((inoff % inblksz) != 0 || (outoff % inblksz) != 0) { if ((inoff % inblksz) != 0 || (outoff % inblksz) != 0) {
error = SET_ERROR(EXDEV); error = SET_ERROR(EINVAL);
goto unlock; goto unlock;
} }
/* /*
@ -1187,7 +1191,7 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
*/ */
if ((len % inblksz) != 0 && if ((len % inblksz) != 0 &&
(len < inzp->z_size - inoff || len < outzp->z_size - outoff)) { (len < inzp->z_size - inoff || len < outzp->z_size - outoff)) {
error = SET_ERROR(EXDEV); error = SET_ERROR(EINVAL);
goto unlock; goto unlock;
} }
@ -1246,9 +1250,6 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
* in the current transaction group. Return an error, * in the current transaction group. Return an error,
* so the caller can fallback to just copying the data. * so the caller can fallback to just copying the data.
*/ */
if (error == EAGAIN) {
error = SET_ERROR(EXDEV);
}
break; break;
} }
/* /*