From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56777) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVhoh-0001Gr-8Y for qemu-devel@nongnu.org; Tue, 16 Feb 2016 10:45:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aVhoe-0002r2-IE for qemu-devel@nongnu.org; Tue, 16 Feb 2016 10:45:31 -0500 Received: from mail-wm0-x22d.google.com ([2a00:1450:400c:c09::22d]:37396) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVhoe-0002qj-7l for qemu-devel@nongnu.org; Tue, 16 Feb 2016 10:45:28 -0500 Received: by mail-wm0-x22d.google.com with SMTP id g62so158666531wme.0 for ; Tue, 16 Feb 2016 07:45:28 -0800 (PST) Sender: Paolo Bonzini References: <1455470231-5223-1-git-send-email-pbonzini@redhat.com> <1455470231-5223-6-git-send-email-pbonzini@redhat.com> <20160215185818.14c47ef5.cornelia.huck@de.ibm.com> From: Paolo Bonzini Message-ID: <56C34414.90809@redhat.com> Date: Tue, 16 Feb 2016 16:45:24 +0100 MIME-Version: 1.0 In-Reply-To: <20160215185818.14c47ef5.cornelia.huck@de.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/8] virtio-blk: fix "disabled data plane" mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: qemu-devel@nongnu.org, stefanha@redhat.com, mst@redhat.com 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/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index cc521c1..29db94b 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -263,7 +263,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s) VirtQueue *vq; int r; - if (vblk->dataplane_started || s->starting) { + if (s->starting) { return; } @@ -295,7 +295,6 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s) vblk->complete_request = complete_request_vring; s->starting = false; - vblk->dataplane_started = true; trace_virtio_blk_data_plane_start(s); blk_set_aio_context(s->conf->conf.blk, s->ctx); @@ -317,7 +316,6 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s) fail_vring: s->disabled = true; s->starting = false; - vblk->dataplane_started = true; } /* Context: QEMU global mutex held */ @@ -327,14 +325,13 @@ 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 (s->stopping) { return; } /* Better luck next time. */ if (s->disabled) { s->disabled = false; - vblk->dataplane_started = false; return; } s->stopping = true; @@ -361,6 +358,5 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s) /* Clean up guest notifier (irq) */ k->set_guest_notifiers(qbus->parent, 1, false); - vblk->dataplane_started = false; s->stopping = false; } 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? Paolo