Re: [Qemu-devel] [PATCH 5/8] virtio-blk: fix "disabled data plane" mode

[Cornelia Huck]

On Tue, 16 Feb 2016 16:45:24 +0100
Paolo Bonzini <address@hidden> wrote:

> On 15/02/2016 18:58, Cornelia Huck wrote:
> > It seems a bit odd to me that ->started is the only state that is not
> > inside the dataplane struct... this approach saves a function call for
> > an accessor, though.
> 
> Actually, I can do better by moving the flag entirely within
> hw/block/virtio-blk.c:
> 

> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index e04c8f5..34bae8e 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -590,6 +590,7 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, 
> VirtQueue *vq)
>       * dataplane here instead of waiting for .set_status().
>       */
>      if (s->dataplane && !s->dataplane_started) {
> +        s->dataplane_started = true;
>          virtio_blk_data_plane_start(s->dataplane);
>          return;
>      }
> @@ -658,8 +659,9 @@ static void virtio_blk_reset(VirtIODevice *vdev)
>      aio_context_acquire(ctx);
>      blk_drain(s->blk);
> 
> -    if (s->dataplane) {
> +    if (s->dataplane && s->dataplane_started) {
>          virtio_blk_data_plane_stop(s->dataplane);
> +        s->dataplane_started = false;
>      }
>      aio_context_release(ctx);
> 
> 
> Does it look better?

I think yes.

Hm... this seems to guarantee that _start()/_stop() never runs
concurrently, doesn't it? Could we get rid of the ->starting/->stopping
flags as well?

...and ->disabled as well, since we try just once until we stop?