Hello,
Max Reitz, on lun. 27 févr. 2017 17:12:47 +0100, wrote:
> > #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
> > - if (s->has_discard && s->has_fallocate) {
> > + if (s->has_discard && (s->has_fallocate || open_flags &
> > BDRV_O_SPARSE)) {
>
> s->has_fallocate has a meaning. I wouldn't try to call do_fallocate() if
> s->has_fallocate is false.
Ah, sorry, I didn't realize that that test wasn't only to check that
we'll be able to call fallocate(0) further down.
> Therefore, I consider this to effectively be a no-op.
Yes.
> > @@ -1098,7 +1102,8 @@ static ssize_t
> > handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
> > #endif
> >
> > #ifdef CONFIG_FALLOCATE
> > - if (s->has_fallocate && aiocb->aio_offset >=
> > bdrv_getlength(aiocb->bs)) {
> > + if (s->has_fallocate && !(open_flags & BDRV_O_SPARSE)
> > + && aiocb->aio_offset >= bdrv_getlength(aiocb->bs)) {
>
> First, this part is only invoked if everything before it has failed.
I misread the code indeed.
> Unless I'm mistaken, unmap/trim requests from the guest should result in
> a discard request in the block layer. This should always trigger
> handle_aiocb_discard() here and that should do what you want it to.
Mmm, indeed. I guess I got lost in the hairy block code.
> Could you maybe give me the configuration that results in the issue
> you're describing in the commit message?
Actually I can't reproduce the issue any more. I'm now wondering how I
ended up there.
Anyway, I'm really sorry for the noise, and thanks for the good work :)
Samuel
