Re: [Qemu-devel] [Qemu-block] [RFC PATCH 0/7] Fix transactional snapshot with virtio-blk dataplane

[Stefan Hajnoczi]

On Wed, May 06, 2015 at 07:23:32PM +0800, Fam Zheng wrote:
> Reported by Paolo.
> 
> Unlike the iohandler in main loop, iothreads currently process the event
> notifier used as virtio-blk ioeventfd in all nested aio_poll. This is 
> dangerous
> without proper protection, because guest requests could sneak to block layer
> where they mustn't.
> 
> For example, a QMP transaction may involve multiple bdrv_drain_all() in
> handling the list of AioContext it works on. If an aio_poll in one of the
> bdrv_drain_all() happens to process a guest VQ kick by dispatching the
> ioeventfd event, a new guest write is then submitted, and voila, the
> transaction semantics is violated.
> 
> This series avoids this problem by disabling virtio-blk handlers during
> bdrv_drain_all() and transactions.
> 
> Notes:
> 
> If the general approach is right, other transaction types could get the
> blockers similarly, in next revision. And some related bdrv_drain_all() could
> also be changed to bdrv_drain().
> 
> virtio-scsi-dataplane will be a bit more complicated, but still doable.  It
> would probably need one more interface abstraction between scsi-disk, scsi-bus
> and virtio-scsi.
> 
> Although other devices don't have a pause/resume callback yet, the
> blk_check_request, which returns -EBUSY if "device io" op blocker is set, 
> could
> hopefully cover most cases already.
> 
> Timers and block jobs also generate IO, but it should be fine as long as they
> don't change guest visible data, which is true AFAICT.
> 
> 
> Fam Zheng (7):
>   block: Add op blocker type "device IO"
>   block: Block "device IO" during bdrv_drain and bdrv_drain_all
>   block: Add op blocker notifier list
>   block-backend: Add blk_op_blocker_add_notifier
>   virtio-blk: Move complete_request to 'ops' structure
>   virtio-blk: Don't handle output when there is "device IO" op blocker
>   blockdev: Add "device IO" op blocker during snapshot transaction
> 
>  block.c                         | 20 ++++++++++++
>  block/block-backend.c           | 10 ++++++
>  block/io.c                      | 12 +++++++
>  blockdev.c                      |  7 +++++
>  hw/block/dataplane/virtio-blk.c | 36 ++++++++++++++++++---
>  hw/block/virtio-blk.c           | 69 
> +++++++++++++++++++++++++++++++++++++++--
>  include/block/block.h           |  9 ++++++
>  include/block/block_int.h       |  3 ++
>  include/hw/virtio/virtio-blk.h  | 17 ++++++++--
>  include/sysemu/block-backend.h  |  2 ++
>  10 files changed, 174 insertions(+), 11 deletions(-)

Please also add a listener to nbd.c so NBD exports cannot submit I/O
during bdrv_drain_all().

pgpRVkhBWIkT7.pgp
Description: PGP signature