diff mbox

[5/8] virtio-blk: fix "disabled data plane" mode

Message ID 1455470231-5223-6-git-send-email-pbonzini@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paolo Bonzini Feb. 14, 2016, 5:17 p.m. UTC
In disabled mode, virtio-blk dataplane seems to be enabled, but flow
actually goes through the normal virtio path.  This patch simplifies a bit
the handling of disabled mode.  In disabled mode, virtio_blk_handle_output
might be called even if s->dataplane is not NULL.

This is a bit tricky, because the current check for s->dataplane will
always trigger, causing a continuous stream of calls to
virtio_blk_data_plane_start.  Unfortunately, these calls will not
do anything.  To fix this, set the "started" flag even in disabled
mode, and skip virtio_blk_data_plane_start if the started flag is true.
The resulting changes also prepare the code for the next patch, were
virtio-blk dataplane will reuse the same virtio_blk_handle_output function
as "regular" virtio-blk.

Because struct VirtIOBlockDataPlane is opaque in virtio-blk.c, we have
to move s->dataplane->started inside struct VirtIOBlock.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 21 +++++++++------------
 hw/block/virtio-blk.c           |  2 +-
 include/hw/virtio/virtio-blk.h  |  1 +
 3 files changed, 11 insertions(+), 13 deletions(-)

Comments

Cornelia Huck Feb. 15, 2016, 5:58 p.m. UTC | #1
On Sun, 14 Feb 2016 18:17:08 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> In disabled mode, virtio-blk dataplane seems to be enabled, but flow
> actually goes through the normal virtio path.  This patch simplifies a bit
> the handling of disabled mode.  In disabled mode, virtio_blk_handle_output
> might be called even if s->dataplane is not NULL.
> 
> This is a bit tricky, because the current check for s->dataplane will
> always trigger, causing a continuous stream of calls to
> virtio_blk_data_plane_start.  Unfortunately, these calls will not
> do anything.  
> To fix this, set the "started" flag even in disabled
> mode, and skip virtio_blk_data_plane_start if the started flag is true.
> The resulting changes also prepare the code for the next patch, were
> virtio-blk dataplane will reuse the same virtio_blk_handle_output function
> as "regular" virtio-blk.
> 
> Because struct VirtIOBlockDataPlane is opaque in virtio-blk.c, we have
> to move s->dataplane->started inside struct VirtIOBlock.

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.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/block/dataplane/virtio-blk.c | 21 +++++++++------------
>  hw/block/virtio-blk.c           |  2 +-
>  include/hw/virtio/virtio-blk.h  |  1 +
>  3 files changed, 11 insertions(+), 13 deletions(-)
> 

> @@ -319,9 +314,10 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      k->set_guest_notifiers(qbus->parent, 1, false);
>    fail_guest_notifiers:
>      vring_teardown(&s->vring, s->vdev, 0);
> -    s->disabled = true;
>    fail_vring:
> +    s->disabled = true;

I originally introduced ->disabled to cover (possibly temporary)
failures to set up notifiers, but I guess this doesn't hurt.

>      s->starting = false;
> +    vblk->dataplane_started = true;
>  }
> 
>  /* Context: QEMU global mutex held */

