Re: [Qemu-devel] Bug in virtio_net_load

[Robin Geuze]

Hey,

So over the weekend we did a bunch of migrations with a bunch of 
different guest OSes and such and it all worked fine, so I would say the 
patch is working properly :)

Regards,

Robin Geuze

On 7/1/2016 10:48, Cornelia Huck wrote:
> On Thu, 30 Jun 2016 20:23:08 +0300
> "Michael S. Tsirkin" <address@hidden> wrote:
>
> > I'm not sure what was I thinking when I applied this:
> > it changes load without changing save - how can this work?
> The ordering implications are easy to miss :(
>
> > I am inclined to revert 1f8828ef573c83365b4a87a776daf8bcef1caa21 and
> > apply this instead:
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 7ed06ea..18153d5 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -1499,6 +1499,16 @@ int virtio_load(VirtIODevice *vdev, QEMUFile
> > *f, int version_id) }
> >       qemu_get_be32s(f, &features);
> >
> > +    /*
> > +     * Temporarily set guest_features low bits - needed by
> > +     * virtio net load code testing for
> > VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> > +     * VIRTIO_NET_F_GUEST_ANNOUNCE and VIRTIO_NET_F_CTRL_VQ.
> > +     *
> > +     * Note: devices should always test host features in future -
> > don't create
> > +     * new dependencies like this.
> docs/virtio-migration.txt should probably talk about that as well. And
> any conditional stuff needs to go into a subsection in the future.
>
> > +     */
> > +    vdev->guest_features = features;
> > +
> >       config_len = qemu_get_be32(f);
> >
> >       /*
> >
> > Could you please confirm whether this help?
> > Jason, Cornelia - any comments?
> After staring at the code, I'm inclined to think that this will work.
>
> virtio migration: Frying unsuspecting brains since 2008.
> <To be fair, the original code wasn't that convoluted.>