Re: [Qemu-devel] [PATCH 04/21] block: Add bdrv_close_all() handlers

[Max Reitz]

On 2015-01-28 at 17:44, Eric Blake wrote:
> On 01/26/2015 12:27 PM, Max Reitz wrote:
> > Every time a reference to a BlockBackend is taken, a notifier for
> > bdrv_close_all() has to be deposited so the reference holder can
> > relinquish its reference when bdrv_close_all() is called. That notifier
> > should be revoked on a bdrv_unref() call.
> In addition to the design question about whether NBD exports should be
> their own new BB,
>
> > @@ -198,8 +207,12 @@ void blk_ref(BlockBackend *blk)
> >    * If this drops it to zero, destroy @blk.
> >    * For convenience, do nothing if @blk is null.
> >    */
> > -void blk_unref(BlockBackend *blk)
> > +void blk_unref(BlockBackend *blk, Notifier *close_all_notifier)
> >   {
> > +    if (close_all_notifier) {
> > +        notifier_remove(close_all_notifier);
> > +    }
> > +
> >       if (blk) {
> Does removing a notifier when blk is NULL make sense?

Considering the pattern is probably "allocate notifier; if that fails, 
goto fail; create BB; if that fails, goto fail; ...; fail: blk_unref(); 
free notifier", the is indeed a case where the notifier may be non-NULL 
but not yet registered (if "create BB" failed). Therefore, to simplify 
the caller code, notifier_remove() should only be called if blk is not 
NULL, right.

Will fix.

Max