After spending some time wrapping my head around it again, this looks
sane to me.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Fam Zheng Feb. 16, 2016, 7:26 a.m. UTC | #2
On Sun, 02/14 18:17, Paolo Bonzini wrote:
> In disabled mode, virtio-blk dataplane seems to be enabled, but flow
> actually goes through the normal virtio path.  This patch simplifies a bit
> the handling of disabled mode.  In disabled mode, virtio_blk_handle_output
> might be called even if s->dataplane is not NULL.
> 
> This is a bit tricky, because the current check for s->dataplane will
> always trigger, causing a continuous stream of calls to
> virtio_blk_data_plane_start.  Unfortunately, these calls will not
> do anything.  To fix this, set the "started" flag even in disabled
> mode, and skip virtio_blk_data_plane_start if the started flag is true.
> The resulting changes also prepare the code for the next patch, were
> virtio-blk dataplane will reuse the same virtio_blk_handle_output function
> as "regular" virtio-blk.
> 
> Because struct VirtIOBlockDataPlane is opaque in virtio-blk.c, we have
> to move s->dataplane->started inside struct VirtIOBlock.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/block/dataplane/virtio-blk.c | 21 +++++++++------------
>  hw/block/virtio-blk.c           |  2 +-
>  include/hw/virtio/virtio-blk.h  |  1 +
>  3 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index 03b81bc..cc521c1 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -28,7 +28,6 @@
>  #include "qom/object_interfaces.h"
>  
>  struct VirtIOBlockDataPlane {
> -    bool started;
>      bool starting;
>      bool stopping;
>      bool disabled;
> @@ -264,11 +263,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      VirtQueue *vq;
>      int r;
>  
> -    if (s->started || s->disabled) {
> -        return;
> -    }
> -
> -    if (s->starting) {
> +    if (vblk->dataplane_started || s->starting) {
>          return;
>      }
>  
> @@ -300,7 +295,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      vblk->complete_request = complete_request_vring;
>  
>      s->starting = false;
> -    s->started = true;
> +    vblk->dataplane_started = true;
>      trace_virtio_blk_data_plane_start(s);
>  
>      blk_set_aio_context(s->conf->conf.blk, s->ctx);
> @@ -319,9 +314,10 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      k->set_guest_notifiers(qbus->parent, 1, false);
>    fail_guest_notifiers:
>      vring_teardown(&s->vring, s->vdev, 0);
> -    s->disabled = true;
>    fail_vring:
> +    s->disabled = true;
>      s->starting = false;

Worth a comment here, or at definition of dataplane_started, explaining the
trick said in the commit message?

Reviewed-by: Fam Zheng <famz@redhat.com>

> +    vblk->dataplane_started = true;
>  }
>  

<...>
Christian Borntraeger March 9, 2016, 12:21 p.m. UTC | #3
On 02/14/2016 06:17 PM, Paolo Bonzini wrote:
> In disabled mode, virtio-blk dataplane seems to be enabled, but flow
> actually goes through the normal virtio path.  This patch simplifies a bit
> the handling of disabled mode.  In disabled mode, virtio_blk_handle_output
> might be called even if s->dataplane is not NULL.
> 
> This is a bit tricky, because the current check for s->dataplane will
> always trigger, causing a continuous stream of calls to
> virtio_blk_data_plane_start.  Unfortunately, these calls will not
> do anything.  To fix this, set the "started" flag even in disabled
> mode, and skip virtio_blk_data_plane_start if the started flag is true.
> The resulting changes also prepare the code for the next patch, were
> virtio-blk dataplane will reuse the same virtio_blk_handle_output function
> as "regular" virtio-blk.
> 
> Because struct VirtIOBlockDataPlane is opaque in virtio-blk.c, we have
> to move s->dataplane->started inside struct VirtIOBlock.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

I have some random crashes at startup 
                
                Stack trace of thread 48326:
                #0  0x000002aa2e0cce46 bdrv_co_do_rw (qemu-system-s390x)
                #1  0x000002aa2e159e8e coroutine_trampoline (qemu-system-s390x)
                #2  0x000003ffbc35150a __makecontext_ret (libc.so.6)


that I was able to bisect.
commit 2906cddfecff21af20eedab43288b485a679f9ac does crash regularly, 
2906cddfecff21af20eedab43288b485a679f9ac^ does not.

I will try to find somebody that looks into that - unless you have an idea.

Christian

> ---

>  hw/block/dataplane/virtio-blk.c | 21 +++++++++------------
>  hw/block/virtio-blk.c           |  2 +-
>  include/hw/virtio/virtio-blk.h  |  1 +
>  3 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index 03b81bc..cc521c1 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -28,7 +28,6 @@
>  #include "qom/object_interfaces.h"
> 
>  struct VirtIOBlockDataPlane {
> -    bool started;
>      bool starting;
>      bool stopping;
>      bool disabled;
> @@ -264,11 +263,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      VirtQueue *vq;
>      int r;
> 
> -    if (s->started || s->disabled) {
> -        return;
> -    }
> -
> -    if (s->starting) {
> +    if (vblk->dataplane_started || s->starting) {
>          return;
>      }
> 
> @@ -300,7 +295,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      vblk->complete_request = complete_request_vring;
> 
>      s->starting = false;
> -    s->started = true;
> +    vblk->dataplane_started = true;
>      trace_virtio_blk_data_plane_start(s);
> 
>      blk_set_aio_context(s->conf->conf.blk, s->ctx);
> @@ -319,9 +314,10 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      k->set_guest_notifiers(qbus->parent, 1, false);
>    fail_guest_notifiers:
>      vring_teardown(&s->vring, s->vdev, 0);
> -    s->disabled = true;
>    fail_vring:
> +    s->disabled = true;
>      s->starting = false;
> +    vblk->dataplane_started = true;
>  }
> 
>  /* Context: QEMU global mutex held */
> @@ -331,13 +327,14 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>      VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
> 
> +    if (!vblk->dataplane_started || s->stopping) {
> +        return;
> +    }
> 
>      /* Better luck next time. */
>      if (s->disabled) {
>          s->disabled = false;
> -        return;
> -    }
> -    if (!s->started || s->stopping) {
> +        vblk->dataplane_started = false;
>          return;
>      }
>      s->stopping = true;
> @@ -364,6 +361,6 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>      /* Clean up guest notifier (irq) */
>      k->set_guest_notifiers(qbus->parent, 1, false);
> 
> -    s->started = false;
> +    vblk->dataplane_started = false;
>      s->stopping = false;
>  }
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index c427698..e04c8f5 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -589,7 +589,7 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
>      /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
>       * dataplane here instead of waiting for .set_status().
>       */
> -    if (s->dataplane) {
> +    if (s->dataplane && !s->dataplane_started) {
>          virtio_blk_data_plane_start(s->dataplane);
>          return;
>      }
> diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
> index 199bb0e..781969d 100644
> --- a/include/hw/virtio/virtio-blk.h
> +++ b/include/hw/virtio/virtio-blk.h
> @@ -56,6 +56,7 @@ typedef struct VirtIOBlock {
>      /* Function to push to vq and notify guest */
>      void (*complete_request)(struct VirtIOBlockReq *req, unsigned char status);
>      Notifier migration_state_notifier;
> +    bool dataplane_started;
>      struct VirtIOBlockDataPlane *dataplane;
>  } VirtIOBlock;
>
Paolo Bonzini March 9, 2016, 12:55 p.m. UTC | #4
On 09/03/2016 13:21, Christian Borntraeger wrote:
> I have some random crashes at startup 
>                 
>                 Stack trace of thread 48326:
>                 #0  0x000002aa2e0cce46 bdrv_co_do_rw (qemu-system-s390x)
>                 #1  0x000002aa2e159e8e coroutine_trampoline (qemu-system-s390x)
>                 #2  0x000003ffbc35150a __makecontext_ret (libc.so.6)
> 
> 
> that I was able to bisect.
> commit 2906cddfecff21af20eedab43288b485a679f9ac does crash regularly, 
> 2906cddfecff21af20eedab43288b485a679f9ac^ does not.
> 
> I will try to find somebody that looks into that - unless you have an idea.

The only random idea is to move

    vblk->dataplane_started = true

to the beginning of virtio_blk_data_plane_start rather than the end.

Paolo
Christian Borntraeger March 9, 2016, 1:05 p.m. UTC | #5
On 03/09/2016 02:02 PM, Christian Borntraeger wrote:
> On 03/09/2016 01:55 PM, Paolo Bonzini wrote:
>>
>>
>> On 09/03/2016 13:21, Christian Borntraeger wrote:
>>> I have some random crashes at startup 
>>>                 
>>>                 Stack trace of thread 48326:
>>>                 #0  0x000002aa2e0cce46 bdrv_co_do_rw (qemu-system-s390x)
>>>                 #1  0x000002aa2e159e8e coroutine_trampoline (qemu-system-s390x)
>>>                 #2  0x000003ffbc35150a __makecontext_ret (libc.so.6)
>>>
>>>
>>> that I was able to bisect.
>>> commit 2906cddfecff21af20eedab43288b485a679f9ac does crash regularly, 
>>> 2906cddfecff21af20eedab43288b485a679f9ac^ does not.
>>>
>>> I will try to find somebody that looks into that - unless you have an idea.
>>
>> The only random idea is to move
>>
>>     vblk->dataplane_started = true
>>
>> to the beginning of virtio_blk_data_plane_start rather than the end.
>>
>> Paolo
>>
> 
> Indeed
> 
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index 36f3d2b..1908d59 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -195,6 +195,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      if (vblk->dataplane_started || s->starting) {
>          return;
>      }
> +    vblk->dataplane_started = true;
>  
>      s->starting = true;
>      s->vq = virtio_get_queue(s->vdev, 0);
> @@ -235,7 +236,6 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>    fail_guest_notifiers:
>      s->disabled = true;
>      s->starting = false;
> -    vblk->dataplane_started = true;
>  }
>  
>  /* Context: QEMU global mutex held */
> 
> seems to fix the issue. 

Hmmm, no another crash, just seems to happen less often.
Cornelia Huck March 9, 2016, 1:12 p.m. UTC | #6
On Wed, 9 Mar 2016 14:05:20 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 03/09/2016 02:02 PM, Christian Borntraeger wrote:
> > On 03/09/2016 01:55 PM, Paolo Bonzini wrote:
> >>
> >>
> >> On 09/03/2016 13:21, Christian Borntraeger wrote:
> >>> I have some random crashes at startup 
> >>>                 
> >>>                 Stack trace of thread 48326:
> >>>                 #0  0x000002aa2e0cce46 bdrv_co_do_rw (qemu-system-s390x)
> >>>                 #1  0x000002aa2e159e8e coroutine_trampoline (qemu-system-s390x)
> >>>                 #2  0x000003ffbc35150a __makecontext_ret (libc.so.6)
> >>>
> >>>
> >>> that I was able to bisect.
> >>> commit 2906cddfecff21af20eedab43288b485a679f9ac does crash regularly, 
> >>> 2906cddfecff21af20eedab43288b485a679f9ac^ does not.
> >>>
> >>> I will try to find somebody that looks into that - unless you have an idea.
> >>
> >> The only random idea is to move
> >>
> >>     vblk->dataplane_started = true
> >>
> >> to the beginning of virtio_blk_data_plane_start rather than the end.
> >>
> >> Paolo
> >>
> > 
> > Indeed
> > 
> > diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> > index 36f3d2b..1908d59 100644
> > --- a/hw/block/dataplane/virtio-blk.c
> > +++ b/hw/block/dataplane/virtio-blk.c
> > @@ -195,6 +195,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
> >      if (vblk->dataplane_started || s->starting) {
> >          return;
> >      }
> > +    vblk->dataplane_started = true;
> >  
> >      s->starting = true;
> >      s->vq = virtio_get_queue(s->vdev, 0);
> > @@ -235,7 +236,6 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
> >    fail_guest_notifiers:
> >      s->disabled = true;
> >      s->starting = false;
> > -    vblk->dataplane_started = true;
> >  }
> >  
> >  /* Context: QEMU global mutex held */
> > 
> > seems to fix the issue. 
> 
> Hmmm, no another crash, just seems to happen less often. 

