Re: [Qemu-devel] Re: [PATCH 1/3] Only call aio flush handler if set

[Anthony Liguori <aliguori@us.ibm.com>]

Ryan Harper wrote:
> * Anthony Liguori <aliguori@us.ibm.com> [2008-09-22 21:49]:
>   
>> Ryan Harper wrote:
>>     
>>> If the aio handler doesn't register an io_flush handler, we'd SEGV; fix 
>>> that by
>>> only calling the flush handler if set.  BTW, aio handlers *should* 
>>> register an
>>> io_flush routine.
>>>
>>> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
>>>
>>> diff --git a/aio.c b/aio.c
>>> index 687e4be..2bb3ed4 100644
>>> --- a/aio.c
>>> +++ b/aio.c
>>> @@ -105,7 +105,8 @@ void qemu_aio_flush(void)
>>>         ret = 0;
>>>
>>>         LIST_FOREACH(node, &aio_handlers, node) {
>>> -            ret |= node->io_flush(node->opaque);
>>> +            if (node->io_flush)
>>> +                ret |= node->io_flush(node->opaque);
>>>         }
>>>  
>>>       
>> Just not doing an io_flush is just hiding the real bug--that the user 
>> didn't register an io_flush handler.  If the inevitable SEGV is not your 
>>     
>
> That may be true, but it it is no different than the check for read and
> write handlers in qemu_aio_wait().
>   

Read and write handlers are optional.  I guess in practice one or the 
other should be set but neither one is individually required.  The 
problem with your patch is that it takes something that is a bug, and 
makes it more difficult to spot.  So it actually makes things worse.

Regards,

Anthony Liguori