qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of F


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP
Date: Thu, 13 Nov 2014 09:26:01 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Fam Zheng <address@hidden> writes:

> On Wed, 11/12 20:27, Markus Armbruster wrote:
>> Commit 5500316 (May 2012) implemented raw_co_is_allocated() as
>> follows:
>> 
>> 1. If defined(CONFIG_FIEMAP), use the FS_IOC_FIEMAP ioctl
>> 
>> 2. Else if defined(SEEK_HOLE) && defined(SEEK_DATA), use lseek()
>> 
>> 3. Else pretend there are no holes
>> 
>> Later on, raw_co_is_allocated() was generalized to
>> raw_co_get_block_status().
>> 
>> Commit 4f11aa8 (May 2014) changed it to try the three methods in order
>> until success, because "there may be implementations which support
>> [SEEK_HOLE/SEEK_DATA] but not [FIEMAP] (e.g., NFSv4.2) as well as vice
>> versa."
>> 
>> Unfortunately, we used FIEMAP incorrectly: we lacked FIEMAP_FLAG_SYNC.
>> Commit 38c4d0a (Sep 2014) added it.  Because that's a significant
>> speed hit, the next commit 38c4d0a put SEEK_HOLE/SEEK_DATA first.
>
> s/38c4d0a/7c159037/

Fixing...

>> As you see, the obvious use of FIEMAP is wrong, and the correct use is
>> slow.  I guess this puts it somewhere between -7 "The obvious use is
>> wrong" and -10 "It's impossible to get right" on Rusty Russel's Hard
>> to Misuse scale[*].
>
> Nice reading :)

Adapted from a comment Paolo made in a discussion preceding this patch
:)

[...]
> Other than the wrong commit id in message,
>
> Reviewed-by: Fam Zheng <address@hidden>

Thanks!



reply via email to

[Prev in Thread] Current Thread [Next in Thread]