What about the proposal in <56C34414.90809@redhat.com>, i.e. move
setting the started flag out of dataplane entirely?
Christian Borntraeger March 9, 2016, 2:29 p.m. UTC | #7
On 03/09/2016 01:55 PM, Paolo Bonzini wrote:
> 
> 
> On 09/03/2016 13:21, Christian Borntraeger wrote:
>> I have some random crashes at startup 
>>                 
>>                 Stack trace of thread 48326:
>>                 #0  0x000002aa2e0cce46 bdrv_co_do_rw (qemu-system-s390x)
>>                 #1  0x000002aa2e159e8e coroutine_trampoline (qemu-system-s390x)
>>                 #2  0x000003ffbc35150a __makecontext_ret (libc.so.6)
>>
>>
>> that I was able to bisect.
>> commit 2906cddfecff21af20eedab43288b485a679f9ac does crash regularly, 
>> 2906cddfecff21af20eedab43288b485a679f9ac^ does not.
>>
>> I will try to find somebody that looks into that - unless you have an idea.
> 
> The only random idea is to move
> 
>     vblk->dataplane_started = true
> 
> to the beginning of virtio_blk_data_plane_start rather than the end.
> 
> Paolo

FWIW, it seems that this patch triggers this error, the "tracked_request_begin"
that I reported yesterday and / or some early read issues from the bootloader
in a random fashion.
Using 2906cddfecff21af20eedab43288b485a679f9ac^ seems to work all the time,
moving around vblk->dataplane_started = true also triggers all 3 types
of bugs, e.g.

