[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block/raw-posix: Fall back to SEEK_HOLE/SEEK_DA
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] block/raw-posix: Fall back to SEEK_HOLE/SEEK_DATA |
Date: |
Tue, 06 May 2014 06:27:04 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 05/06/2014 05:49 AM, Stefan Hajnoczi wrote:
> On Mon, May 05, 2014 at 10:01:39PM +0200, Max Reitz wrote:
>> The current version of raw-posix always uses ioctl(FS_IOC_FIEMAP) if
>> FIEMAP is available; lseek with SEEK_HOLE/SEEK_DATA are not even
>> compiled in in this case. However, there may be implementations which
>> support the latter but not the former (e.g., NFSv4.2). In this case,
>> raw-posix should fall back to lseek with SEEK_HOLE/SEEK_DATA if FIEMAP
>> does not work.
>>
>
> A bigger cleanup is extracting the FIEMAP and SEEK_HOLE/SEEK_DATA
> implementations into their own static functions. Then
> raw_co_get_block_status() becomes simpler and doesn't need ifdefs:
>
> ret = try_fiemap(...);
> if (ret < 0) {
> ret = try_seekhole(...);
> }
> if (ret < 0) {
> ...report every block allocated by default....
> }
>
> In other words, let normal C control flow describe the relationships
> between these code paths. Use ifdef only to nop out try_fiemap() and
> try_seekhole().
>
> What do you think?
I like the idea - separating control flow from #ifdefs (by having stubs
on the other end of the ifdef) definitely makes algorithms easier to
understand.
More things to consider: GNU Coreutils has support for both fiemap and
seek_hole, but favors seek_hole first, for a couple reasons. First,
FIEMAP has not always been reliable: on some older kernel/filesystem
pairs, fiemap could return stale results, which led cp(1) to cause data
loss unless it did an fsync() first to get the fiemap to be stable - but
the cost of the fsync() made the operation slower than if fiemap were
never used. Second, POSIX will be standardizing seek_hole in its next
revision [1] (still several years out, but the fact that it is an
announced intention means people are starting to implement it now).
fiemap, on the other hand, remains a Linux-only extension. Yes, fiemap
provides more details than seek_hole (and is the ONLY way to know the
difference between a hole that has reserved space on the disk vs a hole
that will require allocation if is written to), but if all you need to
know is whether a hole exists (rather than what type of hole), then
seek_hole is MUCH simpler.
[1] http://austingroupbugs.net/view.php?id=415
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature