Patchwork qemu-kvm-0.11 regression, crashes on older guests with virtio network

login
register
mail settings
Submitter Dustin Kirkland
Date Oct. 29, 2009, 3:01 p.m.
Message ID <1256828478.25064.126.camel@x200>
Download mbox | patch
Permalink /patch/37199/
State New
Headers show

Comments

Mark McLoughlin - Oct. 29, 2009, 3:01 p.m.
On Thu, 2009-10-29 at 10:01 -0500, Dustin Kirkland wrote:
> On Thu, 2009-10-29 at 14:48 +0000, Mark McLoughlin wrote:
> > Ah, it all makes sense now.
> > 
> > I was getting confused between HOST_* and GUEST_*
> > 
> > this should have been:
> > 
> >     features |= (1 << VIRTIO_NET_F_MAC);
> >     features |= (1 << VIRTIO_NET_F_HOST_CSUM);
> >     features |= (1 << VIRTIO_NET_F_HOST_TSO4);
> >     features |= (1 << VIRTIO_NET_F_HOST_TSO6);
> >     features |= (1 << VIRTIO_NET_F_HOST_ECN);
> > 
> > Could you try that Dustin?
> 
> Hmm, not sure I'm doing this correctly...  I tried changing the
> following, but looks like I might also have to define these as well,
> since:
> 
> /tmp/qemu-kvm/qemu-kvm/hw/virtio-net.c:167: error:
> ‘VIRTIO_NET_F_HOST_CSUM’ undeclared (first use in this function)

Sorry, should be VIRTIO_NET_F_CSUM ... the rest is correct

Thanks,
mark.
Dustin Kirkland - Oct. 29, 2009, 3:01 p.m.
On Thu, 2009-10-29 at 14:48 +0000, Mark McLoughlin wrote:
> Ah, it all makes sense now.
> 
> I was getting confused between HOST_* and GUEST_*
> 
> this should have been:
> 
>     features |= (1 << VIRTIO_NET_F_MAC);
>     features |= (1 << VIRTIO_NET_F_HOST_CSUM);
>     features |= (1 << VIRTIO_NET_F_HOST_TSO4);
>     features |= (1 << VIRTIO_NET_F_HOST_TSO6);
>     features |= (1 << VIRTIO_NET_F_HOST_ECN);
> 
> Could you try that Dustin?

Hmm, not sure I'm doing this correctly...  I tried changing the
following, but looks like I might also have to define these as well,
since:

/tmp/qemu-kvm/qemu-kvm/hw/virtio-net.c:167: error:
‘VIRTIO_NET_F_HOST_CSUM’ undeclared (first use in this function)
Dustin Kirkland - Oct. 29, 2009, 3:13 p.m.
On Thu, 2009-10-29 at 15:01 +0000, Mark McLoughlin wrote:
> Sorry, should be VIRTIO_NET_F_CSUM ... the rest is correct

Brilliant!

Works like a champ.  I'll send a patch in a subsequent email.  Would you
add a signed-off-by (or whatever), Mark?

:-Dustin
Mark McLoughlin - Oct. 29, 2009, 3:15 p.m.
On Thu, 2009-10-29 at 10:13 -0500, Dustin Kirkland wrote:
> On Thu, 2009-10-29 at 15:01 +0000, Mark McLoughlin wrote:
> > Sorry, should be VIRTIO_NET_F_CSUM ... the rest is correct
> 
> Brilliant!
> 
> Works like a champ.  I'll send a patch in a subsequent email.  Would you
> add a signed-off-by (or whatever), Mark?

Sure,

Signed-off-by: Mark McLoughlin <markmc@redhat.com>

or:

Acked-by: Mark McLoughlin <markmc@redhat.com>

whichever works :)

Cheers,
Mark.

Patch

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ce8e6cb..6582e69 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -164,10 +164,10 @@  static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
     /* Linux kernel 2.6.25.  It understood MAC (as everyone must),
      * but also these: */
     features |= (1 << VIRTIO_NET_F_MAC);
-    features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
-    features |= (1 << VIRTIO_NET_F_GUEST_ECN);
+    features |= (1 << VIRTIO_NET_F_HOST_CSUM);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO4);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO6);
+    features |= (1 << VIRTIO_NET_F_HOST_ECN);
 
     return features & virtio_net_get_features(vdev);
 }