Am 03.05.2021 um 15:09 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 03.05.2021 15:41, Kevin Wolf wrote:
> > Am 03.05.2021 um 13:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > 03.05.2021 14:05, Kevin Wolf wrote:
> > > > Like other error paths, this one needs to call tran_finalize() and clean
> > > > up the BlockReopenQueue, too.
> > >
> > > We don't need the "abort" loop on that path. And clean-up of
> > > BlockReopenQueue is at "cleanup:" label.
> >
> > The cleanup of the BlockReopenQueue itself is there, but not of all
> > fields in it. Specifically, these lines are needed:
> >
> > qobject_unref(bs_entry->state.explicit_options);
> > qobject_unref(bs_entry->state.options);
> >
> > The references are taken in bdrv_reopen_queue_child() and either used in
> > commit or released on abort. Doing nothing with them leaks them.
>
> Oops. Somehow I decided they are set in _prepare.
>
> >
> > > So I'd prefer Peter's suggestion (my "[PATCH 2/6] block:
> > > bdrv_reopen_multiple(): fix leak of tran object")
> >
> > I don't like it because I think every call that doesn't end in a commit,
> > should jump to the abort label to make sure that everything that remains
> > unused because of this is correctly cleaned up.
> >
>
>
> agree now..
>
> Still, don't we miss these two qobject_unref() calls on success path?
No, they are put to use in bdrv_reopen_commit():
qobject_unref(bs->explicit_options);
qobject_unref(bs->options);
bs->explicit_options = reopen_state->explicit_options;
bs->options = reopen_state->options;
Kevin
