Jim Meyering wrote:
> jeff.liu wrote:
> ...
>>>> + uint64_t ext_len = fm_ext[i].fe_length;
>>>> +
>>>> + if (lseek (src_fd, (off_t) ext_logical, SEEK_SET) < 0LL)
>>>> + {
>>>> + error (0, errno, _("cannot lseek %s"), quote (src_name));
>>>> + return_val = false;
>>>> + }
>>>> +
>>>> + if (lseek (dest_fd, (off_t) ext_logical, SEEK_SET) < 0LL)
>>>> + {
>>>> + error (0, errno, _("cannot lseek %s"), quote (dst_name));
>>>> + return_val = false;
>>>> + }
>>> If either lseek fails, then we must not proceed to read or write.
>>> Setting "return_val" is insufficient.
>>>
>>> This makes me think we'll have to redesign a little, to avoid
>>> the possibility of corruption.
>>>
>>> Consider this change:
>>>
>>> If the fiemap ioctl succeeds: use only this specialized code to
>>> perform the copy and do not fall back on normal copy. However, if
>>> any operation fails here (like these lseeks, read or write), we
>>> diagnose it as usual. Not falling back upon failure has the added
>>> benefit that there's no risk of a duplicate diagnostic if the same
>>> failure occurs again during the regular copy.
>>>
>>> Otherwise, imagine that lseek advances the source FD,
>>> but the destination lseek fails. Now the FDs are inconsistent.
>>> We would have to realign (or restore) them, or risk writing
>>> data at the wrong offset: corruption.
>> So, do you means if fiemap ioctl(2) succeeds, no matter what error(read,
>> write, lseek) occurred
>> afterwards, we should always diagnosing it and then abort with the
>> corresponding error msg?
>>
>> My original design really cause the file corruption if the lseek(2) already
>> advances the source FD
>> but the destination lseek(2) fails.
>>
>> But is it make sense to seek back to the start offset for both source and
>> destination FDs, then fall
>> back to the regular copy?
>
> Your changes should make cp handle these cases:
>
> ioctl succeeds and fiemap-copying succeeds
> nothing more to do for this pair of files
> ioctl succeeds and any aspect of fiemap-copying fails
> fail: do not try to copy any other way
> ioctl fails
> attempt to copy as usual
>
> In each case, continue with any remaining files.
Thanks for the detailed info!
-Jeff
