Re: [PATCH] virtio-blk: don't start dataplane during the stop of dataplane

[Stefan Hajnoczi]

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
>

signature.asc
Description: PGP signature