Message ID | 202310111414266586398@zte.com.cn |
---|---|
State | New |
Headers | show |
Series | virtio-blk: don't start dataplane during the stop of dataplane | expand |
On Wed, Oct 11, 2023 at 02:14:26PM +0800, lv.mengzhao@zte.com.cn wrote: > From: hujian <hu.jian@zte.com.cn> > > During the stop of dataplane for virtio-blk, virtio_bus_cleanup_host_notifier() is be > called to clean up notifier at the end, if polled ioeventfd, virtio_blk_handle_output() > is used to handle io request. But due to s->dataplane_disabled is false, it will be > returned directly, which drops io request. > Backtrace: > ->virtio_blk_data_plane_stop > ->virtio_bus_cleanup_host_notifier > ->virtio_queue_host_notifier_read > ->virtio_queue_notify_vq > ->vq->handle_output > ->virtio_blk_handle_output > ->if (s->dataplane && !s->dataplane_stoped) > ->if (!s->dataplane_disabled) > ->return * > ->virtio_blk_handle_output_do > The above problem can occur when using "virsh reset" cmdline to reset guest, while > guest does io. > To fix this problem, don't try to start dataplane if s->stopping is true, and io would > be handled by virtio_blk_handle_vq(). > > Signed-off-by: hujian <hu.jian@zte.com.cn> > --- > hw/block/virtio-blk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks, applied to my block tree: https://gitlab.com/stefanha/qemu/commits/block Stefan
On Wed, Oct 11, 2023 at 02:14:26PM +0800, lv.mengzhao@zte.com.cn wrote: > From: hujian <hu.jian@zte.com.cn> > > During the stop of dataplane for virtio-blk, virtio_bus_cleanup_host_notifier() is be > called to clean up notifier at the end, if polled ioeventfd, virtio_blk_handle_output() > is used to handle io request. But due to s->dataplane_disabled is false, it will be > returned directly, which drops io request. > Backtrace: > ->virtio_blk_data_plane_stop > ->virtio_bus_cleanup_host_notifier > ->virtio_queue_host_notifier_read > ->virtio_queue_notify_vq > ->vq->handle_output > ->virtio_blk_handle_output > ->if (s->dataplane && !s->dataplane_stoped) > ->if (!s->dataplane_disabled) > ->return * > ->virtio_blk_handle_output_do > The above problem can occur when using "virsh reset" cmdline to reset guest, while > guest does io. > To fix this problem, don't try to start dataplane if s->stopping is true, and io would > be handled by virtio_blk_handle_vq(). > > Signed-off-by: hujian <hu.jian@zte.com.cn> > --- > hw/block/virtio-blk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I have dropped this patch again after Fiona pointed out it does not compile and Kevin noticed that handling requests from the main loop thread while the I/O is still being processed in the IOThread is going to cause thread-safety issues. Can you explain the problem you are seeing in more detail? You run "virsh reset" while the guest is doing I/O. Then what happens? Stefan > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 39e7f23..c2d5938 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -1166,7 +1166,7 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) > { > VirtIOBlock *s = (VirtIOBlock *)vdev; > > - if (s->dataplane && !s->dataplane_started) { > + if (s->dataplane && !s->dataplane_started && !s->stopping) { > /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start > * dataplane here instead of waiting for .set_status(). > */ > -- > 1.8.3.1 >
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 39e7f23..c2d5938 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1166,7 +1166,7 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) { VirtIOBlock *s = (VirtIOBlock *)vdev; - if (s->dataplane && !s->dataplane_started) { + if (s->dataplane && !s->dataplane_started && !s->stopping) { /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start * dataplane here instead of waiting for .set_status(). */