[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC for-2.7] block: keep AioContext pointer in BlockBa
Re: [Qemu-devel] [RFC for-2.7] block: keep AioContext pointer in BlockBackend
Tue, 17 May 2016 12:52:54 +0200
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0
On 11.04.2016 16:14, Stefan Hajnoczi wrote:
> blk_get/set_aio_context() delegate to BlockDriverState without storing
> the AioContext pointer in BlockBackend.
> There are two flaws:
> 1. BlockBackend falls back to the QEMU main loop AioContext when there
> is no root BlockDriverState. This means the drive loses its
> AioContext during media change and would break dataplane.
> 2. BlockBackend state used from multiple threads has no lock. Race
> conditions will creep in as functionality is moved from
> BlockDriverState to BlockBackend due to the absense of a lock. The
> monitor cannot access BlockBackend state safely while an IOThread is
> also accessing the state.
> Both issue #1 and #2 are mostly theoretical at the moment. I haven't
> figured out a way to trigger #1 with virtio-blk (does not support media
> change) or virtio-scsi (blocks the eject operation). #2 may be possible
> with block accounting statistics in BlockBackend but I'm not aware of a
> crash that can be triggered.
> This patch stores the AioContext pointer in BlockBackend and puts newly
> inserted BlockDriverStates into the AioContext.
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> block/block-backend.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
We can have multiple BBs per BDS tree, and in the future (after Kevin's
"block: Remove BlockDriverState.blk" series) we may even have multiple
BBs per BDS. Therefore, we have to make sure to propagate the AIO
context to all BBs attached to the BDS tree, just as the AIO context is
propagated to all BDSs in the BDS tree.
A trivial way to do this would be to query the BDS's AIO context in
blk_get_aio_context() and update the BB's context if they don't match.
The more correct way would probably be to make bdrv_attach_aio_context()
update its BB's (or BBs', in the future) AIO context.
Description: OpenPGP digital signature