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
