[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of F
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP |
Date: |
Thu, 13 Nov 2014 12:40:30 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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?
Kevin
pgpXTLztfSE1W.pgp
Description: PGP signature
- [Qemu-devel] [PATCH 0/2] raw-posix: Get rid of FIEMAP, Markus Armbruster, 2014/11/12
- [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Markus Armbruster, 2014/11/12
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Fam Zheng, 2014/11/12
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Eric Blake, 2014/11/12
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Markus Armbruster, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Max Reitz, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Kevin Wolf, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Max Reitz, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Markus Armbruster, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Kevin Wolf, 2014/11/13
Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Max Reitz, 2014/11/13
[Qemu-devel] [PATCH 1/2] raw-posix: Fix comment for raw_co_get_block_status(), Markus Armbruster, 2014/11/12