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

[Ryan Harper <ryanh@us.ibm.com>]

* Anthony Liguori <aliguori@us.ibm.com> [2008-09-23 09:36]:
> 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.

I disagree that anything is worse off by not SEGV'ing, In any case, what
do you want here?

Read or Write must be set along with flush or we error in fd
registration?


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253   T/L: 678-9253
ryanh@us.ibm.com