Re: [Qemu-devel] [PATCH v4 00/11] block: Rework bdrv_close_all()

[Kevin Wolf <kwolf@redhat.com>]

Am 27.02.2015 um 17:43 hat Max Reitz geschrieben:
> Currently, bdrv_close_all() force-closes all BDSs with a BlockBackend,
> which can lead to data corruption (see the iotest added in the final
> patch of this series) and is most certainly very ugly.
> 
> This series reworks bdrv_close_all() to instead eject the BDS trees from
> all BlockBackends and then close the monitor-owned BDS trees, which are
> the only BDSs without a BB. In effect, all BDSs are closed just by
> getting closed automatically due to their reference count becoming 0.
> 
> The benefit over the approach taken in v1 and v2 is that in device
> models we often cannot simply drop the reference to a BB because there
> may be some user which we forgot about. By ejecting the BDS trees from
> the BB, the BB itself becomes unusable, but in a clean way (it will
> return errors when accessed, but nothing will crash). Also, it is much
> simpler (no reference tracking necessary).
> 
> The only disadvantage (I can see) is that the BBs are leaked; but this
> does not matter because the qemu process is about to exit anyway.

I haven't looked at the actual patches yet, but just from this
description and the diffstat: We need to make sure that the refcount
really drops to 0. That is, if there are NBD servers, block jobs, etc.
that hold an additional reference, we must make sure to stop them. It
doesn't look like this series takes care of this, does it?

Hm... Perhaps we could even install an atexit handler that asserts that
all BDSes are really gone in the end?

Kevin