[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] clean up callback when del virtqueue

From: Jason Wang
Subject: Re: [Qemu-devel] [PATCH] clean up callback when del virtqueue
Date: Mon, 17 Sep 2018 10:42:45 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

On 2018年09月14日 21:14, liujunjie (A) wrote:

-----Original Message-----
From: Jason Wang [mailto:address@hidden
Sent: Friday, September 14, 2018 8:45 PM
To: liujunjie (A) <address@hidden>; address@hidden
Cc: Huangweidong (C) <address@hidden>; wangxin (U)
<address@hidden>; address@hidden; Gonglei (Arei)
<address@hidden>; Zhoujian (jay) <address@hidden>
Subject: Re: [Qemu-devel] [PATCH] clean up callback when del virtqueue

On 2018年09月08日 21:04, liujunjie wrote:
Before, we did not clear callback like handle_output when delete the
virtqueue which may result be segmentfault.
The scene is as follows:
1. Start a vm with multiqueue vhost-net, 2. then we write
VIRTIO_PCI_GUEST_FEATURES in PCI configuration to triger multiqueue
disable in this vm which will delete the virtqueue.
In this step, the tx_bh is deleted but the callback
virtio_net_handle_tx_bh still exist.
3. Finally, we write VIRTIO_PCI_QUEUE_NOTIFY in PCI configuration to
notify the deleted virtqueue. In this way, virtio_net_handle_tx_bh
will be called and qemu will be crashed.
Good catch.

Although the way described above is uncommon, we had better reinforce it.

Signed-off-by: liujunjie <address@hidden>
   hw/net/virtio-net.c | 4 +++-
   hw/virtio/virtio.c  | 3 +++
   2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index
f154756..9bb20e3 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1467,7 +1467,9 @@ static void virtio_net_handle_tx_bh(VirtIODevice
*vdev, VirtQueue *vq)
       virtio_queue_set_notification(vq, 0);
-    qemu_bh_schedule(q->tx_bh);
+    if (q->tx_bh) {
+        qemu_bh_schedule(q->tx_bh);
+    }
I believe this is not necessary since handle_output is NULL now?
Yes, it is not necessary.


   static void virtio_net_tx_timer(void *opaque) diff --git
a/hw/virtio/virtio.c b/hw/virtio/virtio.c index d4e4d98..7577518
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1604,6 +1604,9 @@ void virtio_del_queue(VirtIODevice *vdev, int n)

       vdev->vq[n].vring.num = 0;
       vdev->vq[n].vring.num_default = 0;
+    vdev->vq[n].vring.align = 0;
Is this related or fix for another bug?
It's not related for another bug. So it is also unnecessary to add this, too.
The reason why add it is only trying to correspond to the function 
<virtio_add_queue> in the same file.

Ok, please send V2 and cc qemu stable.



+    vdev->vq[n].handle_output = NULL;
+    vdev->vq[n].handle_aio_output = NULL;

   static void virtio_set_isr(VirtIODevice *vdev, int value)

reply via email to

[Prev in Thread] Current Thread [Next in Thread]