Re: [Qemu-block] [PATCH] blk: Add discard=sparse mode

[Max Reitz]

On 27.02.2017 17:33, Samuel Thibault wrote:
> 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.

I can understand that very well. ;-)

>> 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 :)

Not a problem at all. In case you happen to encounter the issue again,
just send a report to the qemu-block list.

Max

signature.asc
Description: OpenPGP digital signature