[Top][All Lists]

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

Re: [Qemu-devel] [PATCH] block: Close a BlockDriverState completely even

From: Kevin Wolf
Subject: Re: [Qemu-devel] [PATCH] block: Close a BlockDriverState completely even when bs->drv is NULL
Date: Wed, 8 Nov 2017 15:33:54 +0100
User-agent: Mutt/1.9.1 (2017-09-22)

Am 06.11.2017 um 15:53 hat Alberto Garcia geschrieben:
> bdrv_close() skips much of its logic when bs->drv is NULL. This is
> fine when we're closing a BlockDriverState that has just been created
> (because e.g the initialization process failed), but it's not enough
> in other cases.
> For example, when a valid qcow2 image is found to be corrupted then
> QEMU marks it as such in the file header and then sets bs->drv to
> NULL in order to make the BlockDriverState unusable. When that BDS is
> later closed then many of its data structures are not freed (leaking
> their memory) and none of its children are detached. This results in
> bdrv_close_all() failing to close all BDSs and making this assertion
> fail when QEMU is being shut down:
>    bdrv_close_all: Assertion `QTAILQ_EMPTY(&all_bdrv_states)' failed.
> This patch makes bdrv_close() do the full uninitialization process
> in all cases. This fixes the problem with corrupted images and still
> works fine with freshly created BDSs.
> Signed-off-by: Alberto Garcia <address@hidden>

This doesn't apply cleanly in the test case. Does it depend on your
qcow2 fixes?

Also, while I think the change makes sense, it's also clear that we're
trying to fix up an inconsistent state here. Maybe we could also improve
the state that block drivers leave behind when marking an image as
corrupt. Just setting bs->drv = NULL means that at least any internal
data structures will not get cleaned up.

On the other hand, we can't just call bdrv_close() from a failing
request because closing requires that we drain the request first. Maybe
it would be possible to call drv->bdrv_close() with a BH or something.


reply via email to

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