Re: [gentoo-portage-dev] [PATCH] movefile: support in-kernel file copying on Linux (bug 607868)

[Zac Medico]

On 03/02/2017 08:24 AM, Michał Górny wrote:
> W dniu 01.03.2017, śro o godzinie 19∶03 -0800, użytkownik Zac Medico
> napisał:
>> +def _test_optimized_copyfile():
>> +	"""
>> +	Test if _optimized_copyfile works. It will fail for Linux versions
>> +	from 2.6.0 to 2.6.32, because sendfile does not support writing
>> +	to regular files. It will also fail for Linux versions less than
>> +	3.1 if portage was compiled 3.1 or later, due to missing support
>> +	for lseek SEEK_DATA/SEEK_HOLE.
>
> I don't really like the idea of relying on one-time attempt to determine
> whether a complex mechanism lacking proper fallback will work. But I
> guess you're only aiming at catching corner cases here.

In v2, there's a complete fallback mechanism in the native code, and the
_test_optimized_copyfile function has been eliminated.

>> +            /* Revert SEEK_HOLE offset change, since we're going
>> +             * to copy the data that comes before the hole.
>> +             */
>> +            if (lseek(fd_in, offset_out, SEEK_SET) < 0) {
>> +                error = 1;
>> +                break;
>> +            }
>> +
>> +            copyfunc_ret = copyfunc(fd_out,
>> +                                    fd_in,
>> +                                    &offset_out,
>> +                                    offset_hole - offset_data);
>> +
>> +            if (copyfunc_ret < 0) {
>> +                if ((errno == EXDEV || errno == ENOSYS) &&
>> +                    copyfunc == cfr_wrapper) {
>> +                    /* Use sendfile instead of copy_file_range for
>> +                     * cross-device copies, or when the copy_file_range
>> +                     * syscall is not available (less than Linux 4.5).
>> +                     */
>> +                    copyfunc = sendfile;
>> +                    copyfunc_ret = copyfunc(fd_out,
>> +                                            fd_in,
>> +                                            &offset_out,
>> +                                            offset_hole - offset_data);
>> +
>> +                    if (copyfunc_ret < 0) {
>
> I think you still should have a proper fallback for sendfile() refusing
> to do its job.

In the lseek loop, sendfile should always work if lseek SEEK_DATA works,
so there should be no need to fallback for sendfile failure here. In v2,
I've added a comment about this.

>> +    while (eintr_retry) {
>> +
>> +        error = 0;
>> +
>> +        Py_BEGIN_ALLOW_THREADS
>> +
>> +        if (!stat_acquired && fstat(fd_in, &sb) < 0) {
>> +            error = 1;
>> +        }
>> +        else {
>> +            stat_acquired = 1;
>> +            while (offset_out < sb.st_size) {
>> +                copyfunc_ret = sendfile(fd_out,
>> +                                        fd_in,
>> +                                        &offset_out,
>> +                                        sb.st_size - offset_out);
>> +
>> +                if (copyfunc_ret < 0) {
>
> Likewise, especially that old kernels may refuse file-to-file copy here.

In v2, it will fallback to a plain read/write loop.
-- 
Thanks,
Zac