Re: [PATCH v3 02/33] block/nbd: fix how state is cleared on nbd_open() failure paths

[Roman Kagan]

On Thu, Apr 22, 2021 at 01:27:22AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 21.04.2021 17:00, Roman Kagan wrote:
> > On Fri, Apr 16, 2021 at 11:08:40AM +0300, Vladimir Sementsov-Ogievskiy 
> > wrote:
> > > @@ -2305,20 +2301,23 @@ static int nbd_open(BlockDriverState *bs, QDict 
> > > *options, int flags,
> > >           return -EEXIST;
> > >       }
> > > +    ret = nbd_process_options(bs, options, errp);
> > > +    if (ret < 0) {
> > > +        goto fail;
> > > +    }
> > > +
> > >       /*
> > >        * establish TCP connection, return error if it fails
> > >        * TODO: Configurable retry-until-timeout behaviour.
> > >        */
> > >       if (nbd_establish_connection(bs, s->saddr, errp) < 0) {
> > > -        yank_unregister_instance(BLOCKDEV_YANK_INSTANCE(bs->node_name));
> > > -        return -ECONNREFUSED;
> > > +        ret = -ECONNREFUSED;
> > > +        goto fail;
> > >       }
> > >       ret = nbd_client_handshake(bs, errp);
> > Not that this was introduced by this patch, but once you're at it:
> > AFAICT nbd_client_handshake() calls yank_unregister_instance() on some
> > error path(s); I assume this needs to go too, otherwise it's called
> > twice (and asserts).
> > 
> > Roman.
> > 
> 
> No, nbd_client_handshake() only calls yank_unregister_function(), not 
> instance.

Indeed.  Sorry for confusion.

Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>