On Fri, Mar 12, 2021 at 02:30:17AM +0100, David Disseldorp wrote:
> This change sees --reflink behave similar to coreutils cp
> --reflink=auto. The kernel may attempt a copy-on-write clone of the
> range and may fallback to splice. cpio fallback to regular read/write
> is needed in case the source and destination reside on separate
> filesystems.
>
> Signed-off-by: David Disseldorp <[email protected]>
> ---
> src/copyin.c | 2 +-
> src/copyout.c | 33 ++++++++++++++++++++++++++++++---
> 2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/src/copyin.c b/src/copyin.c
> index c8ea278..f6d7292 100644
> --- a/src/copyin.c
> +++ b/src/copyin.c
> @@ -510,7 +510,7 @@ copyin_regular_file (struct cpio_file_stat* file_hdr, int
> in_file_des)
> }
> copy_files_tape_to_disk (in_file_des, out_file_des, file_hdr->c_filesize);
> disk_empty_output_buffer (out_file_des, true);
> -
> +
> if (to_stdout_option)
> {
> if (archive_format == arf_crcascii)
> diff --git a/src/copyout.c b/src/copyout.c
> index 5ca587f..076478e 100644
> --- a/src/copyout.c
> +++ b/src/copyout.c
> @@ -718,9 +718,36 @@ process_copy_out ()
>
> if (write_out_header (&file_hdr, out_file_des))
> continue;
> - copy_files_disk_to_tape (in_file_des,
> - out_file_des, file_hdr.c_filesize,
> - orig_file_name);
> +#ifdef HAVE_CPIO_REFLINK
> + /*
> + * copy_file_range(2) should only be used if:
> + * - it has explicitly been requested via --reflink
> + * - crc calculations for aren't needed
> + * - output is a regular seekable file
> + */
> + if (reflink_flag && !crc_i_flag && output_is_seekable)
> + {
> + ssize_t ret;
> + /* flush output buffer before copy_file_range I/O */
> + tape_empty_output_buffer (out_file_des);
> + ret = copy_files_range (in_file_des,
(nit: spaces/tabs inconsistent here?)
> + out_file_des,
> + file_hdr.c_filesize);
> + /* -EXDEV: can't use copy_file_range, fallback to read/write
> */
> + if (ret == -EXDEV)
> + copy_files_disk_to_tape (in_file_des,
> + out_file_des, file_hdr.c_filesize,
> + orig_file_name);
> + else if (ret < 0)
> + error (PAXEXIT_FAILURE, -ret, _("copy_file_range failed."));
Maybe you could log the error but, instead of exiting, simply try the
copy_file_disk_to_tape() instead of doing so only if -EXDEV. This is
particularly relevant because in copy_files_range() you're doing:
...
ret = copy_file_range(...);
/* simplify read/write fallback: don't partially seek on error */
if (ret == 0)
return -ENODATA;
(Unfortunately, not doing the seek() on error will force the full
read+write instead of trying to continue if there was already some data
copied. But yeah it greatly simplifies things too.)
Cheers,
--
Luís
> + }
> + else
> +#endif /* HAVE_CPIO_REFLINK */
> + {
> + copy_files_disk_to_tape (in_file_des,
> + out_file_des, file_hdr.c_filesize,
> + orig_file_name);
> + }
> warn_if_file_changed(orig_file_name, file_hdr.c_filesize,
> file_hdr.c_mtime);
>
> --
> 2.26.2
>
