Re: [Qemu-devel] [PATCH V2 18/20] virtio-net: multiqueue support

[Aurelien Jarno]

On Fri, Jan 25, 2013 at 06:35:41PM +0800, Jason Wang wrote:
> This patch implements both userspace and vhost support for multiple queue
> virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of
> VirtIONetQueue to VirtIONet.
> 
> Signed-off-by: Jason Wang <address@hidden>
> ---
>  hw/virtio-net.c |  317 
> +++++++++++++++++++++++++++++++++++++++++++------------
>  hw/virtio-net.h |   28 +++++-
>  2 files changed, 275 insertions(+), 70 deletions(-)

This patch breaks virtio-net in Minix, even with multiqueue disable. I
don't know virtio enough to know if it is a Minix or a QEMU problem.
However I have been able to identify the part of the commit causing the
failure:

> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index ef522d5..cec91a7 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c

...

> +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
> +{
> +    VirtIODevice *vdev = &n->vdev;
> +    int i, max = multiqueue ? n->max_queues : 1;
> +
> +    n->multiqueue = multiqueue;
> +
> +    for (i = 2; i <= n->max_queues * 2 + 1; i++) {
> +        virtio_del_queue(vdev, i);
> +    }
> +

The for loop above is something which is new, even with multiqueue
disabled. Even with max_queues=1 it calls virtio_del_queue with i = 2
and i = 3. Disabling this loop makes the code to work as before.

On the Minix side it triggers the following assertion:

| virtio.c:370: assert "q->vaddr != NULL" failed, function "free_phys_queue"    
                                                                                
                                             
| virtio_net(73141): panic: assert failed

This correspond to this function in lib/libvirtio/virtio.c:

| static void
| free_phys_queue(struct virtio_queue *q)
| {
|         assert(q != NULL);
|         assert(q->vaddr != NULL);
| 
|         free_contig(q->vaddr, q->ring_size);
|         q->vaddr = NULL;
|         q->paddr = 0;
|         q->num = 0;
|         free_contig(q->data, sizeof(q->data[0]));
|         q->data = NULL;
| }

Do you have an idea if the problem is on the Minix side or on the QEMU
side?

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net