Re: [Qemu-devel] [Qemu-block] [PATCH] block: use drained section in bdrv_close

[Paolo Bonzini <pbonzini@redhat.com>]

> On 23.12.2015 11:48, Paolo Bonzini wrote:
> > bdrv_close is used when ejecting a medium.
> 
> Is it still? Other than it maybe being indirectly called through
> bdrv_delete(), it shouldn't be.

Yes, through blk_remove_bs -> bdrv_unref -> bdrv_delete.

> >                                             Use a drained section to ensure
> > that all I/O goes to either the old medium or the bitbucket.
> 
> Since the BDS will be deleted after bdrv_close() has been called, where
> else would the I/O go now?

The ioeventfd could be processed during bs->drv->bdrv_close.  For
example I/O could be submitted while qcow2_close's qcow2_cache_flush
yields, and the result would probably be corrupted metadata.

Paolo

> Max
> 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  block.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/block.c b/block.c
> > index 411edbf..01655de 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -2154,9 +2154,10 @@ void bdrv_close(BlockDriverState *bs)
> >          bdrv_io_limits_disable(bs);
> >      }
> >  
> > -    bdrv_drain(bs); /* complete I/O */
> > +    bdrv_drained_begin(bs); /* complete I/O */
> >      bdrv_flush(bs);
> >      bdrv_drain(bs); /* in case flush left pending I/O */
> > +
> >      notifier_list_notify(&bs->close_notifiers, bs);
> >  
> >      if (bs->blk) {
> > @@ -2206,6 +2207,7 @@ void bdrv_close(BlockDriverState *bs)
> >          g_free(ban);
> >      }
> >      QLIST_INIT(&bs->aio_notifiers);
> > +    bdrv_drained_end(bs);
> >  }
> >  
> >  void bdrv_close_all(void)
> > 
> 
> 
>