Re: [Qemu-devel] [PATCH 9/9] virtio: remove starting/stopping checks

[Christian Borntraeger]

On 04/01/2016 03:19 PM, Paolo Bonzini wrote:
> Reentrancy cannot happen while the BQL is being held.
> 
> Reviewed-by: Cornelia Huck <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>

Reverting this patch makes the segfaults go away.





> ---
>  hw/block/dataplane/virtio-blk.c | 12 ++----------
>  hw/scsi/virtio-scsi-dataplane.c |  9 +--------
>  include/hw/virtio/virtio-scsi.h |  2 --
>  3 files changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index 74c6d37..cb00cdc 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -27,9 +27,6 @@
>  #include "qom/object_interfaces.h"
> 
>  struct VirtIOBlockDataPlane {
> -    bool starting;
> -    bool stopping;
> -
>      VirtIOBlkConf *conf;
> 
>      VirtIODevice *vdev;
> @@ -203,11 +200,10 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane 
> *s)
>      VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
>      int r;
> 
> -    if (vblk->dataplane_started || s->starting) {
> +    if (vblk->dataplane_started) {
>          return;
>      }
> 
> -    s->starting = true;
>      s->vq = virtio_get_queue(s->vdev, 0);
> 
>      /* Set up guest notifier (irq) */
> @@ -226,7 +222,6 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>          goto fail_host_notifier;
>      }
> 
> -    s->starting = false;
>      vblk->dataplane_started = true;
>      trace_virtio_blk_data_plane_start(s);
> 
> @@ -246,7 +241,6 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      k->set_guest_notifiers(qbus->parent, 1, false);
>    fail_guest_notifiers:
>      vblk->dataplane_disabled = true;
> -    s->starting = false;
>      vblk->dataplane_started = true;
>  }
> 
> @@ -257,7 +251,7 @@ 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) {
> +    if (!vblk->dataplane_started) {
>          return;
>      }
> 
> @@ -267,7 +261,6 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>          vblk->dataplane_started = false;
>          return;
>      }
> -    s->stopping = true;
>      trace_virtio_blk_data_plane_stop(s);
> 
>      aio_context_acquire(s->ctx);
> @@ -286,5 +279,4 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>      k->set_guest_notifiers(qbus->parent, 1, false);
> 
>      vblk->dataplane_started = false;
> -    s->stopping = false;
>  }
> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> index 5494dcc..7511447 100644
> --- a/hw/scsi/virtio-scsi-dataplane.c
> +++ b/hw/scsi/virtio-scsi-dataplane.c
> @@ -115,14 +115,11 @@ void virtio_scsi_dataplane_start(VirtIOSCSI *s)
>      VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
> 
>      if (s->dataplane_started ||
> -        s->dataplane_starting ||
>          s->dataplane_fenced ||
>          s->ctx != iothread_get_aio_context(vs->conf.iothread)) {
>          return;
>      }
> 
> -    s->dataplane_starting = true;
> -
>      /* Set up guest notifier (irq) */
>      rc = k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, true);
>      if (rc != 0) {
> @@ -150,7 +147,6 @@ void virtio_scsi_dataplane_start(VirtIOSCSI *s)
>          }
>      }
> 
> -    s->dataplane_starting = false;
>      s->dataplane_started = true;
>      aio_context_release(s->ctx);
>      return;
> @@ -164,7 +160,6 @@ fail_vrings:
>      k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
>  fail_guest_notifiers:
>      s->dataplane_fenced = true;
> -    s->dataplane_starting = false;
>      s->dataplane_started = true;
>  }
> 
> @@ -176,7 +171,7 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
>      VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
>      int i;
> 
> -    if (!s->dataplane_started || s->dataplane_stopping) {
> +    if (!s->dataplane_started) {
>          return;
>      }
> 
> @@ -186,7 +181,6 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
>          s->dataplane_started = false;
>          return;
>      }
> -    s->dataplane_stopping = true;
>      assert(s->ctx == iothread_get_aio_context(vs->conf.iothread));
> 
>      aio_context_acquire(s->ctx);
> @@ -203,6 +197,5 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
> 
>      /* Clean up guest notifier (irq) */
>      k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
> -    s->dataplane_stopping = false;
>      s->dataplane_started = false;
>  }
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index ba2f5ce..d5352d8 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -89,8 +89,6 @@ typedef struct VirtIOSCSI {
>      QTAILQ_HEAD(, VirtIOSCSIBlkChangeNotifier) remove_notifiers;
> 
>      bool dataplane_started;
> -    bool dataplane_starting;
> -    bool dataplane_stopping;
>      bool dataplane_fenced;
>      Error *blocker;
>      uint32_t host_features;
>