[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/8] virtio-blk: fix "disabled data plane" mode
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 5/8] virtio-blk: fix "disabled data plane" mode |
Date: |
Tue, 16 Feb 2016 15:26:26 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> ---
> 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 <address@hidden>
> + vblk->dataplane_started = true;
> }
>
<...>
[Qemu-devel] [PATCH 2/8] vring: make vring_enable_notification return void, Paolo Bonzini, 2016/02/14
[Qemu-devel] [PATCH 3/8] virtio: add AioContext-specific function for host notifiers, Paolo Bonzini, 2016/02/14
[Qemu-devel] [PATCH 4/8] virtio: export vring_notify as virtio_should_notify, Paolo Bonzini, 2016/02/14
[Qemu-devel] [PATCH 6/8] virtio-blk: do not use vring in dataplane, Paolo Bonzini, 2016/02/14