From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57369) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WheSk-0006tN-Sa for qemu-devel@nongnu.org; Tue, 06 May 2014 08:27:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WheSg-0000Ym-CL for qemu-devel@nongnu.org; Tue, 06 May 2014 08:27:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11862) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WheSg-0000XL-3v for qemu-devel@nongnu.org; Tue, 06 May 2014 08:27:06 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s46CR5rC015217 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 6 May 2014 08:27:05 -0400 Message-ID: <5368D518.9000007@redhat.com> Date: Tue, 06 May 2014 06:27:04 -0600 From: Eric Blake MIME-Version: 1.0 References: <1399320099-32457-1-git-send-email-mreitz@redhat.com> <20140506114958.GE15810@stefanha-thinkpad.redhat.com> In-Reply-To: <20140506114958.GE15810@stefanha-thinkpad.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="poT16oQSTu07XKll1Ewu5nwd8m93qsHkC" Subject: Re: [Qemu-devel] [PATCH] block/raw-posix: Fall back to SEEK_HOLE/SEEK_DATA List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , Max Reitz Cc: Kevin Wolf , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --poT16oQSTu07XKll1Ewu5nwd8m93qsHkC Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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. >> >=20 > 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: >=20 > ret =3D try_fiemap(...); > if (ret < 0) { > ret =3D try_seekhole(...); > } > if (ret < 0) { > ...report every block allocated by default.... > } >=20 > 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(). >=20 > 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=3D415 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --poT16oQSTu07XKll1Ewu5nwd8m93qsHkC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTaNUYAAoJEKeha0olJ0NqiFwH/iXMto3rpdi2Sw+R3sIVOd+3 D03CFwKOTH3yD9Axo7VimJe+beI5I6Vo6oN/SLBDxZmCureSOfS7jpjIgo2K7cNL bKeQELBv5rtGAKdVVspwCKE8h8CrgOLT6edEyLtzdgzu4IqIALU+HNC5ppvfnLtN oKI93+CYRfGGmc9SqySpxrb6N6IgWI/eSkOzmwylQbUXJIApmm5yqlgYeQzSSIgL HZjR0LkHia/pVZetztr1CNv6JfklVHcNYROohUADKsF8e/MIPOSQUAo4FejC4+3a EQbrUC6hKfEj8cC3N0a4oL9LgSqjVzimSskU43f7XkmKQ7YY48AXfQgKDqbOpXQ= =Y7eC -----END PGP SIGNATURE----- --poT16oQSTu07XKll1Ewu5nwd8m93qsHkC--