[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v4] Do not delete BlockDriverState when deleting

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v4] Do not delete BlockDriverState when deleting the drive
Date: Wed, 30 Mar 2011 09:36:40 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux)

[Cc: Justin, because I feel it should go into stable-0.14 as well]

Ryan Harper <address@hidden> writes:

> When removing a drive from the host-side via drive_del we currently have
> the following path:
> drive_del
> qemu_aio_flush()
> bdrv_close()    // zaps bs->drv, which makes any subsequent I/O get
>                 // dropped.  Works as designed
> drive_uninit()
> bdrv_delete()   // frees the bs.  Since the device is still connected to
>                 // bs, any subsequent I/O is a use-after-free.
> The value of bs->drv becomes unpredictable on free.  As long as it
> remains null, I/O still gets dropped, however it could become non-null
> at any point after the free resulting SEGVs or other QEMU state
> corruption.
> To resolve this issue as simply as possible, we can chose to not
> actually delete the BlockDriverState pointer.  Since bdrv_close()
> handles setting the drv pointer to NULL, we just need to remove the
> BlockDriverState from the QLIST that is used to enumerate the block
> devices.  This is currently handled within bdrv_delete, so move this
> into its own function, bdrv_make_anon().
> The result is that we can now invoke drive_del, this closes the file
> descriptors and sets BlockDriverState->drv to NULL which prevents futher
> IO to the device, and since we do not free BlockDriverState, we don't
> have to worry about the copy retained in the block devices.
> We also don't attempt to remove the qdev property since we are no longer
> deleting the BlockDriverState on drives with associated drives.  This
> also allows for removing Drives with no devices associated either.
> Reported-by: Markus Armbruster <address@hidden>
> Signed-off-by: Ryan Harper <address@hidden>

Acked-by: Markus Armbruster <address@hidden>

reply via email to

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