From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57129) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVG7i-0008Fr-Qe for qemu-devel@nongnu.org; Thu, 25 Apr 2013 02:57:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UVG7c-0004wy-8u for qemu-devel@nongnu.org; Thu, 25 Apr 2013 02:57:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48395) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVG7b-0004wt-Vs for qemu-devel@nongnu.org; Thu, 25 Apr 2013 02:57:36 -0400 Date: Thu, 25 Apr 2013 09:57:20 +0300 From: "Michael S. Tsirkin" Message-ID: <20130425065720.GA7299@redhat.com> References: <1366870793-769-1-git-send-email-jasowang@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1366870793-769-1-git-send-email-jasowang@redhat.com> Subject: Re: [Qemu-devel] [PATCH] virtio-net: unbreak the minix guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, Aurelien Jarno On Thu, Apr 25, 2013 at 02:19:53PM +0800, Jason Wang wrote: > Multiqueue patchset conditionally add control vq only when guest negotiate the > feature. Though the spec is not clear on this but it breaks the minix guest > since it will identify the ctrl vq even if it does not support it. Though this > behavior seems a violation on the spec "If the VIRTIO_NET_F_CTRL_VQ feature bit > is negotiated, identify the control virtqueue.", to keep the backward > compatibility, always add the ctrl vq at end of the queues. > > Also remove the meaningless ctrl_vq initialization and vq deletion. > > Reported-by: Aurelien Jarno > Cc: Aurelien Jarno > Signed-off-by: Jason Wang > --- > hw/net/virtio-net.c | 9 ++++----- > 1 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 4d2cdd2..1068f4e 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -1045,7 +1045,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl) > > n->multiqueue = multiqueue; > > - for (i = 2; i <= n->max_queues * 2 + 1; i++) { > + for (i = 2; i <= n->max_queues * 2; i++) { > virtio_del_queue(vdev, i); > } > Unrelated cleanup? Separate patch pls. > @@ -1067,9 +1067,9 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl) > n->vqs[i].n = n; > } > > - if (ctrl) { > - n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl); > - } > + /* Some guest (e.g minix) may identifiy ctrl vq even if it does not > + * support. */ /* * Note: Minux Guests (version XYZ) use ctrl vq but don't ack * VIRTIO_NET_F_CTRL_VQ. Create ctrl vq unconditionally to avoid * breaking them. */ > + n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl); > > virtio_net_set_queues(n); > } > @@ -1317,7 +1317,6 @@ static int virtio_net_device_init(VirtIODevice *vdev) > virtio_net_handle_tx_bh); > n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]); > } > - n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl); Why drop this here? Unrelated cleanup? Separate patch pls. > qemu_macaddr_default_if_unset(&n->nic_conf.macaddr); > memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac)); > n->status = VIRTIO_NET_S_LINK_UP; > -- > 1.7.1