| Message ID | 1409667966-16907-1-git-send-email-mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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?
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;
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(-)