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

[Anthony Liguori]

Ryan Harper wrote:
> * Anthony Liguori <address@hidden> [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 <address@hidden>
> > >
> > > 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