On 11/13/2014 07:52 AM, Eric Blake wrote:
> On 11/13/2014 06:03 AM, Kevin Wolf wrote:
>> Am 13.11.2014 um 11:17 hat Markus Armbruster geschrieben:
>>> When SEEK_HOLE tells us we're in a hole, we try SEEK_DATA to find its
>>> end. When that fails, we pretend the hole extends to the end of file.
>>> Wrong.
>>
>> Wrong only in some cases, see below.
>>
>>> Except when SEEK_END fails, we screw up and claim it extends
>>> to offset -1. More wrong.
>
>
>>> +++ b/block/raw-posix.c
>>> @@ -1494,8 +1494,9 @@ static int try_seek_hole(BlockDriverState *bs, off_t
>>> start, off_t *data,
>>> } else {
>>> /* On a hole. We need another syscall to find its end. */
>>> *data = lseek(s->fd, start, SEEK_DATA);
>>> - if (*data == -1) {
>>> - *data = lseek(s->fd, 0, SEEK_END);
>>> + if (*data < 0) {
>>> + /* no idea where the hole ends, give up (unlikely to happen) */
>>
>> Not quite unlikely. If the file ends with a sparse area, we'll get
>> -1/ENXIO here.
>>
>> lseek() with SEEK_DATA starting in a hole when there is no data until
>> EOF is actually the part that isn't documented in the man page, but
>> ENXIO is what I'm seeing here on RHEL 7.
>
> Here's the (proposed) POSIX wording:
>
> http://austingroupbugs.net/view.php?id=415
>
> And ENXIO is indeed the expected error for SEEK_DATA on a trailing hole,
> so maybe we should special case it.
> Uggh. Historical practice on Solaris (and therefore the POSIX wording) says that SEEK_HOLE in a trailing hole is allowed (but not required) to seek to EOF instead of reporting the offset requested. I have no clue why this was done, but it is VERY annoying - it means that if you provide an offset within a tail hole of a file, you cannot reliably tell if the file ends in a hole or with data, without ALSO trying SEEK_DATA. For applications that are reading a file sequentially but skipping over holes, this behavior is fine (it short-circuits the hole/data search points and might shave an iteration off a lop). But for OUR purposes, where we are merely trying to ascertain whether we are in a hole, we have an inaccurate response - since SEEK_HOLE does NOT return the offset we passed in, we are prone to treat the offset as belonging to data, which is a pessimization (you never get wrong results by treating a hole as data and reading it, but it is definitely slower). I think you HAVE to call lseek() twice, both with SEEK_HOLE and with SEEK_DATA, if you want to accurately determine whether an offset happens to live within a trailing hole. (By the way, I really wish Solaris had implemented a variant that queried, but did NOT change the file offset - maybe Linux can add that as an extension, and give it sane semantics of not special casing trailing holes...) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
