From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58261) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVGCb-0003eS-4G for qemu-devel@nongnu.org; Thu, 25 Apr 2013 03:02:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UVGCW-0006QD-5D for qemu-devel@nongnu.org; Thu, 25 Apr 2013 03:02:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49077) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVGCV-0006Q4-UE for qemu-devel@nongnu.org; Thu, 25 Apr 2013 03:02:40 -0400 Message-ID: <5178D500.1030506@redhat.com> Date: Thu, 25 Apr 2013 15:02:24 +0800 From: Jason Wang MIME-Version: 1.0 References: <1366870793-769-1-git-send-email-jasowang@redhat.com> <20130425065720.GA7299@redhat.com> In-Reply-To: <20130425065720.GA7299@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio-net: unbreak the minix guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, Aurelien Jarno On 04/25/2013 02:57 PM, Michael S. Tsirkin wrote: > 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. Ok. > >> @@ -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. > */ Sure. >> + 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. Ok, will send v2. Thanks >> 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