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.
