Re: [Qemu-block] [Qemu-devel] Is the use of bdrv_getlength() in handle_aiocb_write_zeroes() kosher?

[Markus Armbruster]

"Denis V. Lunev" <address@hidden> writes:

> On 08/04/2017 03:16 PM, Markus Armbruster wrote:
>> Denis, you added this in commit d50d822:
>>
>> #ifdef CONFIG_FALLOCATE
>>     if (s->has_fallocate && aiocb->aio_offset >= bdrv_getlength(aiocb->bs)) {
>>         int ret = do_fallocate(s->fd, 0, aiocb->aio_offset, 
>> aiocb->aio_nbytes);
>>         if (ret == 0 || ret != -ENOTSUP) {
>>             return ret;
>>         }
>>         s->has_fallocate = false;
>>     }
>> #endif
>>
>> bdrv_getlength() can fail.  Does it do the right thing then?  For what
>> it's worth, the comparison of its value is signed.
> fallocate() with 0 flags can work only beyond end of file
> or on top of the hole. Thus the check is made to validate
> that we are beyond EOF.
>
> Technically fallocate should fail if that condition will be
> violated. But you right, we can add sanity check here.
> This would not harm.
>
> Should I send it?

I figure an explicit check for bdrv_getlength() failure would make the
code easier to understand.  In short: yes, please!