Re: [PATCH 6/8] attempt copy_file_range if explicitly requested

[David Disseldorp]

On Thu, 18 Mar 2021 14:58:35 +0000, Luis Henriques wrote:

> On Fri, Mar 12, 2021 at 02:30:17AM +0100, David Disseldorp wrote:
...
> > +#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?)

Oops, will fix...

> > +                                     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;

Good catch, will add -ENODATA fallback alongside -EXDEV.

Cheers, David