Re: [Qemu-block] [PATCH] block: Close a BlockDriverState completely even when bs->drv is NULL

[Kevin Wolf]

Am 17.11.2017 um 17:19 hat Alberto Garcia geschrieben:
> On Fri 17 Nov 2017 05:14:08 PM CET, Max Reitz wrote:
> > On 2017-11-06 15:53, Alberto Garcia wrote:
> >> 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>
> >> ---
> >>  block.c                    | 57 
> >> +++++++++++++++++++++++-----------------------
> >>  tests/qemu-iotests/060     | 13 +++++++++++
> >>  tests/qemu-iotests/060.out | 12 ++++++++++
> >>  3 files changed, 53 insertions(+), 29 deletions(-)
> >
> > Sooo...  What's the exact status of this patch? :-)
> 
> I can resend it rebased on top of your block branch, but I'm fine if you
> merge the iotest manually (it's a trivial merge).
> 
> I'm not sure about Kevin's comments though, it wasn't clear to me if
> he's fine if we apply this patch or not.

I'm not sure if it's enough, but I think the patch is good anyway.

Kevin