| Message ID | 1432711146-28405-4-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/27/2015 01:19 AM, Fam Zheng wrote: > Currently the host notifier is checked by all aio_poll, which is not > safe. For example, in a qmp transaction that takes snapshots or starts > drive-backup on multiple dataplane disks, the atomicity could be broken: > There could be one or more bdrv_drain_all() calls in each transaction > opeartion, which will unconditinally calls one or more aio_poll on the s/opeartion/operation/ s/unconditinally calls/unconditionally call/ > AioContext: > > qmp transaction > backup A prepare > ... > bdrv_drain_all() > aio_poll(A) > aio_poll(B) > aio_poll(C) > ... > ... > backup B prepare > ... > bdrv_drain_all() > aio_poll(A) > aio_poll(B) > -> aio_poll(C) > ... > snapshot C prepare > ... > bdrv_drain_all() > aio_poll(A) > aio_poll(B) > aio_poll(C) > ... > > If the aio_poll(C) in the middle receives a new virtio-blk request from > ioeventfd, a new request will be submitted to C, then snapshot C is > inconsistent. > > To avoid that, use aio_set_io_event_notifier so the behavior is the same > as in main loop. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > hw/block/dataplane/virtio-blk.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > again, letting others comment on the technical merits
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 3db139b..027c5c5 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -283,7 +283,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s) /* Get this show started by hooking up our callbacks */ aio_context_acquire(s->ctx); - aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify); + aio_set_io_event_notifier(s->ctx, &s->host_notifier, handle_notify); aio_context_release(s->ctx); return; @@ -319,7 +319,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s) aio_context_acquire(s->ctx); /* Stop notifications for new requests from guest */ - aio_set_event_notifier(s->ctx, &s->host_notifier, NULL); + aio_set_io_event_notifier(s->ctx, &s->host_notifier, NULL); /* Drain and switch bs back to the QEMU main loop */ blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
Currently the host notifier is checked by all aio_poll, which is not safe. For example, in a qmp transaction that takes snapshots or starts drive-backup on multiple dataplane disks, the atomicity could be broken: There could be one or more bdrv_drain_all() calls in each transaction opeartion, which will unconditinally calls one or more aio_poll on the AioContext: qmp transaction backup A prepare ... bdrv_drain_all() aio_poll(A) aio_poll(B) aio_poll(C) ... ... backup B prepare ... bdrv_drain_all() aio_poll(A) aio_poll(B) -> aio_poll(C) ... snapshot C prepare ... bdrv_drain_all() aio_poll(A) aio_poll(B) aio_poll(C) ... If the aio_poll(C) in the middle receives a new virtio-blk request from ioeventfd, a new request will be submitted to C, then snapshot C is inconsistent. To avoid that, use aio_set_io_event_notifier so the behavior is the same as in main loop. Signed-off-by: Fam Zheng <famz@redhat.com> --- hw/block/dataplane/virtio-blk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)