[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Kevin Wolf
Subject: Re: [Qemu-block] [PATCH v4 00/11] block: Rework bdrv_close_all()
Date: Mon, 2 Mar 2015 10:25:42 +0100
User-agent: Mutt/1.5.21 (2010-09-15)

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?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]