Re: [Qemu-devel] [Qemu-block] [RFC PATCH 0/7] Fix transactional snapshot with virtio-blk dataplane
On Thu, May 07, 2015 at 02:43:43PM +0100, Stefan Hajnoczi wrote: 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(). qmp_transaction probably also needs this. Stefan pgpAQPw3DSGnC.pgp Description: PGP signature
Re: [Qemu-devel] [Qemu-block] [RFC PATCH 0/7] Fix transactional snapshot with virtio-blk dataplane
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(). pgpUR96AN6jPf.pgp Description: PGP signature