Thread 1 (Thread 0x3ffaabff910 (LWP 32782)):
#0  0x0000000010329a70 in bdrv_co_do_rw (opaque=0x0) at /home/cborntra/REPOS/qemu/block/io.c:2170
#1  0x00000000103b2e7a in coroutine_trampoline (i0=1023, i1=-2147470992) at /home/cborntra/REPOS/qemu/util/coroutine-ucontext.c:79
#2  0x000003ffac85150a in __makecontext_ret () from /lib64/libc.so.6
(gdb) list
2165	
2166	/* Invoke bdrv_co_do_readv/bdrv_co_do_writev */
2167	static void coroutine_fn bdrv_co_do_rw(void *opaque)
2168	{
2169	    BlockAIOCBCoroutine *acb = opaque;
2170	    BlockDriverState *bs = acb->common.bs;
2171	
2172	    if (!acb->is_write) {
2173	        acb->req.error = bdrv_co_do_readv(bs, acb->req.sector,
2174	            acb->req.nb_sectors, acb->req.qiov, acb->req.flags);



I will try to find somebody to work on this.
Paolo Bonzini March 9, 2016, 4:17 p.m. UTC | #8
On 09/03/2016 15:29, Christian Borntraeger wrote:
> FWIW, it seems that this patch triggers this error, the "tracked_request_begin"
> that I reported yesterday and / or some early read issues from the bootloader
> in a random fashion.
> Using 2906cddfecff21af20eedab43288b485a679f9ac^ seems to work all the time,
> moving around vblk->dataplane_started = true also triggers all 3 types
> of bugs

In all likelihood, the bug is that virtio_blk_handle_output is being
called in two threads.

It's not clear to me how that's possible, though.

Paolo
diff mbox

Patch

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 03b81bc..cc521c1 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -28,7 +28,6 @@ 
 #include "qom/object_interfaces.h"
 
 struct VirtIOBlockDataPlane {
-    bool started;
     bool starting;
     bool stopping;
     bool disabled;
@@ -264,11 +263,7 @@  void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     VirtQueue *vq;
     int r;
 
-    if (s->started || s->disabled) {
-        return;
-    }
-
-    if (s->starting) {
+    if (vblk->dataplane_started || s->starting) {
         return;
     }
 
@@ -300,7 +295,7 @@  void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     vblk->complete_request = complete_request_vring;
 
     s->starting = false;
-    s->started = true;
+    vblk->dataplane_started = true;
     trace_virtio_blk_data_plane_start(s);
 
     blk_set_aio_context(s->conf->conf.blk, s->ctx);
@@ -319,9 +314,10 @@  void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     k->set_guest_notifiers(qbus->parent, 1, false);
   fail_guest_notifiers:
     vring_teardown(&s->vring, s->vdev, 0);
-    s->disabled = true;
   fail_vring:
+    s->disabled = true;
     s->starting = false;
+    vblk->dataplane_started = true;
 }
 
 /* Context: QEMU global mutex held */
@@ -331,13 +327,14 @@  void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
 
+    if (!vblk->dataplane_started || s->stopping) {
+        return;
+    }
 
     /* Better luck next time. */
     if (s->disabled) {
         s->disabled = false;
-        return;
-    }
-    if (!s->started || s->stopping) {
+        vblk->dataplane_started = false;
         return;
     }
     s->stopping = true;
@@ -364,6 +361,6 @@  void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
     /* Clean up guest notifier (irq) */
     k->set_guest_notifiers(qbus->parent, 1, false);
 
-    s->started = false;
+    vblk->dataplane_started = false;
     s->stopping = false;
 }
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c427698..e04c8f5 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -589,7 +589,7 @@  static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
     /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
      * dataplane here instead of waiting for .set_status().
      */
-    if (s->dataplane) {
+    if (s->dataplane && !s->dataplane_started) {
         virtio_blk_data_plane_start(s->dataplane);
         return;
     }
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 199bb0e..781969d 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -56,6 +56,7 @@  typedef struct VirtIOBlock {
     /* Function to push to vq and notify guest */
     void (*complete_request)(struct VirtIOBlockReq *req, unsigned char status);
     Notifier migration_state_notifier;
+    bool dataplane_started;
     struct VirtIOBlockDataPlane *dataplane;
 } VirtIOBlock;