[Qemu-devel] [PATCH] virtio-net: don't run bh on vm stopped

[Qemu-devel] [PATCH] virtio-net: don't run bh on vm stopped

[Michael S. Tsirkin]

commit 783e7706937fe15523b609b545587a028a2bdd03
    virtio-net: stop/start bh when appropriate

is incomplete: BH might execute within the same main loop iteration but
after vmstop, so in theory, we might trigger an assertion.
I was unable to reproduce this in practice,
but it seems clear enough that the potential is there, so worth fixing.

Cc: qemu-stable@nongnu.org
Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 268eff9..365e266 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1224,7 +1224,12 @@ static void virtio_net_tx_timer(void *opaque)
     VirtIONetQueue *q = opaque;
     VirtIONet *n = q->n;
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
-    assert(vdev->vm_running);
+    /* This happens when device was stopped but BH wasn't. */
+    if (!vdev->vm_running) {
+        /* Make sure tx waiting is set, so we'll run when restarted. */
+        assert(q->tx_waiting);
+        return;
+    }
 
     q->tx_waiting = 0;
 
@@ -1244,7 +1249,12 @@ static void virtio_net_tx_bh(void *opaque)
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
     int32_t ret;
 
-    assert(vdev->vm_running);
+    /* This happens when device was stopped but BH wasn't. */
+    if (!vdev->vm_running) {
+        /* Make sure tx waiting is set, so we'll run when restarted. */
+        assert(q->tx_waiting);
+        return;
+    }
 
     q->tx_waiting = 0;
 
-- 
MST

Re: [Qemu-devel] [PATCH] virtio-net: don't run bh on vm stopped

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 770 bytes --]

On Tue, Sep 02, 2014 at 05:26:12PM +0300, Michael S. Tsirkin wrote:
> commit 783e7706937fe15523b609b545587a028a2bdd03
>     virtio-net: stop/start bh when appropriate
> 
> is incomplete: BH might execute within the same main loop iteration but
> after vmstop, so in theory, we might trigger an assertion.
> I was unable to reproduce this in practice,
> but it seems clear enough that the potential is there, so worth fixing.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/net/virtio-net.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] virtio-net: don't run bh on vm stopped

[Michael S. Tsirkin]

On Wed, Sep 03, 2014 at 11:45:57AM +0100, Stefan Hajnoczi wrote:
> On Tue, Sep 02, 2014 at 05:26:12PM +0300, Michael S. Tsirkin wrote:
> > commit 783e7706937fe15523b609b545587a028a2bdd03
> >     virtio-net: stop/start bh when appropriate
> > 
> > is incomplete: BH might execute within the same main loop iteration but
> > after vmstop, so in theory, we might trigger an assertion.
> > I was unable to reproduce this in practice,
> > but it seems clear enough that the potential is there, so worth fixing.
> > 
> > Cc: qemu-stable@nongnu.org
> > Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  hw/net/virtio-net.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> Thanks, applied to my net tree:
> https://github.com/stefanha/qemu/commits/net
> 
> Stefan

It was actually on my tree already but since
I recalled my last pull request, no problem.
Can you send pull request with this today please,
so it can go into 2.1.1?