GNU bug report logs - #5915
[coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2

Previous Next

Full log

Message #23 received at 5915 <at> debbugs.gnu.org (full text, mbox):

From: "jeff.liu" <jeff.liu <at> oracle.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: Sunil Mushran <sunil.mushran <at> oracle.com>, 5915 <at> debbugs.gnu.org,
	Joel Becker <Joel.Becker <at> oracle.com>, Tao Ma <tao.ma <at> oracle.com>,
	Pádraig Brady <P <at> draigBrady.com>, chris.mason <at> oracle.com
Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1)
	and mv(1) v2
Date: Sat, 17 Apr 2010 23:10:22 +0800

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.