From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60892) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xoslb-0004rS-9L for qemu-devel@nongnu.org; Thu, 13 Nov 2014 06:40:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XoslT-00061s-TH for qemu-devel@nongnu.org; Thu, 13 Nov 2014 06:40:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34102) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XoslT-00061S-89 for qemu-devel@nongnu.org; Thu, 13 Nov 2014 06:40:39 -0500 Date: Thu, 13 Nov 2014 12:40:30 +0100 From: Kevin Wolf Message-ID: <20141113114030.GA3933@noname.redhat.com> References: <1415820422-17796-1-git-send-email-armbru@redhat.com> <1415820422-17796-3-git-send-email-armbru@redhat.com> <5463EC70.1030107@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="BOKacYhQ+x31HxR3" Content-Disposition: inline In-Reply-To: <5463EC70.1030107@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: tony@bakeyournoodle.com, qemu-devel@nongnu.org, Markus Armbruster , mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com --BOKacYhQ+x31HxR3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 13.11.2014 um 00:25 hat Eric Blake geschrieben: > On 11/12/2014 01:27 PM, Markus Armbruster wrote: > > + /* in hole, end not yet known */ > > + offs =3D lseek(s->fd, start, SEEK_DATA); > > + if (offs < 0) { > > + /* no idea where the hole ends, give up (unlikely to happen) */ > > + goto dunno; > > + } > > + assert(offs >=3D start); > > + *hole =3D start; > > + *data =3D offs; >=20 > This assertion feels like an off-by-one. The same offset cannot be both > a hole and data (except in some racy situation where some other process > is writing data to that offset in between our two lseek calls, but > that's already in no-man's land because no one else should be writing > the file while qemu has it open). Is it worth using 'assert(offs > > start)' instead? As soon as you say "except", it's wrong to assert this at all. We can't guarantee that the condition is true and it's not a programming error in qemu if it's false. Sounds to me as if it should be a normal error check rather than an assertion. Also, what happens after EOF? I haven't read the patch yet, maybe it handles the situation already earlier, but if it doesn't, won't we get offset =3D=3D start then? Kevin --BOKacYhQ+x31HxR3 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJUZJitAAoJEH8JsnLIjy/WF5kQALY+Akh1eOutk7jwAdLJVWPs l+7eaiEXwIlf+vknzziNmH9RskjHbXEwjuXji63YVR7Xjz1Tp3pz3vHJUhP0bBbF pYY2PjaXfOHkFCLQGUNhUi97UHYBhgy9qqF5HFHKrM9Q1gqwHTDWiq9R50wNZ9TL gw3wCgfTDBTYuzucOB2oYFqmNPyXxNUXMh8rFA4jC6+AoQISXcv1FqIgGAcydHH0 gHImCsSfWjkO+5ICs6TKbdbGUzphTMHMHfHxyQ8eXI4F+69cMxTKX0CPTHOgZ38H 093qsnyZkpq+XUn31R0lC4MKx+tfn6Q/Y02VgTTvM1s42nrIdfXSTArjEih25oiI 4rUNbYdO44Xwi6/1HvZKWCRgG6F04BCDhE+2ssaCrQnsZpa23/jmdgLzgiNh+MlM LGWDqbbu9Jh1GgA8x7Xx7kYFxqGycVcm989e6dg3zncyRPjD79MDQpj01kk253iR f+PgkMbetUxWV7jiOR0/4Yjaf6Be8Xdezt3XYi0FrZXdo9TA3YpIvdloshIBe0dK 8LlbTnttBIoan/2BovHL1wYjyGUaoQhyoQtFmxutr9SFeHHjU8JuF9kmsJLAP/PO CtzuYsls0GZhJ5/cDLFQ8rB7kmPabgmoH6U/Y/9cNnQT41noYdxk4GVbu55VSS8Z JFVA9h4IcnF9QhFqxz4O =tGeh -----END PGP SIGNATURE----- --BOKacYhQ+x31HxR3--