From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34583) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XowKn-0007Iw-DO for qemu-devel@nongnu.org; Thu, 13 Nov 2014 10:29:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XowKf-00048W-Mn for qemu-devel@nongnu.org; Thu, 13 Nov 2014 10:29:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43874) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XowKf-00048S-EG for qemu-devel@nongnu.org; Thu, 13 Nov 2014 10:29:13 -0500 Message-ID: <5464CE42.4000809@redhat.com> Date: Thu, 13 Nov 2014 08:29:06 -0700 From: Eric Blake MIME-Version: 1.0 References: <1415873823-13844-1-git-send-email-armbru@redhat.com> <1415873823-13844-4-git-send-email-armbru@redhat.com> <20141113130327.GD3933@noname.redhat.com> <5464C59A.4000602@redhat.com> In-Reply-To: <5464C59A.4000602@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="UjOKx8AEnPXPrDsGaPHqo1PoI5imE1URj" Subject: Re: [Qemu-devel] [PATCH v2 3/4] raw-posix: Fix try_seek_hole()'s handling of SEEK_DATA failure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , Markus Armbruster Cc: famz@redhat.com, tony@bakeyournoodle.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, mreitz@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --UjOKx8AEnPXPrDsGaPHqo1PoI5imE1URj Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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= =2E >>> 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. >=20 >=20 >>> +++ 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 =3D lseek(s->fd, start, SEEK_DATA); >>> - if (*data =3D=3D -1) { >>> - *data =3D lseek(s->fd, 0, SEEK_END); >>> + if (*data < 0) { >>> + /* no idea where the hole ends, give up (unlikely to hap= pen) */ >> >> 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. >=20 > Here's the (proposed) POSIX wording: >=20 > http://austingroupbugs.net/view.php?id=3D415 >=20 > And ENXIO is indeed the expected error for SEEK_DATA on a trailing hole= , > so maybe we should special case it. >=20 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...) --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --UjOKx8AEnPXPrDsGaPHqo1PoI5imE1URj 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 iQEcBAEBCAAGBQJUZM5CAAoJEKeha0olJ0Nqpl0IAKTzKhKloqb5xW0Df3bKfV3J tuKeMUXLrl2FXsoo1KipStAbJsIn7X1uxw//RVm1dJ/T2nR2zdgVwmfkERncKAD7 OEl1VOqlgmIdzLZwl0OJMMPqXWKraGB7FLIYJoes5gyV59VyR5ir4wDsdyuuJWlb OfW1OWdZKqA6NwXJzsZTMLdrcNwZDYULPEaMA8lUTyiE7o6xczR6zAiEU1fHSCXJ JSKIhQgMUdjrJe8yt/868olJGdy9nArngymabkNuYLKvPyD2AVO54PFXcyD59zTy qYmNeurrM7YlE56tS0nc3lGQvtyLJcEZU0quNbV/TR3Vz1TmtEohMiMyMwH4HZI= =BNia -----END PGP SIGNATURE----- --UjOKx8AEnPXPrDsGaPHqo1PoI5imE1URj--