From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34147) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xosqp-0000vc-Ki for qemu-devel@nongnu.org; Thu, 13 Nov 2014 06:46:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xosqj-0007t4-BP for qemu-devel@nongnu.org; Thu, 13 Nov 2014 06:46:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43240) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xosqj-0007sy-2k for qemu-devel@nongnu.org; Thu, 13 Nov 2014 06:46:05 -0500 Message-ID: <546499F6.4050201@redhat.com> Date: Thu, 13 Nov 2014 12:45:58 +0100 From: Max Reitz MIME-Version: 1.0 References: <1415820422-17796-1-git-send-email-armbru@redhat.com> <1415820422-17796-3-git-send-email-armbru@redhat.com> <5463EC70.1030107@redhat.com> <20141113114030.GA3933@noname.redhat.com> In-Reply-To: <20141113114030.GA3933@noname.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: Kevin Wolf , Eric Blake Cc: pbonzini@redhat.com, stefanha@redhat.com, Markus Armbruster , tony@bakeyournoodle.com, qemu-devel@nongnu.org On 2014-11-13 at 12:40, Kevin Wolf wrote: > 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 = lseek(s->fd, start, SEEK_DATA); >>> + if (offs < 0) { >>> + /* no idea where the hole ends, give up (unlikely to happen) */ >>> + goto dunno; >>> + } >>> + assert(offs >= start); >>> + *hole = start; >>> + *data = offs; >> 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 == start then? raw_co_get_block_status() already bails out if start is at or beyond EOF. Max